openrewrite / rewrite-static-analysis

OpenRewrite recipes for identifying and fixing static analysis issues.
Apache License 2.0
32 stars 54 forks source link

org.openrewrite.staticanalysis.RemoveUnusedPrivateMethods ignores class level @SuppressWarnings("unused") #294

Closed blipper closed 6 months ago

blipper commented 8 months ago

What version of OpenRewrite are you using?

6.6.3

How are you running OpenRewrite?

What is the smallest, simplest way to reproduce the problem?

@SuppressWarnings("unused")
class A {
    @SuppressWarnings("unused")
    private static void foo() {
    }

    private void bar() {
    }

}

What did you expect to see?

@SuppressWarnings("unused")
class A {
    @SuppressWarnings("unused")
    private static void foo() {
    }

    private void bar() {
    }

}

What did you see instead?

@SuppressWarnings("unused")
class A {
    @SuppressWarnings("unused")
    private static void foo() {
    }

}

The looks like that https://github.com/openrewrite/rewrite-static-analysis/blob/main/src/main/java/org/openrewrite/staticanalysis/RemoveUnusedPrivateMethods.java#L62 doesn't look at the class level annotation and is also just assuming that any class annotation means don't do anything. IT should look for @SuppressWarnings("unused") at the class level.

What is the full stack trace of any errors you encountered?

stacktrace output here

Are you interested in contributing a fix to OpenRewrite?

timtebeek commented 7 months ago

Thanks for the report @blipper ! Seems like this could be resolved by adding an override that visits the class, and checking there if unused warnings are suppressed. If so: do not call super and return the input class directly to avoid traversing down at method declarations.

Dinozavvvr commented 6 months ago

Hey! I want to try to do it

timtebeek commented 6 months ago

Sure, thanks a lot! Open a draft PR with just a test and I'll assign the issue to you.

Dinozavvvr commented 6 months ago

Opened draft PR: https://github.com/openrewrite/rewrite-static-analysis/pull/293