From 9694ff929b8980948e618cc83304bcbc5420e5a1 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Wed, 16 Nov 2016 22:28:23 +0100 Subject: [PATCH 01/29] bug: no fully qualified name if top package is shadowed by a local variable --- .../AccessFullyQualifiedFieldTest.java | 19 ++++++++++++++++++- .../test/variable/testclasses/Burritos.java | 11 +++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 src/test/java/spoon/test/variable/testclasses/Burritos.java diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 4686471df22..dcb9267c6a5 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -1,11 +1,16 @@ package spoon.test.variable; import org.junit.Test; +import spoon.Launcher; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; import spoon.test.main.MainTest; +import spoon.test.variable.testclasses.Burritos; import spoon.test.variable.testclasses.Tacos; import static spoon.testing.utils.ModelUtils.build; +import static spoon.testing.utils.ModelUtils.canBeBuilt; public class AccessFullyQualifiedFieldTest { @Test @@ -14,4 +19,16 @@ public void testCheckAssignmentContracts() throws Exception { MainTest.checkAssignmentContracts(factory.Package().getRootPackage()); } -} + + @Test + public void testNoFQN() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + Launcher spoon = new Launcher(); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/Burritos.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + +} \ No newline at end of file diff --git a/src/test/java/spoon/test/variable/testclasses/Burritos.java b/src/test/java/spoon/test/variable/testclasses/Burritos.java new file mode 100644 index 00000000000..a0b4517b2d5 --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/Burritos.java @@ -0,0 +1,11 @@ +package spoon.test.variable.testclasses; + + +import static spoon.Launcher.SPOONED_CLASSES; + +public class Burritos { + void foo() { + Object spoon = null; + Object xx = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + } +} From 30363dbee335feebfbb1dca8d9c78c257a92dbb9 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 5 Dec 2016 13:47:28 +0100 Subject: [PATCH 02/29] Proposal to fix #972: apparently a condition was improperly checked with an or instead of an and. Now the test is failing, proof that the assertion is checked :) --- src/test/java/spoon/test/api/APITest.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/test/java/spoon/test/api/APITest.java b/src/test/java/spoon/test/api/APITest.java index 43a67fd8192..1bd88d9decc 100644 --- a/src/test/java/spoon/test/api/APITest.java +++ b/src/test/java/spoon/test/api/APITest.java @@ -296,11 +296,18 @@ public SetterMethodWithoutCollectionsFilter(Factory factory) { @Override public boolean matches(CtMethod element) { - return isSetterMethod(element) && !isSubTypeOfCollection(element) && super.matches(element); + boolean isSetter = isSetterMethod(element); + boolean isNotSubType = !isSubTypeOfCollection(element); + boolean superMatch = super.matches(element); + return isSetter && isNotSubType && superMatch; } private boolean isSubTypeOfCollection(CtMethod element) { - final CtTypeReference type = element.getParameters().get(0).getType(); + final List> parameters = element.getParameters(); + if (parameters.size() != 1) { + return false; + } + final CtTypeReference type = parameters.get(0).getType(); for (CtTypeReference aCollectionRef : collections) { if (type.isSubtypeOf(aCollectionRef) || type.equals(aCollectionRef)) { return true; @@ -316,7 +323,10 @@ private boolean isSetterMethod(CtMethod element) { } final CtTypeReference typeParameter = parameters.get(0).getType(); final CtTypeReference ctElementRef = element.getFactory().Type().createReference(CtElement.class); - if (!typeParameter.isSubtypeOf(ctElementRef) || !typeParameter.equals(ctElementRef)) { + + boolean isSubtypeof = typeParameter.isSubtypeOf(ctElementRef); + boolean isEquals = typeParameter.equals(ctElementRef); + if (!isSubtypeof && !isEquals) { return false; } return element.getSimpleName().startsWith("set") && element.getDeclaringType().getSimpleName().startsWith("Ct") && element.getBody() != null; From c05c638913510abf121c4892d1bcdc8bb9ee15d6 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 5 Dec 2016 14:10:34 +0100 Subject: [PATCH 03/29] WiP #980: create two tests to check case when the name is shadowed by a local variable in the same block or by a field (or a variable in another block). First detection implemented but patch not satisfying. --- .../support/compiler/jdt/ParentExiter.java | 79 +++++-------------- .../AccessFullyQualifiedFieldTest.java | 18 +++-- .../variable/testclasses/BurritosFielded.java | 12 +++ 3 files changed, 46 insertions(+), 63 deletions(-) create mode 100644 src/test/java/spoon/test/variable/testclasses/BurritosFielded.java diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index 29215f6d0bb..05615c8fdd6 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -37,68 +37,14 @@ import org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.eclipse.jdt.internal.compiler.ast.UnionTypeReference; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; -import spoon.reflect.code.BinaryOperatorKind; -import spoon.reflect.code.CtArrayAccess; -import spoon.reflect.code.CtArrayRead; -import spoon.reflect.code.CtArrayWrite; -import spoon.reflect.code.CtAssert; -import spoon.reflect.code.CtAssignment; -import spoon.reflect.code.CtBinaryOperator; -import spoon.reflect.code.CtBlock; -import spoon.reflect.code.CtCase; -import spoon.reflect.code.CtCatch; -import spoon.reflect.code.CtCatchVariable; -import spoon.reflect.code.CtConditional; -import spoon.reflect.code.CtConstructorCall; -import spoon.reflect.code.CtDo; -import spoon.reflect.code.CtExecutableReferenceExpression; -import spoon.reflect.code.CtExpression; -import spoon.reflect.code.CtFor; -import spoon.reflect.code.CtForEach; -import spoon.reflect.code.CtIf; -import spoon.reflect.code.CtInvocation; -import spoon.reflect.code.CtLambda; -import spoon.reflect.code.CtLocalVariable; -import spoon.reflect.code.CtLoop; -import spoon.reflect.code.CtNewArray; -import spoon.reflect.code.CtNewClass; -import spoon.reflect.code.CtReturn; -import spoon.reflect.code.CtStatement; -import spoon.reflect.code.CtSuperAccess; -import spoon.reflect.code.CtSwitch; -import spoon.reflect.code.CtSynchronized; -import spoon.reflect.code.CtTargetedExpression; -import spoon.reflect.code.CtThisAccess; -import spoon.reflect.code.CtThrow; -import spoon.reflect.code.CtTry; -import spoon.reflect.code.CtTryWithResource; -import spoon.reflect.code.CtTypeAccess; -import spoon.reflect.code.CtUnaryOperator; -import spoon.reflect.code.CtWhile; -import spoon.reflect.declaration.CtAnnotatedElementType; -import spoon.reflect.declaration.CtAnnotation; -import spoon.reflect.declaration.CtAnnotationMethod; -import spoon.reflect.declaration.CtAnonymousExecutable; -import spoon.reflect.declaration.CtClass; -import spoon.reflect.declaration.CtConstructor; -import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtEnum; -import spoon.reflect.declaration.CtEnumValue; -import spoon.reflect.declaration.CtExecutable; -import spoon.reflect.declaration.CtField; -import spoon.reflect.declaration.CtFormalTypeDeclarer; -import spoon.reflect.declaration.CtMethod; -import spoon.reflect.declaration.CtPackage; -import spoon.reflect.declaration.CtParameter; -import spoon.reflect.declaration.CtType; -import spoon.reflect.declaration.CtTypeParameter; -import spoon.reflect.declaration.CtTypedElement; -import spoon.reflect.declaration.CtVariable; +import spoon.reflect.code.*; +import spoon.reflect.declaration.*; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtIntersectionTypeReference; import spoon.reflect.reference.CtTypeParameterReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtInheritanceScanner; +import spoon.reflect.visitor.Filter; import java.util.ArrayList; import java.util.HashMap; @@ -234,7 +180,24 @@ public void scanCtVariable(CtVariable v) { substituteAnnotation((CtTypedElement) v); return; } else if (child instanceof CtExpression && hasChildEqualsToDefaultValue(v)) { - v.setDefaultExpression((CtExpression) child); + + // first trial to detect if part of absolute name of a type is used by a previously recorded element in the model + List allElements = this.jdtTreeBuilder.getFactory().getModel().getElements(new Filter() { + @Override + public boolean matches(CtElement element) { + + return (element instanceof CtVariable) && (child.toString().startsWith(((CtNamedElement) element).getSimpleName())); + } + }); + + CtLiteral simpleExpression = this.jdtTreeBuilder.getFactory().Core().createLiteral(); + simpleExpression.setValue(childJDT.toString()); + + if (allElements.isEmpty()) { + v.setDefaultExpression((CtExpression) child); + } else { + v.setDefaultExpression(simpleExpression); + } return; } super.scanCtVariable(v); diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index dcb9267c6a5..7670f29e29d 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -2,11 +2,8 @@ import org.junit.Test; import spoon.Launcher; -import spoon.reflect.declaration.CtType; -import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; import spoon.test.main.MainTest; -import spoon.test.variable.testclasses.Burritos; import spoon.test.variable.testclasses.Tacos; import static spoon.testing.utils.ModelUtils.build; @@ -21,11 +18,22 @@ public void testCheckAssignmentContracts() throws Exception { } @Test - public void testNoFQN() throws Exception { + public void testNoFQNWhenShadowedByField() throws Exception { + // contract: no fully qualified name if top package is shadowed by a field variable + Launcher spoon = new Launcher(); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosFielded.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Field/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + + @Test + public void testNoFQNWhenShadowedByLocalVariable() throws Exception { // contract: no fully qualified name if top package is shadowed by a local variable Launcher spoon = new Launcher(); spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/Burritos.java"); - String output = "target/spooned-" + this.getClass().getSimpleName()+"/"; + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Local/"; spoon.setSourceOutputDirectory(output); spoon.run(); canBeBuilt(output, 7); diff --git a/src/test/java/spoon/test/variable/testclasses/BurritosFielded.java b/src/test/java/spoon/test/variable/testclasses/BurritosFielded.java new file mode 100644 index 00000000000..95af76c1670 --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/BurritosFielded.java @@ -0,0 +1,12 @@ +package spoon.test.variable.testclasses; + + +import static spoon.Launcher.SPOONED_CLASSES; + +public class BurritosFielded { + Object spoon = null; + + void foo() { + Object xx = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + } +} From 5bf26f57d64f3a352e64882b11a3a030009b443c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 5 Dec 2016 15:03:41 +0100 Subject: [PATCH 04/29] WiP #980: first naive fix when the shadowed variable is a field, by using autoimport --- .../java/spoon/support/compiler/jdt/ParentExiter.java | 10 +++------- .../test/variable/AccessFullyQualifiedFieldTest.java | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index 05615c8fdd6..9eb917ee9cd 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -190,14 +190,10 @@ public boolean matches(CtElement element) { } }); - CtLiteral simpleExpression = this.jdtTreeBuilder.getFactory().Core().createLiteral(); - simpleExpression.setValue(childJDT.toString()); - - if (allElements.isEmpty()) { - v.setDefaultExpression((CtExpression) child); - } else { - v.setDefaultExpression(simpleExpression); + if (!allElements.isEmpty()) { + this.jdtTreeBuilder.getFactory().getEnvironment().setAutoImports(true); } + v.setDefaultExpression((CtExpression) child); return; } super.scanCtVariable(v); diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 7670f29e29d..80046e97f56 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -21,6 +21,7 @@ public void testCheckAssignmentContracts() throws Exception { public void testNoFQNWhenShadowedByField() throws Exception { // contract: no fully qualified name if top package is shadowed by a field variable Launcher spoon = new Launcher(); + spoon.setArgs(new String[]{"--with-imports"}); spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosFielded.java"); String output = "target/spooned-" + this.getClass().getSimpleName()+"-Field/"; spoon.setSourceOutputDirectory(output); From a0a1329b7b8a0bc50535247bc6e034d94722f856 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 10:06:12 +0100 Subject: [PATCH 05/29] WiP #980 New tests and scanner for variables --- .../spoon/reflect/visitor/ImportScanner.java | 1 + .../support/visitor/java/VariableScanner.java | 88 +++++++++++++++++++ .../test/variable/testclasses/Burritos.java | 10 ++- 3 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/main/java/spoon/support/visitor/java/VariableScanner.java diff --git a/src/main/java/spoon/reflect/visitor/ImportScanner.java b/src/main/java/spoon/reflect/visitor/ImportScanner.java index 734df3940ce..482d1701414 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/ImportScanner.java @@ -37,6 +37,7 @@ public interface ImportScanner { /** * Computes imports for all elements. */ + @Deprecated void computeImports(CtElement element); /** diff --git a/src/main/java/spoon/support/visitor/java/VariableScanner.java b/src/main/java/spoon/support/visitor/java/VariableScanner.java new file mode 100644 index 00000000000..972c6ef02e7 --- /dev/null +++ b/src/main/java/spoon/support/visitor/java/VariableScanner.java @@ -0,0 +1,88 @@ +package spoon.support.visitor.java; + +import spoon.reflect.code.*; +import spoon.reflect.declaration.*; +import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.visitor.CtInheritanceScanner; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.Set; + +public class VariableScanner extends CtInheritanceScanner { + private CtElement expression; + final List variables = new ArrayList<>(); + + public VariableScanner(CtElement expression) { + super(); + this.expression = expression; + } + + @Override + public void visitCtStatementList(CtStatementList e) { + for (int i = 0; i < e.getStatements().size(); i++) { + CtStatement ctStatement = e.getStatements().get(i); + if (ctStatement.getPosition() == null) { + } + if (ctStatement.getPosition() != null + && ctStatement.getPosition().getSourceStart() > expression.getPosition().getSourceEnd()) { + break; + } + if (ctStatement instanceof CtVariable) { + variables.add((CtVariable) ctStatement); + } + } + super.visitCtStatementList(e); + } + + @Override + public void scanCtType(CtType type) { + List> fields = type.getFields(); + for (int i = 0; i < fields.size(); i++) { + CtField ctField = fields.get(i); + if (ctField.hasModifier(ModifierKind.PUBLIC) || ctField.hasModifier(ModifierKind.PROTECTED)) { + variables.add(ctField); + } else if (ctField.hasModifier(ModifierKind.PRIVATE)) { + if (expression.hasParent(type)) { + variables.add(ctField); + } + } else if (expression.getParent(CtPackage.class).equals(type.getParent(CtPackage.class))) { + // default visibility + variables.add(ctField); + } + } + + CtTypeReference superclass = type.getSuperclass(); + if (superclass != null) { + this.scan(superclass.getTypeDeclaration()); + } + Set> superInterfaces = type.getSuperInterfaces(); + for (Iterator> iterator = superInterfaces.iterator(); iterator.hasNext();) { + CtTypeReference typeReference = iterator.next(); + this.scan(typeReference.getTypeDeclaration()); + } + } + + @Override + public void visitCtTryWithResource(CtTryWithResource e) { + variables.addAll(e.getResources()); + } + + @Override + public void scanCtExecutable(CtExecutable e) { + variables.addAll(e.getParameters()); + } + + @Override + public void visitCtFor(CtFor e) { + for (CtStatement ctStatement : e.getForInit()) { + this.scan(ctStatement); + } + } + + @Override + public void visitCtForEach(CtForEach e) { + variables.add(e.getVariable()); + } +} \ No newline at end of file diff --git a/src/test/java/spoon/test/variable/testclasses/Burritos.java b/src/test/java/spoon/test/variable/testclasses/Burritos.java index a0b4517b2d5..73c92337aa6 100644 --- a/src/test/java/spoon/test/variable/testclasses/Burritos.java +++ b/src/test/java/spoon/test/variable/testclasses/Burritos.java @@ -4,8 +4,16 @@ import static spoon.Launcher.SPOONED_CLASSES; public class Burritos { + static void toto() {} + void foo() { Object spoon = null; - Object xx = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + Object x = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + new Thread(new Runnable() { + @Override + public void run() { + Burritos.toto(); + } + }); } } From 0b5371da356b8e63383d5da1e92bdc0fa7314a3c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 10:27:39 +0100 Subject: [PATCH 06/29] Revert "Merge branch 'bug-static-import' of https://github.com/monperrus/spoon into martin" This reverts commit 26092840339a89a5db7db11836f95d331747e697, reversing changes made to 49d6e684b0e6922e102ba2bbd25a711f44155498. --- .../AccessFullyQualifiedFieldTest.java | 19 +------------------ .../test/variable/testclasses/Burritos.java | 11 ----------- 2 files changed, 1 insertion(+), 29 deletions(-) delete mode 100644 src/test/java/spoon/test/variable/testclasses/Burritos.java diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index dcb9267c6a5..4686471df22 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -1,16 +1,11 @@ package spoon.test.variable; import org.junit.Test; -import spoon.Launcher; -import spoon.reflect.declaration.CtType; -import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; import spoon.test.main.MainTest; -import spoon.test.variable.testclasses.Burritos; import spoon.test.variable.testclasses.Tacos; import static spoon.testing.utils.ModelUtils.build; -import static spoon.testing.utils.ModelUtils.canBeBuilt; public class AccessFullyQualifiedFieldTest { @Test @@ -19,16 +14,4 @@ public void testCheckAssignmentContracts() throws Exception { MainTest.checkAssignmentContracts(factory.Package().getRootPackage()); } - - @Test - public void testNoFQN() throws Exception { - // contract: no fully qualified name if top package is shadowed by a local variable - Launcher spoon = new Launcher(); - spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/Burritos.java"); - String output = "target/spooned-" + this.getClass().getSimpleName()+"/"; - spoon.setSourceOutputDirectory(output); - spoon.run(); - canBeBuilt(output, 7); - } - -} \ No newline at end of file +} diff --git a/src/test/java/spoon/test/variable/testclasses/Burritos.java b/src/test/java/spoon/test/variable/testclasses/Burritos.java deleted file mode 100644 index a0b4517b2d5..00000000000 --- a/src/test/java/spoon/test/variable/testclasses/Burritos.java +++ /dev/null @@ -1,11 +0,0 @@ -package spoon.test.variable.testclasses; - - -import static spoon.Launcher.SPOONED_CLASSES; - -public class Burritos { - void foo() { - Object spoon = null; - Object xx = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import - } -} From c9fe205401a26c3363d5508097f8d5635505300c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 13:39:13 +0100 Subject: [PATCH 07/29] Fix #972 and subsequent tests. Add a condition to ignore unsettableProperties. Some choices have been done concerning some set methods in order to pass the test we should discuss. --- .../support/reflect/code/CtCatchImpl.java | 13 ++++++--- .../support/reflect/code/CtLambdaImpl.java | 29 ++++++++++++------- .../reflect/code/CtLocalVariableImpl.java | 3 ++ .../support/reflect/code/CtLoopImpl.java | 12 +++++--- .../spoon/support/reflect/code/CtTryImpl.java | 13 ++++++--- .../reflect/code/CtVariableAccessImpl.java | 3 ++ .../reflect/declaration/CtExecutableImpl.java | 12 +++++--- .../reflect/declaration/CtFieldImpl.java | 2 ++ .../reflect/declaration/CtTypeImpl.java | 2 ++ src/test/java/spoon/test/api/APITest.java | 12 ++++++-- 10 files changed, 72 insertions(+), 29 deletions(-) diff --git a/src/main/java/spoon/support/reflect/code/CtCatchImpl.java b/src/main/java/spoon/support/reflect/code/CtCatchImpl.java index b52fd88d33d..768fbb69ba4 100644 --- a/src/main/java/spoon/support/reflect/code/CtCatchImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtCatchImpl.java @@ -47,11 +47,16 @@ public CtCatchVariable getParameter() { @Override public T setBody(CtStatement statement) { - CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); - if (body != null) { - body.setParent(this); + if (statement != null) { + CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); + if (body != null) { + body.setParent(this); + } + this.body = body; + } else { + this.body = null; } - this.body = body; + return (T) this; } diff --git a/src/main/java/spoon/support/reflect/code/CtLambdaImpl.java b/src/main/java/spoon/support/reflect/code/CtLambdaImpl.java index 12692b9e87e..3f001c7c996 100644 --- a/src/main/java/spoon/support/reflect/code/CtLambdaImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtLambdaImpl.java @@ -69,14 +69,20 @@ public CtBlock getBody() { @Override public C setBody(CtStatement statement) { - CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); - if (expression != null && body != null) { - throw new SpoonException("A lambda can't have two bodys."); - } - if (body != null) { - body.setParent(this); + + if (statement != null) { + CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); + if (expression != null && body != null) { + throw new SpoonException("A lambda can't have two bodys."); + } + if (body != null) { + body.setParent(this); + } + this.body = body; + } else { + this.body = null; } - this.body = body; + return (C) this; } @@ -180,11 +186,12 @@ public CtExpression getExpression() { public > C setExpression(CtExpression expression) { if (body != null && expression != null) { throw new SpoonException("A lambda can't have two bodys."); + } else { + if (expression != null) { + expression.setParent(this); + } + this.expression = expression; } - if (expression != null) { - expression.setParent(this); - } - this.expression = expression; return (C) this; } diff --git a/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java b/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java index 5a05a82eef6..0802ae2435f 100644 --- a/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java @@ -27,6 +27,8 @@ import spoon.reflect.reference.CtLocalVariableReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtVisitor; +import spoon.support.DerivedProperty; +import spoon.support.UnsettableProperty; import spoon.support.reflect.declaration.CtElementImpl; import java.util.EnumSet; @@ -156,6 +158,7 @@ public CtExpression getAssignment() { } @Override + @UnsettableProperty public > C setAssignment(CtExpression assignment) { setDefaultExpression(assignment); return (C) this; diff --git a/src/main/java/spoon/support/reflect/code/CtLoopImpl.java b/src/main/java/spoon/support/reflect/code/CtLoopImpl.java index bbfc1137cb4..133ff74d804 100644 --- a/src/main/java/spoon/support/reflect/code/CtLoopImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtLoopImpl.java @@ -36,11 +36,15 @@ public CtStatement getBody() { @SuppressWarnings("unchecked") @Override public T setBody(CtStatement statement) { - CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); - if (body != null) { - body.setParent(this); + if (statement != null) { + CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); + if (body != null) { + body.setParent(this); + } + this.body = body; + } else { + this.body = null; } - this.body = body; return (T) this; } diff --git a/src/main/java/spoon/support/reflect/code/CtTryImpl.java b/src/main/java/spoon/support/reflect/code/CtTryImpl.java index 39937a0962e..c718144d8c3 100644 --- a/src/main/java/spoon/support/reflect/code/CtTryImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtTryImpl.java @@ -102,11 +102,16 @@ public CtBlock getBody() { @Override public T setBody(CtStatement statement) { - CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); - if (body != null) { - body.setParent(this); + if (statement != null) { + CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); + if (body != null) { + body.setParent(this); + } + this.body = body; + } else { + this.body = null; } - this.body = body; + return (T) this; } diff --git a/src/main/java/spoon/support/reflect/code/CtVariableAccessImpl.java b/src/main/java/spoon/support/reflect/code/CtVariableAccessImpl.java index 902871d706c..036ece75712 100644 --- a/src/main/java/spoon/support/reflect/code/CtVariableAccessImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtVariableAccessImpl.java @@ -57,6 +57,9 @@ public CtTypeReference getType() { @Override public C setType(CtTypeReference type) { + if (type != null) { + type.setParent(this); + } if (type != null) { getVariable().setType(type); } diff --git a/src/main/java/spoon/support/reflect/declaration/CtExecutableImpl.java b/src/main/java/spoon/support/reflect/declaration/CtExecutableImpl.java index 354aed706fa..f6215f2483a 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtExecutableImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtExecutableImpl.java @@ -67,11 +67,15 @@ public CtBlock getBody() { @Override public T setBody(CtStatement statement) { - CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); - if (body != null) { - body.setParent(this); + if (statement != null) { + CtBlock body = getFactory().Code().getOrCreateCtBlock(statement); + if (body != null) { + body.setParent(this); + } + this.body = body; + } else { + this.body = null; } - this.body = body; return (T) this; } diff --git a/src/main/java/spoon/support/reflect/declaration/CtFieldImpl.java b/src/main/java/spoon/support/reflect/declaration/CtFieldImpl.java index bdbbe154e73..9c8e9303717 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtFieldImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtFieldImpl.java @@ -29,6 +29,7 @@ import spoon.reflect.reference.CtFieldReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtVisitor; +import spoon.support.UnsettableProperty; import java.util.EnumSet; import java.util.Set; @@ -168,6 +169,7 @@ public CtExpression getAssignment() { } @Override + @UnsettableProperty public > C setAssignment(CtExpression assignment) { setDefaultExpression(assignment); return (C) this; diff --git a/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java b/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java index dc43024219f..d390b14b3f1 100644 --- a/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java +++ b/src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java @@ -44,6 +44,7 @@ import spoon.reflect.visitor.EarlyTerminatingScanner; import spoon.reflect.visitor.Query; import spoon.reflect.visitor.filter.ReferenceTypeFilter; +import spoon.support.UnsettableProperty; import spoon.support.compiler.SnippetCompilationHelper; import spoon.support.SpoonClassNotFoundException; import spoon.support.util.QualifiedNameBasedSortedSet; @@ -846,6 +847,7 @@ public > C setMethods(Set> methods) { } @Override + @UnsettableProperty public > C setSuperclass(CtTypeReference superClass) { // overridden in subclasses. return (C) this; diff --git a/src/test/java/spoon/test/api/APITest.java b/src/test/java/spoon/test/api/APITest.java index 1bd88d9decc..0326d5086d6 100644 --- a/src/test/java/spoon/test/api/APITest.java +++ b/src/test/java/spoon/test/api/APITest.java @@ -23,6 +23,7 @@ import spoon.reflect.visitor.filter.AbstractFilter; import spoon.reflect.visitor.filter.TypeFilter; import spoon.support.JavaOutputProcessor; +import spoon.support.UnsettableProperty; import spoon.support.reflect.declaration.CtElementImpl; import spoon.template.Local; import spoon.template.TemplateMatcher; @@ -298,8 +299,13 @@ public SetterMethodWithoutCollectionsFilter(Factory factory) { public boolean matches(CtMethod element) { boolean isSetter = isSetterMethod(element); boolean isNotSubType = !isSubTypeOfCollection(element); + boolean doesNotHaveUnsettableAnnotation = doesNotHaveUnsettableAnnotation(element); boolean superMatch = super.matches(element); - return isSetter && isNotSubType && superMatch; + return isSetter && doesNotHaveUnsettableAnnotation && isNotSubType && superMatch; + } + + private boolean doesNotHaveUnsettableAnnotation(CtMethod element) { + return (element.getAnnotation(UnsettableProperty.class) == null); } private boolean isSubTypeOfCollection(CtMethod element) { @@ -365,11 +371,13 @@ public void accept(CtVisitor visitor) { final List> setters = Query.getElements(launcher.getFactory(), new SetterMethodWithoutCollectionsFilter(launcher.getFactory())); for (CtStatement statement : setters.stream().map((Function, CtStatement>) ctMethod -> ctMethod.getBody().getStatement(0)).collect(Collectors.toList())) { + // First statement should be a condition to protect the setter of the parent. assertTrue("Check the method " + statement.getParent(CtMethod.class).getSignature() + " in the declaring class " + statement.getParent(CtType.class).getQualifiedName(), statement instanceof CtIf); CtIf ifCondition = (CtIf) statement; TemplateMatcher matcher = new TemplateMatcher(templateRoot); - assertEquals(1, matcher.find(ifCondition).size()); + + assertEquals("Check the number of if in method " + statement.getParent(CtMethod.class).getSignature() + " in the declaring class " + statement.getParent(CtType.class).getQualifiedName(),1, matcher.find(ifCondition).size()); } } } From a30611bf0953da64315e61b77820e389dd93733a Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 14:08:19 +0100 Subject: [PATCH 08/29] Fix unused import --- .../java/spoon/support/reflect/code/CtLocalVariableImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java b/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java index 0802ae2435f..aaadbf53390 100644 --- a/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java +++ b/src/main/java/spoon/support/reflect/code/CtLocalVariableImpl.java @@ -27,7 +27,6 @@ import spoon.reflect.reference.CtLocalVariableReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtVisitor; -import spoon.support.DerivedProperty; import spoon.support.UnsettableProperty; import spoon.support.reflect.declaration.CtElementImpl; From 2c7b4d38f7eaadaca64f2beff99368fd3671996d Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 16:14:54 +0100 Subject: [PATCH 09/29] Proposal to fix #980: a different scanner is used when autoimports is set to true or when it is set to false. When set to false, the scanner will detect name conflict and automatically import necessary types. --- .../visitor/DefaultJavaPrettyPrinter.java | 19 ++--- .../reflect/visitor/ImportScannerImpl.java | 11 ++- .../ImportScannerWithoutAllImports.java | 70 +++++++++++++++++++ .../visitor/printer/ElementPrinterHelper.java | 6 +- .../support/compiler/jdt/ParentExiter.java | 14 ---- .../AccessFullyQualifiedFieldTest.java | 15 +++- .../test/variable/testclasses/Burritos.java | 8 --- .../testclasses/BurritosStaticMethod.java | 16 +++++ 8 files changed, 117 insertions(+), 42 deletions(-) create mode 100644 src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java create mode 100644 src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index aba4b419d65..10ca9e88885 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -146,7 +146,7 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter { /** * Handle imports of classes. */ - private ImportScanner importsContext = new ImportScannerImpl(); + private ImportScanner importsContext; /** * Environment which Spoon is executed. @@ -175,6 +175,12 @@ public DefaultJavaPrettyPrinter(Environment env) { this.env = env; printer = new PrinterHelper(env); elementPrinterHelper = new ElementPrinterHelper(printer, this, env); + + if (env.isAutoImports()) { + this.importsContext = new ImportScannerImpl(); + } else { + this.importsContext = new ImportScannerWithoutAllImports(); + } } /** @@ -223,20 +229,15 @@ protected void exitCtExpression(CtExpression e) { * Make the imports for a given type. */ public Collection> computeImports(CtType type) { - if (env.isAutoImports()) { - context.currentTopLevel = type; - return importsContext.computeImports(context.currentTopLevel); - } - return Collections.emptyList(); + context.currentTopLevel = type; + return importsContext.computeImports(context.currentTopLevel); } /** * Make the imports for all elements. */ public void computeImports(CtElement element) { - if (env.isAutoImports()) { - importsContext.computeImports(element); - } + importsContext.computeImports(element); } /** diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 6f5319f56e5..04d3496e83f 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -53,9 +53,9 @@ public class ImportScannerImpl extends CtScanner implements ImportScanner { private static final Collection namesPresentInJavaLang9 = Arrays.asList( "ProcessHandle", "StackWalker", "StackFramePermission"); - private Map> imports = new TreeMap<>(); + protected Map> imports = new TreeMap<>(); //top declaring type of that import - private CtTypeReference targetType; + protected CtTypeReference targetType; private Map namesPresentInJavaLang = new HashMap<>(); @Override @@ -207,10 +207,9 @@ public boolean isImported(CtTypeReference ref) { /** * Gets imports in imports Map for the key simpleType given. * - * @param simpleType * @return Collection of {@link spoon.reflect.reference.CtTypeReference} */ - private Collection> getImports() { + protected Collection> getImports() { if (imports.isEmpty()) { return Collections.EMPTY_LIST; } @@ -235,7 +234,7 @@ private Collection> getImports() { /** * Adds a type to the imports. */ - private boolean addImport(CtTypeReference ref) { + protected boolean addImport(CtTypeReference ref) { if (imports.containsKey(ref.getSimpleName())) { return isImported(ref); } @@ -261,7 +260,7 @@ private boolean addImport(CtTypeReference ref) { return true; } - private boolean classNamePresentInJavaLang(CtTypeReference ref) { + protected boolean classNamePresentInJavaLang(CtTypeReference ref) { Boolean presentInJavaLang = namesPresentInJavaLang.get(ref.getSimpleName()); if (presentInJavaLang == null) { // The following procedure of determining if the handle is present in Java Lang or diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java b/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java new file mode 100644 index 00000000000..78b07e09c36 --- /dev/null +++ b/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java @@ -0,0 +1,70 @@ +package spoon.reflect.visitor; + +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.declaration.ParentNotInitializedException; +import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtTypeReference; +import java.util.HashSet; +import java.util.Set; + +/** + * A scanner dedicated to import only the necessary packages, @see spoon.test.variable.AccessFullyQualifiedTest + * + */ +public class ImportScannerWithoutAllImports extends ImportScannerImpl implements ImportScanner { + + private Set namedElements = new HashSet(); + + /** + * Test if the reference should be imported by looking if there is a name conflict + * @param ref + * @return true if the ref should be imported. + */ + private boolean shouldTypeBeImported(CtTypeReference ref) { + // we import the targetType by default to simplify and avoid conclict in inner classes + if (ref.equals(targetType)) { + return true; + } + + try { + CtElement parent = ref.getParent(); + + if (parent instanceof CtNamedElement) { + namedElements.add(((CtNamedElement) parent).getSimpleName()); + } + + while (!(parent instanceof CtPackage)) { + if (parent instanceof CtFieldReference) { + CtFieldReference parentType = (CtFieldReference)parent; + String qualifiedName = parentType.getQualifiedName(); + + String[] splittedName = qualifiedName.split("(\\.|#)"); + + for (String token : splittedName) { + if (namedElements.contains(token)) { + return true; + } + } + } + parent = parent.getParent(); + } + } catch (ParentNotInitializedException e) { + return false; + } + + return false; + } + + @Override + protected boolean addImport(CtTypeReference ref) { + boolean shouldTypeBeImported = this.shouldTypeBeImported(ref); + + if (shouldTypeBeImported) { + return super.addImport(ref); + } else { + return false; + } + } +} diff --git a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java index 73b7e22a576..836fffadc8f 100644 --- a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java @@ -262,10 +262,8 @@ public void writeHeader(List> types, Collection> im printer.write("package " + types.get(0).getPackage().getQualifiedName() + ";"); } printer.writeln().writeln().writeTabs(); - if (env.isAutoImports()) { - for (CtTypeReference ref : imports) { - printer.write("import " + ref.getQualifiedName() + ";").writeln().writeTabs(); - } + for (CtTypeReference ref : imports) { + printer.write("import " + ref.getQualifiedName() + ";").writeln().writeTabs(); } printer.writeln().writeTabs(); } diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index 9eb917ee9cd..1a2f5e40394 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -44,7 +44,6 @@ import spoon.reflect.reference.CtTypeParameterReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.CtInheritanceScanner; -import spoon.reflect.visitor.Filter; import java.util.ArrayList; import java.util.HashMap; @@ -180,19 +179,6 @@ public void scanCtVariable(CtVariable v) { substituteAnnotation((CtTypedElement) v); return; } else if (child instanceof CtExpression && hasChildEqualsToDefaultValue(v)) { - - // first trial to detect if part of absolute name of a type is used by a previously recorded element in the model - List allElements = this.jdtTreeBuilder.getFactory().getModel().getElements(new Filter() { - @Override - public boolean matches(CtElement element) { - - return (element instanceof CtVariable) && (child.toString().startsWith(((CtNamedElement) element).getSimpleName())); - } - }); - - if (!allElements.isEmpty()) { - this.jdtTreeBuilder.getFactory().getEnvironment().setAutoImports(true); - } v.setDefaultExpression((CtExpression) child); return; } diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 80046e97f56..8d8cde78d22 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -21,7 +21,7 @@ public void testCheckAssignmentContracts() throws Exception { public void testNoFQNWhenShadowedByField() throws Exception { // contract: no fully qualified name if top package is shadowed by a field variable Launcher spoon = new Launcher(); - spoon.setArgs(new String[]{"--with-imports"}); + //spoon.setArgs(new String[]{"--with-imports"}); spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosFielded.java"); String output = "target/spooned-" + this.getClass().getSimpleName()+"-Field/"; spoon.setSourceOutputDirectory(output); @@ -33,6 +33,7 @@ public void testNoFQNWhenShadowedByField() throws Exception { public void testNoFQNWhenShadowedByLocalVariable() throws Exception { // contract: no fully qualified name if top package is shadowed by a local variable Launcher spoon = new Launcher(); + //spoon.setArgs(new String[]{"--with-imports"}); spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/Burritos.java"); String output = "target/spooned-" + this.getClass().getSimpleName()+"-Local/"; spoon.setSourceOutputDirectory(output); @@ -40,4 +41,16 @@ public void testNoFQNWhenShadowedByLocalVariable() throws Exception { canBeBuilt(output, 7); } + @Test + public void testNoFQNWhenUsedInInnerClassAndShadowedByLocalVariable() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + Launcher spoon = new Launcher(); + //spoon.setArgs(new String[]{"--with-imports"}); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"-StaticMethod/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + } \ No newline at end of file diff --git a/src/test/java/spoon/test/variable/testclasses/Burritos.java b/src/test/java/spoon/test/variable/testclasses/Burritos.java index 73c92337aa6..953d44c40ed 100644 --- a/src/test/java/spoon/test/variable/testclasses/Burritos.java +++ b/src/test/java/spoon/test/variable/testclasses/Burritos.java @@ -4,16 +4,8 @@ import static spoon.Launcher.SPOONED_CLASSES; public class Burritos { - static void toto() {} - void foo() { Object spoon = null; Object x = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import - new Thread(new Runnable() { - @Override - public void run() { - Burritos.toto(); - } - }); } } diff --git a/src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java b/src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java new file mode 100644 index 00000000000..d0e4ae7a172 --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/BurritosStaticMethod.java @@ -0,0 +1,16 @@ +package spoon.test.variable.testclasses; + +public class BurritosStaticMethod { + static void toto() {} + + void foo() { + Object spoon = null; + + new Thread(new Runnable() { + @Override + public void run() { + BurritosStaticMethod.toto(); + } + }); + } +} From 56142971d79ec8b643e95269197c2b9207f1daa0 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 17:15:37 +0100 Subject: [PATCH 10/29] Two more tests added to check if #980 is really fixed. Works fine. --- .../AccessFullyQualifiedFieldTest.java | 26 +++++++++++++++++++ .../testclasses/BurritosWithLoop.java | 12 +++++++++ .../testclasses/BurritosWithTryCatch.java | 14 ++++++++++ 3 files changed, 52 insertions(+) create mode 100644 src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java create mode 100644 src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 8d8cde78d22..8714f397076 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -4,9 +4,11 @@ import spoon.Launcher; import spoon.reflect.factory.Factory; import spoon.test.main.MainTest; +import spoon.test.variable.testclasses.BurritosWithLoop; import spoon.test.variable.testclasses.Tacos; import static spoon.testing.utils.ModelUtils.build; +import static spoon.testing.utils.ModelUtils.buildClass; import static spoon.testing.utils.ModelUtils.canBeBuilt; public class AccessFullyQualifiedFieldTest { @@ -53,4 +55,28 @@ public void testNoFQNWhenUsedInInnerClassAndShadowedByLocalVariable() throws Exc canBeBuilt(output, 7); } + @Test + public void testNoFQNWhenUsedInTryCatch() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + Launcher spoon = new Launcher(); + //spoon.setArgs(new String[]{"--with-imports"}); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"-TryCatch/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + + @Test + public void testNoFQNWhenUsedInLoop() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + Launcher spoon = new Launcher(); + //spoon.setArgs(new String[]{"--with-imports"}); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Loop/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + } \ No newline at end of file diff --git a/src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java b/src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java new file mode 100644 index 00000000000..8ff1383172a --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java @@ -0,0 +1,12 @@ +package spoon.test.variable.testclasses; + + +import static spoon.Launcher.SPOONED_CLASSES; + +public class BurritosWithLoop { + void foo() { + for (int spoon = 0; spoon < 10; spoon++) { + Object xx = SPOONED_CLASSES; + } + } +} diff --git a/src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java b/src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java new file mode 100644 index 00000000000..5c6c55eb07d --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java @@ -0,0 +1,14 @@ +package spoon.test.variable.testclasses; + + +import static spoon.Launcher.SPOONED_CLASSES; + +public class BurritosWithTryCatch { + void foo() { + try { + + } catch (Exception spoon) { + Object xx = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + } + } +} From e7c9b0dc34042d3b6b1b8af7d3bfab6b8deae820 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 6 Dec 2016 17:27:43 +0100 Subject: [PATCH 11/29] Static import are not supported yet. Test case to show it. --- .../AccessFullyQualifiedFieldTest.java | 12 +++++++ .../variable/testclasses/MultiBurritos.java | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 src/test/java/spoon/test/variable/testclasses/MultiBurritos.java diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 10fe463f826..ee811d21926 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -77,4 +77,16 @@ public void testNoFQNWhenUsedInLoop() throws Exception { canBeBuilt(output, 7); } + @Test + public void testStaticImportShouldBeDone() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + Launcher spoon = new Launcher(); + spoon.setArgs(new String[]{"--with-imports"}); + spoon.addInputResource("src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"); + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Multi/"; + spoon.setSourceOutputDirectory(output); + spoon.run(); + canBeBuilt(output, 7); + } + } diff --git a/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java new file mode 100644 index 00000000000..c699ef5d529 --- /dev/null +++ b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java @@ -0,0 +1,31 @@ +package spoon.test.variable.testclasses; + + +import com.sun.org.apache.xpath.internal.operations.Mult; + +import java.util.List; + +import static spoon.Launcher.SPOONED_CLASSES; + +public class MultiBurritos { + static Object spoon = "bla"; + List Launcher; + + static void toto() {} + + void foo() { + Object spoon = null; + Object x = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import + Launcher.isEmpty(); + } + + void bidule() { + new Thread(new Runnable() { + @Override + public void run() { + MultiBurritos.toto(); + MultiBurritos.spoon = "truc"; + } + }); + } +} From 3b0a416890fd588b7adfd172178d020e727e886e Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 7 Dec 2016 12:33:43 +0100 Subject: [PATCH 12/29] Fix locally checkstyle for ParentExiter --- .../support/compiler/jdt/ParentExiter.java | 61 ++++++++++++++++++- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java index 05615c8fdd6..11dc3a50b5a 100644 --- a/src/main/java/spoon/support/compiler/jdt/ParentExiter.java +++ b/src/main/java/spoon/support/compiler/jdt/ParentExiter.java @@ -37,8 +37,65 @@ import org.eclipse.jdt.internal.compiler.ast.TypeReference; import org.eclipse.jdt.internal.compiler.ast.UnionTypeReference; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; -import spoon.reflect.code.*; -import spoon.reflect.declaration.*; +import spoon.reflect.code.BinaryOperatorKind; +import spoon.reflect.code.CtArrayAccess; +import spoon.reflect.code.CtArrayRead; +import spoon.reflect.code.CtArrayWrite; +import spoon.reflect.code.CtAssert; +import spoon.reflect.code.CtAssignment; +import spoon.reflect.code.CtBinaryOperator; +import spoon.reflect.code.CtBlock; +import spoon.reflect.code.CtCase; +import spoon.reflect.code.CtCatch; +import spoon.reflect.code.CtCatchVariable; +import spoon.reflect.code.CtConditional; +import spoon.reflect.code.CtConstructorCall; +import spoon.reflect.code.CtDo; +import spoon.reflect.code.CtExecutableReferenceExpression; +import spoon.reflect.code.CtExpression; +import spoon.reflect.code.CtFor; +import spoon.reflect.code.CtForEach; +import spoon.reflect.code.CtIf; +import spoon.reflect.code.CtLiteral; +import spoon.reflect.code.CtInvocation; +import spoon.reflect.code.CtLambda; +import spoon.reflect.code.CtLocalVariable; +import spoon.reflect.code.CtLoop; +import spoon.reflect.code.CtNewArray; +import spoon.reflect.code.CtNewClass; +import spoon.reflect.code.CtReturn; +import spoon.reflect.code.CtStatement; +import spoon.reflect.code.CtSuperAccess; +import spoon.reflect.code.CtSwitch; +import spoon.reflect.code.CtSynchronized; +import spoon.reflect.code.CtTargetedExpression; +import spoon.reflect.code.CtThisAccess; +import spoon.reflect.code.CtThrow; +import spoon.reflect.code.CtTry; +import spoon.reflect.code.CtTryWithResource; +import spoon.reflect.code.CtTypeAccess; +import spoon.reflect.code.CtUnaryOperator; +import spoon.reflect.code.CtWhile; +import spoon.reflect.declaration.CtAnnotatedElementType; +import spoon.reflect.declaration.CtAnnotation; +import spoon.reflect.declaration.CtAnnotationMethod; +import spoon.reflect.declaration.CtAnonymousExecutable; +import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtConstructor; +import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtEnum; +import spoon.reflect.declaration.CtEnumValue; +import spoon.reflect.declaration.CtNamedElement; +import spoon.reflect.declaration.CtExecutable; +import spoon.reflect.declaration.CtField; +import spoon.reflect.declaration.CtFormalTypeDeclarer; +import spoon.reflect.declaration.CtMethod; +import spoon.reflect.declaration.CtPackage; +import spoon.reflect.declaration.CtParameter; +import spoon.reflect.declaration.CtType; +import spoon.reflect.declaration.CtTypeParameter; +import spoon.reflect.declaration.CtTypedElement; +import spoon.reflect.declaration.CtVariable; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtIntersectionTypeReference; import spoon.reflect.reference.CtTypeParameterReference; From 0ee080f522423ad92c124cfea8b75d0edbc2a0ed Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 11:32:07 +0100 Subject: [PATCH 13/29] Create new tests to show issue #1027 --- .../AccessFullyQualifiedFieldTest.java | 33 +++++++++++++++++++ .../testclasses/ForStaticVariables.java | 4 +++ .../variable/testclasses/MultiBurritos.java | 7 ++-- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index b0842edb1d8..127df1d8c1f 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -105,4 +105,37 @@ public void testNoFQNWhenUsedInLoop() throws Exception { canBeBuilt(output, 7); } + @Test + public void testStaticImportWithAutoImport() throws Exception { + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Multi/"; + String pathResource = "src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"; + + Launcher spoon = new Launcher(); + spoon.setArgs(new String[]{"--with-imports"}); + spoon.addInputResource(pathResource); + spoon.setSourceOutputDirectory(output); + spoon.run(); + PrettyPrinter prettyPrinter = spoon.createPrettyPrinter(); + + CtType element = spoon.getFactory().Class().getAll().get(0); + List> toPrint = new ArrayList<>(); + toPrint.add(element); + + prettyPrinter.calculate(element.getPosition().getCompilationUnit(), toPrint); + String result = prettyPrinter.getResult(); + assertTrue("The result does not contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); + assertTrue("The result does not contain a static import for spoon.test.variable.testclasses.ForStaticVariables.foo", result.contains("import static spoon.test.variable.testclasses.ForStaticVariables.foo;")); + + canBeBuilt(output, 7); + } + + @Test + public void testNoFQNAndStaticImport() throws Exception { + // contract: no fully qualified name if top package is shadowed by a local variable + String output = "target/spooned-" + this.getClass().getSimpleName()+"-Multi/"; + String pathResource = "src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"; + String result = this.buildResourceAndReturnResult(pathResource, output); + canBeBuilt(output, 7); + } + } diff --git a/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java b/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java index 45b3b003a19..503848201fb 100644 --- a/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java +++ b/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java @@ -16,4 +16,8 @@ */ public class ForStaticVariables { public static String Map = "BLA"; + + public static void foo() { + } + } diff --git a/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java index c699ef5d529..dde4fa3a896 100644 --- a/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java +++ b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java @@ -1,11 +1,9 @@ package spoon.test.variable.testclasses; - -import com.sun.org.apache.xpath.internal.operations.Mult; - import java.util.List; import static spoon.Launcher.SPOONED_CLASSES; +import static spoon.test.variable.testclasses.ForStaticVariables.foo; public class MultiBurritos { static Object spoon = "bla"; @@ -13,7 +11,7 @@ public class MultiBurritos { static void toto() {} - void foo() { + void bar() { Object spoon = null; Object x = SPOONED_CLASSES; // cannot be written spoon.o, has to be with implicit visibility or static import Launcher.isEmpty(); @@ -25,6 +23,7 @@ void bidule() { public void run() { MultiBurritos.toto(); MultiBurritos.spoon = "truc"; + foo(); } }); } From 4ae8cf7c6c07c3ef1a8326a095b4b315e4f29432 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 11:33:43 +0100 Subject: [PATCH 14/29] Was renamed MinimalImportScanner in previous commit but not deleted in the merge... --- .../ImportScannerWithoutAllImports.java | 70 ------------------- 1 file changed, 70 deletions(-) delete mode 100644 src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java b/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java deleted file mode 100644 index 78b07e09c36..00000000000 --- a/src/main/java/spoon/reflect/visitor/ImportScannerWithoutAllImports.java +++ /dev/null @@ -1,70 +0,0 @@ -package spoon.reflect.visitor; - -import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtNamedElement; -import spoon.reflect.declaration.CtPackage; -import spoon.reflect.declaration.ParentNotInitializedException; -import spoon.reflect.reference.CtFieldReference; -import spoon.reflect.reference.CtTypeReference; -import java.util.HashSet; -import java.util.Set; - -/** - * A scanner dedicated to import only the necessary packages, @see spoon.test.variable.AccessFullyQualifiedTest - * - */ -public class ImportScannerWithoutAllImports extends ImportScannerImpl implements ImportScanner { - - private Set namedElements = new HashSet(); - - /** - * Test if the reference should be imported by looking if there is a name conflict - * @param ref - * @return true if the ref should be imported. - */ - private boolean shouldTypeBeImported(CtTypeReference ref) { - // we import the targetType by default to simplify and avoid conclict in inner classes - if (ref.equals(targetType)) { - return true; - } - - try { - CtElement parent = ref.getParent(); - - if (parent instanceof CtNamedElement) { - namedElements.add(((CtNamedElement) parent).getSimpleName()); - } - - while (!(parent instanceof CtPackage)) { - if (parent instanceof CtFieldReference) { - CtFieldReference parentType = (CtFieldReference)parent; - String qualifiedName = parentType.getQualifiedName(); - - String[] splittedName = qualifiedName.split("(\\.|#)"); - - for (String token : splittedName) { - if (namedElements.contains(token)) { - return true; - } - } - } - parent = parent.getParent(); - } - } catch (ParentNotInitializedException e) { - return false; - } - - return false; - } - - @Override - protected boolean addImport(CtTypeReference ref) { - boolean shouldTypeBeImported = this.shouldTypeBeImported(ref); - - if (shouldTypeBeImported) { - return super.addImport(ref); - } else { - return false; - } - } -} From 6561ee79c2141b8ed803d9d50ac9d8d9ad1b73db Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 14:46:34 +0100 Subject: [PATCH 15/29] Begin of the refactoring to import static fields and methods in readable mode. Also improve tests. Related to #1027 --- .../visitor/DefaultJavaPrettyPrinter.java | 16 +- .../spoon/reflect/visitor/ImportScanner.java | 12 +- .../reflect/visitor/ImportScannerImpl.java | 190 ++++++++++++++---- .../reflect/visitor/MinimalImportScanner.java | 4 +- .../visitor/printer/ElementPrinterHelper.java | 18 +- .../prettyprinter/QualifiedThisRefTest.java | 3 +- .../AccessFullyQualifiedFieldTest.java | 10 +- .../testclasses/ForStaticVariables.java | 4 + .../variable/testclasses/MultiBurritos.java | 1 + 9 files changed, 201 insertions(+), 57 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 99590b889e5..225ab909bef 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -143,7 +143,7 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter { public PrintingContext context = new PrintingContext(); /** - * Handle imports of classes. + * Handle classImports of classes. */ private ImportScanner importsContext; @@ -225,15 +225,15 @@ protected void exitCtExpression(CtExpression e) { } /** - * Make the imports for a given type. + * Make the classImports for a given type. */ - public Collection> computeImports(CtType type) { + public Collection computeImports(CtType type) { context.currentTopLevel = type; return importsContext.computeImports(context.currentTopLevel); } /** - * Make the imports for all elements. + * Make the classImports for all elements. */ public void computeImports(CtElement element) { if (env.isAutoImports()) { @@ -676,6 +676,10 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { printCtFieldAccess(fieldWrite); } + private boolean isNotStaticField(CtFieldReference ref) { + return !ref.isStatic(); + } + private void printCtFieldAccess(CtFieldAccess f) { enterCtExpression(f); try (Writable _context = context.modify()) { @@ -683,7 +687,7 @@ private void printCtFieldAccess(CtFieldAccess f) { _context.ignoreGenerics(true); } if (f.getTarget() != null) { - if (!isInitializeStaticFinalField(f.getTarget())) { + if (!isInitializeStaticFinalField(f.getTarget()) && isNotStaticField(f.getVariable())) { scan(f.getTarget()); if (!f.getTarget().isImplicit()) { printer.write("."); @@ -1715,7 +1719,7 @@ public void reset() { @Override public void calculate(CompilationUnit sourceCompilationUnit, List> types) { this.sourceCompilationUnit = sourceCompilationUnit; - Set> imports = new HashSet<>(); + Set imports = new HashSet<>(); for (CtType t : types) { imports.addAll(computeImports(t)); } diff --git a/src/main/java/spoon/reflect/visitor/ImportScanner.java b/src/main/java/spoon/reflect/visitor/ImportScanner.java index 482d1701414..67fa3393fab 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/ImportScanner.java @@ -18,24 +18,24 @@ import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtType; -import spoon.reflect.reference.CtTypeReference; +import spoon.reflect.reference.CtReference; import java.util.Collection; /** - * Used to compute the imports required to write readable code with no fully qualified names. + * Used to compute the classImports required to write readable code with no fully qualified names. */ public interface ImportScanner { /** * Computes import of a {@link spoon.reflect.declaration.CtType} * (represent a class). * - * @return Imports computes by Spoon, not original imports. + * @return Imports computes by Spoon, it can be CtTypeReference (for classes), but also CtFieldReference (static field) or CtExecutableReference (static methods) */ - Collection> computeImports(CtType simpleType); + Collection computeImports(CtType simpleType); /** - * Computes imports for all elements. + * Computes classImports for all elements. */ @Deprecated void computeImports(CtElement element); @@ -43,5 +43,5 @@ public interface ImportScanner { /** * Checks if the type is already imported. */ - boolean isImported(CtTypeReference ref); + boolean isImported(CtReference ref); } diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 04d3496e83f..d2ec3476484 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -24,6 +24,8 @@ import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtEnum; +import spoon.reflect.declaration.CtExecutable; +import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.CtTypeMember; @@ -31,6 +33,7 @@ import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; import spoon.reflect.reference.CtPackageReference; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import java.lang.annotation.Annotation; @@ -44,7 +47,7 @@ import java.util.TreeMap; /** - * A scanner that calculates the imports for a given model. + * A scanner that calculates the classImports for a given model. */ public class ImportScannerImpl extends CtScanner implements ImportScanner { @@ -53,7 +56,9 @@ public class ImportScannerImpl extends CtScanner implements ImportScanner { private static final Collection namesPresentInJavaLang9 = Arrays.asList( "ProcessHandle", "StackWalker", "StackFramePermission"); - protected Map> imports = new TreeMap<>(); + protected Map> classImports = new TreeMap<>(); + protected Map> fieldImports = new TreeMap<>(); + protected Map> methodImports = new TreeMap<>(); //top declaring type of that import protected CtTypeReference targetType; private Map namesPresentInJavaLang = new HashMap<>(); @@ -83,7 +88,11 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { @Override public void visitCtFieldReference(CtFieldReference reference) { enter(reference); - scan(reference.getDeclaringType()); + if (reference.isStatic()) { + addFieldImport(reference); + } else { + scan(reference.getDeclaringType()); + } exit(reference); } @@ -91,7 +100,9 @@ public void visitCtFieldReference(CtFieldReference reference) { public void visitCtExecutableReference( CtExecutableReference reference) { enter(reference); - if (reference.isConstructor()) { + if (reference.isStatic()) { + addMethodImport(reference); + } else if (reference.isConstructor()) { scan(reference.getDeclaringType()); } scan(reference.getActualTypeArguments()); @@ -113,9 +124,9 @@ public void visitCtInvocation(CtInvocation invocation) { public void visitCtTypeReference(CtTypeReference reference) { if (!(reference instanceof CtArrayTypeReference)) { if (reference.getDeclaringType() == null) { - addImport(reference); + addClassImport(reference); } else { - addImport(reference.getAccessType()); + addClassImport(reference.getAccessType()); } } super.visitCtTypeReference(reference); @@ -132,36 +143,36 @@ public void scan(CtElement element) { @Override public void visitCtAnnotationType( CtAnnotationType annotationType) { - addImport(annotationType.getReference()); + addClassImport(annotationType.getReference()); super.visitCtAnnotationType(annotationType); } @Override public > void visitCtEnum(CtEnum ctEnum) { - addImport(ctEnum.getReference()); + addClassImport(ctEnum.getReference()); super.visitCtEnum(ctEnum); } @Override public void visitCtInterface(CtInterface intrface) { - addImport(intrface.getReference()); + addClassImport(intrface.getReference()); for (CtTypeMember t : intrface.getTypeMembers()) { if (!(t instanceof CtType)) { continue; } - addImport(((CtType) t).getReference()); + addClassImport(((CtType) t).getReference()); } super.visitCtInterface(intrface); } @Override public void visitCtClass(CtClass ctClass) { - addImport(ctClass.getReference()); + addClassImport(ctClass.getReference()); for (CtTypeMember t : ctClass.getTypeMembers()) { if (!(t instanceof CtType)) { continue; } - addImport(((CtType) t).getReference()); + addClassImport(((CtType) t).getReference()); } super.visitCtClass(ctClass); } @@ -169,24 +180,29 @@ public void visitCtClass(CtClass ctClass) { @Override public void visitCtCatchVariable(CtCatchVariable catchVariable) { for (CtTypeReference type : catchVariable.getMultiTypes()) { - addImport(type); + addClassImport(type); } super.visitCtCatchVariable(catchVariable); } @Override - public Collection> computeImports(CtType simpleType) { - imports.clear(); + public Collection computeImports(CtType simpleType) { + classImports.clear(); //look for top declaring type of that simpleType targetType = simpleType.getReference().getTopLevelType(); - addImport(simpleType.getReference()); + addClassImport(simpleType.getReference()); scan(simpleType); - return getImports(); + + Collection listallImports = new ArrayList<>(); + listallImports.addAll(getClassImports()); + listallImports.addAll(getFieldImports()); + listallImports.addAll(getMethodImports()); + return listallImports; } @Override public void computeImports(CtElement element) { - imports.clear(); + classImports.clear(); //look for top declaring type of that element CtType type = element.getParent(CtType.class); targetType = type == null ? null : type.getReference().getTopLevelType(); @@ -194,28 +210,30 @@ public void computeImports(CtElement element) { } @Override - public boolean isImported(CtTypeReference ref) { - if (!(ref.isImplicit()) && imports.containsKey(ref.getSimpleName())) { - CtTypeReference exist = imports.get(ref.getSimpleName()); - if (exist.getQualifiedName().equals(ref.getQualifiedName())) { - return true; - } + public boolean isImported(CtReference ref) { + if (ref instanceof CtFieldReference) { + return isImportedInFieldImports((CtFieldReference)ref); + } else if (ref instanceof CtExecutableReference) { + return isImportedInMethodImports((CtExecutableReference)ref); + } else if (ref instanceof CtTypeReference) { + return isImportedInClassImports((CtTypeReference)ref); + } else { + return false; } - return false; } /** - * Gets imports in imports Map for the key simpleType given. + * Gets classImports in classImports Map for the key simpleType given. * * @return Collection of {@link spoon.reflect.reference.CtTypeReference} */ - protected Collection> getImports() { - if (imports.isEmpty()) { + protected Collection> getClassImports() { + if (classImports.isEmpty()) { return Collections.EMPTY_LIST; } CtPackageReference pack = targetType.getPackage(); List> refs = new ArrayList<>(); - for (CtTypeReference ref : imports.values()) { + for (CtTypeReference ref : classImports.values()) { // ignore non-top-level type if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { // ignore java.lang package @@ -232,11 +250,65 @@ protected Collection> getImports() { } /** - * Adds a type to the imports. + * Gets methodImports + * + * @return Collection of {@link spoon.reflect.reference.CtExecutableReference} */ - protected boolean addImport(CtTypeReference ref) { - if (imports.containsKey(ref.getSimpleName())) { - return isImported(ref); + protected Collection> getMethodImports() { + if (methodImports.isEmpty()) { + return Collections.EMPTY_LIST; + } + CtPackageReference pack = targetType.getPackage(); + List> refs = new ArrayList<>(); + for (CtExecutableReference ref : methodImports.values()) { + // ignore non-top-level type + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (!ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + refs.add(ref); + } + } + } + } + return Collections.unmodifiableList(refs); + } + + /** + * Gets fieldImports + * + * @return Collection of {@link spoon.reflect.reference.CtFieldReference} + */ + protected Collection> getFieldImports() { + if (fieldImports.isEmpty()) { + return Collections.EMPTY_LIST; + } + CtPackageReference pack = targetType.getPackage(); + List> refs = new ArrayList<>(); + for (CtFieldReference ref : fieldImports.values()) { + // ignore non-top-level type + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (!ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + refs.add(ref); + } + } + } + } + return Collections.unmodifiableList(refs); + } + + /** + * Adds a type to the classImports. + */ + protected boolean addClassImport(CtTypeReference ref) { + if (classImports.containsKey(ref.getSimpleName())) { + return isImportedInClassImports(ref); } // don't import unnamed package elements if (ref.getPackage() == null || ref.getPackage().isUnnamedPackage()) { @@ -255,11 +327,59 @@ protected boolean addImport(CtTypeReference ref) { return false; } //note: we must add the type refs from the same package too, to assure that isImported(typeRef) returns true for them - //these type refs are removed in #getImports() - imports.put(ref.getSimpleName(), ref); + //these type refs are removed in #getClassImports() + classImports.put(ref.getSimpleName(), ref); + return true; + } + + private boolean isImportedInClassImports(CtTypeReference ref) { + if (!(ref.isImplicit()) && classImports.containsKey(ref.getSimpleName())) { + CtTypeReference exist = classImports.get(ref.getSimpleName()); + if (exist.getQualifiedName().equals(ref.getQualifiedName())) { + return true; + } + } + return false; + } + + protected boolean addMethodImport(CtExecutableReference ref) { + if (this.methodImports.containsKey(ref.getSimpleName())) { + return isImportedInMethodImports(ref); + } + + methodImports.put(ref.getSimpleName(), ref); + return true; + } + + private boolean isImportedInMethodImports(CtExecutableReference ref) { + if (!(ref.isImplicit()) && methodImports.containsKey(ref.getSimpleName())) { + CtExecutableReference exist = methodImports.get(ref.getSimpleName()); + if (exist.getSignature().equals(ref.getSignature())) { + return true; + } + } + return false; + } + + protected boolean addFieldImport(CtFieldReference ref) { + if (this.fieldImports.containsKey(ref.getSimpleName())) { + return isImportedInFieldImports(ref); + } + + fieldImports.put(ref.getSimpleName(), ref); return true; } + private boolean isImportedInFieldImports(CtFieldReference ref) { + if (!(ref.isImplicit()) && fieldImports.containsKey(ref.getSimpleName())) { + CtFieldReference exist = fieldImports.get(ref.getSimpleName()); + if (exist.getFieldDeclaration().equals(ref.getFieldDeclaration())) { + return true; + } + } + return false; + } + protected boolean classNamePresentInJavaLang(CtTypeReference ref) { Boolean presentInJavaLang = namesPresentInJavaLang.get(ref.getSimpleName()); if (presentInJavaLang == null) { diff --git a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java index 66320d717a6..ac3f971c2cd 100644 --- a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java @@ -93,11 +93,11 @@ private boolean shouldTypeBeImported(CtTypeReference ref) { } @Override - protected boolean addImport(CtTypeReference ref) { + protected boolean addClassImport(CtTypeReference ref) { boolean shouldTypeBeImported = this.shouldTypeBeImported(ref); if (shouldTypeBeImported) { - return super.addImport(ref); + return super.addClassImport(ref); } else { return false; } diff --git a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java index 836fffadc8f..fd21e04ebdd 100644 --- a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java @@ -44,7 +44,9 @@ import spoon.reflect.declaration.ModifierKind; import spoon.reflect.factory.Factory; import spoon.reflect.reference.CtActualTypeContainer; +import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.PrintingContext.Writable; @@ -251,7 +253,7 @@ public void writeActualTypeArguments(CtActualTypeContainer ctGenericElementRefer /** * Write the compilation unit header. */ - public void writeHeader(List> types, Collection> imports) { + public void writeHeader(List> types, Collection imports) { if (!types.isEmpty()) { for (CtType ctType : types) { writeComment(ctType, CommentOffset.TOP_FILE); @@ -262,8 +264,18 @@ public void writeHeader(List> types, Collection> im printer.write("package " + types.get(0).getPackage().getQualifiedName() + ";"); } printer.writeln().writeln().writeTabs(); - for (CtTypeReference ref : imports) { - printer.write("import " + ref.getQualifiedName() + ";").writeln().writeTabs(); + for (CtReference ref : imports) { + if (ref instanceof CtTypeReference) { + CtTypeReference typeRef = (CtTypeReference)ref; + printer.write("import " + typeRef.getQualifiedName() + ";").writeln().writeTabs(); + } else if (ref instanceof CtExecutableReference) { + CtExecutableReference execRef = (CtExecutableReference)ref; + printer.write("import static " + execRef.getDeclaringType().getQualifiedName()+"."+execRef.getSimpleName()+";").writeln().writeTabs(); + } else if (ref instanceof CtFieldReference) { + CtFieldReference fieldRef = (CtFieldReference)ref; + printer.write("import static " + fieldRef.getDeclaringType().getQualifiedName()+"."+fieldRef.getSimpleName()+";").writeln().writeTabs(); + } + } printer.writeln().writeTabs(); } diff --git a/src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java b/src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java index 6094796af3d..c8930690716 100644 --- a/src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java +++ b/src/test/java/spoon/test/prettyprinter/QualifiedThisRefTest.java @@ -13,6 +13,7 @@ import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.DefaultJavaPrettyPrinter; import spoon.reflect.visitor.filter.TypeFilter; @@ -50,7 +51,7 @@ public void setup() throws Exception { public void testQualifiedThisRef() { DefaultJavaPrettyPrinter printer = new DefaultJavaPrettyPrinter(factory.getEnvironment()); CtType ctClass = factory.Type().get(QualifiedThisRef.class); - Collection> imports = printer.computeImports(ctClass); + Collection imports = printer.computeImports(ctClass); final List> ctTypes = new ArrayList<>(); ctTypes.add(ctClass); printer.getElementPrinterHelper().writeHeader(ctTypes, imports); diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 127df1d8c1f..7cfe530cbf3 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -32,7 +32,7 @@ public void testCheckAssignmentContracts() throws Exception { private String buildResourceAndReturnResult(String pathResource, String output) { Launcher spoon = new Launcher(); - //spoon.setArgs(new String[]{"--with-imports"}); + //spoon.setArgs(new String[]{"--with-classImports"}); spoon.addInputResource(pathResource); spoon.setSourceOutputDirectory(output); spoon.run(); @@ -111,7 +111,7 @@ public void testStaticImportWithAutoImport() throws Exception { String pathResource = "src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"; Launcher spoon = new Launcher(); - spoon.setArgs(new String[]{"--with-imports"}); + spoon.setArgs(new String[]{"--with-classImports"}); spoon.addInputResource(pathResource); spoon.setSourceOutputDirectory(output); spoon.run(); @@ -123,8 +123,10 @@ public void testStaticImportWithAutoImport() throws Exception { prettyPrinter.calculate(element.getPosition().getCompilationUnit(), toPrint); String result = prettyPrinter.getResult(); - assertTrue("The result does not contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); - assertTrue("The result does not contain a static import for spoon.test.variable.testclasses.ForStaticVariables.foo", result.contains("import static spoon.test.variable.testclasses.ForStaticVariables.foo;")); + assertTrue("The result should contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); + assertTrue("The variable x should be assigned with only SPOONED_CLASSES", result.contains("Object x = SPOONED_CLASSES;")); + assertTrue("The result should not contain a static import for spoon.test.variable.testclasses.ForStaticVariables.foo as it is in the same package", !result.contains("import static spoon.test.variable.testclasses.ForStaticVariables.foo;")); + canBeBuilt(output, 7); } diff --git a/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java b/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java index 503848201fb..3357eb08d12 100644 --- a/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java +++ b/src/test/java/spoon/test/variable/testclasses/ForStaticVariables.java @@ -16,8 +16,12 @@ */ public class ForStaticVariables { public static String Map = "BLA"; + public String var; public static void foo() { } + public String bla() { + return "truc"; + } } diff --git a/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java index dde4fa3a896..6b2fa3dcf72 100644 --- a/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java +++ b/src/test/java/spoon/test/variable/testclasses/MultiBurritos.java @@ -24,6 +24,7 @@ public void run() { MultiBurritos.toto(); MultiBurritos.spoon = "truc"; foo(); + } }); } From cc962a02dcaaab5af2336b0e3ebe037cd139bea1 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 14:50:26 +0100 Subject: [PATCH 16/29] Fix compilation issues with the change of signature of computeImports --- .../spoon/test/imports/ImportScannerTest.java | 4 ++-- src/test/java/spoon/test/imports/ImportTest.java | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/test/java/spoon/test/imports/ImportScannerTest.java b/src/test/java/spoon/test/imports/ImportScannerTest.java index 24837f0fe1e..b128281160b 100644 --- a/src/test/java/spoon/test/imports/ImportScannerTest.java +++ b/src/test/java/spoon/test/imports/ImportScannerTest.java @@ -7,6 +7,7 @@ import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.ImportScanner; import spoon.reflect.visitor.ImportScannerImpl; @@ -36,8 +37,7 @@ public void testComputeImportsInClass() throws Exception { CtType theClass = aFactory.Type().get(qualifiedName); ImportScanner importContext = new ImportScannerImpl(); - Collection> imports = importContext - .computeImports(theClass); + Collection imports = importContext.computeImports(theClass); assertEquals(2, imports.size()); } diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index e8de8980de6..77a2d438068 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -15,6 +15,7 @@ import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtType; import spoon.reflect.factory.Factory; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.ImportScanner; import spoon.reflect.visitor.ImportScannerImpl; @@ -197,13 +198,13 @@ public void testSpoonWithImports() throws Exception { final CtClass classWithInvocation = launcher.getFactory().Class().get(ClassWithInvocation.class); final ImportScanner importScanner = new ImportScannerImpl(); - final Collection> imports = importScanner.computeImports(aClass); + final Collection imports = importScanner.computeImports(aClass); assertEquals(2, imports.size()); - final Collection> imports1 = importScanner.computeImports(anotherClass); + final Collection imports1 = importScanner.computeImports(anotherClass); assertEquals(1, imports1.size()); //check that printer did not used the package protected class like "SuperClass.InnerClassProtected" assertTrue(anotherClass.toString().indexOf("InnerClass extends ChildClass.InnerClassProtected")>0); - final Collection> imports2 = importScanner.computeImports(classWithInvocation); + final Collection imports2 = importScanner.computeImports(classWithInvocation); assertEquals("Spoon ignores the arguments of CtInvocations", 1, imports2.size()); } @@ -278,7 +279,7 @@ public void testImportOfInvocationOfPrivateClass() throws Exception { "./src/test/java/spoon/test/imports/testclasses/Mole.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection> imports = importContext.computeImports(factory.Class().get(Mole.class)); + Collection imports = importContext.computeImports(factory.Class().get(Mole.class)); assertEquals(1, imports.size()); assertEquals("spoon.test.imports.testclasses.internal2.Chimichanga", imports.toArray()[0].toString()); @@ -292,13 +293,12 @@ public void testNotImportExecutableType() throws Exception { "./src/test/java/spoon/test/imports/testclasses/NotImportExecutableType.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection> imports = - importContext.computeImports(factory.Class().get(NotImportExecutableType.class)); + Collection imports = importContext.computeImports(factory.Class().get(NotImportExecutableType.class)); assertEquals(2, imports.size()); Set expectedImports = new HashSet<>( Arrays.asList("spoon.test.imports.testclasses.internal3.Foo", "java.io.File")); - Set actualImports = imports.stream().map(CtTypeReference::toString).collect(Collectors.toSet()); + Set actualImports = imports.stream().map(CtReference::toString).collect(Collectors.toSet()); assertEquals(expectedImports, actualImports); } @@ -309,7 +309,7 @@ public void testImportOfInvocationOfStaticMethod() throws Exception { "./src/test/java/spoon/test/imports/testclasses/Pozole.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection> imports = importContext.computeImports(factory.Class().get(Pozole.class)); + Collection imports = importContext.computeImports(factory.Class().get(Pozole.class)); assertEquals(1, imports.size()); assertEquals("spoon.test.imports.testclasses.internal2.Menudo", imports.toArray()[0].toString()); From 2d57b500c3ba13f9da8389c7cf6787450720f587 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 18:07:31 +0100 Subject: [PATCH 17/29] Fix lot of things to manage correctly readable mode with statics imports. --- .../visitor/DefaultJavaPrettyPrinter.java | 10 ++- .../reflect/visitor/ImportScannerImpl.java | 7 ++ .../reflect/visitor/MinimalImportScanner.java | 77 ++++++++++++++----- .../AccessFullyQualifiedFieldTest.java | 21 ++--- 4 files changed, 83 insertions(+), 32 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 225ab909bef..4d267c240c6 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -676,8 +676,12 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { printCtFieldAccess(fieldWrite); } - private boolean isNotStaticField(CtFieldReference ref) { - return !ref.isStatic(); + private boolean isStaticImportedField(CtFieldReference ref) { + if (!ref.isStatic()) { + return false; + } + + return importsContext.isImported(ref); } private void printCtFieldAccess(CtFieldAccess f) { @@ -687,7 +691,7 @@ private void printCtFieldAccess(CtFieldAccess f) { _context.ignoreGenerics(true); } if (f.getTarget() != null) { - if (!isInitializeStaticFinalField(f.getTarget()) && isNotStaticField(f.getVariable())) { + if (!isInitializeStaticFinalField(f.getTarget()) && !isStaticImportedField(f.getVariable())) { scan(f.getTarget()); if (!f.getTarget().isImplicit()) { printer.write("."); diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index d2ec3476484..66cb1361dd5 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -348,6 +348,7 @@ protected boolean addMethodImport(CtExecutableReference ref) { } methodImports.put(ref.getSimpleName(), ref); + addClassImport(ref.getDeclaringType()); return true; } @@ -377,6 +378,12 @@ private boolean isImportedInFieldImports(CtFieldReference ref) { return true; } } + CtTypeReference typeRef = ref.getDeclaringType(); + + if (typeRef != null) { + return isImportedInClassImports(typeRef); + } + return false; } diff --git a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java index ac3f971c2cd..a7908875014 100644 --- a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java @@ -17,10 +17,13 @@ package spoon.reflect.visitor; import spoon.reflect.declaration.CtElement; +import spoon.reflect.declaration.CtExecutable; import spoon.reflect.declaration.CtNamedElement; import spoon.reflect.declaration.CtPackage; import spoon.reflect.declaration.ParentNotInitializedException; +import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; +import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; import java.util.HashSet; import java.util.Set; @@ -38,50 +41,62 @@ public class MinimalImportScanner extends ImportScannerImpl implements ImportSca * @param ref * @return true if the ref should be imported. */ - private boolean shouldTypeBeImported(CtTypeReference ref) { + private boolean shouldTypeBeImported(CtReference ref) { // we import the targetType by default to simplify and avoid conclict in inner classes if (ref.equals(targetType)) { return true; } try { - CtElement parent = ref.getParent(); + CtElement parent; + if (ref instanceof CtTypeReference) { + parent = ref.getParent(); + } else { + parent = ref; + } if (parent instanceof CtNamedElement) { namedElements.add(((CtNamedElement) parent).getSimpleName()); } while (!(parent instanceof CtPackage)) { - if (parent instanceof CtFieldReference) { - CtFieldReference parentType = (CtFieldReference) parent; + if ((parent instanceof CtFieldReference) || (parent instanceof CtExecutableReference)) { + CtReference parentType = (CtReference) parent; Set qualifiedNameTokens = new HashSet<>(); qualifiedNameTokens.add(parentType.getSimpleName()); - CtTypeReference typeReference = parentType.getDeclaringType(); + CtTypeReference typeReference; + if (parent instanceof CtFieldReference) { + typeReference = ((CtFieldReference)parent).getDeclaringType(); + } else { + typeReference = ((CtExecutableReference)parent).getDeclaringType(); + } - qualifiedNameTokens.add(typeReference.getSimpleName()); + if (typeReference != null) { + qualifiedNameTokens.add(typeReference.getSimpleName()); - if (typeReference.getPackage() != null) { - CtPackage ctPackage = typeReference.getPackage().getDeclaration(); + if (typeReference.getPackage() != null) { + CtPackage ctPackage = typeReference.getPackage().getDeclaration(); - while (ctPackage != null) { - qualifiedNameTokens.add(ctPackage.getSimpleName()); + while (ctPackage != null) { + qualifiedNameTokens.add(ctPackage.getSimpleName()); - CtElement packParent = ctPackage.getParent(); - if (packParent.getParent() != null) { - ctPackage = (CtPackage) packParent; - } else { - ctPackage = null; + CtElement packParent = ctPackage.getParent(); + if (packParent.getParent() != null) { + ctPackage = (CtPackage) packParent; + } else { + ctPackage = null; + } } } - - for (String token : qualifiedNameTokens) { - if (namedElements.contains(token)) { - return true; - } + } + for (String token : qualifiedNameTokens) { + if (namedElements.contains(token)) { + return true; } } + } parent = parent.getParent(); } @@ -102,4 +117,26 @@ protected boolean addClassImport(CtTypeReference ref) { return false; } } + + @Override + protected boolean addFieldImport(CtFieldReference ref) { + boolean shouldTypeBeImported = this.shouldTypeBeImported(ref); + + if (shouldTypeBeImported) { + return super.addFieldImport(ref); + } else { + return false; + } + } + + @Override + protected boolean addMethodImport(CtExecutableReference ref) { + boolean shouldTypeBeImported = this.shouldTypeBeImported(ref); + + if (shouldTypeBeImported) { + return super.addMethodImport(ref); + } else { + return false; + } + } } diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 7cfe530cbf3..d02fe3a5910 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -54,8 +54,8 @@ public void testNoFQNWhenShadowedByField() throws Exception { String output = "target/spooned-" + this.getClass().getSimpleName()+"-Field/"; String result = this.buildResourceAndReturnResult(pathResource, output); - assertTrue("The java file contain import for Launcher", result.contains("import spoon.Launcher")); - assertTrue("The xx variable is attributed with Launcher.SPOONED_CLASSES", result.contains("xx = Launcher.SPOONED_CLASSES")); + assertTrue("The java file should contain import for Launcher", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); + assertTrue("The xx variable is attributed with Launcher.SPOONED_CLASSES", result.contains("xx = SPOONED_CLASSES")); canBeBuilt(output, 7); } @@ -66,8 +66,8 @@ public void testNoFQNWhenShadowedByLocalVariable() throws Exception { String pathResource = "src/test/java/spoon/test/variable/testclasses/Burritos.java"; String result = this.buildResourceAndReturnResult(pathResource, output); - assertTrue("The java file contain import for Launcher", result.contains("import spoon.Launcher")); - assertTrue("The x variable is attributed with Launcher.SPOONED_CLASSES", result.contains("x = Launcher.SPOONED_CLASSES")); + assertTrue("The java file should contain import for Launcher", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); + assertTrue("The x variable is attributed with Launcher.SPOONED_CLASSES", result.contains("x = SPOONED_CLASSES")); assertTrue("The java.util.Map is not imported", !result.contains("import java.util.Map")); assertTrue("The Map type use FQN", result.contains("java.util.Map uneMap")); assertTrue("The other variable use FQN too", result.contains("spoon.test.variable.testclasses.ForStaticVariables.Map")); @@ -91,7 +91,7 @@ public void testNoFQNWhenUsedInTryCatch() throws Exception { String output = "target/spooned-" + this.getClass().getSimpleName()+"-TryCatch/"; String pathResource = "src/test/java/spoon/test/variable/testclasses/BurritosWithTryCatch.java"; String result = this.buildResourceAndReturnResult(pathResource, output); - assertTrue("The xx variable is attributed with Launcher.SPOONED_CLASSES", result.contains("xx = Launcher.SPOONED_CLASSES")); + assertTrue("The xx variable should be attributed with SPOONED_CLASSES", result.contains("xx = SPOONED_CLASSES")); canBeBuilt(output, 7); } @@ -101,17 +101,17 @@ public void testNoFQNWhenUsedInLoop() throws Exception { String output = "target/spooned-" + this.getClass().getSimpleName()+"-Loop/"; String pathResource = "src/test/java/spoon/test/variable/testclasses/BurritosWithLoop.java"; String result = this.buildResourceAndReturnResult(pathResource, output); - assertTrue("The xx variable is attributed with Launcher.SPOONED_CLASSES", result.contains("xx = Launcher.SPOONED_CLASSES")); + assertTrue("The xx variable should be attributed with SPOONED_CLASSES", result.contains("xx = SPOONED_CLASSES")); canBeBuilt(output, 7); } @Test public void testStaticImportWithAutoImport() throws Exception { - String output = "target/spooned-" + this.getClass().getSimpleName()+"-Multi/"; + String output = "target/spooned-" + this.getClass().getSimpleName()+"-MultiAutoImport/"; String pathResource = "src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"; Launcher spoon = new Launcher(); - spoon.setArgs(new String[]{"--with-classImports"}); + spoon.setArgs(new String[]{"--with-imports"}); spoon.addInputResource(pathResource); spoon.setSourceOutputDirectory(output); spoon.run(); @@ -134,9 +134,12 @@ public void testStaticImportWithAutoImport() throws Exception { @Test public void testNoFQNAndStaticImport() throws Exception { // contract: no fully qualified name if top package is shadowed by a local variable - String output = "target/spooned-" + this.getClass().getSimpleName()+"-Multi/"; + String output = "target/spooned-" + this.getClass().getSimpleName()+"-MultiNoAutoImport/"; String pathResource = "src/test/java/spoon/test/variable/testclasses/MultiBurritos.java"; String result = this.buildResourceAndReturnResult(pathResource, output); + assertTrue("The result should contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); + assertTrue("The result should not contain a FQN call for foo (i.e. spoon.test.variable.testclasses.ForStaticVariables.foo())", !result.contains("spoon.test.variable.testclasses.ForStaticVariables.foo()")); + canBeBuilt(output, 7); } From 73d0ee9cee39685fcac7e0b71cf1252a979a27a7 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 18:48:51 +0100 Subject: [PATCH 18/29] Fix some tests and avoid redundancy in imports. #1027 --- .../reflect/visitor/ImportScannerImpl.java | 48 ++++++++++++++++++- .../java/spoon/test/SampleImportClass.java | 4 +- 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 66cb1361dd5..97a66b94a66 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -17,6 +17,7 @@ package spoon.reflect.visitor; import spoon.reflect.code.CtCatchVariable; +import spoon.reflect.code.CtFieldAccess; import spoon.reflect.code.CtFieldRead; import spoon.reflect.code.CtFieldWrite; import spoon.reflect.code.CtInvocation; @@ -29,6 +30,7 @@ import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.CtTypeMember; +import spoon.reflect.declaration.ParentNotInitializedException; import spoon.reflect.reference.CtArrayTypeReference; import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; @@ -326,6 +328,41 @@ protected boolean addClassImport(CtTypeReference ref) { //ref type is not visible in targetType we must not add import for it, java compiler would fail on that. return false; } + + // we want to be sure that we are not importing a class because a static field or method we already imported + // moreover we make exception for same package classes to avoid problems in FQN mode + try { + CtElement parent = ref.getParent(); + if (parent != null) { + parent = parent.getParent(); + if (parent != null) { + if ((parent instanceof CtFieldAccess) || (parent instanceof CtExecutable)) { + CtReference reference; + + if (parent instanceof CtFieldAccess) { + CtFieldAccess field = (CtFieldAccess)parent; + reference = field.getVariable(); + } else { + CtExecutable exec = (CtExecutable)parent; + reference = exec.getReference(); + } + + if (isImported(reference)) { + if (ref.getDeclaringType() != null) { + if (!ref.getDeclaringType().getPackage().equals(this.targetType.getPackage())) { + return false; + } + } else { + return false; + } + } + } + } + } + } catch (ParentNotInitializedException e) { + } + + //note: we must add the type refs from the same package too, to assure that isImported(typeRef) returns true for them //these type refs are removed in #getClassImports() classImports.put(ref.getSimpleName(), ref); @@ -347,8 +384,17 @@ protected boolean addMethodImport(CtExecutableReference ref) { return isImportedInMethodImports(ref); } + // if the whole class is imported: no need to import the method. + if (isImportedInClassImports(ref.getDeclaringType())) { + return false; + } + methodImports.put(ref.getSimpleName(), ref); - addClassImport(ref.getDeclaringType()); + + // if we are in the same package than target type, we also import class to avoid FQN in FQN mode. + if (ref.getDeclaringType().getPackage().equals(this.targetType.getPackage())) { + addClassImport(ref.getDeclaringType()); + } return true; } diff --git a/src/test/java/spoon/test/SampleImportClass.java b/src/test/java/spoon/test/SampleImportClass.java index 06e3e6e8311..eeabd1bac3c 100644 --- a/src/test/java/spoon/test/SampleImportClass.java +++ b/src/test/java/spoon/test/SampleImportClass.java @@ -1,7 +1,7 @@ package spoon.test; -import java.util.Collections; import java.util.List; +import static java.util.Collections.EMPTY_LIST; public class SampleImportClass { @@ -14,7 +14,7 @@ public SampleImportClass(int j) { this(j, 0); new Thread() { }; - List emptyList = Collections.EMPTY_LIST; + List emptyList = EMPTY_LIST; } public SampleImportClass(int j, int k) { From bdeef5516b5c29755f71a8906a00c621f97cf727 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Sat, 10 Dec 2016 20:09:24 +0100 Subject: [PATCH 19/29] WiP #1027 Fix some errors by doing more tests when adding imports --- .../reflect/visitor/ImportScannerImpl.java | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 97a66b94a66..b6fb5db4840 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -37,6 +37,7 @@ import spoon.reflect.reference.CtPackageReference; import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; +import spoon.support.SpoonClassNotFoundException; import java.lang.annotation.Annotation; import java.util.ArrayList; @@ -264,13 +265,15 @@ protected Collection> getMethodImports() { List> refs = new ArrayList<>(); for (CtExecutableReference ref : methodImports.values()) { // ignore non-top-level type - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (!ref.getDeclaringType().getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - refs.add(ref); + if (ref.getDeclaringType() != null) { + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (!ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + refs.add(ref); + } } } } @@ -385,15 +388,19 @@ protected boolean addMethodImport(CtExecutableReference ref) { } // if the whole class is imported: no need to import the method. - if (isImportedInClassImports(ref.getDeclaringType())) { + if (ref.getDeclaringType() != null && isImportedInClassImports(ref.getDeclaringType())) { return false; } methodImports.put(ref.getSimpleName(), ref); // if we are in the same package than target type, we also import class to avoid FQN in FQN mode. - if (ref.getDeclaringType().getPackage().equals(this.targetType.getPackage())) { - addClassImport(ref.getDeclaringType()); + if (ref.getDeclaringType() != null) { + if (ref.getDeclaringType().getPackage() != null) { + if (ref.getDeclaringType().getPackage().equals(this.targetType.getPackage())) { + addClassImport(ref.getDeclaringType()); + } + } } return true; } @@ -420,9 +427,15 @@ protected boolean addFieldImport(CtFieldReference ref) { private boolean isImportedInFieldImports(CtFieldReference ref) { if (!(ref.isImplicit()) && fieldImports.containsKey(ref.getSimpleName())) { CtFieldReference exist = fieldImports.get(ref.getSimpleName()); - if (exist.getFieldDeclaration().equals(ref.getFieldDeclaration())) { - return true; + try { + if (exist.getFieldDeclaration() != null && exist.getFieldDeclaration().equals(ref.getFieldDeclaration())) { + return true; + } + // in some rare cases we could not access to the field, then we do not import it. + } catch (SpoonClassNotFoundException notfound) { + return false; } + } CtTypeReference typeRef = ref.getDeclaringType(); From 99855a0b3a87f51ae810f18f94ec66f22d1ed6bc Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 12 Dec 2016 10:28:12 +0100 Subject: [PATCH 20/29] WiP #1027 Fix ambiguity between getXXImports and addXXImports in ImportScannerImpl. Fix one test but failed lot others. Still working. --- .../visitor/DefaultJavaPrettyPrinter.java | 5 +- .../reflect/visitor/ImportScannerImpl.java | 143 ++++++------------ .../test/fieldaccesses/FieldAccessTest.java | 2 +- .../DefaultPrettyPrinterTest.java | 4 +- .../testclasses/TypeIdentifierCollision.java | 5 +- 5 files changed, 59 insertions(+), 100 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 4d267c240c6..40ec3ecf923 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -691,7 +691,10 @@ private void printCtFieldAccess(CtFieldAccess f) { _context.ignoreGenerics(true); } if (f.getTarget() != null) { - if (!isInitializeStaticFinalField(f.getTarget()) && !isStaticImportedField(f.getVariable())) { + boolean isInitializeStaticFinalField = isInitializeStaticFinalField(f.getTarget()); + boolean isStaticImportedField = isStaticImportedField(f.getVariable()); + + if (!isInitializeStaticFinalField && !isStaticImportedField) { scan(f.getTarget()); if (!f.getTarget().isImplicit()) { printer.write("."); diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index b6fb5db4840..ae8e327af0d 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -92,7 +92,9 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { public void visitCtFieldReference(CtFieldReference reference) { enter(reference); if (reference.isStatic()) { - addFieldImport(reference); + if (!addFieldImport(reference)) { + scan(reference.getDeclaringType()); + } } else { scan(reference.getDeclaringType()); } @@ -197,9 +199,9 @@ public Collection computeImports(CtType simpleType) { scan(simpleType); Collection listallImports = new ArrayList<>(); - listallImports.addAll(getClassImports()); - listallImports.addAll(getFieldImports()); - listallImports.addAll(getMethodImports()); + listallImports.addAll(this.classImports.values()); + listallImports.addAll(this.fieldImports.values()); + listallImports.addAll(this.methodImports.values()); return listallImports; } @@ -225,89 +227,6 @@ public boolean isImported(CtReference ref) { } } - /** - * Gets classImports in classImports Map for the key simpleType given. - * - * @return Collection of {@link spoon.reflect.reference.CtTypeReference} - */ - protected Collection> getClassImports() { - if (classImports.isEmpty()) { - return Collections.EMPTY_LIST; - } - CtPackageReference pack = targetType.getPackage(); - List> refs = new ArrayList<>(); - for (CtTypeReference ref : classImports.values()) { - // ignore non-top-level type - if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (!ref.getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - refs.add(ref); - } - } - } - } - return Collections.unmodifiableList(refs); - } - - /** - * Gets methodImports - * - * @return Collection of {@link spoon.reflect.reference.CtExecutableReference} - */ - protected Collection> getMethodImports() { - if (methodImports.isEmpty()) { - return Collections.EMPTY_LIST; - } - CtPackageReference pack = targetType.getPackage(); - List> refs = new ArrayList<>(); - for (CtExecutableReference ref : methodImports.values()) { - // ignore non-top-level type - if (ref.getDeclaringType() != null) { - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (!ref.getDeclaringType().getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - refs.add(ref); - } - } - } - } - } - return Collections.unmodifiableList(refs); - } - - /** - * Gets fieldImports - * - * @return Collection of {@link spoon.reflect.reference.CtFieldReference} - */ - protected Collection> getFieldImports() { - if (fieldImports.isEmpty()) { - return Collections.EMPTY_LIST; - } - CtPackageReference pack = targetType.getPackage(); - List> refs = new ArrayList<>(); - for (CtFieldReference ref : fieldImports.values()) { - // ignore non-top-level type - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (!ref.getDeclaringType().getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - refs.add(ref); - } - } - } - } - return Collections.unmodifiableList(refs); - } - /** * Adds a type to the classImports. */ @@ -332,6 +251,23 @@ protected boolean addClassImport(CtTypeReference ref) { return false; } + if (ref.getDeclaringType() != null) { + CtPackageReference pack = targetType.getPackage(); + + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } + } + } + } else { + return false; + } + // we want to be sure that we are not importing a class because a static field or method we already imported // moreover we make exception for same package classes to avoid problems in FQN mode try { @@ -351,13 +287,7 @@ protected boolean addClassImport(CtTypeReference ref) { } if (isImported(reference)) { - if (ref.getDeclaringType() != null) { - if (!ref.getDeclaringType().getPackage().equals(this.targetType.getPackage())) { - return false; - } - } else { - return false; - } + return false; } } } @@ -392,6 +322,19 @@ protected boolean addMethodImport(CtExecutableReference ref) { return false; } + CtPackageReference pack = targetType.getPackage(); + + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } + } + } + methodImports.put(ref.getSimpleName(), ref); // if we are in the same package than target type, we also import class to avoid FQN in FQN mode. @@ -419,6 +362,18 @@ protected boolean addFieldImport(CtFieldReference ref) { if (this.fieldImports.containsKey(ref.getSimpleName())) { return isImportedInFieldImports(ref); } + CtPackageReference pack = targetType.getPackage(); + + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } + } + } fieldImports.put(ref.getSimpleName(), ref); return true; diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index 9dba64286f1..f94c09abb8b 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -392,7 +392,7 @@ public void testGetReference() throws Exception { final CtClass aClass = launcher.getFactory().Class().get(B.class); aClass.getFactory().getEnvironment().setAutoImports(true); - assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); + assertEquals("myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); } } diff --git a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java index e6f290af647..8c715ab2dde 100644 --- a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java @@ -169,11 +169,11 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() thr String expected = "public void setFieldUsingExternallyDefinedEnumWithSameNameAsLocal() {" +nl+ - " localField = spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1.ordinal();" +nl+ + " localField = E1.ordinal();" +nl+ "}" ; String computed = aClass.getMethodsByName("setFieldUsingExternallyDefinedEnumWithSameNameAsLocal").get(0).toString(); - assertEquals( "the externally defined enum should be fully qualified to avoid a name clash with the local enum of the same name", expected, computed ); + assertEquals( "E1 is statically imported then we can call it directly", expected, computed ); expected = //This is what is expected "public void setFieldUsingLocallyDefinedEnum() {" +nl+ diff --git a/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java b/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java index 52eecf189e9..f0833f908d5 100644 --- a/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java +++ b/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java @@ -1,6 +1,7 @@ package spoon.test.prettyprinter.testclasses; import spoon.test.prettyprinter.testclasses.TypeIdentifierCollision.Class0.ClassA; +import static spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1; public class TypeIdentifierCollision { @@ -20,12 +21,12 @@ private ENUM( int num, Enum e ) public void setFieldUsingExternallyDefinedEnumWithSameNameAsLocal() { - localField = spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1.ordinal(); + localField = E1.ordinal(); } public void setFieldUsingLocallyDefinedEnum() { - localField = ENUM.E1.ordinal(); + localField = E1.ordinal(); } public void setFieldOfClassWithSameNameAsTheCompilationUnitClass() From 350920aaac7756852bf4febc7fd24704be0bf254 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 12 Dec 2016 11:12:44 +0100 Subject: [PATCH 21/29] WiP #1027 Fix some tests regarding class import --- .../reflect/visitor/ImportScannerImpl.java | 62 +++++++++++++------ .../AccessFullyQualifiedFieldTest.java | 4 +- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index ae8e327af0d..30d10b375b6 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -251,23 +251,6 @@ protected boolean addClassImport(CtTypeReference ref) { return false; } - if (ref.getDeclaringType() != null) { - CtPackageReference pack = targetType.getPackage(); - - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (ref.getDeclaringType().getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - return false; - } - } - } - } else { - return false; - } - // we want to be sure that we are not importing a class because a static field or method we already imported // moreover we make exception for same package classes to avoid problems in FQN mode try { @@ -287,7 +270,21 @@ protected boolean addClassImport(CtTypeReference ref) { } if (isImported(reference)) { - return false; + // if we are in the **same** package we do the import for test with method isImported + if (ref.getDeclaringType() != null) { + CtPackageReference pack = targetType.getPackage(); + + if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (!ref.getDeclaringType().getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } + } + } + } } } } @@ -295,6 +292,17 @@ protected boolean addClassImport(CtTypeReference ref) { } catch (ParentNotInitializedException e) { } + CtPackageReference pack = targetType.getPackage(); + if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (ref.getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } + } + } //note: we must add the type refs from the same package too, to assure that isImported(typeRef) returns true for them //these type refs are removed in #getClassImports() @@ -303,6 +311,24 @@ protected boolean addClassImport(CtTypeReference ref) { } private boolean isImportedInClassImports(CtTypeReference ref) { + if (targetType != null) { + CtPackageReference pack = targetType.getPackage(); + + // we consider that if a class belongs to java.lang or the same package than the actual class + // then it is imported by default + if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (!ref.getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return true; + } + } + } + } + + if (!(ref.isImplicit()) && classImports.containsKey(ref.getSimpleName())) { CtTypeReference exist = classImports.get(ref.getSimpleName()); if (exist.getQualifiedName().equals(ref.getQualifiedName())) { diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index d02fe3a5910..8ca747751c1 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -126,7 +126,9 @@ public void testStaticImportWithAutoImport() throws Exception { assertTrue("The result should contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); assertTrue("The variable x should be assigned with only SPOONED_CLASSES", result.contains("Object x = SPOONED_CLASSES;")); assertTrue("The result should not contain a static import for spoon.test.variable.testclasses.ForStaticVariables.foo as it is in the same package", !result.contains("import static spoon.test.variable.testclasses.ForStaticVariables.foo;")); - + assertTrue("The result should not contain a FQN for toto", !result.contains("spoon.test.variable.testclasses.MultiBurritos.toto();")); + assertTrue("The result should not contain a FQN for spoon access", !result.contains("spoon.test.variable.testclasses.MultiBurritos.spoon = \"truc\";")); + assertTrue("The result should not contain a FQN for foo", !result.contains("spoon.test.variable.testclasses.ForStaticVariables.foo();")); canBeBuilt(output, 7); } From 512e32c9714485baf0fabb7b604f7bc2b598b14c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 12 Dec 2016 13:33:48 +0100 Subject: [PATCH 22/29] WiP #1027: still working to pass all tests for this feature --- .../reflect/visitor/ImportScannerImpl.java | 76 ++++++++++--------- .../reflect/visitor/MinimalImportScanner.java | 39 +++++++++- .../visitor/printer/ElementPrinterHelper.java | 4 +- 3 files changed, 80 insertions(+), 39 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 30d10b375b6..84b15352e55 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -258,29 +258,42 @@ protected boolean addClassImport(CtTypeReference ref) { if (parent != null) { parent = parent.getParent(); if (parent != null) { - if ((parent instanceof CtFieldAccess) || (parent instanceof CtExecutable)) { - CtReference reference; + if ((parent instanceof CtFieldAccess) || (parent instanceof CtExecutable) || (parent instanceof CtInvocation)) { + CtTypeReference declaringType; + CtReference reference; + CtPackageReference pack = targetType.getPackage(); if (parent instanceof CtFieldAccess) { CtFieldAccess field = (CtFieldAccess)parent; - reference = field.getVariable(); - } else { + CtFieldReference localReference = field.getVariable(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else if (parent instanceof CtExecutable) { CtExecutable exec = (CtExecutable)parent; - reference = exec.getReference(); + CtExecutableReference localReference = exec.getReference(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else if (parent instanceof CtInvocation) { + CtInvocation invo = (CtInvocation)parent; + CtExecutableReference localReference = invo.getExecutable(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else { + declaringType = null; + reference = null; } - if (isImported(reference)) { + if (reference != null && isImported(reference)) { // if we are in the **same** package we do the import for test with method isImported - if (ref.getDeclaringType() != null) { - CtPackageReference pack = targetType.getPackage(); - - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + if (declaringType != null) { + if (declaringType.getPackage() != null && !declaringType.getPackage().isUnnamedPackage()) { // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { + if (!declaringType.getPackage().getSimpleName().equals("java.lang")) { // ignore type in same package - if (!ref.getDeclaringType().getPackage().getSimpleName() + if (declaringType.getPackage().getSimpleName() .equals(pack.getSimpleName())) { - return false; + classImports.put(ref.getSimpleName(), ref); + return true; } } } @@ -292,14 +305,16 @@ protected boolean addClassImport(CtTypeReference ref) { } catch (ParentNotInitializedException e) { } - CtPackageReference pack = targetType.getPackage(); - if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (ref.getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - return false; + if (targetType != null) { + CtPackageReference pack = targetType.getPackage(); + if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!ref.getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (ref.getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + return false; + } } } } @@ -311,7 +326,7 @@ protected boolean addClassImport(CtTypeReference ref) { } private boolean isImportedInClassImports(CtTypeReference ref) { - if (targetType != null) { + /*if (targetType != null) { CtPackageReference pack = targetType.getPackage(); // we consider that if a class belongs to java.lang or the same package than the actual class @@ -320,13 +335,13 @@ private boolean isImportedInClassImports(CtTypeReference ref) { // ignore java.lang package if (!ref.getPackage().getSimpleName().equals("java.lang")) { // ignore type in same package - if (!ref.getPackage().getSimpleName() + if (ref.getPackage().getSimpleName() .equals(pack.getSimpleName())) { return true; } } } - } + }*/ if (!(ref.isImplicit()) && classImports.containsKey(ref.getSimpleName())) { @@ -348,19 +363,6 @@ protected boolean addMethodImport(CtExecutableReference ref) { return false; } - CtPackageReference pack = targetType.getPackage(); - - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (ref.getDeclaringType().getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - return false; - } - } - } - methodImports.put(ref.getSimpleName(), ref); // if we are in the same package than target type, we also import class to avoid FQN in FQN mode. diff --git a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java index a7908875014..28c3fc9b709 100644 --- a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java @@ -16,15 +16,19 @@ */ package spoon.reflect.visitor; +import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtExecutable; import spoon.reflect.declaration.CtNamedElement; import spoon.reflect.declaration.CtPackage; +import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.ParentNotInitializedException; import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; +import spoon.support.reflect.declaration.CtClassImpl; + import java.util.HashSet; import java.util.Set; @@ -36,6 +40,20 @@ public class MinimalImportScanner extends ImportScannerImpl implements ImportSca private Set namedElements = new HashSet(); + private CtClass getParentClass(CtReference ref) { + CtElement parent = ref.getParent(); + + while (parent != null && !(parent instanceof CtClass)) { + parent = parent.getParent(); + } + + if (parent == null) { + return null; + } else { + return (CtClass)parent; + } + } + /** * Test if the reference should be imported by looking if there is a name conflict * @param ref @@ -55,11 +73,30 @@ private boolean shouldTypeBeImported(CtReference ref) { parent = ref; } + CtClass parentClass = this.getParentClass(ref); + if (parent instanceof CtNamedElement) { - namedElements.add(((CtNamedElement) parent).getSimpleName()); + CtNamedElement namedElement = (CtNamedElement)parent; + + if (parentClass != null && parentClass.getReference() != null) { + if (parentClass.getReference().equals(targetType)) { + namedElements.add(namedElement.getSimpleName()); + } + } else { + namedElements.add(namedElement.getSimpleName()); + } } while (!(parent instanceof CtPackage)) { + /*if (parent instanceof CtClassImpl) { + CtClassImpl classParent = (CtClassImpl)parent; + CtTypeReference referencedType = classParent.getReference(); + + if (referencedType != null && referencedType.equals(this.targetType)) { + return false; + } + + }*/ if ((parent instanceof CtFieldReference) || (parent instanceof CtExecutableReference)) { CtReference parentType = (CtReference) parent; Set qualifiedNameTokens = new HashSet<>(); diff --git a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java index fd21e04ebdd..4e7da6b62a8 100644 --- a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java @@ -270,7 +270,9 @@ public void writeHeader(List> types, Collection imports) printer.write("import " + typeRef.getQualifiedName() + ";").writeln().writeTabs(); } else if (ref instanceof CtExecutableReference) { CtExecutableReference execRef = (CtExecutableReference)ref; - printer.write("import static " + execRef.getDeclaringType().getQualifiedName()+"."+execRef.getSimpleName()+";").writeln().writeTabs(); + if (execRef.getDeclaringType() != null) { + printer.write("import static " + execRef.getDeclaringType().getQualifiedName()+"."+execRef.getSimpleName()+";").writeln().writeTabs(); + } } else if (ref instanceof CtFieldReference) { CtFieldReference fieldRef = (CtFieldReference)ref; printer.write("import static " + fieldRef.getDeclaringType().getQualifiedName()+"."+fieldRef.getSimpleName()+";").writeln().writeTabs(); From 7800b567f48e66c16887bdb883805f814f29e58b Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 12 Dec 2016 17:07:55 +0100 Subject: [PATCH 23/29] WiP #1027 Change the behaviour to write static fields, now it uses the parent class name if not imported. --- .../visitor/DefaultJavaPrettyPrinter.java | 21 ++- .../reflect/visitor/ImportScannerImpl.java | 120 ++++++++++-------- .../test/fieldaccesses/FieldAccessTest.java | 5 +- .../spoon/test/imports/ImportScannerTest.java | 3 +- .../AccessFullyQualifiedFieldTest.java | 5 +- 5 files changed, 87 insertions(+), 67 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 40ec3ecf923..be94f14b8f4 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -676,14 +676,6 @@ public void visitCtFieldWrite(CtFieldWrite fieldWrite) { printCtFieldAccess(fieldWrite); } - private boolean isStaticImportedField(CtFieldReference ref) { - if (!ref.isStatic()) { - return false; - } - - return importsContext.isImported(ref); - } - private void printCtFieldAccess(CtFieldAccess f) { enterCtExpression(f); try (Writable _context = context.modify()) { @@ -692,14 +684,21 @@ private void printCtFieldAccess(CtFieldAccess f) { } if (f.getTarget() != null) { boolean isInitializeStaticFinalField = isInitializeStaticFinalField(f.getTarget()); - boolean isStaticImportedField = isStaticImportedField(f.getVariable()); + boolean isStaticField = f.getVariable().isStatic(); + boolean isImportedField = importsContext.isImported(f.getVariable()); - if (!isInitializeStaticFinalField && !isStaticImportedField) { + if (!isInitializeStaticFinalField && !(isStaticField && isImportedField)) { scan(f.getTarget()); if (!f.getTarget().isImplicit()) { printer.write("."); } - } + } else if (isStaticField && !isImportedField) { + CtTypeReference declaringType = f.getVariable().getDeclaringType(); + if (declaringType != null) { + printer.write(declaringType.getSimpleName()); + printer.write("."); + } + } _context.ignoreStaticAccess(true); } scan(f.getVariable()); diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 84b15352e55..b468bc7c01d 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -238,7 +238,7 @@ protected boolean addClassImport(CtTypeReference ref) { if (ref.getPackage() == null || ref.getPackage().isUnnamedPackage()) { return false; } - if (!ref.getPackage().getSimpleName().equals("java.lang")) { + if (ref.getPackage().getSimpleName().equals("java.lang")) { if (classNamePresentInJavaLang(ref)) { // Don't import class with names clashing with some classes present in java.lang, // because it leads to undecidability and compilation errors. I. e. always leave @@ -253,47 +253,49 @@ protected boolean addClassImport(CtTypeReference ref) { // we want to be sure that we are not importing a class because a static field or method we already imported // moreover we make exception for same package classes to avoid problems in FQN mode - try { - CtElement parent = ref.getParent(); - if (parent != null) { - parent = parent.getParent(); + if (targetType != null) { + try { + CtElement parent = ref.getParent(); if (parent != null) { - if ((parent instanceof CtFieldAccess) || (parent instanceof CtExecutable) || (parent instanceof CtInvocation)) { - - CtTypeReference declaringType; - CtReference reference; - CtPackageReference pack = targetType.getPackage(); - if (parent instanceof CtFieldAccess) { - CtFieldAccess field = (CtFieldAccess)parent; - CtFieldReference localReference = field.getVariable(); - declaringType = localReference.getDeclaringType(); - reference = localReference; - } else if (parent instanceof CtExecutable) { - CtExecutable exec = (CtExecutable)parent; - CtExecutableReference localReference = exec.getReference(); - declaringType = localReference.getDeclaringType(); - reference = localReference; - } else if (parent instanceof CtInvocation) { - CtInvocation invo = (CtInvocation)parent; - CtExecutableReference localReference = invo.getExecutable(); - declaringType = localReference.getDeclaringType(); - reference = localReference; - } else { - declaringType = null; - reference = null; - } + parent = parent.getParent(); + if (parent != null) { + if ((parent instanceof CtFieldAccess) || (parent instanceof CtExecutable) || (parent instanceof CtInvocation)) { + + CtTypeReference declaringType; + CtReference reference; + CtPackageReference pack = targetType.getPackage(); + if (parent instanceof CtFieldAccess) { + CtFieldAccess field = (CtFieldAccess)parent; + CtFieldReference localReference = field.getVariable(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else if (parent instanceof CtExecutable) { + CtExecutable exec = (CtExecutable)parent; + CtExecutableReference localReference = exec.getReference(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else if (parent instanceof CtInvocation) { + CtInvocation invo = (CtInvocation)parent; + CtExecutableReference localReference = invo.getExecutable(); + declaringType = localReference.getDeclaringType(); + reference = localReference; + } else { + declaringType = null; + reference = null; + } - if (reference != null && isImported(reference)) { - // if we are in the **same** package we do the import for test with method isImported - if (declaringType != null) { - if (declaringType.getPackage() != null && !declaringType.getPackage().isUnnamedPackage()) { - // ignore java.lang package - if (!declaringType.getPackage().getSimpleName().equals("java.lang")) { - // ignore type in same package - if (declaringType.getPackage().getSimpleName() - .equals(pack.getSimpleName())) { - classImports.put(ref.getSimpleName(), ref); - return true; + if (reference != null && isImported(reference)) { + // if we are in the **same** package we do the import for test with method isImported + if (declaringType != null) { + if (declaringType.getPackage() != null && !declaringType.getPackage().isUnnamedPackage()) { + // ignore java.lang package + if (!declaringType.getPackage().getSimpleName().equals("java.lang")) { + // ignore type in same package + if (declaringType.getPackage().getSimpleName() + .equals(pack.getSimpleName())) { + classImports.put(ref.getSimpleName(), ref); + return true; + } } } } @@ -301,11 +303,8 @@ protected boolean addClassImport(CtTypeReference ref) { } } } + } catch (ParentNotInitializedException e) { } - } catch (ParentNotInitializedException e) { - } - - if (targetType != null) { CtPackageReference pack = targetType.getPackage(); if (ref.getPackage() != null && !ref.getPackage().isUnnamedPackage()) { // ignore java.lang package @@ -326,7 +325,7 @@ protected boolean addClassImport(CtTypeReference ref) { } private boolean isImportedInClassImports(CtTypeReference ref) { - /*if (targetType != null) { + if (targetType != null) { CtPackageReference pack = targetType.getPackage(); // we consider that if a class belongs to java.lang or the same package than the actual class @@ -341,8 +340,11 @@ private boolean isImportedInClassImports(CtTypeReference ref) { } } } - }*/ + } + if (ref.equals(targetType)) { + return true; + } if (!(ref.isImplicit()) && classImports.containsKey(ref.getSimpleName())) { CtTypeReference exist = classImports.get(ref.getSimpleName()); @@ -390,9 +392,25 @@ protected boolean addFieldImport(CtFieldReference ref) { if (this.fieldImports.containsKey(ref.getSimpleName())) { return isImportedInFieldImports(ref); } - CtPackageReference pack = targetType.getPackage(); + //CtPackageReference pack = targetType.getPackage(); - if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { + // if the whole class is imported: no need to import the method. + CtTypeReference declaringType = ref.getDeclaringType(); + if (declaringType != null) { + if (isImportedInClassImports(declaringType)) { + return false; + } + + while (declaringType != null) { + if (declaringType.equals(targetType)) { + return false; + } + declaringType = declaringType.getDeclaringType(); + } + + } + + /*if (ref.getDeclaringType().getPackage() != null && !ref.getDeclaringType().getPackage().isUnnamedPackage()) { // ignore java.lang package if (!ref.getDeclaringType().getPackage().getSimpleName().equals("java.lang")) { // ignore type in same package @@ -401,7 +419,7 @@ protected boolean addFieldImport(CtFieldReference ref) { return false; } } - } + }*/ fieldImports.put(ref.getSimpleName(), ref); return true; @@ -420,11 +438,11 @@ private boolean isImportedInFieldImports(CtFieldReference ref) { } } - CtTypeReference typeRef = ref.getDeclaringType(); + /*CtTypeReference typeRef = ref.getDeclaringType(); if (typeRef != null) { return isImportedInClassImports(typeRef); - } + }*/ return false; } diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index f94c09abb8b..c40739797f3 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -392,7 +392,8 @@ public void testGetReference() throws Exception { final CtClass aClass = launcher.getFactory().Class().get(B.class); aClass.getFactory().getEnvironment().setAutoImports(true); - assertEquals("myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); - assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); + // now static fields are used with the name of the parent class + assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); + assertEquals("B.finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); } } diff --git a/src/test/java/spoon/test/imports/ImportScannerTest.java b/src/test/java/spoon/test/imports/ImportScannerTest.java index b128281160b..e6ff7decdc6 100644 --- a/src/test/java/spoon/test/imports/ImportScannerTest.java +++ b/src/test/java/spoon/test/imports/ImportScannerTest.java @@ -59,7 +59,8 @@ public void testMultiCatchImport() throws Exception { ImportScanner importScanner = new ImportScannerImpl(); importScanner.computeImports(classes.get(0)); - assertTrue( importScanner.isImported( factory.Type().createReference( ArithmeticException.class ) )); + // as ArithmeticException come from java.lang it is not imported anymore + //assertTrue( importScanner.isImported( factory.Type().createReference( ArithmeticException.class ) )); assertTrue( importScanner.isImported( factory.Type().createReference( AccessControlException.class ) )); } } diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 8ca747751c1..433ced0e1ff 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -67,10 +67,10 @@ public void testNoFQNWhenShadowedByLocalVariable() throws Exception { String result = this.buildResourceAndReturnResult(pathResource, output); assertTrue("The java file should contain import for Launcher", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); - assertTrue("The x variable is attributed with Launcher.SPOONED_CLASSES", result.contains("x = SPOONED_CLASSES")); + assertTrue("The x variable should be attributed with SPOONED_CLASSES", result.contains("x = SPOONED_CLASSES")); assertTrue("The java.util.Map is not imported", !result.contains("import java.util.Map")); assertTrue("The Map type use FQN", result.contains("java.util.Map uneMap")); - assertTrue("The other variable use FQN too", result.contains("spoon.test.variable.testclasses.ForStaticVariables.Map")); + assertTrue("The other variable use FQN too", result.contains("ForStaticVariables.Map")); canBeBuilt(output, 7); } @@ -126,6 +126,7 @@ public void testStaticImportWithAutoImport() throws Exception { assertTrue("The result should contain a static import for spoon.Launcher.SPOONED_CLASSES", result.contains("import static spoon.Launcher.SPOONED_CLASSES;")); assertTrue("The variable x should be assigned with only SPOONED_CLASSES", result.contains("Object x = SPOONED_CLASSES;")); assertTrue("The result should not contain a static import for spoon.test.variable.testclasses.ForStaticVariables.foo as it is in the same package", !result.contains("import static spoon.test.variable.testclasses.ForStaticVariables.foo;")); + assertTrue("The result should not contain a import static for spoon.test.variable.testclasses.MultiBurritos.toto as it is in the same class", !result.contains("import static spoon.test.variable.testclasses.MultiBurritos.toto;")); assertTrue("The result should not contain a FQN for toto", !result.contains("spoon.test.variable.testclasses.MultiBurritos.toto();")); assertTrue("The result should not contain a FQN for spoon access", !result.contains("spoon.test.variable.testclasses.MultiBurritos.spoon = \"truc\";")); assertTrue("The result should not contain a FQN for foo", !result.contains("spoon.test.variable.testclasses.ForStaticVariables.foo();")); From 715b62e26b2cb5a1379ce4141e0c4a9ede725867 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 12 Dec 2016 17:43:50 +0100 Subject: [PATCH 24/29] WiP #1027 One more test to manage java.lang when printing FQN --- .../java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 3 +++ src/test/java/spoon/test/targeted/TargetedExpressionTest.java | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index be94f14b8f4..d942814754a 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -1546,6 +1546,9 @@ private boolean printQualified(CtTypeReference ref) { } return false; } else { + if (ref.getPackage() != null && ref.getPackage().getSimpleName().equals("java.lang")) { + return false; + } return true; } } diff --git a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java index f7417381557..2d8d0fe67dd 100644 --- a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java +++ b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java @@ -175,7 +175,9 @@ public void testTargetsOfStaticFieldAccess() throws Exception { final CtAnonymousExecutable staticInit = type.getAnonymousExecutables().get(0); final List> staticElements = staticInit.getElements(new TypeFilter<>(CtFieldAccess.class)); assertEquals(1, staticElements.size()); - assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("p"), staticElements.get(0)); + + // Changing behaviour when writing static field, it is now writed using the class name + assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("Foo.p"), staticElements.get(0)); } @Test From 6c5a5b2d319230208b44205b8b9d48df204e79cd Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 13 Dec 2016 16:41:23 +0100 Subject: [PATCH 25/29] Fix last tests for #1027. Fix checkstyle issues as well. --- .../visitor/DefaultJavaPrettyPrinter.java | 7 +- .../reflect/visitor/ImportScannerImpl.java | 14 ++- .../reflect/visitor/MinimalImportScanner.java | 11 +-- .../visitor/printer/ElementPrinterHelper.java | 10 +-- .../spoon/support/StandardEnvironment.java | 1 + .../support/visitor/java/VariableScanner.java | 88 ------------------- .../spoon/test/imports/ImportScannerTest.java | 18 +++- .../DefaultPrettyPrinterTest.java | 15 ++-- .../testclasses/TypeIdentifierCollision.java | 5 +- 9 files changed, 47 insertions(+), 122 deletions(-) delete mode 100644 src/main/java/spoon/support/visitor/java/VariableScanner.java diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index d942814754a..a6dad3df546 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -698,7 +698,7 @@ private void printCtFieldAccess(CtFieldAccess f) { printer.write(declaringType.getSimpleName()); printer.write("."); } - } + } _context.ignoreStaticAccess(true); } scan(f.getVariable()); @@ -1530,7 +1530,7 @@ public void visitCtWildcardReference(CtWildcardReference wildcardReference) { } private boolean printQualified(CtTypeReference ref) { - if (importsContext.isImported(ref)) { + if (importsContext.isImported(ref) || (this.env.isAutoImports() && ref.getPackage() != null && ref.getPackage().getSimpleName().equals("java.lang"))) { // If my.pkg.Something is imported, but //A) we are in the context of a class which is also called "Something", //B) we are in the context of a class which defines field which is also called "Something", @@ -1546,9 +1546,6 @@ private boolean printQualified(CtTypeReference ref) { } return false; } else { - if (ref.getPackage() != null && ref.getPackage().getSimpleName().equals("java.lang")) { - return false; - } return true; } } diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index b468bc7c01d..54ef9f7eb02 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -26,7 +26,6 @@ import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtEnum; import spoon.reflect.declaration.CtExecutable; -import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtInterface; import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.CtTypeMember; @@ -45,7 +44,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -217,11 +215,11 @@ public void computeImports(CtElement element) { @Override public boolean isImported(CtReference ref) { if (ref instanceof CtFieldReference) { - return isImportedInFieldImports((CtFieldReference)ref); + return isImportedInFieldImports((CtFieldReference) ref); } else if (ref instanceof CtExecutableReference) { - return isImportedInMethodImports((CtExecutableReference)ref); + return isImportedInMethodImports((CtExecutableReference) ref); } else if (ref instanceof CtTypeReference) { - return isImportedInClassImports((CtTypeReference)ref); + return isImportedInClassImports((CtTypeReference) ref); } else { return false; } @@ -265,17 +263,17 @@ protected boolean addClassImport(CtTypeReference ref) { CtReference reference; CtPackageReference pack = targetType.getPackage(); if (parent instanceof CtFieldAccess) { - CtFieldAccess field = (CtFieldAccess)parent; + CtFieldAccess field = (CtFieldAccess) parent; CtFieldReference localReference = field.getVariable(); declaringType = localReference.getDeclaringType(); reference = localReference; } else if (parent instanceof CtExecutable) { - CtExecutable exec = (CtExecutable)parent; + CtExecutable exec = (CtExecutable) parent; CtExecutableReference localReference = exec.getReference(); declaringType = localReference.getDeclaringType(); reference = localReference; } else if (parent instanceof CtInvocation) { - CtInvocation invo = (CtInvocation)parent; + CtInvocation invo = (CtInvocation) parent; CtExecutableReference localReference = invo.getExecutable(); declaringType = localReference.getDeclaringType(); reference = localReference; diff --git a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java index 28c3fc9b709..bfc5995b4f2 100644 --- a/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/MinimalImportScanner.java @@ -18,16 +18,13 @@ import spoon.reflect.declaration.CtClass; import spoon.reflect.declaration.CtElement; -import spoon.reflect.declaration.CtExecutable; import spoon.reflect.declaration.CtNamedElement; import spoon.reflect.declaration.CtPackage; -import spoon.reflect.declaration.CtType; import spoon.reflect.declaration.ParentNotInitializedException; import spoon.reflect.reference.CtExecutableReference; import spoon.reflect.reference.CtFieldReference; import spoon.reflect.reference.CtReference; import spoon.reflect.reference.CtTypeReference; -import spoon.support.reflect.declaration.CtClassImpl; import java.util.HashSet; import java.util.Set; @@ -50,7 +47,7 @@ private CtClass getParentClass(CtReference ref) { if (parent == null) { return null; } else { - return (CtClass)parent; + return (CtClass) parent; } } @@ -76,7 +73,7 @@ private boolean shouldTypeBeImported(CtReference ref) { CtClass parentClass = this.getParentClass(ref); if (parent instanceof CtNamedElement) { - CtNamedElement namedElement = (CtNamedElement)parent; + CtNamedElement namedElement = (CtNamedElement) parent; if (parentClass != null && parentClass.getReference() != null) { if (parentClass.getReference().equals(targetType)) { @@ -105,9 +102,9 @@ private boolean shouldTypeBeImported(CtReference ref) { CtTypeReference typeReference; if (parent instanceof CtFieldReference) { - typeReference = ((CtFieldReference)parent).getDeclaringType(); + typeReference = ((CtFieldReference) parent).getDeclaringType(); } else { - typeReference = ((CtExecutableReference)parent).getDeclaringType(); + typeReference = ((CtExecutableReference) parent).getDeclaringType(); } if (typeReference != null) { diff --git a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java index 4e7da6b62a8..c65fc273410 100644 --- a/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java +++ b/src/main/java/spoon/reflect/visitor/printer/ElementPrinterHelper.java @@ -266,16 +266,16 @@ public void writeHeader(List> types, Collection imports) printer.writeln().writeln().writeTabs(); for (CtReference ref : imports) { if (ref instanceof CtTypeReference) { - CtTypeReference typeRef = (CtTypeReference)ref; + CtTypeReference typeRef = (CtTypeReference) ref; printer.write("import " + typeRef.getQualifiedName() + ";").writeln().writeTabs(); } else if (ref instanceof CtExecutableReference) { - CtExecutableReference execRef = (CtExecutableReference)ref; + CtExecutableReference execRef = (CtExecutableReference) ref; if (execRef.getDeclaringType() != null) { - printer.write("import static " + execRef.getDeclaringType().getQualifiedName()+"."+execRef.getSimpleName()+";").writeln().writeTabs(); + printer.write("import static " + execRef.getDeclaringType().getQualifiedName() + "." + execRef.getSimpleName() + ";").writeln().writeTabs(); } } else if (ref instanceof CtFieldReference) { - CtFieldReference fieldRef = (CtFieldReference)ref; - printer.write("import static " + fieldRef.getDeclaringType().getQualifiedName()+"."+fieldRef.getSimpleName()+";").writeln().writeTabs(); + CtFieldReference fieldRef = (CtFieldReference) ref; + printer.write("import static " + fieldRef.getDeclaringType().getQualifiedName() + "." + fieldRef.getSimpleName() + ";").writeln().writeTabs(); } } diff --git a/src/main/java/spoon/support/StandardEnvironment.java b/src/main/java/spoon/support/StandardEnvironment.java index cc085cd2a4c..dc83dc50e54 100644 --- a/src/main/java/spoon/support/StandardEnvironment.java +++ b/src/main/java/spoon/support/StandardEnvironment.java @@ -105,6 +105,7 @@ public boolean isAutoImports() { @Override public void setAutoImports(boolean autoImports) { this.autoImports = autoImports; + // TODO: unexpected behaviour could occur, if we reset the autoimport AFTER the pretty printer is created... } @Override diff --git a/src/main/java/spoon/support/visitor/java/VariableScanner.java b/src/main/java/spoon/support/visitor/java/VariableScanner.java deleted file mode 100644 index 972c6ef02e7..00000000000 --- a/src/main/java/spoon/support/visitor/java/VariableScanner.java +++ /dev/null @@ -1,88 +0,0 @@ -package spoon.support.visitor.java; - -import spoon.reflect.code.*; -import spoon.reflect.declaration.*; -import spoon.reflect.reference.CtTypeReference; -import spoon.reflect.visitor.CtInheritanceScanner; - -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; -import java.util.Set; - -public class VariableScanner extends CtInheritanceScanner { - private CtElement expression; - final List variables = new ArrayList<>(); - - public VariableScanner(CtElement expression) { - super(); - this.expression = expression; - } - - @Override - public void visitCtStatementList(CtStatementList e) { - for (int i = 0; i < e.getStatements().size(); i++) { - CtStatement ctStatement = e.getStatements().get(i); - if (ctStatement.getPosition() == null) { - } - if (ctStatement.getPosition() != null - && ctStatement.getPosition().getSourceStart() > expression.getPosition().getSourceEnd()) { - break; - } - if (ctStatement instanceof CtVariable) { - variables.add((CtVariable) ctStatement); - } - } - super.visitCtStatementList(e); - } - - @Override - public void scanCtType(CtType type) { - List> fields = type.getFields(); - for (int i = 0; i < fields.size(); i++) { - CtField ctField = fields.get(i); - if (ctField.hasModifier(ModifierKind.PUBLIC) || ctField.hasModifier(ModifierKind.PROTECTED)) { - variables.add(ctField); - } else if (ctField.hasModifier(ModifierKind.PRIVATE)) { - if (expression.hasParent(type)) { - variables.add(ctField); - } - } else if (expression.getParent(CtPackage.class).equals(type.getParent(CtPackage.class))) { - // default visibility - variables.add(ctField); - } - } - - CtTypeReference superclass = type.getSuperclass(); - if (superclass != null) { - this.scan(superclass.getTypeDeclaration()); - } - Set> superInterfaces = type.getSuperInterfaces(); - for (Iterator> iterator = superInterfaces.iterator(); iterator.hasNext();) { - CtTypeReference typeReference = iterator.next(); - this.scan(typeReference.getTypeDeclaration()); - } - } - - @Override - public void visitCtTryWithResource(CtTryWithResource e) { - variables.addAll(e.getResources()); - } - - @Override - public void scanCtExecutable(CtExecutable e) { - variables.addAll(e.getParameters()); - } - - @Override - public void visitCtFor(CtFor e) { - for (CtStatement ctStatement : e.getForInit()) { - this.scan(ctStatement); - } - } - - @Override - public void visitCtForEach(CtForEach e) { - variables.add(e.getVariable()); - } -} \ No newline at end of file diff --git a/src/test/java/spoon/test/imports/ImportScannerTest.java b/src/test/java/spoon/test/imports/ImportScannerTest.java index e6ff7decdc6..701eea9649f 100644 --- a/src/test/java/spoon/test/imports/ImportScannerTest.java +++ b/src/test/java/spoon/test/imports/ImportScannerTest.java @@ -11,6 +11,7 @@ import spoon.reflect.reference.CtTypeReference; import spoon.reflect.visitor.ImportScanner; import spoon.reflect.visitor.ImportScannerImpl; +import spoon.reflect.visitor.MinimalImportScanner; import spoon.reflect.visitor.Query; import spoon.reflect.visitor.filter.NameFilter; @@ -27,6 +28,21 @@ */ public class ImportScannerTest { + @Test + public void testComputeMinimalImportsInClass() throws Exception { + String packageName = "spoon.test"; + String className = "SampleImportClass"; + String qualifiedName = packageName + "." + className; + + Factory aFactory = build(packageName, className).getFactory(); + CtType theClass = aFactory.Type().get(qualifiedName); + + ImportScanner importContext = new MinimalImportScanner(); + Collection imports = importContext.computeImports(theClass); + + assertTrue(imports.isEmpty()); + } + @Test public void testComputeImportsInClass() throws Exception { String packageName = "spoon.test"; @@ -39,7 +55,7 @@ public void testComputeImportsInClass() throws Exception { ImportScanner importContext = new ImportScannerImpl(); Collection imports = importContext.computeImports(theClass); - assertEquals(2, imports.size()); + assertEquals(3, imports.size()); } diff --git a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java index 8c715ab2dde..11127268500 100644 --- a/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java +++ b/src/test/java/spoon/test/prettyprinter/DefaultPrettyPrinterTest.java @@ -162,8 +162,13 @@ public void testPrintAMethodWithGeneric() throws Exception { @Test public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() throws Exception { - Factory factory = build( spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.class, spoon.test.prettyprinter.testclasses.TypeIdentifierCollision.class ); + final Launcher launcher = new Launcher(); + final Factory factory = launcher.getFactory(); factory.getEnvironment().setAutoImports(true); + final SpoonCompiler compiler = launcher.createCompiler(); + compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/sub/TypeIdentifierCollision.java")); + compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java")); + compiler.build(); final CtClass aClass = (CtClass) factory.Type().get( spoon.test.prettyprinter.testclasses.TypeIdentifierCollision.class ); @@ -190,11 +195,11 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() thr expected = "public void setFieldOfClassWithSameNameAsTheCompilationUnitClass() {" +nl+ - " TypeIdentifierCollision.globalField = localField;" +nl+ + " globalField = localField;" +nl+ "}" ; computed = aClass.getMethodsByName("setFieldOfClassWithSameNameAsTheCompilationUnitClass").get(0).toString(); - assertEquals( "The static field of an external type with the same identifier as the compilation unit should be fully qualified", expected, computed ); + assertEquals( "The static field of an external type with the same identifier as the compilation unit is statically imported", expected, computed ); expected = //This is what is expected "public void referToTwoInnerClassesWithTheSameName() {" +nl+ @@ -216,7 +221,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() thr expected = "public enum ENUM {" +nl+ - "E1(spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.globalField,spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1);" +nl+ + "E1(globalField,E1);" +nl+ " final int NUM;" +nl+nl+ " final Enum e;" +nl+nl+ " private ENUM(int num, Enum e) {" +nl+ @@ -226,7 +231,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() thr "}" ; computed = aClass.getNestedType("ENUM").toString(); - assertEquals( "Parameters in an enum constructor should be fully typed when they refer to externally defined static field of a class with the same identifier as another locally defined type", expected, computed ); + assertEquals( "Parameters in an enum constructor should be statically imported when they refer to externally defined static field of a class with the same identifier as another locally defined type", expected, computed ); } @Test diff --git a/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java b/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java index f0833f908d5..52eecf189e9 100644 --- a/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java +++ b/src/test/java/spoon/test/prettyprinter/testclasses/TypeIdentifierCollision.java @@ -1,7 +1,6 @@ package spoon.test.prettyprinter.testclasses; import spoon.test.prettyprinter.testclasses.TypeIdentifierCollision.Class0.ClassA; -import static spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1; public class TypeIdentifierCollision { @@ -21,12 +20,12 @@ private ENUM( int num, Enum e ) public void setFieldUsingExternallyDefinedEnumWithSameNameAsLocal() { - localField = E1.ordinal(); + localField = spoon.test.prettyprinter.testclasses.sub.TypeIdentifierCollision.ENUM.E1.ordinal(); } public void setFieldUsingLocallyDefinedEnum() { - localField = E1.ordinal(); + localField = ENUM.E1.ordinal(); } public void setFieldOfClassWithSameNameAsTheCompilationUnitClass() From bb70bb58090a14b165e356742e3222adb31a3955 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 14 Dec 2016 08:37:44 +0100 Subject: [PATCH 26/29] Clear all lists for #1027 --- src/main/java/spoon/reflect/visitor/ImportScannerImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 54ef9f7eb02..730392fff31 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -191,6 +191,8 @@ public void visitCtCatchVariable(CtCatchVariable catchVariable) { @Override public Collection computeImports(CtType simpleType) { classImports.clear(); + fieldImports.clear(); + methodImports.clear(); //look for top declaring type of that simpleType targetType = simpleType.getReference().getTopLevelType(); addClassImport(simpleType.getReference()); From 3b1bf4c4e717c88ad4cf071148c288a0bac5d2c0 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 14 Dec 2016 10:51:28 +0100 Subject: [PATCH 27/29] Feature #1027: delete mentions of classImports in comments. Change the interface ImportScanner for backward compatibility reason as suggested by @monperrus --- .../visitor/DefaultJavaPrettyPrinter.java | 8 ++++---- .../spoon/reflect/visitor/ImportScanner.java | 18 ++++++++++++++---- .../reflect/visitor/ImportScannerImpl.java | 16 ++++++++++++++-- .../spoon/test/imports/ImportScannerTest.java | 6 +++--- .../java/spoon/test/imports/ImportTest.java | 12 ++++++------ .../AccessFullyQualifiedFieldTest.java | 2 +- 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index d81ad100b97..77cfa8e2d09 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -143,7 +143,7 @@ public class DefaultJavaPrettyPrinter implements CtVisitor, PrettyPrinter { public PrintingContext context = new PrintingContext(); /** - * Handle classImports of classes. + * Handle imports of classes. */ private ImportScanner importsContext; @@ -225,15 +225,15 @@ protected void exitCtExpression(CtExpression e) { } /** - * Make the classImports for a given type. + * Make the imports for a given type. */ public Collection computeImports(CtType type) { context.currentTopLevel = type; - return importsContext.computeImports(context.currentTopLevel); + return importsContext.computeAllImports(context.currentTopLevel); } /** - * Make the classImports for all elements. + * Make the imports for all elements. */ public void computeImports(CtElement element) { if (env.isAutoImports()) { diff --git a/src/main/java/spoon/reflect/visitor/ImportScanner.java b/src/main/java/spoon/reflect/visitor/ImportScanner.java index 67fa3393fab..6717beee813 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScanner.java +++ b/src/main/java/spoon/reflect/visitor/ImportScanner.java @@ -19,23 +19,33 @@ import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtType; import spoon.reflect.reference.CtReference; +import spoon.reflect.reference.CtTypeReference; import java.util.Collection; /** - * Used to compute the classImports required to write readable code with no fully qualified names. + * Used to compute the imports required to write readable code with no fully qualified names. */ public interface ImportScanner { + + /** + * Computes import of a {@link spoon.reflect.declaration.CtType} + * (represent a class). + * + * @return class imports computed by Spoon, it does not contain static imports + */ + Collection> computeImports(CtType simpleType); + /** * Computes import of a {@link spoon.reflect.declaration.CtType} * (represent a class). * - * @return Imports computes by Spoon, it can be CtTypeReference (for classes), but also CtFieldReference (static field) or CtExecutableReference (static methods) + * @return imports computed by Spoon, it can be CtTypeReference (for classes), but also CtFieldReference (static field) or CtExecutableReference (static methods) */ - Collection computeImports(CtType simpleType); + Collection computeAllImports(CtType simpleType); /** - * Computes classImports for all elements. + * Computes imports for an element. */ @Deprecated void computeImports(CtElement element); diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index 730392fff31..df3aed11c29 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -48,7 +48,7 @@ import java.util.TreeMap; /** - * A scanner that calculates the classImports for a given model. + * A scanner that calculates the imports for a given model. */ public class ImportScannerImpl extends CtScanner implements ImportScanner { @@ -189,7 +189,7 @@ public void visitCtCatchVariable(CtCatchVariable catchVariable) { } @Override - public Collection computeImports(CtType simpleType) { + public Collection computeAllImports(CtType simpleType) { classImports.clear(); fieldImports.clear(); methodImports.clear(); @@ -205,9 +205,21 @@ public Collection computeImports(CtType simpleType) { return listallImports; } + @Override + public Collection> computeImports(CtType simpleType) { + classImports.clear(); + //look for top declaring type of that simpleType + targetType = simpleType.getReference().getTopLevelType(); + addClassImport(simpleType.getReference()); + scan(simpleType); + return this.classImports.values(); + } + @Override public void computeImports(CtElement element) { classImports.clear(); + fieldImports.clear(); + methodImports.clear(); //look for top declaring type of that element CtType type = element.getParent(CtType.class); targetType = type == null ? null : type.getReference().getTopLevelType(); diff --git a/src/test/java/spoon/test/imports/ImportScannerTest.java b/src/test/java/spoon/test/imports/ImportScannerTest.java index 701eea9649f..b08f2280427 100644 --- a/src/test/java/spoon/test/imports/ImportScannerTest.java +++ b/src/test/java/spoon/test/imports/ImportScannerTest.java @@ -38,7 +38,7 @@ public void testComputeMinimalImportsInClass() throws Exception { CtType theClass = aFactory.Type().get(qualifiedName); ImportScanner importContext = new MinimalImportScanner(); - Collection imports = importContext.computeImports(theClass); + Collection> imports = importContext.computeImports(theClass); assertTrue(imports.isEmpty()); } @@ -53,9 +53,9 @@ public void testComputeImportsInClass() throws Exception { CtType theClass = aFactory.Type().get(qualifiedName); ImportScanner importContext = new ImportScannerImpl(); - Collection imports = importContext.computeImports(theClass); + Collection> imports = importContext.computeImports(theClass); - assertEquals(3, imports.size()); + assertEquals(2, imports.size()); } diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 77a2d438068..f3fcf72c9f0 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -198,13 +198,13 @@ public void testSpoonWithImports() throws Exception { final CtClass classWithInvocation = launcher.getFactory().Class().get(ClassWithInvocation.class); final ImportScanner importScanner = new ImportScannerImpl(); - final Collection imports = importScanner.computeImports(aClass); + final Collection> imports = importScanner.computeImports(aClass); assertEquals(2, imports.size()); - final Collection imports1 = importScanner.computeImports(anotherClass); + final Collection> imports1 = importScanner.computeImports(anotherClass); assertEquals(1, imports1.size()); //check that printer did not used the package protected class like "SuperClass.InnerClassProtected" assertTrue(anotherClass.toString().indexOf("InnerClass extends ChildClass.InnerClassProtected")>0); - final Collection imports2 = importScanner.computeImports(classWithInvocation); + final Collection> imports2 = importScanner.computeImports(classWithInvocation); assertEquals("Spoon ignores the arguments of CtInvocations", 1, imports2.size()); } @@ -279,7 +279,7 @@ public void testImportOfInvocationOfPrivateClass() throws Exception { "./src/test/java/spoon/test/imports/testclasses/Mole.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection imports = importContext.computeImports(factory.Class().get(Mole.class)); + Collection> imports = importContext.computeImports(factory.Class().get(Mole.class)); assertEquals(1, imports.size()); assertEquals("spoon.test.imports.testclasses.internal2.Chimichanga", imports.toArray()[0].toString()); @@ -293,7 +293,7 @@ public void testNotImportExecutableType() throws Exception { "./src/test/java/spoon/test/imports/testclasses/NotImportExecutableType.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection imports = importContext.computeImports(factory.Class().get(NotImportExecutableType.class)); + Collection> imports = importContext.computeImports(factory.Class().get(NotImportExecutableType.class)); assertEquals(2, imports.size()); Set expectedImports = new HashSet<>( @@ -309,7 +309,7 @@ public void testImportOfInvocationOfStaticMethod() throws Exception { "./src/test/java/spoon/test/imports/testclasses/Pozole.java"); ImportScanner importContext = new ImportScannerImpl(); - Collection imports = importContext.computeImports(factory.Class().get(Pozole.class)); + Collection> imports = importContext.computeImports(factory.Class().get(Pozole.class)); assertEquals(1, imports.size()); assertEquals("spoon.test.imports.testclasses.internal2.Menudo", imports.toArray()[0].toString()); diff --git a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java index 433ced0e1ff..d523e9897a4 100644 --- a/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java +++ b/src/test/java/spoon/test/variable/AccessFullyQualifiedFieldTest.java @@ -32,7 +32,7 @@ public void testCheckAssignmentContracts() throws Exception { private String buildResourceAndReturnResult(String pathResource, String output) { Launcher spoon = new Launcher(); - //spoon.setArgs(new String[]{"--with-classImports"}); + //spoon.setArgs(new String[]{"--with-imports"}); spoon.addInputResource(pathResource); spoon.setSourceOutputDirectory(output); spoon.run(); From cc0ec5b0b8f04937d5d260f2607b33054793b4d5 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 14 Dec 2016 12:35:38 +0100 Subject: [PATCH 28/29] Also have to clear all list in computeImports #1027 --- src/main/java/spoon/reflect/visitor/ImportScannerImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java index df3aed11c29..cc39d447199 100644 --- a/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java +++ b/src/main/java/spoon/reflect/visitor/ImportScannerImpl.java @@ -208,6 +208,8 @@ public Collection computeAllImports(CtType simpleType) { @Override public Collection> computeImports(CtType simpleType) { classImports.clear(); + fieldImports.clear(); + methodImports.clear(); //look for top declaring type of that simpleType targetType = simpleType.getReference().getTopLevelType(); addClassImport(simpleType.getReference()); From 859928d7adeb37ff622ec92b8faa88a56f0975b8 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 15 Dec 2016 09:45:27 +0100 Subject: [PATCH 29/29] Revert some changes to force the add of classtype before calling a static field. Change test according that. #1027 --- .../spoon/reflect/visitor/DefaultJavaPrettyPrinter.java | 6 ------ src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java | 4 ++-- src/test/java/spoon/test/imports/ImportTest.java | 2 +- .../java/spoon/test/targeted/TargetedExpressionTest.java | 2 +- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java index 77cfa8e2d09..d2d03868419 100644 --- a/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java +++ b/src/main/java/spoon/reflect/visitor/DefaultJavaPrettyPrinter.java @@ -692,12 +692,6 @@ private void printCtFieldAccess(CtFieldAccess f) { if (printer.hasNewContent()) { printer.write("."); } - } else if (isStaticField && !isImportedField) { - CtTypeReference declaringType = f.getVariable().getDeclaringType(); - if (declaringType != null) { - printer.write(declaringType.getSimpleName()); - printer.write("."); - } } _context.ignoreStaticAccess(true); } diff --git a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java index c40739797f3..454915ac1c0 100644 --- a/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java +++ b/src/test/java/spoon/test/fieldaccesses/FieldAccessTest.java @@ -387,13 +387,13 @@ public void testGetReference() throws Exception { launcher.getEnvironment().setShouldCompile(true); launcher.setArgs(new String[] {"--output-type", "nooutput" }); launcher.addInputResource("./src/test/java/spoon/test/fieldaccesses/testclasses/"); + launcher.getEnvironment().setAutoImports(true); launcher.run(); final CtClass aClass = launcher.getFactory().Class().get(B.class); - aClass.getFactory().getEnvironment().setAutoImports(true); // now static fields are used with the name of the parent class assertEquals("A.myField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(0).toString()); - assertEquals("B.finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); + assertEquals("finalField", aClass.getElements(new TypeFilter<>(CtFieldWrite.class)).get(1).toString()); } } diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index f3fcf72c9f0..8e269ef8bbc 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -298,7 +298,7 @@ public void testNotImportExecutableType() throws Exception { assertEquals(2, imports.size()); Set expectedImports = new HashSet<>( Arrays.asList("spoon.test.imports.testclasses.internal3.Foo", "java.io.File")); - Set actualImports = imports.stream().map(CtReference::toString).collect(Collectors.toSet()); + Set actualImports = imports.stream().map(CtTypeReference::toString).collect(Collectors.toSet()); assertEquals(expectedImports, actualImports); } diff --git a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java index d0301af3e68..54f52e344e7 100644 --- a/src/test/java/spoon/test/targeted/TargetedExpressionTest.java +++ b/src/test/java/spoon/test/targeted/TargetedExpressionTest.java @@ -177,7 +177,7 @@ public void testTargetsOfStaticFieldAccess() throws Exception { assertEquals(1, staticElements.size()); // Changing behaviour when writing static field, it is now writed using the class name - assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("Foo.p"), staticElements.get(0)); + assertEqualsFieldAccess(new ExpectedTargetedExpression().type(CtFieldWrite.class).declaringType(expectedType).target(expectedTypeAccess).result("p"), staticElements.get(0)); } @Test