From adc085795cdf81acd9cdeb0578056f456cd38c14 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 13 Jun 2017 20:38:53 +0200 Subject: [PATCH 1/7] fix: ignore null items in ReplacementVisitor --- .../replace/ReplacementVisitor.java | 22 ++++++++++++++----- .../visitor/replace/ReplacementVisitor.java | 22 ++++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/main/java/spoon/generating/replace/ReplacementVisitor.java b/src/main/java/spoon/generating/replace/ReplacementVisitor.java index 5fc186faf1e..17a5287c335 100644 --- a/src/main/java/spoon/generating/replace/ReplacementVisitor.java +++ b/src/main/java/spoon/generating/replace/ReplacementVisitor.java @@ -74,8 +74,12 @@ private void replaceInMapIfExist(Map mapProtected throw new SpoonException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName()); } V val = (V) replace[0]; - map.put(key, val); - val.setParent(shouldBeDeleted.getParent()); + if (val != null) { + map.put(key, val); + val.setParent(shouldBeDeleted.getParent()); + } else { + map.remove(key); + } } else { map.remove(key); } @@ -95,8 +99,10 @@ private void replaceInSetIfExist(Set setProtected, Repl if (shouldBeDeleted != null) { set.remove(shouldBeDeleted); for (CtElement ele : replace) { - set.add((T) ele); - ele.setParent(shouldBeDeleted.getParent()); + if (ele != null) { + set.add((T) ele); + ele.setParent(shouldBeDeleted.getParent()); + } } listener.set(set); } @@ -117,8 +123,12 @@ private void replaceInListIfExist(List listProtected, R list.remove(index); if (replace.length > 0) { for (int i = 0; i < replace.length; i++) { - list.add(index + i, (T) replace[i]); - replace[i].setParent(shouldBeDeleted.getParent()); + T ele = (T) replace[i]; + if (ele != null) { + list.add(index, ele); + ele.setParent(shouldBeDeleted.getParent()); + index = index + 1; + } } } listener.set(list); diff --git a/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java b/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java index c9d9a52217d..e37a39934bc 100644 --- a/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java +++ b/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java @@ -65,8 +65,12 @@ private void replaceInMapIfEx throw new spoon.SpoonException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName()))); } V val = ((V) (replace[0])); - map.put(key, val); - val.setParent(shouldBeDeleted.getParent()); + if (val != null) { + map.put(key, val); + val.setParent(shouldBeDeleted.getParent()); + }else { + map.remove(key); + } }else { map.remove(key); } @@ -86,8 +90,10 @@ private void replaceInSetIfExist if (shouldBeDeleted != null) { set.remove(shouldBeDeleted); for (spoon.reflect.declaration.CtElement ele : replace) { - set.add(((T) (ele))); - ele.setParent(shouldBeDeleted.getParent()); + if (ele != null) { + set.add(((T) (ele))); + ele.setParent(shouldBeDeleted.getParent()); + } } listener.set(set); } @@ -108,8 +114,12 @@ private void replaceInListIfExis list.remove(index); if ((replace.length) > 0) { for (int i = 0; i < (replace.length); i++) { - list.add((index + i), ((T) (replace[i]))); - replace[i].setParent(shouldBeDeleted.getParent()); + T ele = ((T) (replace[i])); + if (ele != null) { + list.add(index, ele); + ele.setParent(shouldBeDeleted.getParent()); + index = index + 1; + } } } listener.set(list); From 287d58e862aa25b75e99dcfa26f057503e988ed3 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Thu, 15 Jun 2017 19:58:56 +0200 Subject: [PATCH 2/7] test nulls in list --- .../java/spoon/test/replace/ReplaceTest.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/test/java/spoon/test/replace/ReplaceTest.java b/src/test/java/spoon/test/replace/ReplaceTest.java index 636c9405586..78a01fa7d63 100644 --- a/src/test/java/spoon/test/replace/ReplaceTest.java +++ b/src/test/java/spoon/test/replace/ReplaceTest.java @@ -33,6 +33,7 @@ import spoon.test.replace.testclasses.Mole; import spoon.test.replace.testclasses.Tacos; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -163,6 +164,45 @@ public void testReplaceStmtByList() { assertEquals(2, ((CtBlock) sample.getMethod("retry").getBody().getStatement(0)).getStatements().size()); } + @Test + public void testReplaceStmtByListStatements() { + CtClass sample = factory.Package().get("spoon.test.replace.testclasses") + .getType("Foo"); + + // replace retry content by statements + CtStatement stmt = sample.getMethod("retry").getBody().getStatement(0); + List lst = sample.getMethod("statements").getBody().getStatements(); + + // replace a single statement by a statement list + stmt.replace(lst); + + // we should have only 2 statements after (from the stmt list) + assertEquals(2, sample.getMethod("retry").getBody().getStatements().size()); + } + + @Test + public void testReplaceStmtByListStatementsAndNull() { + //contract: null elements in list are ignored + CtClass sample = factory.Package().get("spoon.test.replace.testclasses") + .getType("Foo"); + + // replace retry content by statements + CtStatement stmt = sample.getMethod("retry").getBody().getStatement(0); + List lst = sample.getMethod("statements").getBody().getStatements(); + List lstWithNulls = new ArrayList<>(); + lstWithNulls.add(null); + lstWithNulls.add(lst.get(0)); + lstWithNulls.add(null); + lstWithNulls.add(lst.get(1)); + lstWithNulls.add(null); + + // replace a single statement by a statement list + stmt.replace(lstWithNulls); + + // we should have only 2 statements after (from the stmt list) + assertEquals(2, sample.getMethod("retry").getBody().getStatements().size()); + } + @Test public void testReplaceField() { CtClass sample = factory.Package().get("spoon.test.replace.testclasses") From e9510c159ea8f1b789b68908dc764fa4ee8924d8 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 20 Jun 2017 18:03:09 +0200 Subject: [PATCH 3/7] fix throw exception if replace is impossible --- .../replace/ReplacementVisitor.java | 9 +++- .../replace/InvalidReplaceException.java | 41 +++++++++++++++++++ .../visitor/replace/ReplacementVisitor.java | 8 +++- 3 files changed, 54 insertions(+), 4 deletions(-) create mode 100644 src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java diff --git a/src/main/java/spoon/generating/replace/ReplacementVisitor.java b/src/main/java/spoon/generating/replace/ReplacementVisitor.java index 17a5287c335..1205e2c1038 100644 --- a/src/main/java/spoon/generating/replace/ReplacementVisitor.java +++ b/src/main/java/spoon/generating/replace/ReplacementVisitor.java @@ -19,6 +19,7 @@ import spoon.SpoonException; import spoon.reflect.declaration.CtElement; import spoon.reflect.visitor.CtScanner; +import spoon.support.visitor.replace.InvalidReplaceException; import java.util.ArrayList; import java.util.Collection; @@ -37,12 +38,16 @@ class ReplacementVisitor extends CtScanner { public static void replace(CtElement original, CtElement replace) { try { new ReplacementVisitor(original, replace == null ? EMPTY : new CtElement[]{replace}).scan(original.getParent()); + } catch (InvalidReplaceException e) { + throw e; } catch (SpoonException ignore) { } } public static void replace(CtElement original, Collection replaces) { try { new ReplacementVisitor(original, replaces.toArray(new CtElement[replaces.size()])).scan(original.getParent()); + } catch (InvalidReplaceException e) { + throw e; } catch (SpoonException ignore) { } } @@ -71,7 +76,7 @@ private void replaceInMapIfExist(Map mapProtected if (shouldBeDeleted != null) { if (replace.length > 0) { if (replace.length > 1) { - throw new SpoonException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName()); + throw new InvalidReplaceException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName()); } V val = (V) replace[0]; if (val != null) { @@ -140,7 +145,7 @@ private void replaceElementIfExist(CtElement candidate, ReplaceListener listener CtElement val = null; if (replace.length > 0) { if (replace.length > 1) { - throw new SpoonException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName()); + throw new InvalidReplaceException("Cannot replace single value by multiple values in " + listener.getClass().getSimpleName()); } val = replace[0]; } diff --git a/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java b/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java new file mode 100644 index 00000000000..81df90edad3 --- /dev/null +++ b/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java @@ -0,0 +1,41 @@ +/** + * Copyright (C) 2006-2017 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.support.visitor.replace; + +import spoon.SpoonException; + +/** + * Thrown when replacing of element by another element is not possible + */ +public class InvalidReplaceException extends SpoonException { + + public InvalidReplaceException() { + } + + public InvalidReplaceException(String msg) { + super(msg); + } + + public InvalidReplaceException(Throwable e) { + super(e); + } + + public InvalidReplaceException(String msg, Throwable e) { + super(msg, e); + } + +} diff --git a/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java b/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java index e37a39934bc..caa26af3aa2 100644 --- a/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java +++ b/src/main/java/spoon/support/visitor/replace/ReplacementVisitor.java @@ -26,6 +26,8 @@ public class ReplacementVisitor extends spoon.reflect.visitor.CtScanner { public static void replace(spoon.reflect.declaration.CtElement original, spoon.reflect.declaration.CtElement replace) { try { new spoon.support.visitor.replace.ReplacementVisitor(original, (replace == null ? spoon.support.visitor.replace.ReplacementVisitor.EMPTY : new spoon.reflect.declaration.CtElement[]{ replace })).scan(original.getParent()); + } catch (spoon.support.visitor.replace.InvalidReplaceException e) { + throw e; } catch (spoon.SpoonException ignore) { } } @@ -33,6 +35,8 @@ public static void replace(spoon.reflect.declaration.CtElement original, spoon.r public static void replace(spoon.reflect.declaration.CtElement original, java.util.Collection replaces) { try { new spoon.support.visitor.replace.ReplacementVisitor(original, replaces.toArray(new spoon.reflect.declaration.CtElement[replaces.size()])).scan(original.getParent()); + } catch (spoon.support.visitor.replace.InvalidReplaceException e) { + throw e; } catch (spoon.SpoonException ignore) { } } @@ -62,7 +66,7 @@ private void replaceInMapIfEx if (shouldBeDeleted != null) { if ((replace.length) > 0) { if ((replace.length) > 1) { - throw new spoon.SpoonException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName()))); + throw new spoon.support.visitor.replace.InvalidReplaceException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName()))); } V val = ((V) (replace[0])); if (val != null) { @@ -131,7 +135,7 @@ private void replaceElementIfExist(spoon.reflect.declaration.CtElement candidate spoon.reflect.declaration.CtElement val = null; if ((replace.length) > 0) { if ((replace.length) > 1) { - throw new spoon.SpoonException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName()))); + throw new spoon.support.visitor.replace.InvalidReplaceException(("Cannot replace single value by multiple values in " + (listener.getClass().getSimpleName()))); } val = replace[0]; } From a32823351be83ac13a69bf7a008e632d967fad6d Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 20 Jun 2017 18:03:26 +0200 Subject: [PATCH 4/7] test replace in Map --- .../replace/InvalidReplaceException.java | 82 +++++++++---------- .../spoon/test/annotation/AnnotationTest.java | 58 +++++++++++++ 2 files changed, 99 insertions(+), 41 deletions(-) diff --git a/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java b/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java index 81df90edad3..d88d5fe50d1 100644 --- a/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java +++ b/src/main/java/spoon/support/visitor/replace/InvalidReplaceException.java @@ -1,41 +1,41 @@ -/** - * Copyright (C) 2006-2017 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.support.visitor.replace; - -import spoon.SpoonException; - -/** - * Thrown when replacing of element by another element is not possible - */ -public class InvalidReplaceException extends SpoonException { - - public InvalidReplaceException() { - } - - public InvalidReplaceException(String msg) { - super(msg); - } - - public InvalidReplaceException(Throwable e) { - super(e); - } - - public InvalidReplaceException(String msg, Throwable e) { - super(msg, e); - } - -} +/** + * Copyright (C) 2006-2017 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.support.visitor.replace; + +import spoon.SpoonException; + +/** + * Thrown when replacing of element by another element is not possible + */ +public class InvalidReplaceException extends SpoonException { + + public InvalidReplaceException() { + } + + public InvalidReplaceException(String msg) { + super(msg); + } + + public InvalidReplaceException(Throwable e) { + super(e); + } + + public InvalidReplaceException(String msg, Throwable e) { + super(msg, e); + } + +} diff --git a/src/test/java/spoon/test/annotation/AnnotationTest.java b/src/test/java/spoon/test/annotation/AnnotationTest.java index 70b6344f66d..c21c5699257 100644 --- a/src/test/java/spoon/test/annotation/AnnotationTest.java +++ b/src/test/java/spoon/test/annotation/AnnotationTest.java @@ -4,6 +4,7 @@ import org.junit.Test; import spoon.Launcher; import spoon.OutputType; +import spoon.SpoonException; import spoon.processing.AbstractAnnotationProcessor; import spoon.processing.ProcessingManager; import spoon.reflect.annotations.PropertyGetter; @@ -66,6 +67,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.Target; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Set; @@ -77,6 +79,7 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static spoon.testing.utils.ModelUtils.buildClass; import static spoon.testing.utils.ModelUtils.canBeBuilt; @@ -1028,4 +1031,59 @@ public void testCreateAnnotation() throws Exception { assertTrue(type.isAnnotationType()); assertSame(type, type.getReference().getDeclaration()); } + + @Test + public void testReplaceAnnotationValue() throws Exception { + CtType type = this.factory.Type().get("spoon.test.annotation.testclasses.Main"); + + CtMethod m1 = type.getElements(new NameFilter>("m1")).get(0); + + List> annotations = m1.getAnnotations(); + assertEquals(1, annotations.size()); + + CtAnnotation a = annotations.get(0); + AnnotParamTypes annot = (AnnotParamTypes) a.getActualAnnotation(); + + //contract: test replace of single value + CtExpression integerValue = a.getValue("integer"); + assertEquals(42, ((CtLiteral) integerValue).getValue().intValue()); + assertEquals(42, annot.integer()); + integerValue.replace(factory.createLiteral(17)); + CtExpression newIntegerValue = a.getValue("integer"); + assertEquals(17, ((CtLiteral) newIntegerValue).getValue().intValue()); + assertEquals(17, annot.integer()); + + //contract: replacing of single value of map by multiple values must fail + //even if second value is null + try { + a.getValue("integer").replace(Arrays.asList(factory.createLiteral(18), null)); + fail(); + } catch (SpoonException e) { + //OK + } + + //contract: replacing of single value by no value + a.getValue("integer").delete(); + assertNull(a.getValue("integer")); + try { + annot.integer(); + fail(); + } catch (NullPointerException e) { + //OK - fails because int cannot be null + } + a.getValue("string").delete(); + assertNull(a.getValue("string")); + assertNull(annot.string()); + + //contract: test replace of item in collection + assertEquals(1, annot.integers().length); + assertEquals(42, annot.integers()[0]); + CtNewArray integersNewArray = (CtNewArray)a.getValue("integers"); + integersNewArray.getElements().get(0).replace(Arrays.asList(null, factory.createLiteral(101), null, factory.createLiteral(102))); + assertEquals(2, annot.integers().length); + assertEquals(101, annot.integers()[0]); + assertEquals(102, annot.integers()[1]); + } + + } From 47cf6cdf837606adada8d44ebe5420f3c5fa033b Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 20 Jun 2017 18:44:04 +0200 Subject: [PATCH 5/7] test invalid replace of single value by multiple --- src/test/java/spoon/test/replace/ReplaceTest.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/test/java/spoon/test/replace/ReplaceTest.java b/src/test/java/spoon/test/replace/ReplaceTest.java index 78a01fa7d63..659430414bf 100644 --- a/src/test/java/spoon/test/replace/ReplaceTest.java +++ b/src/test/java/spoon/test/replace/ReplaceTest.java @@ -30,6 +30,7 @@ import spoon.reflect.visitor.filter.NameFilter; import spoon.reflect.visitor.filter.ReferenceTypeFilter; import spoon.reflect.visitor.filter.TypeFilter; +import spoon.support.visitor.replace.InvalidReplaceException; import spoon.test.replace.testclasses.Mole; import spoon.test.replace.testclasses.Tacos; @@ -40,6 +41,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; import static spoon.testing.utils.ModelUtils.build; import static spoon.testing.utils.ModelUtils.buildClass; @@ -390,13 +392,21 @@ public void testReplaceExecutableReferenceByAnotherOne() throws Exception { final CtExecutableReference oldExecutable = inv.getExecutable(); final CtExecutableReference newExecutable = factory.Executable().createReference("void java.io.PrintStream#print(java.lang.String)"); - assertEquals(oldExecutable, inv.getExecutable()); + assertSame(oldExecutable, inv.getExecutable()); assertEquals("java.io.PrintStream#println(java.lang.String)", inv.getExecutable().toString()); oldExecutable.replace(newExecutable); - assertEquals(newExecutable, inv.getExecutable()); + assertSame(newExecutable, inv.getExecutable()); assertEquals("java.io.PrintStream#print(java.lang.String)", inv.getExecutable().toString()); + + //contract: replace of single value by multiple values in single value field must fail + try { + newExecutable.replace(Arrays.asList(oldExecutable, null)); + fail(); + } catch (InvalidReplaceException e) { + //OK + } } @Test From 09f413fec9082f3fa54108864c26daee322e736d Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 20 Jun 2017 19:22:25 +0200 Subject: [PATCH 6/7] test replace(null) --- src/test/java/spoon/test/annotation/AnnotationTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/spoon/test/annotation/AnnotationTest.java b/src/test/java/spoon/test/annotation/AnnotationTest.java index c21c5699257..9d629199730 100644 --- a/src/test/java/spoon/test/annotation/AnnotationTest.java +++ b/src/test/java/spoon/test/annotation/AnnotationTest.java @@ -1071,8 +1071,10 @@ public void testReplaceAnnotationValue() throws Exception { } catch (NullPointerException e) { //OK - fails because int cannot be null } - a.getValue("string").delete(); + //contract: replace with null value means remove + a.getValue("string").replace((CtElement) null); assertNull(a.getValue("string")); + //contract: check that null value can be returned assertNull(annot.string()); //contract: test replace of item in collection From cb55a4d530b4c2114dcb6ea00a791445fcb14253 Mon Sep 17 00:00:00 2001 From: Pavel Vojtechovsky Date: Tue, 20 Jun 2017 20:46:38 +0200 Subject: [PATCH 7/7] test replace(Collection.singletonList(null)) --- src/test/java/spoon/test/annotation/AnnotationTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/spoon/test/annotation/AnnotationTest.java b/src/test/java/spoon/test/annotation/AnnotationTest.java index 9d629199730..ba9fd40f6ea 100644 --- a/src/test/java/spoon/test/annotation/AnnotationTest.java +++ b/src/test/java/spoon/test/annotation/AnnotationTest.java @@ -68,6 +68,7 @@ import java.lang.annotation.Target; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -1077,6 +1078,12 @@ public void testReplaceAnnotationValue() throws Exception { //contract: check that null value can be returned assertNull(annot.string()); + //contract: replace with null value in collection means remove + a.getValue("clazz").replace(Collections.singletonList(null)); + assertNull(a.getValue("clazz")); + //contract: check that null value can be returned + assertNull(annot.clazz()); + //contract: test replace of item in collection assertEquals(1, annot.integers().length); assertEquals(42, annot.integers()[0]);