Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: fix pretty-printing of imports for static methods #3716

Merged
merged 7 commits into from
Dec 9, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/java/spoon/reflect/visitor/ImportCleaner.java
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ protected void handleTargetedExpression(CtTargetedExpression<?, ?> targetedExpre
return;
}

if (target != null && target.isImplicit()) {
if (target.isImplicit()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about keeping target != null to avoid crashing with NPE on target.isImplicit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is redundant with the check just above(here):

if (target == null) {
   ...
   return;
}

If target == null, the condition target.isImplicit() will not be hit, isn't?

if (target instanceof CtTypeAccess) {
if (targetedExpression instanceof CtFieldAccess) {
context.addImport(((CtFieldAccess<?>) targetedExpression).getVariable());
Expand Down Expand Up @@ -183,7 +183,8 @@ void addImport(CtReference ref) {
//java.lang is always imported implicitly. Ignore it
return;
}
if (Objects.equals(packageQName, packageRef.getQualifiedName())) {
if (Objects.equals(packageQName, packageRef.getQualifiedName()) &&
!( ref instanceof CtExecutableReference<?> && ((CtExecutableReference<?>) ref).isStatic())) {
//it is reference to a type of the same package. Do not add it
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import spoon.support.compiler.SpoonPom;
import spoon.test.imports.ImportTest;
import spoon.test.prettyprinter.testclasses.AClass;
import spoon.test.prettyprinter.testclasses.ClassUsingStaticMethod;
import spoon.testing.utils.ModelUtils;

import java.io.File;
Expand All @@ -72,18 +73,12 @@
import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -96,6 +91,31 @@
public class DefaultPrettyPrinterTest {
private static final String nl = System.lineSeparator();

@Test
public void testPrintClassWithStaticImportOfMethod() {
final Launcher launcher = new Launcher();
final Factory factory = launcher.getFactory();
factory.getEnvironment().setAutoImports(true);
final SpoonModelBuilder compiler = launcher.createCompiler();
compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/ClassWithStaticMethod.java"));
compiler.addInputSource(new File("./src/test/java/spoon/test/prettyprinter/testclasses/ClassUsingStaticMethod.java"));
compiler.build();

final String expected =
"package spoon.test.prettyprinter.testclasses;" + nl +
"import static spoon.test.prettyprinter.testclasses.ClassWithStaticMethod.findFirst;" + nl +
"public class ClassUsingStaticMethod {" + nl +
" public void callFindFirst() {" + nl +
" findFirst();" + nl +
" new ClassWithStaticMethod().notStaticFindFirst();" + nl +
" }" + nl +
"}";

final CtClass<?> classUsingStaticMethod = (CtClass<?>) factory.Type().get(ClassUsingStaticMethod.class);
final String printed = factory.getEnvironment().createPrettyPrinter().printTypes(classUsingStaticMethod);
assertEquals(expected, printed);
}

@Test
public void printerCanPrintInvocationWithoutException() throws Exception {
String packageName = "spoon.test.subclass.prettyprinter";
Expand Down Expand Up @@ -162,7 +182,7 @@ public void testPrintAClassWithImports() {
final CtClass<?> aClass = (CtClass<?>) factory.Type().get(AClass.class);
//TODO remove that after implicit is set correctly for these cases
assertTrue(factory.getEnvironment().createPrettyPrinter().printTypes(aClass).contains(expected));

assertEquals(expected, aClass.prettyprint());

final CtConstructorCall<?> constructorCall = aClass.getElements(new TypeFilter<CtConstructorCall<?>>(CtConstructorCall.class)).get(0);
Expand Down Expand Up @@ -250,7 +270,7 @@ public void autoImportUsesFullyQualifiedNameWhenImportedNameAlreadyPresent() {
String computed = aClass.getMethodsByName("setFieldUsingExternallyDefinedEnumWithSameNameAsLocal").get(0).toString();
assertEquals("We use FQN for E1", expected, computed);

expected =
expected =
"public void setFieldUsingLocallyDefinedEnum() {" + nl
+ " localField = ENUM.E1.ordinal();" + nl
+ "}";
Expand Down Expand Up @@ -403,7 +423,7 @@ public void testThisConstructorCall() throws Exception {
CtConstructor<?> constr = type.getConstructors().stream().filter(c -> c.getParameters().size() == 1).findFirst().get();
assertEquals("this(v, true)", constr.getBody().getStatement(0).toString());
}

@Test
public void testThisConstructorCall2() throws Exception {
// contract: the this(...) call of another constructor is printed well
Expand All @@ -416,8 +436,8 @@ public void testThisConstructorCall2() throws Exception {
CtClass<?> type = (CtClass) f.Class().get("org.apache.commons.math4.linear.ArrayRealVector");
{
CtConstructor<?> constr = type.getConstructors().stream()
.filter(c ->
c.getParameters().size() == 1
.filter(c ->
c.getParameters().size() == 1
&& "ArrayRealVector".equals(c.getParameters().get(0).getType().getSimpleName())
).findFirst().get();
assertEquals("this(v, true)", constr.getBody().getStatement(0).toString());
Expand Down Expand Up @@ -453,7 +473,7 @@ public void testElseIf() {
}

/**
* This test parses Spoon sources (src/main/java) and pretty prints them in a temporary directory
* This test parses Spoon sources (src/main/java) and pretty prints them in a temporary directory
* to check the compliance of the pretty printer to the set of checkstyle rules used by the Spoon repo.
* As the test takes a long time to run, it is only meant to detect exemples of violation that can, then, be
* used as unit test.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package spoon.test.prettyprinter.testclasses;

/**
* @author Benjamin DANGLOT
* [email protected]
* 02/12/2020
*/

import static spoon.test.prettyprinter.testclasses.ClassWithStaticMethod.findFirst;
public class ClassUsingStaticMethod {

public void callFindFirst() {
findFirst();
new ClassWithStaticMethod().notStaticFindFirst();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package spoon.test.prettyprinter.testclasses;

/**
* @author Benjamin DANGLOT
* [email protected]
* 02/12/2020
*/
public class ClassWithStaticMethod {

void notStaticFindFirst() {
System.out.println("");
}

static void findFirst() {
System.out.println("");
}

}