From fbbe9809239d23b4e27a2d2473248d8ff79a585a Mon Sep 17 00:00:00 2001 From: Cal Stephens Date: Thu, 4 Sep 2025 16:44:42 -0700 Subject: [PATCH] Add `preferFinalClasses` rule (#2196) --- Rules.md | 34 ++ Sources/Declaration.swift | 5 + Sources/ParsingHelpers.swift | 23 ++ Sources/RuleRegistry.generated.swift | 1 + Sources/Rules/PreferFinalClasses.swift | 110 ++++++ Tests/ParsingHelpersTests.swift | 29 ++ Tests/Rules/PreferFinalClassesTests.swift | 389 ++++++++++++++++++++++ Tests/XCTestCase+testFormatting.swift | 1 + 8 files changed, 592 insertions(+) create mode 100644 Sources/Rules/PreferFinalClasses.swift create mode 100644 Tests/Rules/PreferFinalClassesTests.swift diff --git a/Rules.md b/Rules.md index 76eaa614..3ecc8e79 100644 --- a/Rules.md +++ b/Rules.md @@ -111,6 +111,7 @@ * [noExplicitOwnership](#noExplicitOwnership) * [noGuardInTests](#noGuardInTests) * [organizeDeclarations](#organizeDeclarations) +* [preferFinalClasses](#preferFinalClasses) * [preferSwiftTesting](#preferSwiftTesting) * [privateStateVariables](#privateStateVariables) * [propertyTypes](#propertyTypes) @@ -1917,6 +1918,39 @@ Prefer `count(where:)` over `filter(_:).count`.
+## preferFinalClasses + +Prefer defining `final` classes. To suppress this rule, add "Base" to the class name, add a doc comment with mentioning "base class" or "subclass", make the class `open`, or use a `// swiftformat:disable:next preferFinalClasses` directive. + +
+Examples + +```diff +- class Foo {} ++ final class Foo {} +``` + +```diff +- public class Bar {} ++ public final class Bar {} +``` + +```diff + // Preserved classes: + open class Baz {} + + class BaseClass {} + + class MyClass {} // Subclassed in this file + class MySubclass: MyClass {} + + /// Base class to be subclassed by other features + class MyCustomizationPoint {} +``` + +
+
+ ## preferForLoop Convert functional `forEach` calls to for loops. diff --git a/Sources/Declaration.swift b/Sources/Declaration.swift index 0a513969..2b82a409 100644 --- a/Sources/Declaration.swift +++ b/Sources/Declaration.swift @@ -176,6 +176,11 @@ extension Declaration { } } + /// The range of the doc comment or regular comment immediately preceding this declaration + var docCommentRange: ClosedRange? { + formatter.parseDocCommentRange(forDeclarationAt: keywordIndex) + } + /// The `CustomDebugStringConvertible` representation of this declaration var debugDescription: String { guard isValid else { diff --git a/Sources/ParsingHelpers.swift b/Sources/ParsingHelpers.swift index 61a233d0..7fe30c3c 100644 --- a/Sources/ParsingHelpers.swift +++ b/Sources/ParsingHelpers.swift @@ -2612,6 +2612,29 @@ extension Formatter { return matches } + /// Parses the range of the doc comment or regular comment immediately preceding the declaration + func parseDocCommentRange(forDeclarationAt keywordIndex: Int) -> ClosedRange? { + let startOfModifiers = startOfModifiers(at: keywordIndex, includingAttributes: true) + + var parseIndex = startOfModifiers + var endOfComment: Int? + + while let endOfPreviousLine = index(of: .linebreak, before: parseIndex), + let endOfPreviousLineContent = index(of: .nonSpace, before: endOfPreviousLine), + tokens[endOfPreviousLineContent].isComment, + let startOfScope = startOfScope(at: endOfPreviousLineContent) + { + parseIndex = startOfScope + + if endOfComment == nil { + endOfComment = endOfPreviousLineContent + } + } + + guard let endOfComment else { return nil } + return parseIndex ... endOfComment + } + /// Parses the prorocol composition typealias declaration starting at the given `typealias` keyword index. /// Returns `nil` if the given index isn't a protocol composition typealias. func parseProtocolCompositionTypealias(at typealiasIndex: Int) diff --git a/Sources/RuleRegistry.generated.swift b/Sources/RuleRegistry.generated.swift index 31ac132d..25924de8 100644 --- a/Sources/RuleRegistry.generated.swift +++ b/Sources/RuleRegistry.generated.swift @@ -60,6 +60,7 @@ let ruleRegistry: [String: FormatRule] = [ "opaqueGenericParameters": .opaqueGenericParameters, "organizeDeclarations": .organizeDeclarations, "preferCountWhere": .preferCountWhere, + "preferFinalClasses": .preferFinalClasses, "preferForLoop": .preferForLoop, "preferKeyPath": .preferKeyPath, "preferSwiftTesting": .preferSwiftTesting, diff --git a/Sources/Rules/PreferFinalClasses.swift b/Sources/Rules/PreferFinalClasses.swift new file mode 100644 index 00000000..9751c994 --- /dev/null +++ b/Sources/Rules/PreferFinalClasses.swift @@ -0,0 +1,110 @@ +// +// PreferFinalClasses.swift +// SwiftFormat +// +// Created by Cal Stephens on 2025-08-25. +// Copyright © 2024 Nick Lockwood. All rights reserved. +// + +import Foundation + +public extension FormatRule { + /// Add the `final` keyword to all classes that are not declared as `open` + static let preferFinalClasses = FormatRule( + help: """ + Prefer defining `final` classes. To suppress this rule, add "Base" to the class name, \ + add a doc comment with mentioning "base class" or "subclass", make the class `open`, \ + or use a `// swiftformat:disable:next preferFinalClasses` directive. + """, + disabledByDefault: true + ) { formatter in + // Parse all declarations to understand inheritance relationships + let declarations = formatter.parseDeclarations() + + // Find all class names that are inherited from in this file + var classesWithSubclasses = Set() + declarations.forEachRecursiveDeclaration { declaration in + guard declaration.keyword == "class" else { return } + + // Check all conformances - any of them could be a superclass + let conformances = formatter.parseConformancesOfType(atKeywordIndex: declaration.keywordIndex) + for conformance in conformances { + // Extract base class name from generic types like "Container" -> "Container" + let baseClassName = conformance.conformance.tokens.first?.string ?? conformance.conformance.string + classesWithSubclasses.insert(baseClassName) + } + } + + // Now process each class declaration + declarations.forEachRecursiveDeclaration { declaration in + guard declaration.keyword == "class", + let className = declaration.name else { return } + + let keywordIndex = declaration.keywordIndex + + // Check if class already has final or open modifiers + let hasFinalModifier = formatter.modifiersForDeclaration(at: keywordIndex, contains: "final") + let hasOpenModifier = formatter.modifiersForDeclaration(at: keywordIndex, contains: "open") + + // Only add final if the class doesn't already have final or open + guard !hasFinalModifier, !hasOpenModifier else { return } + + // Don't add final if this class is inherited from in the same file + guard !classesWithSubclasses.contains(className) else { return } + + // Don't add final to classes that contain "Base" (they're likely meant to be subclassed) + guard !className.contains("Base") else { return } + + // Don't add final to classes with a comment like "// Base class for XYZ functionality" + if let docCommentRange = declaration.docCommentRange { + let subclassRelatedTerms = ["base", "subclass"] + let docComment = formatter.tokens[docCommentRange].string.lowercased() + + for term in subclassRelatedTerms { + if docComment.contains(term) { + return + } + } + } + + formatter.insert(tokenize("final "), at: keywordIndex) + + // Convert any open direct child declarations to public (since final classes can't have open members) + if let classBody = declaration.body { + for childDeclaration in classBody { + guard formatter.modifiersForDeclaration(at: childDeclaration.keywordIndex, contains: "open") else { continue } + + // Replace "open" with "public" for direct child declarations + if let openIndex = formatter.indexOfModifier("open", forDeclarationAt: childDeclaration.keywordIndex) { + formatter.replaceToken(at: openIndex, with: .keyword("public")) + } + } + } + } + } examples: { + """ + ```diff + - class Foo {} + + final class Foo {} + ``` + + ```diff + - public class Bar {} + + public final class Bar {} + ``` + + ```diff + // Preserved classes: + open class Baz {} + + class BaseClass {} + + class MyClass {} // Subclassed in this file + class MySubclass: MyClass {} + + /// Base class to be subclassed by other features + class MyCustomizationPoint {} + ``` + """ + } +} diff --git a/Tests/ParsingHelpersTests.swift b/Tests/ParsingHelpersTests.swift index 388ffa87..416e7333 100644 --- a/Tests/ParsingHelpersTests.swift +++ b/Tests/ParsingHelpersTests.swift @@ -2965,4 +2965,33 @@ class ParsingHelpersTests: XCTestCase { "baaz: baaz.quux", ]) } + + func testParseCommentRange() throws { + let input = """ + import FooLib + + // Class declaration + class MyClass {} + + // Other comment + + /// Foo bar + /// baaz quux + @Foo + struct MyStruct {} + """ + + let formatter = Formatter(tokenize(input)) + let classCommentRange = try XCTUnwrap(formatter.parseDocCommentRange(forDeclarationAt: 9)) // class + let structCommentRange = try XCTUnwrap(formatter.parseDocCommentRange(forDeclarationAt: 30)) // struct + + XCTAssertEqual(formatter.tokens[classCommentRange].string, """ + // Class declaration + """) + + XCTAssertEqual(formatter.tokens[structCommentRange].string, """ + /// Foo bar + /// baaz quux + """) + } } diff --git a/Tests/Rules/PreferFinalClassesTests.swift b/Tests/Rules/PreferFinalClassesTests.swift new file mode 100644 index 00000000..9d47405b --- /dev/null +++ b/Tests/Rules/PreferFinalClassesTests.swift @@ -0,0 +1,389 @@ +// +// PreferFinalClassesTests.swift +// SwiftFormatTests +// +// Created by Cal Stephens on 2025-08-25. +// Copyright © 2024 Nick Lockwood. All rights reserved. +// + +import XCTest +@testable import SwiftFormat + +class PreferFinalClassesTests: XCTestCase { + func testBasicClassMadesFinal() { + let input = """ + class Foo {} + """ + let output = """ + final class Foo {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testPublicClassMadesFinal() { + let input = """ + public class Bar {} + """ + let output = """ + public final class Bar {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testPrivateClassMadesFinal() { + let input = """ + private class Baz {} + """ + let output = """ + private final class Baz {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testInternalClassMadesFinal() { + let input = """ + internal class Qux {} + """ + let output = """ + internal final class Qux {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses, exclude: [.redundantInternal]) + } + + func testOpenClassLeftUnchanged() { + let input = """ + open class OpenClass {} + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testAlreadyFinalClassLeftUnchanged() { + let input = """ + final class FinalClass {} + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testPublicFinalClassLeftUnchanged() { + let input = """ + public final class PublicFinalClass {} + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testPublicOpenClassLeftUnchanged() { + let input = """ + public open class PublicOpenClass {} + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testClassFunctionNotAffected() { + let input = """ + struct Foo { + class func bar() {} + } + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testClassVariableNotAffected() { + let input = """ + struct Foo { + class var bar: String { "bar" } + } + """ + testFormatting(for: input, rule: .preferFinalClasses) + } + + func testNestedClass() { + let input = """ + class OuterClass { + class InnerClass {} + } + """ + let output = """ + final class OuterClass { + final class InnerClass {} + } + """ + testFormatting(for: input, output, rule: .preferFinalClasses, exclude: [.enumNamespaces]) + } + + func testClassWithInheritance() { + let input = """ + class Child: Parent {} + """ + let output = """ + final class Child: Parent {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testClassWithProtocolConformance() { + let input = """ + class MyClass: SomeProtocol {} + """ + let output = """ + final class MyClass: SomeProtocol {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testClassWithMultipleModifiers() { + let input = """ + @objc public class MyClass {} + """ + let output = """ + @objc public final class MyClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testMultipleClasses() { + let input = """ + class FirstClass {} + class SecondClass {} + open class ThirdClass {} + final class FourthClass {} + """ + let output = """ + final class FirstClass {} + final class SecondClass {} + open class ThirdClass {} + final class FourthClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testClassWithComments() { + let input = """ + // This is a class + class MyClass { + // Some content + } + """ + let output = """ + // This is a class + final class MyClass { + // Some content + } + """ + testFormatting(for: input, output, rule: .preferFinalClasses, exclude: [.docComments]) + } + + func testClassWithSubclassNotMadeFinal() { + let input = """ + class BaseClass {} + class SubClass: BaseClass {} + """ + let output = """ + class BaseClass {} + final class SubClass: BaseClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testMultipleInheritanceLevels() { + let input = """ + class GrandParent {} + class Parent: GrandParent {} + class Child: Parent {} + """ + let output = """ + class GrandParent {} + class Parent: GrandParent {} + final class Child: Parent {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testClassWithProtocolConformanceStillMadeFinal() { + let input = """ + protocol SomeProtocol {} + class MyClass: SomeProtocol {} + """ + let output = """ + protocol SomeProtocol {} + final class MyClass: SomeProtocol {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testClassInheritingFromExternalClassMadeFinal() { + let input = """ + class MyViewController: UIViewController {} + """ + let output = """ + final class MyViewController: UIViewController {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testMixedScenario() { + let input = """ + class BaseClass {} + final class AlreadyFinalClass {} + open class OpenClass {} + class SubClass: BaseClass {} + class IndependentClass {} + """ + let output = """ + class BaseClass {} + final class AlreadyFinalClass {} + open class OpenClass {} + final class SubClass: BaseClass {} + final class IndependentClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testGenericClassWithSubclass() { + let input = """ + class Container {} + class StringContainer: Container {} + """ + let output = """ + class Container {} + final class StringContainer: Container {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testGenericClassWithGenericSubclass() { + let input = """ + class BaseContainer {} + class SpecialContainer: BaseContainer {} + """ + let output = """ + class BaseContainer {} + final class SpecialContainer: BaseContainer {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testMultipleGenericParameters() { + let input = """ + class GenericClass {} + class ConcreteClass: GenericClass {} + """ + let output = """ + class GenericClass {} + final class ConcreteClass: GenericClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testComplexGenericInheritanceChain() { + let input = """ + class BaseContainer {} + class MiddleContainer: BaseContainer {} + class FinalContainer: MiddleContainer {} + """ + let output = """ + class BaseContainer {} + class MiddleContainer: BaseContainer {} + final class FinalContainer: MiddleContainer {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testBaseClassNotMadeFinal() { + let input = """ + class BaseClass {} + class ClassBase {} + class SomeBase {} + class BaseSomething {} + class ViewControllerBase {} + class RegularClass {} + """ + let output = """ + class BaseClass {} + class ClassBase {} + class SomeBase {} + class BaseSomething {} + class ViewControllerBase {} + final class RegularClass {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testConvertOpenMembersToPublic() { + let input = """ + public class MyClass { + open var property1: String = "" + open let property2: Int = 0 + open func method1() {} + private var privateProperty: String = "" + public func publicMethod() {} + } + """ + let output = """ + public final class MyClass { + public var property1: String = "" + public let property2: Int = 0 + public func method1() {} + private var privateProperty: String = "" + public func publicMethod() {} + } + """ + testFormatting(for: input, output, rule: .preferFinalClasses) + } + + func testNestedClassWithOpenMembersNotConverted() { + let input = """ + public class OuterClass { + open var outerProperty: String = "" + + public class InnerClass { + open var innerProperty: String = "" + } + } + """ + let output = """ + public final class OuterClass { + public var outerProperty: String = "" + + public final class InnerClass { + public var innerProperty: String = "" + } + } + """ + testFormatting(for: input, output, rule: .preferFinalClasses, exclude: [.enumNamespaces]) + } + + func testMixedScenarioWithBaseAndOpen() { + let input = """ + class BaseController {} + public class MyController { + open var title: String = "" + open func setup() {} + } + class UtilityBase {} + """ + let output = """ + class BaseController {} + public final class MyController { + public var title: String = "" + public func setup() {} + } + class UtilityBase {} + """ + testFormatting(for: input, output, rule: .preferFinalClasses, exclude: [.blankLinesBetweenScopes]) + } + + func testNonFinalClassWithBaseCommentPreserved() { + let input = """ + /// Base class + public class Foo {} + + /// Customization point to be subclassed + public class Foo {} + + //subclass this in your custom implementation + public class Bar {} + """ + + testFormatting(for: input, rule: .preferFinalClasses, exclude: [.docComments, .spaceInsideComments]) + } +} diff --git a/Tests/XCTestCase+testFormatting.swift b/Tests/XCTestCase+testFormatting.swift index 0f1d63d2..7dedaac3 100644 --- a/Tests/XCTestCase+testFormatting.swift +++ b/Tests/XCTestCase+testFormatting.swift @@ -82,6 +82,7 @@ extension XCTestCase { .markTypes, .blockComments, .unusedPrivateDeclarations, + .preferFinalClasses, ] let exclude = exclude + defaultExclusions.filter { !rules.contains($0) } let formatResult: (output: String, changes: [SwiftFormat.Formatter.Change])