From 9694ff929b8980948e618cc83304bcbc5420e5a1 Mon Sep 17 00:00:00 2001 From: Martin Monperrus Date: Wed, 16 Nov 2016 22:28:23 +0100 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 3b0a416890fd588b7adfd172178d020e727e886e Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 7 Dec 2016 12:33:43 +0100 Subject: [PATCH 4/8] 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 9023770a68bc47399178da0f892099677c1bf96c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 20 Dec 2016 13:32:11 +0100 Subject: [PATCH 5/8] Create a file not found exception when a file which not exist is created. Should fix #1042 --- .../support/compiler/FileSystemFile.java | 3 +++ .../spoon/test/api/FileSystemFolderTest.java | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/main/java/spoon/support/compiler/FileSystemFile.java b/src/main/java/spoon/support/compiler/FileSystemFile.java index 76511610486..b272277c436 100644 --- a/src/main/java/spoon/support/compiler/FileSystemFile.java +++ b/src/main/java/spoon/support/compiler/FileSystemFile.java @@ -41,6 +41,9 @@ public FileSystemFile(File file) { super(); try { this.file = file.getCanonicalFile(); + if (!this.file.exists()) { + throw new FileNotFoundException("The following file does not exist: "+this.file.getCanonicalPath()); + } } catch (IOException e) { throw new SpoonException(e); } diff --git a/src/test/java/spoon/test/api/FileSystemFolderTest.java b/src/test/java/spoon/test/api/FileSystemFolderTest.java index 9cc7d5eaf44..94381bb8781 100644 --- a/src/test/java/spoon/test/api/FileSystemFolderTest.java +++ b/src/test/java/spoon/test/api/FileSystemFolderTest.java @@ -3,10 +3,16 @@ import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.FileNotFoundException; import java.util.List; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import spoon.Launcher; +import spoon.LauncherTest; +import spoon.SpoonException; import spoon.compiler.SpoonFolder; import spoon.support.compiler.FileSystemFolder; @@ -19,4 +25,17 @@ public void jarFileIsNotSubfolder() { List subFolders = folder.getSubFolders(); assertTrue(subFolders.isEmpty()); } + + @Rule + public ExpectedException expectedEx = ExpectedException.none(); + + @Test + public void testLauncherWithWrongPathAsInput() { + expectedEx.expect(SpoonException.class); + expectedEx.expectMessage("java.io.FileNotFoundException: The following file does not exist: /Users/urli/Github/spoon/src/wrong/direction/File.java"); + + Launcher spoon = new Launcher(); + spoon.addInputResource("./src/wrong/direction/File.java"); + spoon.buildModel(); + } } From 24d3784d5c52a221cdb913fe4e6627ad6c81968f Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 20 Dec 2016 13:43:17 +0100 Subject: [PATCH 6/8] Fix #1042 by launching the exception only in case of failing getContent --- .../java/spoon/support/compiler/FileSystemFile.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/spoon/support/compiler/FileSystemFile.java b/src/main/java/spoon/support/compiler/FileSystemFile.java index b272277c436..98202eecfad 100644 --- a/src/main/java/spoon/support/compiler/FileSystemFile.java +++ b/src/main/java/spoon/support/compiler/FileSystemFile.java @@ -41,9 +41,6 @@ public FileSystemFile(File file) { super(); try { this.file = file.getCanonicalFile(); - if (!this.file.exists()) { - throw new FileNotFoundException("The following file does not exist: "+this.file.getCanonicalPath()); - } } catch (IOException e) { throw new SpoonException(e); } @@ -51,11 +48,13 @@ public FileSystemFile(File file) { public InputStream getContent() { try { + if (!this.file.exists()) { + throw new FileNotFoundException("The following file does not exist: " + this.file.getCanonicalPath()); + } return new FileInputStream(file); - } catch (FileNotFoundException e) { - Launcher.LOGGER.error(e.getMessage(), e); + } catch (IOException e) { + throw new SpoonException(e); } - return null; } public String getName() { From 876fb55e6b6b157ae3c00cbcfdbaf31587552de2 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 20 Dec 2016 13:49:27 +0100 Subject: [PATCH 7/8] Fix message for the test --- src/test/java/spoon/test/api/FileSystemFolderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/spoon/test/api/FileSystemFolderTest.java b/src/test/java/spoon/test/api/FileSystemFolderTest.java index 94381bb8781..10ffeb741dd 100644 --- a/src/test/java/spoon/test/api/FileSystemFolderTest.java +++ b/src/test/java/spoon/test/api/FileSystemFolderTest.java @@ -32,7 +32,7 @@ public void jarFileIsNotSubfolder() { @Test public void testLauncherWithWrongPathAsInput() { expectedEx.expect(SpoonException.class); - expectedEx.expectMessage("java.io.FileNotFoundException: The following file does not exist: /Users/urli/Github/spoon/src/wrong/direction/File.java"); + expectedEx.expectMessage("java.io.FileNotFoundException"); Launcher spoon = new Launcher(); spoon.addInputResource("./src/wrong/direction/File.java"); From 692019be226c72e6f27cd53200aecf3ee7bdc0d9 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 20 Dec 2016 16:53:46 +0100 Subject: [PATCH 8/8] Change the test to make an assert in the try/catch --- .../java/spoon/test/api/FileSystemFolderTest.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/test/java/spoon/test/api/FileSystemFolderTest.java b/src/test/java/spoon/test/api/FileSystemFolderTest.java index 10ffeb741dd..57c2edb2195 100644 --- a/src/test/java/spoon/test/api/FileSystemFolderTest.java +++ b/src/test/java/spoon/test/api/FileSystemFolderTest.java @@ -26,16 +26,15 @@ public void jarFileIsNotSubfolder() { assertTrue(subFolders.isEmpty()); } - @Rule - public ExpectedException expectedEx = ExpectedException.none(); - @Test public void testLauncherWithWrongPathAsInput() { - expectedEx.expect(SpoonException.class); - expectedEx.expectMessage("java.io.FileNotFoundException"); - Launcher spoon = new Launcher(); spoon.addInputResource("./src/wrong/direction/File.java"); - spoon.buildModel(); + try { + spoon.buildModel(); + } catch (SpoonException spe) { + Throwable containedException = spe.getCause().getCause(); + assertTrue(containedException instanceof FileNotFoundException); + } } }