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 1dd645d8907fbe6a228d2bcba7fd61a44557ba6c Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 8 Dec 2016 17:41:46 +0100 Subject: [PATCH 5/8] Create a new test to show bug #1025 but it does not throw exception. Show another bug with templateMatcher matching itself. --- .../spoon/test/template/TemplateTest.java | 25 ++++++++++++++++ .../template/testclasses/BServiceImpl.java | 30 +++++++++++++++++++ .../template/testclasses/ContextHelper.java | 22 ++++++++++++++ .../testclasses/SecurityCheckerTemplate.java | 29 ++++++++++++++++++ 4 files changed, 106 insertions(+) create mode 100644 src/test/java/spoon/test/template/testclasses/BServiceImpl.java create mode 100644 src/test/java/spoon/test/template/testclasses/ContextHelper.java create mode 100644 src/test/java/spoon/test/template/testclasses/SecurityCheckerTemplate.java diff --git a/src/test/java/spoon/test/template/TemplateTest.java b/src/test/java/spoon/test/template/TemplateTest.java index 6a57b529ae2..c4399e72f37 100644 --- a/src/test/java/spoon/test/template/TemplateTest.java +++ b/src/test/java/spoon/test/template/TemplateTest.java @@ -8,6 +8,7 @@ import spoon.reflect.code.CtStatement; import spoon.reflect.code.CtTry; import spoon.reflect.declaration.CtClass; +import spoon.reflect.declaration.CtElement; import spoon.reflect.declaration.CtField; import spoon.reflect.declaration.CtMethod; import spoon.reflect.declaration.CtParameter; @@ -18,11 +19,14 @@ import spoon.support.template.Parameters; import spoon.template.Substitution; import spoon.template.TemplateMatcher; +import spoon.test.exceptions.ExceptionTest; +import spoon.test.template.testclasses.SecurityCheckerTemplate; import java.io.File; import java.io.Serializable; import java.rmi.Remote; import java.util.Date; +import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -277,4 +281,25 @@ public void testTemplateInterfaces() throws Exception { assertTrue(superc.getSuperInterfaces().contains(factory.Type().createReference(Remote.class))); } + @Test + public void testTemplateMatcher2() throws Exception { + Launcher spoon = new Launcher(); + spoon.addInputResource("./src/test/java/spoon/test/template/testclasses/ContextHelper.java"); + spoon.addInputResource("./src/test/java/spoon/test/template/testclasses/BServiceImpl.java"); + + spoon.addTemplateResource(new FileSystemFile("./src/test/java/spoon/test/template/testclasses/SecurityCheckerTemplate.java")); + + spoon.buildModel(); + Factory factory = spoon.getFactory(); + + CtClass templateKlass = factory.Class().get(SecurityCheckerTemplate.class); + CtMethod templateMethod = (CtMethod) templateKlass.getElements(new NameFilter("matcher1")).get(0); + CtIf templateRoot = (CtIf) templateMethod.getBody().getStatement(0); + TemplateMatcher matcher = new TemplateMatcher(templateRoot); + + List matches = matcher.find(factory.getModel().getRootPackage()); + + assertEquals(1, matches.size()); + } + } diff --git a/src/test/java/spoon/test/template/testclasses/BServiceImpl.java b/src/test/java/spoon/test/template/testclasses/BServiceImpl.java new file mode 100644 index 00000000000..fa0c84fbc83 --- /dev/null +++ b/src/test/java/spoon/test/template/testclasses/BServiceImpl.java @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2006-2016 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and abiding by the rules of distribution of free software. You can use, modify and/or redistribute the software under the terms of the CeCILL-C license as circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had knowledge of the CeCILL-C license and that you accept its terms. + */ + +package spoon.test.template.testclasses; + +/** + * Created by urli on 08/12/2016. + */ +public class BServiceImpl { + + private String bDao; + + private ContextHelper contextHelper; + + public void hello() { + if(!contextHelper.hasPermission("c")) { + throw new SecurityException(); + } + bDao.toString(); + } +} + diff --git a/src/test/java/spoon/test/template/testclasses/ContextHelper.java b/src/test/java/spoon/test/template/testclasses/ContextHelper.java new file mode 100644 index 00000000000..782cc6eb392 --- /dev/null +++ b/src/test/java/spoon/test/template/testclasses/ContextHelper.java @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2006-2016 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and abiding by the rules of distribution of free software. You can use, modify and/or redistribute the software under the terms of the CeCILL-C license as circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had knowledge of the CeCILL-C license and that you accept its terms. + */ + +package spoon.test.template.testclasses; + +/** + * Created by urli on 08/12/2016. + */ +public class ContextHelper { + + public boolean hasPermission(String value) { + return true; + } +} diff --git a/src/test/java/spoon/test/template/testclasses/SecurityCheckerTemplate.java b/src/test/java/spoon/test/template/testclasses/SecurityCheckerTemplate.java new file mode 100644 index 00000000000..31136f4f3b1 --- /dev/null +++ b/src/test/java/spoon/test/template/testclasses/SecurityCheckerTemplate.java @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2006-2016 INRIA and contributors + * Spoon - http://spoon.gforge.inria.fr/ + * + * This software is governed by the CeCILL-C License under French law and abiding by the rules of distribution of free software. You can use, modify and/or redistribute the software under the terms of the CeCILL-C license as circulated by CEA, CNRS and INRIA at http://www.cecill.info. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details. + * + * The fact that you are presently reading this means that you have had knowledge of the CeCILL-C license and that you accept its terms. + */ + +package spoon.test.template.testclasses; + +import spoon.reflect.code.CtLiteral; +import spoon.template.TemplateParameter; + +/** + * Created by urli on 08/12/2016. + */ +public class SecurityCheckerTemplate { + public TemplateParameter _ctx_; + public CtLiteral _p_; + + public void matcher1() { + if (!_ctx_.S().hasPermission(_p_.S())) { + throw new SecurityException(); + } + } +} From 5b3bbca761a03e9741177f6da7936311b6302732 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 10:17:19 +0100 Subject: [PATCH 6/8] Fix bug #1033 and add a test to check the following contract: a template matcher must find itself to prove it runs correctly. --- .../java/spoon/template/TemplateMatcher.java | 6 +++++ .../spoon/test/template/TemplateTest.java | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/main/java/spoon/template/TemplateMatcher.java b/src/main/java/spoon/template/TemplateMatcher.java index 47add010da3..248cd2abf2f 100644 --- a/src/main/java/spoon/template/TemplateMatcher.java +++ b/src/main/java/spoon/template/TemplateMatcher.java @@ -17,6 +17,7 @@ package spoon.template; import spoon.Launcher; +import spoon.SpoonException; import spoon.reflect.code.CtBlock; import spoon.reflect.code.CtFieldAccess; import spoon.reflect.code.CtInvocation; @@ -187,6 +188,11 @@ public void scan(CtElement element) { } }.scan(targetRoot); + if (!finds.contains(templateRoot)) { + throw new SpoonException("TemplateMatcher was unable to find itself, it certainly express a problem somewhere in the template."); + } else { + finds.remove(templateRoot); + } return finds; } diff --git a/src/test/java/spoon/test/template/TemplateTest.java b/src/test/java/spoon/test/template/TemplateTest.java index c4399e72f37..f039856128f 100644 --- a/src/test/java/spoon/test/template/TemplateTest.java +++ b/src/test/java/spoon/test/template/TemplateTest.java @@ -3,6 +3,7 @@ import org.junit.Test; import spoon.Launcher; import spoon.compiler.SpoonResourceHelper; +import spoon.reflect.code.CtBlock; import spoon.reflect.code.CtIf; import spoon.reflect.code.CtInvocation; import spoon.reflect.code.CtStatement; @@ -20,6 +21,7 @@ import spoon.template.Substitution; import spoon.template.TemplateMatcher; import spoon.test.exceptions.ExceptionTest; +import spoon.test.template.testclasses.BServiceImpl; import spoon.test.template.testclasses.SecurityCheckerTemplate; import java.io.File; @@ -300,6 +302,30 @@ public void testTemplateMatcher2() throws Exception { List matches = matcher.find(factory.getModel().getRootPackage()); assertEquals(1, matches.size()); + + CtElement match = matches.get(0); + + assertTrue("Match is not a if", match instanceof CtIf); + + CtElement matchParent = match.getParent(); + + assertTrue("Match parent is not a block", matchParent instanceof CtBlock); + + CtElement matchParentParent = matchParent.getParent(); + + assertTrue("Match grand parent is not a method", matchParentParent instanceof CtMethod); + + CtMethod methodHello = (CtMethod)matchParentParent; + + assertEquals("Match grand parent is not a method called hello", "hello", methodHello.getSimpleName()); + + CtElement methodParent = methodHello.getParent(); + + assertTrue("Parent of the method is not a class",methodParent instanceof CtClass); + + CtClass bservice = (CtClass) methodParent; + + assertEquals("Parent of the method is not a class called BServiceImpl", "BServiceImpl", bservice.getSimpleName()); } } From e9f55c07398d93e770d8ae785b7a7d07f83c2f2e Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 10:36:50 +0100 Subject: [PATCH 7/8] Fix #1033 more properly. --- src/main/java/spoon/template/TemplateMatcher.java | 14 +++++++++++--- .../java/spoon/test/template/TemplateTest.java | 5 +---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/main/java/spoon/template/TemplateMatcher.java b/src/main/java/spoon/template/TemplateMatcher.java index 248cd2abf2f..ebb9d5ea8af 100644 --- a/src/main/java/spoon/template/TemplateMatcher.java +++ b/src/main/java/spoon/template/TemplateMatcher.java @@ -177,7 +177,7 @@ private CtElement checkListStatements(List teList) { * @return the matched elements */ public List find(final CtElement targetRoot) { - new CtScanner() { + CtScanner scanner = new CtScanner() { @Override public void scan(CtElement element) { if (match(element, templateRoot)) { @@ -186,13 +186,21 @@ public void scan(CtElement element) { } super.scan(element); } - }.scan(targetRoot); + }; + scanner.scan(templateRoot); if (!finds.contains(templateRoot)) { throw new SpoonException("TemplateMatcher was unable to find itself, it certainly express a problem somewhere in the template."); - } else { + } + finds.clear(); + + scanner.scan(targetRoot); + + // This case can occur when we are scanning the entire package for example see TemplateTest#testTemplateMatcherWithWholePackage + if (finds.contains(templateRoot)) { finds.remove(templateRoot); } + return finds; } diff --git a/src/test/java/spoon/test/template/TemplateTest.java b/src/test/java/spoon/test/template/TemplateTest.java index f039856128f..e0135ba0eae 100644 --- a/src/test/java/spoon/test/template/TemplateTest.java +++ b/src/test/java/spoon/test/template/TemplateTest.java @@ -20,8 +20,6 @@ import spoon.support.template.Parameters; import spoon.template.Substitution; import spoon.template.TemplateMatcher; -import spoon.test.exceptions.ExceptionTest; -import spoon.test.template.testclasses.BServiceImpl; import spoon.test.template.testclasses.SecurityCheckerTemplate; import java.io.File; @@ -35,7 +33,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static spoon.testing.utils.ModelUtils.build; public class TemplateTest { @@ -284,7 +281,7 @@ public void testTemplateInterfaces() throws Exception { } @Test - public void testTemplateMatcher2() throws Exception { + public void testTemplateMatcherWithWholePackage() throws Exception { Launcher spoon = new Launcher(); spoon.addInputResource("./src/test/java/spoon/test/template/testclasses/ContextHelper.java"); spoon.addInputResource("./src/test/java/spoon/test/template/testclasses/BServiceImpl.java"); From e060213067fcaa5e7f40e00e56a2b8c36ecb65e1 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 9 Dec 2016 10:52:53 +0100 Subject: [PATCH 8/8] Change the SpoonException message when contract is not fulfilled in TemplateMatcher. --- src/main/java/spoon/template/TemplateMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/spoon/template/TemplateMatcher.java b/src/main/java/spoon/template/TemplateMatcher.java index ebb9d5ea8af..ed065cee621 100644 --- a/src/main/java/spoon/template/TemplateMatcher.java +++ b/src/main/java/spoon/template/TemplateMatcher.java @@ -190,7 +190,7 @@ public void scan(CtElement element) { scanner.scan(templateRoot); if (!finds.contains(templateRoot)) { - throw new SpoonException("TemplateMatcher was unable to find itself, it certainly express a problem somewhere in the template."); + throw new SpoonException("TemplateMatcher was unable to find itself, it certainly indicates a bug. Please revise your template or report an issue."); } finds.clear();