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 Failures in UseVar Recipe discovered by moderne.io #285

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

MBoegers
Copy link
Contributor

@MBoegers MBoegers commented Sep 6, 2023

What's changed?

What's your motivation?

To fix failures occurring in moderne.io runs, examples:

  • spring-cloud/spring-cloud-contract @ main: spring-cloud-contract-verifier/src/test/resources/contractsToCompile/contract_multipart.java java.lang.IllegalArgumentException: Unable to parse expression from JavaType Unknown
  • spring-projects/spring-data-commons @ main: src/test/java/org/springframework/data/domain/ManagedTypesUnitTests.java: Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template: var typesSupplier = P.<org.springframework.data.domain.ManagedTypesUnitTests.1>/p1/p()
  • // spring-projects/spring-hateoas @ main src/test/java/org/springframework/hateoas/mediatype/html/HtmlInputTypeUnitTests.java: Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template: var numbers = P.<java.util.stream.Stream<org.springframework.hateoas.mediatype.html.HtmlInputTypeUnitTests..>>/p1/p()

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

no, failures are not an option, here ;)

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ IDEA auto-formatter on affected files
  • I've updated the documentation (if applicable)

@MBoegers
Copy link
Contributor Author

MBoegers commented Sep 6, 2023

I have problems to understand the failure 3.
The test src/test/java/org/openrewrite/java/migrate/lang/UseVarKeywordTest.BugFixing#dusplicateTemplate reproduces failure 3.

The Problem seems to be that the used JavaTemplate var #{} = #{any()} in UseVarForGenericMethodInvocations.UseVarForGenericsVisitor which is applied in UseVarForGenericMethodInvocations.UseVarForGenericsVisitor#transformToVarL109 produces two statements and I am unable to see why or even understand which.

@knutwannheden
Copy link
Contributor

Possibly you need to invoke contextSensitive() on the template builder. This is necessary when to template refers to any elements from the context into which it will be inserted.

@timtebeek timtebeek added the bug Something isn't working label Sep 14, 2023
@timtebeek
Copy link
Contributor

Looks like we already support two of the three new tests covered here; Managed to get the third closer by adding the .contextSensitive() as suggested, and fixed some small test issues. Not yet looked at what's needed for a proper fix to that last failing test. Would you like to continue exploring that @MBoegers ?

@MBoegers
Copy link
Contributor Author

I also played around with it but have no clue yet how to fix...
I would love to have them fixed, because breaking some code bases is not a great option, but TBH I have no time to look into this until 07.October. Feel free to step in 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants