Skip to content

Use lambda parameter counts and block bodies for improved resolution#4757

Closed
johannescoetzee wants to merge 1 commit intojavaparser:masterfrom
johannescoetzee:johannes/use-lambda-param-ret-for-resolve
Closed

Use lambda parameter counts and block bodies for improved resolution#4757
johannescoetzee wants to merge 1 commit intojavaparser:masterfrom
johannescoetzee:johannes/use-lambda-param-ret-for-resolve

Conversation

@johannescoetzee
Copy link
Copy Markdown
Collaborator

@johannescoetzee johannescoetzee commented May 16, 2025

This PR improves lambda-related method resolution by using the lambda parameter count and body for disambiguation in two cases described below. I implemented this by adding the required information to the LambdaArgumentTypePlaceholder class, but made sure that, if this information is not provided (such as for MethodReferenceExpr, type resolution behaviour will be the same as before this PR.

Different parameter counts

import java.util.function.Consumer;

class Test {
    void foo(Consumer<String> consumer) {}
    void foo(Runnable r) {}

    void test() {
        foo(input -> {});
    }
}

Consumer.accept has one parameter while Runnable.run has zero, so since the lambda has one parameter, we know this must resolve to foo(Consumer<String>)

Disambiguation by return type

import java.util.function.Consumer;
import java.util.function.Function;

class Test {
    void foo(Consumer<String> consumer) {}
    void foo(Function<Integer, String> func) {}

    void test() {
        foo(input -> {});
    }
}

In this example, the body of the lambda is a block statement which does not contain any return statements. This means that the lambda can only define a method with a void return type, so it must define Consumer<String> (the same would be true for foo(input -> { return; }).

If it were foo(input -> { return ""; }) instead, then the reverse logic would apply. Since the return type of the lambda is not void, it cannot define Consumer and must therefore define Function.

@codecov
Copy link
Copy Markdown

codecov bot commented May 16, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 31.81818% with 30 lines in your changes missing coverage. Please review.

Project coverage is 58.296%. Comparing base (14479e3) to head (8b35eb5).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...parser/resolution/logic/MethodResolutionLogic.java 0.000% 20 Missing ⚠️
...esolution/model/LambdaArgumentTypePlaceholder.java 0.000% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff               @@
##              master     #4757       +/-   ##
===============================================
- Coverage     58.326%   58.296%   -0.030%     
- Complexity      2514      2516        +2     
===============================================
  Files            671       671               
  Lines          38782     38824       +42     
  Branches        7041      7053       +12     
===============================================
+ Hits           22620     22633       +13     
- Misses         13271     13300       +29     
  Partials        2891      2891
Flag Coverage Δ
AlsoSlowTests 58.296% <31.818%> (-0.030%) ⬇️
javaparser-core 58.296% <31.818%> (-0.030%) ⬇️
javaparser-symbol-solver 58.296% <31.818%> (-0.030%) ⬇️
jdk-10 57.837% <31.818%> (-0.030%) ⬇️
jdk-11 57.847% <31.818%> (-0.030%) ⬇️
jdk-12 57.834% <31.818%> (-0.032%) ⬇️
jdk-13 57.847% <31.818%> (-0.030%) ⬇️
jdk-14 58.084% <31.818%> (-0.030%) ⬇️
jdk-15 58.094% <31.818%> (-0.030%) ⬇️
jdk-16 58.068% <31.818%> (-0.020%) ⬇️
jdk-17 58.223% <31.818%> (-0.030%) ⬇️
jdk-18 58.223% <31.818%> (-0.030%) ⬇️
jdk-8 57.849% <31.818%> (-0.030%) ⬇️
jdk-9 57.837% <31.818%> (-0.030%) ⬇️
macos-latest 58.288% <31.818%> (-0.030%) ⬇️
ubuntu-latest 58.273% <31.818%> (-0.030%) ⬇️
windows-latest 58.278% <31.818%> (-0.030%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...symbolsolver/javaparsermodel/JavaParserFacade.java 77.887% <100.000%> (+0.991%) ⬆️
...esolution/model/LambdaArgumentTypePlaceholder.java 0.000% <0.000%> (ø)
...parser/resolution/logic/MethodResolutionLogic.java 0.000% <0.000%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 555e81b...8b35eb5. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@johannescoetzee johannescoetzee force-pushed the johannes/use-lambda-param-ret-for-resolve branch from 7b848a1 to bfc9760 Compare May 19, 2025 14:57
@johannescoetzee
Copy link
Copy Markdown
Collaborator Author

johannescoetzee commented May 19, 2025

I don't understand why the codecov/patch check is failing. The tests are covered, so the logic it's complaining about must be too, otherwise the tests don't pass

@johannescoetzee johannescoetzee changed the title Use lambda parameter counts and block bodies for improved resolution Draft: Use lambda parameter counts and block bodies for improved resolution May 19, 2025
@johannescoetzee
Copy link
Copy Markdown
Collaborator Author

johannescoetzee commented May 19, 2025

This PR might introduce a new crash. I've changed it to a draft while I look into this, so this shouldn't be merged until I've either fixed the new crash or it turns out to be something else.

@johannescoetzee johannescoetzee force-pushed the johannes/use-lambda-param-ret-for-resolve branch 2 times, most recently from 1885685 to 9c281d0 Compare May 20, 2025 11:53
@johannescoetzee johannescoetzee changed the title Draft: Use lambda parameter counts and block bodies for improved resolution Use lambda parameter counts and block bodies for improved resolution May 20, 2025
@johannescoetzee johannescoetzee force-pushed the johannes/use-lambda-param-ret-for-resolve branch from 9c281d0 to 82a6f16 Compare May 20, 2025 12:14
@johannescoetzee
Copy link
Copy Markdown
Collaborator Author

johannescoetzee commented May 20, 2025

I've found the source of the new crash and added the lambdaUsedAsPolymorphicArgument to test it. The issue was that by the time resolution got to the isConflictingLambdaType method, type variables have not been substituted yet, so LambdaInterfaceLogic.getFunctionalMethod crashed with an UnsupportedOperationException when attempting to find the functional interface for the type variable. I've fixed this by simply not rejecting the candidate in this case (which would be the behaviour before this PR), but this does still result in a MethodAmbiguityException in some cases. I've added a disabled test lambdaUsedAsOverloadedPolymorphicArgument1 to illustrate this. In my opinion, handling this case as well would be better left for a separate PR as it would require a deeper dive into the whole resolution process to avoid doing something that would result in duplicate solving tasks.

@johannescoetzee johannescoetzee force-pushed the johannes/use-lambda-param-ret-for-resolve branch from 82a6f16 to 67cd6ea Compare May 22, 2025 10:14
@johannescoetzee johannescoetzee force-pushed the johannes/use-lambda-param-ret-for-resolve branch from 67cd6ea to 8b35eb5 Compare May 22, 2025 10:15
@johannescoetzee
Copy link
Copy Markdown
Collaborator Author

johannescoetzee commented May 22, 2025

I found another deeper issue that comes up when trying to find the corresponding functional interface, so I'm going to close this PR until I've fixed this one and am confident that there aren't any more

@johannescoetzee johannescoetzee mentioned this pull request Jul 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@johannescoetzee