Improve lambda method discovery Fixes: #477 by gayanper · Pull Request #478 · microsoft/java-debug

Gayan Perera (gayanper) · 2023-04-04T19:15:41Z

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.

Jinbo Wang (testforstephen) · 2023-04-07T03:21:37Z

java-debug/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JdtSourceLookUpProvider.java

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.

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.

Gayan Perera (gayanper) · 2023-04-18T16:17:24Z

Jinbo Wang (@testforstephen) WDYT about the latest change ?

Gayan Perera (gayanper) · 2023-05-11T14:52:38Z

Jinbo Wang (@testforstephen) anything pending for approval and merge ?

Jinbo Wang (testforstephen) · 2023-05-12T13:52:24Z

this PR is on the list for May sprint.

Jinbo Wang (testforstephen)

java-debug/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JdtSourceLookUpProvider.java

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);
            }

- 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.

Gayan Perera (gayanper) · 2023-05-22T16:42:42Z

java-debug/com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JdtSourceLookUpProvider.java

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);
            }

This is a good suggestion, I change the code now.

Jinbo Wang (testforstephen)

LGTM. Thanks for contribution.

Gayan Perera (gayanper) requested review from Shi Chen (CsCherrYY), Yan Zhang (Eskibear), Sheng Chen (jdneo) and Jinbo Wang (testforstephen) as code owners April 4, 2023 19:15

Gayan Perera (gayanper) force-pushed the improve_lambda_mthd_discovery branch from f008742 to 0d93da4 Compare May 11, 2023 07:39

Jinbo Wang (testforstephen) reviewed May 22, 2023

View reviewed changes

Comment thread com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/BindingUtils.java Resolved

Comment thread com.microsoft.java.debug.plugin/src/main/java/com/microsoft/java/debug/BindingUtils.java Resolved

Gayan Perera (gayanper) added 3 commits May 22, 2023 18:40

Improve lambda method discovery Fixes: microsoft#477

fed8402

Improve the solution

453fa03

- 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.

improve fix with suggestions

a06f28b

Gayan Perera (gayanper) force-pushed the improve_lambda_mthd_discovery branch from 0d93da4 to a06f28b Compare May 22, 2023 16:41

Jinbo Wang (testforstephen) reviewed May 23, 2023

View reviewed changes

Comment thread ...g.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JdtSourceLookUpProvider.java Resolved

Gayan Perera (gayanper) added 3 commits May 23, 2023 05:49

Move unit name for both scenarios

7d7546a

Fix format

04fd45c

revert back the method name logic since its better than JDT

de8b811

Jinbo Wang (testforstephen) approved these changes May 24, 2023

View reviewed changes

Jinbo Wang (testforstephen) merged commit a011049 into microsoft:main May 24, 2023

Navigation Menu

Repository navigation

Improve lambda method discovery Fixes: #477 - #478

Improve lambda method discovery Fixes: #477#478
Jinbo Wang (testforstephen) merged 6 commits into
microsoft:mainfrom
gayanper:improve_lambda_mthd_discovery

Gayan Perera (gayanper) commented Apr 4, 2023

Uh oh!

Jinbo Wang (testforstephen) commented Apr 7, 2023

Uh oh!

Gayan Perera (gayanper) commented Apr 18, 2023

Uh oh!

Gayan Perera (gayanper) commented May 11, 2023

Uh oh!

Jinbo Wang (testforstephen) commented May 12, 2023

Uh oh!

Jinbo Wang (testforstephen) left a comment

Uh oh!

Uh oh!

Uh oh!

Gayan Perera (gayanper) commented May 22, 2023

Uh oh!

Uh oh!

Jinbo Wang (testforstephen) left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	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());

Conversation

Gayan Perera (gayanper) commented Apr 4, 2023

Uh oh!

Jinbo Wang (testforstephen) commented Apr 7, 2023

Uh oh!

Gayan Perera (gayanper) commented Apr 18, 2023

Uh oh!

Gayan Perera (gayanper) commented May 11, 2023

Uh oh!

Jinbo Wang (testforstephen) commented May 12, 2023

Uh oh!

Jinbo Wang (testforstephen) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Gayan Perera (gayanper) commented May 22, 2023

Uh oh!

Uh oh!

Jinbo Wang (testforstephen) left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants