eclipse-jdt / eclipse-jdt/eclipse.jdt.core

[Search] LocalVariableLocator fails to find LocalVariable coming from DOM if this has leading comments

Open
#2,310 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
237
Forks
195
Avg merge
1d 10h
Merged PRs (30d)
49

Description

Search can miss occurrence if the input element is coming from ASTParser/DOM.

From JavaSearchGenericFieldsTest, add and run
```java
public void testElementPatternLocalVariables09_ASTParser() throws CoreException {
IJavaSearchScope scope = getJavaSearchScope15("g4.v.ref", false);
ILocalVariable localVar = getLocalVariable_ASTParser("/JavaSearch15/src/g4/v/ref/R5.java", "gen_wld, // simple", "gen_wld");
search(localVar, ALL_OCCURRENCES, scope, this.resultCollector);
localVar = getLocalVariable_ASTParser("/JavaSearch15/src/g4/v/ref/R5.java", "gen_www, // simple", "gen_www");
search(localVar, ALL_OCCURRENCES, scope, this.resultCollector);
localVar = getLocalVariable_ASTParser("/JavaSearch15/src/g4/v/ref/R5.java", "gen_obj) // simple", "gen_obj");
search(localVar, ALL_OCCURRENCES, scope, this.resultCollector);
assertSearchResults(
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>).gen_wld [gen_wld] EXACT_MATCH\n" +
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>) [gen_wld] EXACT_MATCH\n" +
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>).gen_www [gen_www] EXACT_MATCH\n" +
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>) [gen_www] EXACT_MATCH\n" +
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>).gen_obj [gen_obj] EXACT_MATCH\n" +
"src/g4/v/ref/R5.java void g4.v.ref.R5.simple_name(GM,GS,GS>, GM,GS>,GS>>>, GM,GS,GS>) [gen_obj] EXACT_MATCH",
this.resultCollector);
}
private ILocalVariable getLocalVariable_ASTParser(String cuPath, String selectAt, String selection) throws JavaModelException {
ICompilationUnit cu = getCompilationUnit(cuPath);
ASTParser parser = ASTParser.newParser(AST.getJLSLatest());
parser.setResolveBindings(true);
parser.setSource(cu);
CompilationUnit unit = (CompilationUnit)parser.createAST(null);
Name name = (Name)NodeFinder.perform(unit, cu.getSource().indexOf(selectAt), selection.length());
IVariableBinding binding = (IVariableBinding)name.resolveBinding();
return (ILocalVariable)binding.getJavaElement();
}
```

If you compare with existing non-ASTParser-based testElementPatternLocalVariables09, you'll notice that an important difference is the `declarationSourceStart` of the ILocalVariable: when this has prefix comments, the ILocalVariable that is derived from DOM+Bindings ignores those when computing the declarationSourceStart (which is not bad per se).
But later, in LocalVariableLocator, there is this code (and other similar ones) that prevents from finding the element because of this hardcoded expectation
https://github.com/eclipse-jdt/eclipse.jdt.core/blob/8cf2f7acf847042de372ecec29794184ac33c107/org.eclipse.jdt.core/search/org/eclipse/jdt/internal/core/search/matching/LocalVariableLocator.java#L42

One possible remediation would be relax somehow the condition in LocalVariableLocator, either by checking same parent + range overlaps, or by trying a second attempt at comparing the ranges ignoring leading comments.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.