diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalClass.java b/src/main/java/org/openrewrite/staticanalysis/FinalClass.java index 7910ff0d5..e1b88ce71 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalClass.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalClass.java @@ -15,17 +15,42 @@ */ package org.openrewrite.staticanalysis; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Preconditions; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.*; import org.openrewrite.staticanalysis.java.JavaFileChecker; +import java.util.List; import java.util.Set; +import static java.util.Collections.emptyList; import static java.util.Collections.singleton; +@EqualsAndHashCode(callSuper = false) +@Value public class FinalClass extends Recipe { + + @Option(displayName = "Include never-extended classes", + description = "Finalize classes that are never extended anywhere in the codebase", + required = false) + @Nullable + Boolean includeNeverExtended; + + @Option(displayName = "Exclude packages", + description = "Package patterns to exclude from never-extended finalization (e.g., com.example.api.*)", + example = "com.example.api.*", + required = false) + @Nullable + List excludePackages; + + @Option(displayName = "Exclude annotations", + description = "Classes with these annotations won't be finalized when using never-extended mode", + example = "@ExtensionPoint", + required = false) + @Nullable + List excludeAnnotations; + @Override public String getDisplayName() { return "Finalize classes with private constructors"; @@ -33,7 +58,8 @@ public String getDisplayName() { @Override public String getDescription() { - return "Adds the `final` modifier to classes that expose no public or package-private constructors."; + return "Adds the `final` modifier to classes that expose no public or package-private constructors." + + "Optionally, can also finalize classes that are never extended anywhere in the codebase."; } @Override @@ -43,6 +69,10 @@ public Set getTags() { @Override public TreeVisitor getVisitor() { - return Preconditions.check(new JavaFileChecker<>(), new FinalClassVisitor()); + boolean includeNeverExtendedFlag = Boolean.TRUE.equals(includeNeverExtended); + List excludePackagesList = excludePackages != null ? excludePackages : emptyList(); + List excludeAnnotationsList = excludeAnnotations != null ? excludeAnnotations : emptyList(); + FinalClassVisitor visitor = new FinalClassVisitor(includeNeverExtendedFlag, excludePackagesList, excludeAnnotationsList); + return Preconditions.check(new JavaFileChecker<>(), visitor); } } diff --git a/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java b/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java index e123525c5..b3d0044bb 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java +++ b/src/main/java/org/openrewrite/staticanalysis/FinalClassVisitor.java @@ -37,6 +37,21 @@ public class FinalClassVisitor extends JavaIsoVisitor { final Set typesToFinalize = new HashSet<>(); final Set typesToNotFinalize = new HashSet<>(); + private final boolean includeNeverExtended; + private final List excludePackages; + private final List excludeAnnotations; + + public FinalClassVisitor() { + this(false, emptyList(), emptyList()); + } + + public FinalClassVisitor(boolean includeNeverExtended, + List excludePackages, + List excludeAnnotations) { + this.includeNeverExtended = includeNeverExtended; + this.excludePackages = excludePackages != null ? excludePackages : emptyList(); + this.excludeAnnotations = excludeAnnotations != null ? excludeAnnotations : emptyList(); + } @Override public @Nullable J visit(@Nullable Tree tree, ExecutionContext ctx) { @@ -44,6 +59,8 @@ public class FinalClassVisitor extends JavaIsoVisitor { if (visitRoot == null && tree != null) { visitRoot = tree; root = true; + typesToFinalize.clear(); + typesToNotFinalize.clear(); } J result = super.visit(tree, ctx); if (root) { @@ -60,13 +77,47 @@ public class FinalClassVisitor extends JavaIsoVisitor { public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDeclaration, ExecutionContext ctx) { J.ClassDeclaration cd = super.visitClassDeclaration(classDeclaration, ctx); + if (cd.getType() != null) { + excludeSupertypes(cd.getType()); + } + if (cd.getKind() != J.ClassDeclaration.Kind.Type.Class || cd.hasModifier(J.Modifier.Type.Abstract) || cd.hasModifier(J.Modifier.Type.Final) || cd.getType() == null) { return cd; } - excludeSupertypes(cd.getType()); + if (cd.hasModifier(J.Modifier.Type.Sealed) || cd.hasModifier(J.Modifier.Type.NonSealed)) { + return cd; + } + if (!includeNeverExtended) { + boolean allPrivate = true; + int constructorCount = 0; + for (Statement s : cd.getBody().getStatements()) { + if (s instanceof J.MethodDeclaration && ((J.MethodDeclaration) s).isConstructor()) { + J.MethodDeclaration constructor = (J.MethodDeclaration) s; + constructorCount++; + if (!constructor.hasModifier(J.Modifier.Type.Private)) { + allPrivate = false; + } + } + if (constructorCount > 0 && !allPrivate) { + return cd; + } + } + + if (constructorCount > 0 && passesExclusionFilters(cd)) { + typesToFinalize.add(cd.getType().getFullyQualifiedName()); + } + } else { + handleExtendedFinalization(cd); + } + + return cd; + } + + private void handleExtendedFinalization(J.ClassDeclaration cd) { + String fullyQualifiedName = cd.getType().getFullyQualifiedName(); boolean allPrivate = true; int constructorCount = 0; for (Statement s : cd.getBody().getStatements()) { @@ -78,20 +129,68 @@ public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDeclarat } } if (constructorCount > 0 && !allPrivate) { - return cd; + break; } } - if (constructorCount > 0) { - typesToFinalize.add(cd.getType().getFullyQualifiedName()); + if (constructorCount > 0 && allPrivate) { + typesToFinalize.add(fullyQualifiedName); } - return cd; + if (passesExclusionFilters(cd)) { + typesToFinalize.add(fullyQualifiedName); + } + } + + private boolean passesExclusionFilters(J.ClassDeclaration cd) { + if (cd.getType() == null) { + return false; + } + + if (isPackageExcluded(cd.getType().getFullyQualifiedName())) { + return false; + } + + return !hasExcludedAnnotation(cd); + } + + private boolean isPackageExcluded(String className) { + return excludePackages.stream() + .anyMatch(pattern -> matchesPattern(className, pattern)); + } + + private boolean matchesPattern(String className, String pattern) { + if (pattern.endsWith("*")) { + String prefix = pattern.substring(0, pattern.length() - 1); + return className.startsWith(prefix); + } + return className.equals(pattern); + } + + private boolean hasExcludedAnnotation(J.ClassDeclaration classDecl) { + return classDecl.getLeadingAnnotations().stream() + .anyMatch(annotation -> { + String annotationName = annotation.getAnnotationType().toString(); + if (excludeAnnotations.contains(annotationName) || excludeAnnotations.contains("@" + annotationName)) { + return true; + } + + // Check simple name (e.g., "Configuration" for "org.springframework.context.annotation.Configuration") + String simpleName = getSimpleName(annotationName); + if (excludeAnnotations.contains(simpleName) || excludeAnnotations.contains("@" + simpleName)) { + return true; + } + return false; + }); + } + + private String getSimpleName(String fullyQualifiedName) { + int lastDot = fullyQualifiedName.lastIndexOf('.'); + return lastDot >= 0 ? fullyQualifiedName.substring(lastDot + 1) : fullyQualifiedName; } private void excludeSupertypes(JavaType.FullyQualified type) { - if (type.getSupertype() != null && type.getOwningClass() != null && - typesToNotFinalize.add(type.getSupertype().getFullyQualifiedName())) { + if (type.getSupertype() != null && typesToNotFinalize.add(type.getSupertype().getFullyQualifiedName())) { excludeSupertypes(type.getSupertype()); } } @@ -104,31 +203,34 @@ private static class FinalizingVisitor extends JavaIsoVisitor private final Set typesToFinalize; public FinalizingVisitor(Set typesToFinalize) { - this.typesToFinalize = typesToFinalize; + this.typesToFinalize = new HashSet<>(typesToFinalize); } @Override public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { J.ClassDeclaration cd = super.visitClassDeclaration(classDecl, ctx); - if (cd.getType() != null && typesToFinalize.remove(cd.getType().getFullyQualifiedName())) { - List modifiers = new ArrayList<>(cd.getModifiers()); - modifiers.add(new J.Modifier(randomId(), Space.EMPTY, Markers.EMPTY, null, J.Modifier.Type.Final, emptyList())); - modifiers = sortModifiers(modifiers); - cd = cd.withModifiers(modifiers); - if (cd.getType() instanceof JavaType.Class && !cd.getType().hasFlags(Flag.Final)) { - Set flags = new HashSet<>(cd.getType().getFlags()); - flags.add(Flag.Final); - cd = cd.withType(((JavaType.Class) cd.getType()).withFlags(flags)); - } - - // Temporary work around until issue https://github.com/openrewrite/rewrite/issues/2348 is implemented. - if (!cd.getLeadingAnnotations().isEmpty()) { - // Setting the prefix to empty will cause the `Spaces` visitor to fix the formatting. - cd = cd.getPadding().withKind(cd.getPadding().getKind().withPrefix(Space.EMPTY)); + if (cd.getType() != null) { + String fullyQualifiedName = cd.getType().getFullyQualifiedName(); + if (typesToFinalize.remove(fullyQualifiedName)) { + List modifiers = new ArrayList<>(cd.getModifiers()); + modifiers.add(new J.Modifier(randomId(), Space.EMPTY, Markers.EMPTY, null, J.Modifier.Type.Final, emptyList())); + modifiers = sortModifiers(modifiers); + cd = cd.withModifiers(modifiers); + if (cd.getType() instanceof JavaType.Class && !cd.getType().hasFlags(Flag.Final)) { + Set flags = new HashSet<>(cd.getType().getFlags()); + flags.add(Flag.Final); + cd = cd.withType(((JavaType.Class) cd.getType()).withFlags(flags)); + } + + // Temporary work around until issue https://github.com/openrewrite/rewrite/issues/2348 is implemented. + if (!cd.getLeadingAnnotations().isEmpty()) { + // Setting the prefix to empty will cause the `Spaces` visitor to fix the formatting. + cd = cd.getPadding().withKind(cd.getPadding().getKind().withPrefix(Space.EMPTY)); + } + + assert getCursor().getParent() != null; + cd = autoFormat(cd, cd.getName(), ctx, getCursor().getParent()); } - - assert getCursor().getParent() != null; - cd = autoFormat(cd, cd.getName(), ctx, getCursor().getParent()); } return cd; } diff --git a/src/test/java/org/openrewrite/staticanalysis/FinalClassTest.java b/src/test/java/org/openrewrite/staticanalysis/FinalClassTest.java index a2e44f85e..8d322f84a 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FinalClassTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FinalClassTest.java @@ -21,13 +21,16 @@ import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import java.util.Arrays; +import java.util.List; + import static org.openrewrite.java.Assertions.java; class FinalClassTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new FinalClass()); + spec.recipe(new FinalClass(false, null, null)); } @DocumentExample @@ -189,4 +192,506 @@ private B() {} ) ); } + + @Test + void neverExtendedClassesWithIncludeNeverExtended() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + class Person { + private String name; + + public Person(String name) { + this.name = name; + } + + public void setName(String name) { + this.name = name; + } + } + + class Address { + private String street; + + public Address(String street) { + this.street = street; + } + } + """, + """ + final class Person { + private String name; + + public Person(String name) { + this.name = name; + } + + public void setName(String name) { + this.name = name; + } + } + + final class Address { + private String street; + + public Address(String street) { + this.street = street; + } + } + """ + ) + ); + } + + @Test + void extendedClassesNotFinalized() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + class Vehicle { + protected String brand; + + public Vehicle(String brand) { + this.brand = brand; + } + } + + class Car extends Vehicle { + private int doors; + + public Car(String brand, int doors) { + super(brand); + this.doors = doors; + } + } + """, + """ + class Vehicle { + protected String brand; + + public Vehicle(String brand) { + this.brand = brand; + } + } + + final class Car extends Vehicle { + private int doors; + + public Car(String brand, int doors) { + super(brand); + this.doors = doors; + } + } + """ + ) + ); + } + + @Test + void abstractClassesNotFinalized() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + abstract class Animal { + protected String name; + + public Animal(String name) { + this.name = name; + } + + public abstract void makeSound(); + } + + class Dog extends Animal { + public Dog(String name) { + super(name); + } + + @Override + public void makeSound() { + System.out.println("Woof!"); + } + } + """, + """ + abstract class Animal { + protected String name; + + public Animal(String name) { + this.name = name; + } + + public abstract void makeSound(); + } + + final class Dog extends Animal { + public Dog(String name) { + super(name); + } + + @Override + public void makeSound() { + System.out.println("Woof!"); + } + } + """ + ) + ); + } + + @Test + void excludePackagePatterns() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, Arrays.asList("com.example.api.*"), null)), + java( + """ + package com.example.api; + + class PublicApi { + public void doSomething() { + // This should not be finalized due to package exclusion + } + } + """ + ) + ); + } + + @Test + void excludePackagePatternsWithIncludeNeverExtendedFalse() { + rewriteRun( + spec -> spec.recipe(new FinalClass(false, Arrays.asList("com.example.api.*"), null)), + java( + """ + package com.example.api; + + class PublicApi { + private PublicApi() { + } + } + """ + ) + ); + } + + @Test + void excludeAnnotatedClasses() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, Arrays.asList("@ExtensionPoint"))), + java( + """ + @ExtensionPoint + class PluginBase { + public void initialize() { + // This should not be finalized due to annotation exclusion + } + } + + @interface ExtensionPoint {} + """ + ) + ); + } + + @Test + void privateConstructorClassesStillFinalized() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + class UtilityClass { + private UtilityClass() { + // Private constructor - should be finalized by original logic + } + + public static void doSomething() { + System.out.println("Utility method"); + } + } + """, + """ + final class UtilityClass { + private UtilityClass() { + // Private constructor - should be finalized by original logic + } + + public static void doSomething() { + System.out.println("Utility method"); + } + } + """ + ) + ); + } + + @Test + void mixedInheritanceChain() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + class GrandParent { + public GrandParent() {} + } + + class Parent extends GrandParent { + public Parent() {} + } + + class Child extends Parent { + public Child() {} + } + + class StandaloneClass { + public StandaloneClass() {} + } + """, + """ + class GrandParent { + public GrandParent() {} + } + + class Parent extends GrandParent { + public Parent() {} + } + + final class Child extends Parent { + public Child() {} + } + + final class StandaloneClass { + public StandaloneClass() {} + } + """ + ) + ); + } + + @Test + void nestedClassInheritance() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + class Outer { + private Outer() {} + + static class Inner1 { + public Inner1() {} + } + + static class Inner2 extends Inner1 { + public Inner2() {} + } + } + """, + """ + final class Outer { + private Outer() {} + + static class Inner1 { + public Inner1() {} + } + + static final class Inner2 extends Inner1 { + public Inner2() {} + } + } + """ + ) + ); + } + + @Test + void interfacesAndEnumsNotFinalized() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + interface MyInterface { + void method(); + } + + enum MyEnum { + VALUE1, VALUE2 + } + """ + ) + ); + } + + @Test + void excludeMultiplePackagePatterns() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, Arrays.asList("com.example.api.*", "com.example.spi.*"), null)), + java( + """ + package com.example.api; + + class ApiClass { + public void api() {} + } + """ + ), + java( + """ + package com.example.spi; + + class SpiClass { + public void spi() {} + } + """ + ), + java( + """ + package com.example.internal; + + class InternalClass { + public void internal() {} + } + """, + """ + package com.example.internal; + + final class InternalClass { + public void internal() {} + } + """ + ) + ); + } + + @Test + void excludeMultipleAnnotations() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, Arrays.asList("@ExtensionPoint", "@ApiClass"))), + java( + """ + @interface ExtensionPoint {} + @interface ApiClass {} + + @ExtensionPoint + class PluginBase { + public void plugin() {} + } + + @ApiClass + class PublicApi { + public void api() {} + } + + class RegularClass { + public void regular() {} + } + """, + """ + @interface ExtensionPoint {} + @interface ApiClass {} + + @ExtensionPoint + class PluginBase { + public void plugin() {} + } + + @ApiClass + class PublicApi { + public void api() {} + } + + final class RegularClass { + public void regular() {} + } + """ + ) + ); + } + + @Test + void alreadyFinalClassesUnchanged() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + final class AlreadyFinal { + public AlreadyFinal() {} + } + """ + ) + ); + } + + @Test + void shouldNotFinalizeRecords() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + public record Point(int a, int b) { + } + """ + ) + ); + } + + @Test + void shouldNotFinalizeSealedClasses() { + rewriteRun( + spec -> spec.recipe(new FinalClass(true, null, null)), + java( + """ + sealed class S permits X, Y { + S() {} + } + final class X extends S { + X() { + super(); + } + } + non-sealed class Y extends S { + Y() { + super(); + } + } + """ + ) + ); + } + + @Test + void excludeAnnotationWithoutAt() { + rewriteRun( + spec -> spec.recipe(new FinalClass(false, null, List.of("Deprecated"))), + java( + """ + package com.example.config; + + @Deprecated + public class DeprecatedClass { + public DeprecatedClass() {} + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/729") + @Test + void excludeAnnotationWithFullyQualifiedName() { + rewriteRun( + spec -> spec.recipe(new FinalClass(false, null, List.of("@java.lang.Deprecated"))), + java( + """ + package com.example.config; + + @java.lang.Deprecated + public class AnotherDeprecatedClass { + private AnotherDeprecatedClass() {} + } + """ + ) + ); + } }