From a4d7f419a933d9c3aaa21036b4d39c7fe5b0268f Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 16 Oct 2017 16:36:15 +0200 Subject: [PATCH 1/7] Reproduce issue #1607 --- .../java/spoon/test/imports/ImportTest.java | 21 +++++++++++++++++++ .../TestWithGenerics.java | 7 +++++++ 2 files changed, 28 insertions(+) create mode 100644 src/test/resources/import-with-generics/TestWithGenerics.java diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 5d7dec6cf79..5902b17c3f8 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -1230,4 +1230,25 @@ public void testImportStarredPackageWithNonVisibleClass() throws IOException { assertEquals(3, cu.getImports().size()); } + + @Test + public void testImportWithGenerics() throws IOException { + final Launcher launcher = new Launcher(); + launcher.addInputResource("./src/test/resources/import-with-generics/TestWithGenerics.java"); + launcher.getEnvironment().setAutoImports(true); + launcher.getEnvironment().setShouldCompile(true); + launcher.getEnvironment().setNoClasspath(true); + launcher.setSourceOutputDirectory("./target/import-with-generics"); + launcher.run(); + + PrettyPrinter prettyPrinter = launcher.createPrettyPrinter(); + CtType element = launcher.getFactory().Class().get("spoon.test.imports.testclasses.TestWithGenerics"); + List> toPrint = new ArrayList<>(); + toPrint.add(element); + + prettyPrinter.calculate(element.getPosition().getCompilationUnit(), toPrint); + String output = prettyPrinter.getResult(); + + assertTrue(output.contains("import spoon.test.imports.testclasses.withgenerics.Target;")); + } } diff --git a/src/test/resources/import-with-generics/TestWithGenerics.java b/src/test/resources/import-with-generics/TestWithGenerics.java new file mode 100644 index 00000000000..ab60b438c86 --- /dev/null +++ b/src/test/resources/import-with-generics/TestWithGenerics.java @@ -0,0 +1,7 @@ +package spoon.test.imports.testclasses; +/** + * Created by urli on 16/10/2017. + */ +public class TestWithGenerics { + spoon.test.imports.testclasses.withgenerics.Target myfields; +} From 05b69fd7a5e16a4cbf521ba9a6280657fe546f01 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 16 Oct 2017 17:05:19 +0200 Subject: [PATCH 2/7] Bug fix --- .../spoon/support/compiler/jdt/JDTTreeBuilder.java | 4 +++- .../spoon/support/compiler/jdt/ReferenceBuilder.java | 10 +++++++--- .../import-with-generics/TestWithGenerics.java | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java index 64885b43d83..8fd81c86adf 100644 --- a/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java @@ -1244,7 +1244,9 @@ private boolean createParameterizedType(TypeReference parameterizedTypeReference if (skipTypeInAnnotation) { return true; } - context.enter(factory.Code().createTypeAccessWithoutCloningReference(references.buildTypeReference(parameterizedTypeReference, null)), parameterizedTypeReference); + CtTypeReference typeReference = references.buildTypeReference(parameterizedTypeReference, null); + CtTypeAccess typeAccess = factory.Code().createTypeAccessWithoutCloningReference(typeReference); + context.enter(typeAccess, parameterizedTypeReference); return true; } diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index fffe89db9ff..c3ba10be24a 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -130,7 +130,8 @@ CtTypeReference buildTypeReference(TypeReference type, Scope scope) { if (type == null) { return null; } - return buildTypeReferenceInternal(this.getTypeReference(type.resolvedType, type), type, scope); + CtTypeReference typeReference = this.getTypeReference(type.resolvedType, type); + return buildTypeReferenceInternal(typeReference, type, scope); } /** @@ -457,7 +458,8 @@ CtTypeReference getTypeReference(TypeBinding binding, TypeReference ref) insertGenericTypesInNoClasspathFromJDTInSpoon(ref, ctRef); return ctRef; } - return getTypeReference(ref); + CtTypeReference result = getTypeReference(ref); + return result; } CtTypeReference getTypeParameterReference(TypeBinding binding, TypeReference ref) { @@ -542,7 +544,9 @@ CtTypeReference getTypeReference(TypeReference ref) { } if (!res.toString().replace(", ?", ",?").endsWith(CharOperation.toString(ref.getParameterizedTypeName()))) { // verify that we did not match a class that have the same name in a different package - return this.jdtTreeBuilder.getFactory().Type().createReference(CharOperation.toString(ref.getParameterizedTypeName())); + CtTypeReference result = this.jdtTreeBuilder.getFactory().Type().createReference(res.getQualifiedName()); + result.setActualTypeArguments(res.getActualTypeArguments()); + return result; } return res; } diff --git a/src/test/resources/import-with-generics/TestWithGenerics.java b/src/test/resources/import-with-generics/TestWithGenerics.java index ab60b438c86..42164104c81 100644 --- a/src/test/resources/import-with-generics/TestWithGenerics.java +++ b/src/test/resources/import-with-generics/TestWithGenerics.java @@ -3,5 +3,5 @@ * Created by urli on 16/10/2017. */ public class TestWithGenerics { - spoon.test.imports.testclasses.withgenerics.Target myfields; + public spoon.test.imports.testclasses.withgenerics.Target myfields; } From 7fed851cbe53527b7b907271f6efcafb9f132a8b Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 16 Oct 2017 17:24:28 +0200 Subject: [PATCH 3/7] Another bug fix --- .../support/compiler/jdt/ReferenceBuilder.java | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index c3ba10be24a..c460e498bc6 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -537,17 +537,9 @@ CtTypeReference getTypeReference(TypeReference ref) { if (res == null) { return this.jdtTreeBuilder.getFactory().Type().createReference(CharOperation.toString(ref.getParameterizedTypeName())); } - if (inner.getPackage() == null) { - PackageFactory packageFactory = this.jdtTreeBuilder.getFactory().Package(); - CtPackageReference packageReference = index >= 0 ? packageFactory.getOrCreate(concatSubArray(namesParameterized, index)).getReference() : packageFactory.topLevel(); - inner.setPackage(packageReference); - } - if (!res.toString().replace(", ?", ",?").endsWith(CharOperation.toString(ref.getParameterizedTypeName()))) { - // verify that we did not match a class that have the same name in a different package - CtTypeReference result = this.jdtTreeBuilder.getFactory().Type().createReference(res.getQualifiedName()); - result.setActualTypeArguments(res.getActualTypeArguments()); - return result; - } + PackageFactory packageFactory = this.jdtTreeBuilder.getFactory().Package(); + CtPackageReference packageReference = index >= 0 ? packageFactory.getOrCreate(concatSubArray(namesParameterized, index)).getReference() : packageFactory.topLevel(); + inner.setPackage(packageReference); return res; } From 530e4477de2fbac070854ce64cff8f925e13add2 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 16 Oct 2017 17:45:21 +0200 Subject: [PATCH 4/7] Provide another more explicit test and change the bug fix --- .../compiler/jdt/ReferenceBuilder.java | 22 +++++++++++++++---- .../test/reference/TypeReferenceTest.java | 15 +++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index c460e498bc6..d8330fa36bc 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -520,6 +520,13 @@ CtTypeReference getTypeReference(TypeReference ref) { CtTypeReference res = null; CtTypeReference inner = null; final String[] namesParameterized = CharOperation.charArrayToStringArray(ref.getParameterizedTypeName()); + String nameParameterized = CharOperation.toString(ref.getParameterizedTypeName()); + + // JDT return names with parameters like Target, we only want to get Target in this example. + if (nameParameterized.contains("<")) { + nameParameterized = nameParameterized.substring(0, nameParameterized.indexOf("<")); + } + int index = namesParameterized.length - 1; for (; index >= 0; index--) { // Start at the end to get the class name first. @@ -535,11 +542,18 @@ CtTypeReference getTypeReference(TypeReference ref) { inner = main; } if (res == null) { - return this.jdtTreeBuilder.getFactory().Type().createReference(CharOperation.toString(ref.getParameterizedTypeName())); + return this.jdtTreeBuilder.getFactory().Type().createReference(nameParameterized); + } + + if (inner.getPackage() == null) { + PackageFactory packageFactory = this.jdtTreeBuilder.getFactory().Package(); + CtPackageReference packageReference = index >= 0 ? packageFactory.getOrCreate(concatSubArray(namesParameterized, index)).getReference() : packageFactory.topLevel(); + inner.setPackage(packageReference); + } + if (!res.toString().replace(", ?", ",?").endsWith(nameParameterized)) { + // verify that we did not match a class that have the same name in a different package + return this.jdtTreeBuilder.getFactory().Type().createReference(nameParameterized); } - PackageFactory packageFactory = this.jdtTreeBuilder.getFactory().Package(); - CtPackageReference packageReference = index >= 0 ? packageFactory.getOrCreate(concatSubArray(namesParameterized, index)).getReference() : packageFactory.topLevel(); - inner.setPackage(packageReference); return res; } diff --git a/src/test/java/spoon/test/reference/TypeReferenceTest.java b/src/test/java/spoon/test/reference/TypeReferenceTest.java index 35b686b061f..c27b512928d 100644 --- a/src/test/java/spoon/test/reference/TypeReferenceTest.java +++ b/src/test/java/spoon/test/reference/TypeReferenceTest.java @@ -623,4 +623,19 @@ public void testEqualityTypeReference() throws Exception { assertEquals(parameterRef1, parameterRef2); } + + @Test + public void testTypeReferenceWithGenerics() throws Exception { + final Launcher launcher = new Launcher(); + launcher.addInputResource("./src/test/resources/import-with-generics/TestWithGenerics.java"); + launcher.getEnvironment().setAutoImports(true); + launcher.getEnvironment().setNoClasspath(true); + launcher.buildModel(); + + CtField field = launcher.getModel().getElements(new TypeFilter(CtField.class)).get(0); + CtTypeReference fieldTypeRef = field.getType(); + + assertEquals("spoon.test.imports.testclasses.withgenerics.Target", fieldTypeRef.getQualifiedName()); + assertEquals(2, fieldTypeRef.getActualTypeArguments().size()); + } } From ab6e5cdd2d60c99688f79ee67733605b1574d327 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 16 Oct 2017 18:00:39 +0200 Subject: [PATCH 5/7] Use the typename instead of the parameterizedTypeName --- .../java/spoon/support/compiler/jdt/ReferenceBuilder.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java index d8330fa36bc..983dc6a36b7 100644 --- a/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java +++ b/src/main/java/spoon/support/compiler/jdt/ReferenceBuilder.java @@ -521,11 +521,7 @@ CtTypeReference getTypeReference(TypeReference ref) { CtTypeReference inner = null; final String[] namesParameterized = CharOperation.charArrayToStringArray(ref.getParameterizedTypeName()); String nameParameterized = CharOperation.toString(ref.getParameterizedTypeName()); - - // JDT return names with parameters like Target, we only want to get Target in this example. - if (nameParameterized.contains("<")) { - nameParameterized = nameParameterized.substring(0, nameParameterized.indexOf("<")); - } + String typeName = CharOperation.toString(ref.getTypeName()); int index = namesParameterized.length - 1; for (; index >= 0; index--) { @@ -552,7 +548,7 @@ CtTypeReference getTypeReference(TypeReference ref) { } if (!res.toString().replace(", ?", ",?").endsWith(nameParameterized)) { // verify that we did not match a class that have the same name in a different package - return this.jdtTreeBuilder.getFactory().Type().createReference(nameParameterized); + return this.jdtTreeBuilder.getFactory().Type().createReference(typeName); } return res; } From d76d7ef6d6dec9f448b87f704875ef6b9e4e8be5 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 17 Oct 2017 16:58:22 +0200 Subject: [PATCH 6/7] Add contract 1 --- src/test/java/spoon/test/reference/TypeReferenceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/spoon/test/reference/TypeReferenceTest.java b/src/test/java/spoon/test/reference/TypeReferenceTest.java index c27b512928d..b69af0904ac 100644 --- a/src/test/java/spoon/test/reference/TypeReferenceTest.java +++ b/src/test/java/spoon/test/reference/TypeReferenceTest.java @@ -626,6 +626,7 @@ public void testEqualityTypeReference() throws Exception { @Test public void testTypeReferenceWithGenerics() throws Exception { + // contract: in noclasspath, a generic type name should not contain generic information final Launcher launcher = new Launcher(); launcher.addInputResource("./src/test/resources/import-with-generics/TestWithGenerics.java"); launcher.getEnvironment().setAutoImports(true); From fecee9c9163a085252378c3b0e54e0833370c7a8 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 17 Oct 2017 16:59:22 +0200 Subject: [PATCH 7/7] Contract 2 --- src/test/java/spoon/test/imports/ImportTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/spoon/test/imports/ImportTest.java b/src/test/java/spoon/test/imports/ImportTest.java index 5902b17c3f8..14bae826c9c 100644 --- a/src/test/java/spoon/test/imports/ImportTest.java +++ b/src/test/java/spoon/test/imports/ImportTest.java @@ -1233,6 +1233,7 @@ public void testImportStarredPackageWithNonVisibleClass() throws IOException { @Test public void testImportWithGenerics() throws IOException { + // contract: in noclasspath autoimport, we should be able to use generic type final Launcher launcher = new Launcher(); launcher.addInputResource("./src/test/resources/import-with-generics/TestWithGenerics.java"); launcher.getEnvironment().setAutoImports(true);