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.

Sign up for GitHub

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

Jump to bottom

Improve preconditions check #120

Merged

timtebeek merged 38 commits into main from 119-when-using-multiple-before-templates-do-not-require-the-type-present-in-a-single-before-template-to-be-present-for-all-matches

Dec 17, 2024

Contributor

jevanlingen commented Dec 11, 2024 •

edited

Loading

What's changed?

More or less preconditions in recipes, depending on the used types in the @BeforeTemplate methods. Every @BeforeTemplate rules will be its own block of rules, separated by a Preconditions.or block. Possible duplicates will be detected and removed of the final precondition set (called pruning of the preconditions).
Preconditions rules are printed alphabetically (but UsesType comes before UsesMethod).

What's your motivation?

More support for picnics refasten templates.

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files


          Starting point: PreConditionsVerifier

a28be27

jevanlingen linked an issue

that may be closed by this pull request

When using multiple before templates, do not require the type present in a single before template to be present for all matches #119

Closed

jevanlingen added 2 commits

December 12, 2024 08:45


          Starting point: PreConditionsVerifier

495a43e


          Starting point: PreConditionsVerifier

481e084

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifier.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifier.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen commented

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

timtebeek reviewed

View reviewed changes

Contributor

timtebeek left a comment

Great to see the tests already; I've placed some code suggestions to rule out any chance of miscommunication. Thanks for taking this on!

src/test/resources/refaster/PreConditionsVerifier.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen added 3 commits

December 12, 2024 11:29


          Starting point: PreConditionsVerifier (failing tests as intended)

c3ceb41


          Starting point: PreConditionsVerifier (failing tests as intended)

db6081f


          Fix unit test

d5559a9

timtebeek reviewed

View reviewed changes

src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java Outdated Show resolved Hide resolved

jevanlingen added 2 commits

December 12, 2024 15:46


          Implementation

51c5146


          Implementation

3e4bfeb

jevanlingen self-assigned this

jevanlingen marked this pull request as ready for review

December 12, 2024 15:36

timtebeek added the bug label


          improvement

c4528df

timtebeek reviewed

View reviewed changes

src/test/resources/refaster/PreConditionsVerifierRecipes.java Outdated Show resolved Hide resolved

jevanlingen added 4 commits

December 12, 2024 16:59


          improvement

0202ed3


          Fix test

6714f48


          Add extra info about pruning

f8dd36d


          Start new implementation, introduce PreCondition objects

1c9d98e

timtebeek reviewed

View reviewed changes

src/main/java/org/openrewrite/java/template/processor/PreCondition.java Outdated Show resolved Hide resolved


          Start with PreConditionTest

3d9f1a7

jevanlingen and others added 8 commits

December 16, 2024 11:56


          Improve fitsInto

8a392fb


          Apply suggestions from code review

680c7e4

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>


          Merge branch 'main' into 119-when-using-multiple-before-templates-do-…

1e53f66

…not-require-the-type-present-in-a-single-before-template-to-be-present-for-all-matches


          Revert formatting

87bd944


          Revert formatting

d7094c6


          Revert formatting

b65f7c0


          Revert formatting

afe33c0


          Merge branch 'main' into 119-when-using-multiple-before-templates-do-…

b00bb52

…not-require-the-type-present-in-a-single-before-template-to-be-present-for-all-matches

github-actions bot reviewed

View reviewed changes

src/test/resources/refaster/PreConditionsVerifier.java Outdated Show resolved Hide resolved


          Reorder imports in PreConditionsVerifier.java

b512443

timtebeek reviewed

View reviewed changes

src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java Outdated Show resolved Hide resolved

jevanlingen and others added 5 commits

December 16, 2024 13:27


          Change name PreConditionsVerifier => PreconditionsVerifier

0257c95


          Change name PreConditionsVerifier => PreconditionsVerifier

23f9475


          Improvement: extractCommonElements

d458031


          Update src/main/java/org/openrewrite/java/template/processor/Refaster…

…TemplateProcessor.java

Co-authored-by: Tim te Beek <[email protected]>


          Change test accordingly

52d6dbb

github-actions bot reviewed

View reviewed changes

src/test/resources/refaster/PreconditionsVerifier.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreconditionsVerifier.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreconditionsVerifier.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreconditionsVerifier.java Outdated Show resolved Hide resolved

src/test/resources/refaster/PreconditionsVerifier.java Outdated Show resolved Hide resolved


          Revert test

605b1d7

knutwannheden reviewed

View reviewed changes

src/test/resources/refaster/MethodThrowsRecipe.java Show resolved Hide resolved


          Remove unneeded RequiredArgsConstructor

3856a7c

timtebeek reviewed

View reviewed changes

src/main/java/org/openrewrite/java/template/processor/Precondition.java Outdated Show resolved Hide resolved

src/main/java/org/openrewrite/java/template/processor/Precondition.java Show resolved Hide resolved

src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java Outdated Show resolved Hide resolved

jevanlingen and others added 2 commits

December 17, 2024 10:12


          Apply suggestions from code review

7822f2e

Co-authored-by: Tim te Beek <[email protected]>


          Apply suggestions from code review

ef1ed2d

timtebeek approved these changes

View reviewed changes

Contributor

timtebeek left a comment

Great to have the proper minimized preconditions generated now. Thanks a lot!

timtebeek merged commit 6904c9b into main

2 checks passed

timtebeek deleted the 119-when-using-multiple-before-templates-do-not-require-the-type-present-in-a-single-before-template-to-be-present-for-all-matches branch

December 17, 2024 11:38

This was referenced Dec 17, 2024

Generated preconditions not correctly formatted #94

Closed

Upgrade OpenRewrite Templating 1.19.1 -> 1.20.0 PicnicSupermarket/error-prone-support#1470

Merged

This was referenced Dec 19, 2024

Fix indenting #121

Merged

Bugfix: Preconditions of overloaded @BeforeTemplate methods are filtered away #123

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug