From 9399bc9410b9b5f024ad02a7d084a6eead4387b9 Mon Sep 17 00:00:00 2001 From: Nick Fraioli Date: Tue, 23 Apr 2024 23:49:04 -0700 Subject: [PATCH] Update `file_name` rule to match fully-qualified names of nested types This PR is aimed to address Issue #5840. It does the following: 1. Allows the `file_name` rule to match nested types when using fully-qualified names, meaning naming the following file `Nested.MyType.swift` is no longer a violation: ``` // Nested.MyType.swift enum Nested { struct MyType { } } ``` 2. Introduces a new option `require_fully_qualified` to have the `file_name` rule enforce using fully-qualified names, meaning naming the above file `MyType.swift` instead of `Nested.MyType.swift` would become a violation where it wasn't before (naming the file `Nested.swift` would still not be a violation). --- .../Rules/Idiomatic/FileNameRule.swift | 81 +++++++++++++++++-- .../FileNameConfiguration.swift | 2 + .../FileNameRuleTests.swift | 21 ++++- .../Multiple.Levels.Deep.Nested.MyType.swift | 9 +++ .../FileNameRuleFixtures/MyType.swift | 3 + .../FileNameRuleFixtures/Nested.MyType.swift | 4 + 6 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Multiple.Levels.Deep.Nested.MyType.swift create mode 100644 Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/MyType.swift create mode 100644 Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Nested.MyType.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/FileNameRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/FileNameRule.swift index d3554619eb..68487b492b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/FileNameRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/FileNameRule.swift @@ -35,7 +35,7 @@ struct FileNameRule: OptInRule, SourceKitFreeRule { } // Process nested type separator - let allDeclaredTypeNames = TypeNameCollectingVisitor(viewMode: .sourceAccurate) + let allDeclaredTypeNames = TypeNameCollectingVisitor(requireFullyQualifiedNames: configuration.fullyQualified) .walk(tree: file.syntaxTree, handler: \.names) .map { $0.replacingOccurrences(of: ".", with: configuration.nestedTypeSeparator) @@ -56,33 +56,98 @@ struct FileNameRule: OptInRule, SourceKitFreeRule { } private class TypeNameCollectingVisitor: SyntaxVisitor { + // All of a visited node's ancestor type names if that node is nested, starting with the furthest + // ancestor and ending with the direct parent + private var ancestorNames: [String] = [] + + // All of the type names found in the file private(set) var names: Set = [] + // If true, nested types are only allowed in the file name when used by their fully-qualified name + // (e.g. `My.Nested.Type` and not just `Type`) + private let requireFullyQualifiedNames: Bool + + init(requireFullyQualifiedNames: Bool) { + self.requireFullyQualifiedNames = requireFullyQualifiedNames + super.init(viewMode: .sourceAccurate) + } + + private func addVisitedNodeName(_ name: String) { + let fullyQualifiedName = (ancestorNames + [name]).joined(separator: ".") + names.insert(fullyQualifiedName) + + if !requireFullyQualifiedNames { + names.insert(name) + } + } + + override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren + } + override func visitPost(_ node: ClassDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: ActorDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren } override func visitPost(_ node: ActorDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren } override func visitPost(_ node: StructDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: TypeAliasDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren } override func visitPost(_ node: TypeAliasDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren } override func visitPost(_ node: EnumDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.name.text) + return .visitChildren } override func visitPost(_ node: ProtocolDeclSyntax) { - names.insert(node.name.text) + ancestorNames.removeLast() + addVisitedNodeName(node.name.text) + } + + override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind { + ancestorNames.append(node.extendedType.trimmedDescription) + return .visitChildren } override func visitPost(_ node: ExtensionDeclSyntax) { - names.insert(node.extendedType.trimmedDescription) + ancestorNames.removeLast() + addVisitedNodeName(node.extendedType.trimmedDescription) } } diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/FileNameConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/FileNameConfiguration.swift index 08d414eacd..dadde21e2c 100644 --- a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/FileNameConfiguration.swift +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/FileNameConfiguration.swift @@ -14,4 +14,6 @@ struct FileNameConfiguration: SeverityBasedRuleConfiguration { private(set) var suffixPattern = "\\+.*" @ConfigurationElement(key: "nested_type_separator") private(set) var nestedTypeSeparator = "." + @ConfigurationElement(key: "fully_qualified") + private(set) var fullyQualified = false } diff --git a/Tests/SwiftLintFrameworkTests/FileNameRuleTests.swift b/Tests/SwiftLintFrameworkTests/FileNameRuleTests.swift index 30f3167966..8e9f7d5412 100644 --- a/Tests/SwiftLintFrameworkTests/FileNameRuleTests.swift +++ b/Tests/SwiftLintFrameworkTests/FileNameRuleTests.swift @@ -8,7 +8,8 @@ final class FileNameRuleTests: SwiftLintTestCase { excludedOverride: [String]? = nil, prefixPattern: String? = nil, suffixPattern: String? = nil, - nestedTypeSeparator: String? = nil) throws -> [StyleViolation] { + nestedTypeSeparator: String? = nil, + fullyQualified: Bool? = nil) throws -> [StyleViolation] { let file = SwiftLintFile(path: fixturesDirectory.stringByAppendingPathComponent(fileName))! let rule: FileNameRule if let excluded = excludedOverride { @@ -21,6 +22,8 @@ final class FileNameRuleTests: SwiftLintTestCase { rule = try FileNameRule(configuration: ["suffix_pattern": suffixPattern]) } else if let nestedTypeSeparator { rule = try FileNameRule(configuration: ["nested_type_separator": nestedTypeSeparator]) + } else if let fullyQualified { + rule = try FileNameRule(configuration: ["fully_qualified": fullyQualified]) } else { rule = FileNameRule() } @@ -52,6 +55,22 @@ final class FileNameRuleTests: SwiftLintTestCase { XCTAssert(try validate(fileName: "Notification.Name+Extension.swift").isEmpty) } + func testNestedTypeDoesntTrigger() { + XCTAssert(try validate(fileName: "Nested.MyType.swift").isEmpty) + } + + func testMultipleLevelsDeepNestedTypeDoesntTrigger() { + XCTAssert(try validate(fileName: "Multiple.Levels.Deep.Nested.MyType.swift").isEmpty) + } + + func testNestedTypeNotFullyQualifiedDoesntTrigger() { + XCTAssert(try validate(fileName: "MyType.swift").isEmpty) + } + + func testNestedTypeNotFullyQualifiedDoesTriggerWithOverride() { + XCTAssert(try !validate(fileName: "MyType.swift", fullyQualified: true).isEmpty) + } + func testNestedTypeSeparatorDoesntTrigger() { XCTAssert(try validate(fileName: "NotificationName+Extension.swift", nestedTypeSeparator: "").isEmpty) XCTAssert(try validate(fileName: "Notification__Name+Extension.swift", nestedTypeSeparator: "__").isEmpty) diff --git a/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Multiple.Levels.Deep.Nested.MyType.swift b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Multiple.Levels.Deep.Nested.MyType.swift new file mode 100644 index 0000000000..6c8f6b4842 --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Multiple.Levels.Deep.Nested.MyType.swift @@ -0,0 +1,9 @@ +extension Multiple { + enum Levels { + class Deep { + struct Nested { + actor MyType {} + } + } + } +} diff --git a/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/MyType.swift b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/MyType.swift new file mode 100644 index 0000000000..7c72b6af9d --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/MyType.swift @@ -0,0 +1,3 @@ +enum Nested { + struct MyType {} +} diff --git a/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Nested.MyType.swift b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Nested.MyType.swift new file mode 100644 index 0000000000..a57866f72a --- /dev/null +++ b/Tests/SwiftLintFrameworkTests/Resources/FileNameRuleFixtures/Nested.MyType.swift @@ -0,0 +1,4 @@ +enum Nested { + struct MyType { + } +}