Skip to content

Commit 22897d1

Browse files
committed
UnnecessaryThrows should not change API for protected methods
For apache/maven#2291 Needs openrewrite/rewrite#5412
1 parent 1b942fa commit 22897d1

File tree

2 files changed

+57
-3
lines changed

2 files changed

+57
-3
lines changed

src/main/java/org/openrewrite/staticanalysis/UnnecessaryThrows.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.time.Duration;
3030
import java.util.*;
3131

32+
import static org.openrewrite.java.tree.J.Modifier.Type.*;
33+
3234
public class UnnecessaryThrows extends Recipe {
3335

3436
@Override
@@ -146,6 +148,11 @@ private Set<JavaType.FullyQualified> findExceptionCandidates(J.@Nullable MethodD
146148
return Collections.emptySet();
147149
}
148150

151+
// Do not change the API of methods intended for overriding.
152+
if (method.hasModifier(Protected) && !method.getMethodType().isOverride()) {
153+
return Collections.emptySet();
154+
}
155+
149156
//Collect all checked exceptions.
150157
Set<JavaType.FullyQualified> candidates = new TreeSet<>(Comparator.comparing(JavaType.FullyQualified::getFullyQualifiedName));
151158

@@ -166,9 +173,9 @@ private Set<JavaType.FullyQualified> findExceptionCandidates(J.@Nullable MethodD
166173

167174
//noinspection ConstantConditions
168175
if ((method.getMethodType().getDeclaringType() != null && method.getMethodType().getDeclaringType().getFlags().contains(Flag.Final)) ||
169-
method.isAbstract() || method.hasModifier(J.Modifier.Type.Static) ||
170-
method.hasModifier(J.Modifier.Type.Private) ||
171-
method.hasModifier(J.Modifier.Type.Final)) {
176+
method.isAbstract() || method.hasModifier(Static) ||
177+
method.hasModifier(Private) ||
178+
method.hasModifier(Final)) {
172179
//Consider all checked exceptions as candidates if the type/method are final or the method is private or static.
173180
return candidates;
174181
}

src/test/java/org/openrewrite/staticanalysis/UnnecessaryThrowsTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,4 +395,51 @@ public static class MyException extends Exception { }
395395
)
396396
);
397397
}
398+
399+
@Issue("https://github.com/apache/maven/pull/2291")
400+
@Test
401+
void retainProtectedNonOverrides() {
402+
rewriteRun(
403+
//language=java
404+
java(
405+
"""
406+
import java.io.FileNotFoundException;
407+
class A {
408+
// Someone marked this protected, and added exceptions that implementers can optionally use
409+
protected void method() throws FileNotFoundException {
410+
}
411+
}
412+
"""
413+
),
414+
java(
415+
"""
416+
import java.io.FileNotFoundException;
417+
class B extends A {
418+
@Override
419+
protected void method() throws FileNotFoundException {
420+
throw new FileNotFoundException();
421+
}
422+
}
423+
"""
424+
),
425+
java(
426+
"""
427+
import java.io.FileNotFoundException;
428+
class C extends A {
429+
@Override
430+
protected void method() throws FileNotFoundException {
431+
}
432+
}
433+
""",
434+
// Expected to remove here, as we override, but do not use the exceptions
435+
"""
436+
class C extends A {
437+
@Override
438+
protected void method() {
439+
}
440+
}
441+
"""
442+
)
443+
);
444+
}
398445
}

0 commit comments

Comments
 (0)