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: Use a better heuristic for guessing type/field references in no classpath mode #5170

Merged
merged 3 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static void initializeContext() {
/* 22 */ "...\n" +
/* 23 */ "}\n";

ctx = new ZippedCodeBaseTestContext(smpl, "src/test/resources/C4JShouldVibrate.zip", false);
ctx = new ZippedCodeBaseTestContext(smpl, "src/test/resources/C4JShouldVibrate.zip", true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason for this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it makes the test pass.

The test expects types to be imported but did not set the auto-import flag.

}

@Test
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/spoon/support/compiler/jdt/JDTTreeBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -1525,11 +1525,11 @@ public boolean visit(QualifiedNameReference qualifiedNameRef, BlockScope scope)
context.enter(factory.Code().createTypeAccessWithoutCloningReference(typeRef), qualifiedNameRef);
return true;
} else if (qualifiedNameRef.binding instanceof ProblemBinding) {
if (context.stack.peek().element instanceof CtInvocation) {
if (helper.isProblemNameRefProbablyTypeRef(qualifiedNameRef)) {
context.enter(helper.createTypeAccessNoClasspath(qualifiedNameRef), qualifiedNameRef);
return true;
} else {
context.enter(helper.createFieldAccessNoClasspath(qualifiedNameRef), qualifiedNameRef);
}
context.enter(helper.createFieldAccessNoClasspath(qualifiedNameRef), qualifiedNameRef);
return true;
} else {
context.enter(
Expand Down
56 changes: 56 additions & 0 deletions src/main/java/spoon/support/compiler/jdt/JDTTreeBuilderHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
*/
package spoon.support.compiler.jdt;

import java.util.Arrays;
import org.eclipse.jdt.core.compiler.CharOperation;
import org.eclipse.jdt.internal.compiler.ast.AbstractMethodDeclaration;
import org.eclipse.jdt.internal.compiler.ast.Argument;
import org.eclipse.jdt.internal.compiler.ast.ExportsStatement;
import org.eclipse.jdt.internal.compiler.ast.FieldReference;
import org.eclipse.jdt.internal.compiler.ast.ImportReference;
import org.eclipse.jdt.internal.compiler.ast.ModuleDeclaration;
import org.eclipse.jdt.internal.compiler.ast.ModuleReference;
import org.eclipse.jdt.internal.compiler.ast.OpensStatement;
Expand Down Expand Up @@ -392,6 +394,60 @@ <T> CtFieldAccess<T> createFieldAccessNoClasspath(SingleNameReference singleName
return va;
}

boolean isProblemNameRefProbablyTypeRef(QualifiedNameReference qualifiedNameReference) {
ContextBuilder contextBuilder = jdtTreeBuilder.getContextBuilder();
if (contextBuilder.compilationunitdeclaration == null) {
return false;
}
if (contextBuilder.compilationunitdeclaration.imports == null) {
return false;
}
char[][] ourName = qualifiedNameReference.tokens;
for (ImportReference anImport : contextBuilder.compilationunitdeclaration.imports) {
char[][] importName = anImport.getImportName();
int i = indexOfSubList(importName, ourName);
if (i > 0) {
boolean extendsToEndOfImport = i + ourName.length == importName.length;
boolean isStaticImport = anImport.isStatic();
if (!isStaticImport) {
// import foo.bar.baz.A; => "A" is probably a type
// import foo.bar.baz.A; => "baz" is probably a type
return true;
}
// import static foo.Bar.bar; => bar is probably a method/field
// import static foo.Bar; => Bar is probably a type
char[] simpleName = qualifiedNameReference.tokens[qualifiedNameReference.tokens.length - 1];
return !extendsToEndOfImport || !Character.isLowerCase(simpleName[0]);
}
}
return false;
}

/**
* Finds the lowest index where {@code needle} appears in {@code haystack}. This is akin to a
* substring search, but JDT uses a String[] to omit separators and a char[] to represent strings.
* If we want to find out where "String" appears in "java.lang.String", we would call
* {@code indexOfSubList(["java", "lang", "String"], ["lang", "String"])} and receive {@code 1}.
*
* @param haystack the haystack to search in
* @param needle the needle to search
* @return the first index where needle appears in haystack
* @see java.util.Collections#indexOfSubList(List, List) Collections#indexOfSubList for a more
* general version that does not correctly handle array equality
*/
private static int indexOfSubList(char[][] haystack, char[][] needle) {
I-Al-Istannen marked this conversation as resolved.
Show resolved Hide resolved
outer:
for (int i = 0; i < haystack.length - needle.length; i++) {
for (int j = 0; j < needle.length; j++) {
if (!Arrays.equals(haystack[i + j], needle[j])) {
continue outer;
}
}
return i;
}
return -1;
}

/**
* In no classpath mode, when we build a field access, we have a binding typed by ProblemBinding.
* We try to get all information we can get from this binding.
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/spoon/reflect/visitor/CtScannerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,9 @@ public void exit(CtElement o) {
// this is a coarse-grain check to see if the scanner changes
// no more exec ref in paramref
// also takes into account the comments
assertEquals(3631, counter.nElement + countOfCommentsInCompilationUnits);
assertEquals(2423, counter.nEnter + countOfCommentsInCompilationUnits);
assertEquals(2423, counter.nExit + countOfCommentsInCompilationUnits);
assertEquals(3667, counter.nElement + countOfCommentsInCompilationUnits);
assertEquals(2449, counter.nEnter + countOfCommentsInCompilationUnits);
assertEquals(2449, counter.nExit + countOfCommentsInCompilationUnits);

// contract: all AST nodes which are part of Collection or Map are visited first by method "scan(Collection|Map)" and then by method "scan(CtElement)"
Counter counter2 = new Counter();
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/spoon/test/imports/ImportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1529,10 +1529,10 @@ public void testMethodChainAutoImports() {
List<CtStatement> statements = ctor.getBody().getStatements();

assertEquals("super(context, attributeSet)", statements.get(0).toString());
assertEquals("mButton = ((Button) (findViewById(page_button_button)))", statements.get(1).toString());
assertEquals("mCurrentActiveColor = getColor(c4_active_button_color)", statements.get(2).toString());
assertEquals("mCurrentActiveColor = getResources().getColor(c4_active_button_color)", statements.get(3).toString());
assertEquals("mCurrentActiveColor = getData().getResources().getColor(c4_active_button_color)", statements.get(4).toString());
assertEquals("mButton = ((Button) (findViewById(id.page_button_button)))", statements.get(1).toString());
assertEquals("mCurrentActiveColor = getColor(color.c4_active_button_color)", statements.get(2).toString());
assertEquals("mCurrentActiveColor = getResources().getColor(color.c4_active_button_color)", statements.get(3).toString());
assertEquals("mCurrentActiveColor = getData().getResources().getColor(color.c4_active_button_color)", statements.get(4).toString());
}

@Test
Expand Down