From 7e6792c65dacdf49b5a02896192fc49c6da2de33 Mon Sep 17 00:00:00 2001 From: Lucas <24826124+Luro02@users.noreply.github.com> Date: Sat, 26 Aug 2023 11:25:44 +0200 Subject: [PATCH] fix(VariableAccessFilter): ensure all accesses are returned for generic fields #5391 --- .../visitor/filter/VariableAccessFilter.java | 22 +++- .../java/spoon/test/filters/FilterTest.java | 121 +++++++++++------- 2 files changed, 90 insertions(+), 53 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/filter/VariableAccessFilter.java b/src/main/java/spoon/reflect/visitor/filter/VariableAccessFilter.java index 4d0142dcf0f..9337e9792c6 100644 --- a/src/main/java/spoon/reflect/visitor/filter/VariableAccessFilter.java +++ b/src/main/java/spoon/reflect/visitor/filter/VariableAccessFilter.java @@ -12,16 +12,15 @@ import spoon.reflect.visitor.Filter; /** - * This simple filter matches all the accesses to a given field. + * This simple filter matches all the accesses to a given variable. */ public class VariableAccessFilter> implements Filter { CtVariableReference variable; /** - * Creates a new field access filter. + * Creates a new variable access filter. * - * @param variable - * the accessed variable + * @param variable the variable to find accesses to, must not be {@code null} */ public VariableAccessFilter(CtVariableReference variable) { if (variable == null) { @@ -32,7 +31,20 @@ public VariableAccessFilter(CtVariableReference variable) { @Override public boolean matches(T variableAccess) { - return variable.equals(variableAccess.getVariable()); + return this.variable.equals(variableAccess.getVariable()) + // if this.variable is a reference to a generic field, then the reference of the variableAccess might not + // be equal. + // + // For example: + // + // Given class A { T t }, the reference would be to `T t`, + // but an access to `t` could look like this: + // `A a = new A<>(); a.t = "foo";` + // ^^^ reference to `String t` + // => (String t) != (T t), but the same variable is accessed + // Therefore, it is checked that the variable they are referencing is the same. + || this.variable.getDeclaration() != null + && this.variable.getDeclaration().equals(variableAccess.getVariable().getDeclaration()); } } diff --git a/src/test/java/spoon/test/filters/FilterTest.java b/src/test/java/spoon/test/filters/FilterTest.java index a21594544e7..72574bae5b5 100644 --- a/src/test/java/spoon/test/filters/FilterTest.java +++ b/src/test/java/spoon/test/filters/FilterTest.java @@ -32,6 +32,7 @@ import spoon.reflect.code.CtNewClass; import spoon.reflect.code.CtStatement; import spoon.reflect.code.CtSwitch; +import spoon.reflect.code.CtVariableAccess; import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtExecutable; @@ -75,6 +76,7 @@ import spoon.reflect.visitor.filter.ReturnOrThrowFilter; import spoon.reflect.visitor.filter.SubtypeFilter; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.reflect.visitor.filter.VariableAccessFilter; import spoon.support.comparator.DeepRepresentationComparator; import spoon.support.reflect.declaration.CtMethodImpl; import spoon.support.visitor.SubInheritanceHierarchyResolver; @@ -568,7 +570,7 @@ public boolean matches(CtInvocation element) { assertNotNull(declaration); assertEquals("size", declaration.getSimpleName()); } - + @Test public void testReflectionBasedTypeFilter() { final Launcher launcher = new Launcher(); @@ -594,7 +596,7 @@ public boolean matches(CtClass element) { //then do it using Filter implemented by lambda expression List> allClasses3 = launcher.getFactory().Package().getRootPackage().getElements((CtClass element)->true); assertArrayEquals(allClasses.toArray(), allClasses3.toArray()); - + //last try AbstractFilter constructor without class parameter final CtClass aTacos = launcher.getFactory().Class().get(Tacos.class); final CtInvocation invSize = aTacos.getElements(new AbstractFilter>(/*no class is needed here*/) { @@ -614,14 +616,14 @@ public void testQueryStepScannWithConsumer() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { int counter = 0; } Context context = new Context(); - + CtQuery l_qv = launcher.getFactory().getModel().filterChildren(new TypeFilter<>(CtClass.class)); - + assertEquals(0, context.counter); l_qv.forEach(cls->{ assertTrue(cls instanceof CtClass); @@ -629,7 +631,7 @@ class Context { }); assertTrue(context.counter>0); } - + @Test public void testQueryBuilderWithFilterChain() { // contract: query methods can be lazy evaluated in a foreach @@ -637,12 +639,12 @@ public void testQueryBuilderWithFilterChain() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { CtMethod method; int count = 0; } - + Context context = new Context(); // chaining queries @@ -661,14 +663,14 @@ class Context { // sanity check assertTrue(context.count>0); } - + @Test public void testFilterQueryStep() { final Launcher launcher = new Launcher(); launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + //Contract: the filter(Filter) can be used to detect if input of query step should pass to next query step. List realList = launcher.getFactory().Package().getRootPackage().filterChildren(e->{return true;}).select(new TypeFilter<>(CtClass.class)).list(); List expectedList = launcher.getFactory().Package().getRootPackage().filterChildren(new TypeFilter<>(CtClass.class)).list(); @@ -682,7 +684,7 @@ public void testFilterChildrenWithoutFilterQueryStep() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + //Contract: the filterChildren(null) without Filter returns same results like filter which returns true for each input. List list = launcher.getFactory().Package().getRootPackage().filterChildren(null).list(); Iterator iter = list.iterator(); @@ -703,11 +705,11 @@ public void testFunctionQueryStep() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { int count = 0; } - + Context context = new Context(); CtQuery query = launcher.getFactory().Package().getRootPackage().filterChildren((CtClass c)->{return true;}).name("filter CtClass only") @@ -732,7 +734,7 @@ public void testInvalidQueryStep() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + try { launcher.getFactory().Package().getRootPackage().filterChildren((CtClass c)->{return true;}).name("step1") .map((CtMethod m)->m).name("invalidStep2") @@ -756,12 +758,12 @@ public void testInvalidQueryStepFailurePolicyIgnore() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { int count = 0; } Context context = new Context(); - + launcher.getFactory().Package().getRootPackage().filterChildren((CtElement c)->{return true;}).name("step1") .map((CtMethod m)->m).name("invalidStep2") .map((o)->o).name("step3") @@ -779,7 +781,7 @@ public void testElementMapFunction() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + CtClass cls = launcher.getFactory().Class().get(Tacos.class); cls.map((CtClass c)->c.getParent()) .forEach((CtElement e)->{ @@ -794,7 +796,7 @@ public void testElementMapFunctionOtherContracts() { CtQuery q = launcher.getFactory().Query().createQuery().map((String s)->new String[]{"a", null, s}); List list = q.setInput(null).list(); assertEquals(0, list.size()); - + list = q.setInput("c").list(); assertEquals(2, list.size()); assertEquals("a", list.get(0)); @@ -825,7 +827,7 @@ public void testReuseOfQuery() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + CtClass cls = launcher.getFactory().Class().get(Tacos.class); CtClass cls2 = launcher.getFactory().Class().get(Tostada.class); @@ -853,7 +855,7 @@ public void testReuseOfBaseQuery() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + CtClass cls = launcher.getFactory().Class().get(Tacos.class); CtClass cls2 = launcher.getFactory().Class().get(Tostada.class); @@ -928,13 +930,13 @@ public void testQueryInQuery() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { int count = 0; } - + Context context = new Context(); - + CtClass cls = launcher.getFactory().Class().get(Tacos.class); // first query @@ -975,13 +977,13 @@ class Context { }); assertEquals(8, context.count); } - + @Test public void testEmptyQuery() { // contract: unbound or empty query final Launcher launcher = new Launcher(); - + //contract: empty query returns no element assertEquals(0, launcher.getFactory().createQuery().list().size()); assertEquals(0, launcher.getFactory().createQuery((Object) null).list().size()); @@ -995,13 +997,13 @@ public void testEmptyQuery() { assertEquals(0, launcher.getFactory().createQuery().filterChildren(x->{fail();return true;}).list().size()); assertEquals(0, launcher.getFactory().createQuery((Object) null).filterChildren(x->{fail();return true;}).list().size()); } - + @Test public void testBoundQuery() { // contract: bound query, without any mapping final Launcher launcher = new Launcher(); - + //contract: bound query returns bound element List list = launcher.getFactory().createQuery("x").list(); assertEquals(1, list.size()); @@ -1011,17 +1013,17 @@ public void testBoundQuery() { @Test public void testClassCastExceptionOnForEach() { // contract: bound query, without any mapping - // This test could fail with a version of JDK <= 8.0.40. + // This test could fail with a version of JDK <= 8.0.40. final Launcher launcher = new Launcher(); launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { int count = 0; } - + { Context context = new Context(); //contract: if the query produces elements which cannot be cast to forEach consumer, then they are ignored @@ -1155,7 +1157,7 @@ public void apply(CtType input, CtConsumer outputConsumer) { } } } - + @Test public void testEarlyTerminatingQuery() { // contract: a method first evaluates query until first element is found and then terminates the query @@ -1164,16 +1166,16 @@ public void testEarlyTerminatingQuery() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { boolean wasTerminated = false; void failIfTerminated(String place) { assertTrue(wasTerminated==false, "The "+place+" is called after query was terminated."); } } - + Context context = new Context(); - + CtMethod firstMethod = launcher.getFactory().Package().getRootPackage().filterChildren(e->{ context.failIfTerminated("Filter#match of filterChildren"); return true; @@ -1210,28 +1212,28 @@ public void testParentFunction() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + CtClass cls = launcher.getFactory().Class().get(Tacos.class); CtLocalVariable varStrings = cls.filterChildren(new NamedElementFilter<>(CtLocalVariable.class,"strings")).first(); - + class Context { CtElement expectedParent; } - + Context context = new Context(); - + context.expectedParent = varStrings; - + varStrings.map(new ParentFunction()).forEach((parent)->{ context.expectedParent = context.expectedParent.getParent(); assertSame(context.expectedParent, parent); }); - + //context.expectedParent is last visited element - + //Check that last visited element was root package assertSame(launcher.getFactory().getModel().getUnnamedModule(), context.expectedParent); - + //contract: if includingSelf(false), then parent of input element is first element assertSame(varStrings.getParent(), varStrings.map(new ParentFunction().includingSelf(false)).first()); //contract: if includingSelf(true), then input element is first element @@ -1247,14 +1249,14 @@ public void testCtScannerListener() { launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/filters/testclasses"); launcher.run(); - + class Context { long nrOfEnter = 0; long nrOfEnterRetTrue = 0; long nrOfExit = 0; long nrOfResults = 0; } - + Context context1 = new Context(); // scan only packages until top level classes. Do not scan class internals @@ -1271,7 +1273,7 @@ public ScanningMode enter(CtElement element) { public void exit(CtElement element) { context1.nrOfExit++; } - + })).list(); //check that test is visiting some nodes @@ -1281,9 +1283,9 @@ public void exit(CtElement element) { assertEquals(context1.nrOfEnter, context1.nrOfExit); Context context2 = new Context(); - + Iterator iter = result1.iterator(); - + //scan only from packages till top level classes. Do not scan class internals launcher.getFactory().getModel().map(new CtScannerFunction().setListener(new CtScannerListener() { int inClass = 0; @@ -1308,7 +1310,7 @@ public void exit(CtElement element) { } assertTrue(inClass==0 || inClass==1); } - + })).forEach(ele->{ context2.nrOfResults++; assertTrue(ele instanceof CtPackage || ele instanceof CtType || ele instanceof CtModule, "ele instanceof "+ele.getClass()); @@ -1430,4 +1432,27 @@ public boolean matches(CtElement element) { } + @Test + public void testVariableAccessFilterWithGenericField() { + // contract: VariableAccessFilter returns all accesses to a given field, even if it is generic + CtClass ctClass = Launcher.parseClass( + "public class Example {\n" + + " T field;\n" + + "\n" + + " public static void main(String[] args) {\n" + + " Example example = new Example<>();\n" + + "\n" + + " example.field = \"Hello\"; // write access to Example#field\n" + + " System.out.println(example.field); // read access to Example#field\n" + + " }\n" + + "}\n" + ); + + List> accesses = ctClass.getElements( + new VariableAccessFilter<>(ctClass.getField("field").getReference()) + ); + + assertEquals(2, accesses.size()); + } + }