UnnecessaryThrows should not change API for protected methods

[timtebeek]

• For Pull #2291: Modernize codebase with Java improvements - Unnecessary throws apache/maven#2291
• Needs Mark methods overridden from classes as isOverride() rewrite#5412

[github-actions]

Some suggestions could not be made:

• src/main/resources/META-INF/rewrite/examples.yml
  • lines 2337-2336
  • lines 2352-2364
  • lines 2801-2800
  • lines 2820-2831

[Jenson3210]

What should we do when we the code has 3 levels of inheritance?

class PlainText {
    // someone allows for exceptions 
    protected void check() throws CheckedException {
        basicChecks()
    }
}

class ConfidentialText extends PlainText{
    // someone allows for exceptions 
    @Override
    protected void check() throws CheckedException {
        advancedChecks()
    }
}

class TopSecret extends ConfidentialText {
    // someone allows for exceptions 
    @Override
    protected void check() throws CheckedException {
       doesNotThrow()
    }
}

Will this not clean up the throws clause of ConfidentialText and TopSecret whereas it might have been needed by TopSecret?

[timtebeek]

You're right, thanks! I thought the third level would get the exceptions declared on the top level still, but turns out that's not the case. Adjusted the logic to now look for final on the method or the class on protected methods before we change the signature.

[github-actions]

Some suggestions could not be made:

• src/main/resources/META-INF/rewrite/examples.yml
  • lines 2337-2336
  • lines 2352-2364
  • lines 2801-2800
  • lines 2820-2831

[Jenson3210]

LGTM!
Potentially add a C3 which does not get changed in the tests? Non final class and non final method. Although that is also covered by your original test case, it might be very clear how it behaves and what the difference is between these 3.

[timtebeek]

Thanks for the help improving the approach! On mobile here from the plane, so I'll merge as is 😅