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

org.openrewrite.staticanalysis.RemoveUnusedPrivateFields does not recognize @Data Lombok annotation #242

Closed
gwydionmv opened this issue Jan 17, 2024 · 5 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@gwydionmv
Copy link
Contributor

Hello!

Similar to this other open issue here, the recipe RemoveUnusedPrivateFields is deleting private fields even though the class is annotated as @DaTa.

I am using

  • OpenRewrite Maven plugin v5.19.0
  • rewrite-static-analysis v1.2.0

I can help working on fixing this issue if you find it interesting too :D

@gwydionmv gwydionmv added the bug Something isn't working label Jan 17, 2024
@timtebeek
Copy link
Contributor

Hi @gwydionmv ! Thanks for pointing that out and offering to help.

I imagine the fix would be similar to what we do for explicit initialization:

Iterator<Cursor> clz = getCursor().getPathAsCursors(c -> c.getValue() instanceof J.ClassDeclaration);
if (clz.hasNext() && service(AnnotationService.class).matches(clz.next(), LOMBOK_VALUE)) {
return v;
}

Which we then test here
@Test
void ignoreLombokValueField() {
rewriteRun(
spec -> spec.parser(JavaParser.fromJavaVersion().classpath("lombok")),
//language=java
java(
"""
import lombok.Value;
@Value
class Test {
boolean b = false;
}
"""
)
);
}

@timtebeek timtebeek moved this to Backlog in OpenRewrite Jan 17, 2024
@timtebeek
Copy link
Contributor

We would likely need similar handling in RemoveUnusedPrivateFields

public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) {
J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx);

And best to have a matching test as well in
class RemoveUnusedPrivateFieldsTest implements RewriteTest {
@Override
public void defaults(RecipeSpec spec) {
spec.recipe(new RemoveUnusedPrivateFields());
}

@timtebeek timtebeek added the good first issue Good for newcomers label Jan 17, 2024
@timtebeek
Copy link
Contributor

Would be much appreciated if you could provide a fix for that, which I'll gladly review shortly. We can then do the same for #8 too.

gwydionmv pushed a commit to gwydionmv/rewrite-static-analysis that referenced this issue Jan 18, 2024
gwydionmv pushed a commit to gwydionmv/rewrite-static-analysis that referenced this issue Jan 18, 2024
@gwydionmv
Copy link
Contributor Author

I just created the PR @timtebeek 😄 #244

timtebeek added a commit that referenced this issue Jan 18, 2024
* #242 Compatibility with Lombok Data annotation

* #242 Compatibility with Lombok Data annotation - formatting

* Ignore Data, Value, Getter and Setter

* Swap order of check vs variable initializations

* Use AnnotationMatcher of `lombok.*`

---------

Co-authored-by: Gwydion Martin <[email protected]>
Co-authored-by: Tim te Beek <[email protected]>
@timtebeek
Copy link
Contributor

Many thanks! Fixed in

@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenRewrite Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Archived in project
Development

No branches or pull requests

2 participants