-
Notifications
You must be signed in to change notification settings - Fork 181
Improve lambda method discovery Fixes: #477 #478
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
Improve lambda method discovery Fixes: #477 #478
Conversation
Lines 266 to 280 in 31cc033
When parsing the AST for the lambda, we just passed a char array without the context Java model or project, that's why the binding information is not resolved correctly. Your workaround seems OK. Another way to fix this is to find the Java project that the current file belongs to and set the parser to use that project for binding resolver. |
@testforstephen WDYT about the latest change ? |
f008742
to
0d93da4
Compare
@testforstephen anything pending for approval and merge ? |
this PR is on the list for May sprint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lines 266 to 280 in 31cc033
parser.setSource(source.toCharArray()); /** * See the java doc for { @link ASTParser#setResolveBindings(boolean) }. * Binding information is obtained from the Java model. This means that the * compilation unit must be located relative to the Java model. * This happens automatically when the source code comes from either * setSource(ICompilationUnit) or setSource(IClassFile). * When source is supplied by setSource(char[]), the location must be * established explicitly * by setting an environment using setProject(IJavaProject) or * setEnvironment(String [], String [], String [], boolean) * and a unit name setUnitName(String). */ parser.setEnvironment(new String[0], new String[0], null, true); parser.setUnitName(Paths.get(filePath).getFileName().toString()); When parsing the AST for the lambda, we just passed a char array without the context Java model or project, that's why the binding information is not resolved correctly.
A simpler fix is to use the parent Java project as the environment. This works for the case in issue #477.
IFile resource = (IFile) JDTUtils.findResource(JDTUtils.toURI(uri), ResourcesPlugin.getWorkspace().getRoot()::findFilesForLocationURI);
if (resource != null && JdtUtils.isJavaProject(resource.getProject())) {
parser.setProject(JavaCore.create(resource.getProject()));
} else {
parser.setEnvironment(new String[0], new String[0], null, true);
}
com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/BindingUtils.java
Outdated
Show resolved
Hide resolved
com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/BindingUtils.java
Show resolved
Hide resolved
- Use classfile and compilationunit resolution as the first method before fallback to old strategy. - Switch to using jdt.debug lambda helper which resolves lambda method signature with outer local variables.
0d93da4
to
a06f28b
Compare
This is a good suggestion, I change the code now. |
...g.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JdtSourceLookUpProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for contribution.
The issue seems to be when resolving lambda method for reactive operators, the binding resolver is not the default binding resolvers which actually resolves the lambda method.
It seems. the AST construction effects which binding resolver is used at runtime in the node. Therefore switched to the using cache AST when available which seems to resolved the issue. Will do some more testing.