Merge pull request #777 from shawnhyam/upgrade-dont-repeat-type-in-static-properties

Upgrade the DontRepeatTypeInStaticProperties rule.

Author: allevato

Documentation/RuleDocumentation.md

@@ -4,7 +4,7 @@
 
 Use the rules below in the `rules` block of your `.swift-format`
 configuration file, as described in
-[Configuration](Configuration.md). All of these rules can be
+[Configuration](Documentation/Configuration.md). All of these rules can be
 applied in the linter, but only some of them can format your source code
 automatically.
 

Sources/SwiftFormat/Core/Pipelines+Generated.swift

@@ -76,7 +76,6 @@ class LintPipeline: SyntaxVisitor {
     visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
     visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
-    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(NoLeadingUnderscores.visit, for: node)
     visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
     visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
@@ -86,7 +85,6 @@ class LintPipeline: SyntaxVisitor {
     onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
     onVisitPost(rule: AlwaysUseLowerCamelCase.self, for: node)
     onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
-    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: NoLeadingUnderscores.self, for: node)
     onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
     onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
@@ -182,7 +180,6 @@ class LintPipeline: SyntaxVisitor {
 
   override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
-    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(FullyIndirectEnum.visit, for: node)
     visitIfEnabled(NoLeadingUnderscores.visit, for: node)
     visitIfEnabled(OneCasePerLine.visit, for: node)
@@ -192,7 +189,6 @@ class LintPipeline: SyntaxVisitor {
   }
   override func visitPost(_ node: EnumDeclSyntax) {
     onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
-    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: FullyIndirectEnum.self, for: node)
     onVisitPost(rule: NoLeadingUnderscores.self, for: node)
     onVisitPost(rule: OneCasePerLine.self, for: node)
@@ -202,14 +198,12 @@ class LintPipeline: SyntaxVisitor {
 
   override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(AvoidRetroactiveConformances.visit, for: node)
-    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(NoAccessLevelOnExtensionDeclaration.visit, for: node)
     visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
     return .visitChildren
   }
   override func visitPost(_ node: ExtensionDeclSyntax) {
     onVisitPost(rule: AvoidRetroactiveConformances.self, for: node)
-    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: NoAccessLevelOnExtensionDeclaration.self, for: node)
     onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
   }
@@ -423,7 +417,6 @@ class LintPipeline: SyntaxVisitor {
   override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
-    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(NoLeadingUnderscores.visit, for: node)
     visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
     visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
@@ -432,7 +425,6 @@ class LintPipeline: SyntaxVisitor {
   override func visitPost(_ node: ProtocolDeclSyntax) {
     onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
     onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
-    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: NoLeadingUnderscores.self, for: node)
     onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
     onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
@@ -469,7 +461,6 @@ class LintPipeline: SyntaxVisitor {
   override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
     visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
-    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(NoLeadingUnderscores.visit, for: node)
     visitIfEnabled(TypeNamesShouldBeCapitalized.visit, for: node)
     visitIfEnabled(UseSynthesizedInitializer.visit, for: node)
@@ -479,7 +470,6 @@ class LintPipeline: SyntaxVisitor {
   override func visitPost(_ node: StructDeclSyntax) {
     onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
     onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
-    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: NoLeadingUnderscores.self, for: node)
     onVisitPost(rule: TypeNamesShouldBeCapitalized.self, for: node)
     onVisitPost(rule: UseSynthesizedInitializer.self, for: node)
@@ -568,6 +558,7 @@ class LintPipeline: SyntaxVisitor {
     visitIfEnabled(AllPublicDeclarationsHaveDocumentation.visit, for: node)
     visitIfEnabled(AlwaysUseLowerCamelCase.visit, for: node)
     visitIfEnabled(BeginDocumentationCommentWithOneLineSummary.visit, for: node)
+    visitIfEnabled(DontRepeatTypeInStaticProperties.visit, for: node)
     visitIfEnabled(NeverUseImplicitlyUnwrappedOptionals.visit, for: node)
     visitIfEnabled(UseTripleSlashForDocumentationComments.visit, for: node)
     return .visitChildren
@@ -576,6 +567,7 @@ class LintPipeline: SyntaxVisitor {
     onVisitPost(rule: AllPublicDeclarationsHaveDocumentation.self, for: node)
     onVisitPost(rule: AlwaysUseLowerCamelCase.self, for: node)
     onVisitPost(rule: BeginDocumentationCommentWithOneLineSummary.self, for: node)
+    onVisitPost(rule: DontRepeatTypeInStaticProperties.self, for: node)
     onVisitPost(rule: NeverUseImplicitlyUnwrappedOptionals.self, for: node)
     onVisitPost(rule: UseTripleSlashForDocumentationComments.self, for: node)
   }

Sources/SwiftFormat/Rules/AlwaysUseLowerCamelCase.swift

@@ -17,6 +17,7 @@ import SwiftSyntax
 ///
 /// This rule does not apply to test code, defined as code which:
 ///   * Contains the line `import XCTest`
+///   * The function is marked with `@Test` attribute
 ///
 /// Lint: If an identifier contains underscores or begins with a capital letter, a lint error is
 ///       raised.

Sources/SwiftFormat/Rules/DontRepeatTypeInStaticProperties.swift

@@ -22,69 +22,28 @@ import SwiftSyntax
 @_spi(Rules)
 public final class DontRepeatTypeInStaticProperties: SyntaxLintRule {
 
-  public override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
-    return .skipChildren
-  }
-
-  public override func visit(_ node: EnumDeclSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
-    return .skipChildren
-  }
-
-  public override func visit(_ node: ProtocolDeclSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
-    return .skipChildren
-  }
-
-  public override func visit(_ node: StructDeclSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseStaticMembers(node.memberBlock.members, endingWith: node.name.text)
-    return .skipChildren
-  }
-
-  public override func visit(_ node: ExtensionDeclSyntax) -> SyntaxVisitorContinueKind {
-    let members = node.memberBlock.members
-
-    switch Syntax(node.extendedType).as(SyntaxEnum.self) {
-    case .identifierType(let simpleType):
-      diagnoseStaticMembers(members, endingWith: simpleType.name.text)
-    case .memberType(let memberType):
-      // We don't need to drill recursively into this structure because types with more than two
-      // components are constructed left-heavy; that is, `A.B.C.D` is structured as `((A.B).C).D`,
-      // and the final component of the top type is what we want.
-      diagnoseStaticMembers(members, endingWith: memberType.name.text)
-    default:
-      // Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
-      // we'll need to update this if we need to support some subset of those.
-      break
+  /// Visits the static/class properties and diagnoses any where the name has the containing
+  /// type name (excluding possible namespace prefixes, like `NS` or `UI`) as a suffix.
+  public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
+    guard node.modifiers.contains(anyOf: [.class, .static]),
+      let typeName = Syntax(node).containingDeclName
+    else {
+      return .visitChildren
     }
 
-    return .skipChildren
-  }
-
-  /// Iterates over the static/class properties in the given member list and diagnoses any where the
-  /// name has the containing type name (excluding possible namespace prefixes, like `NS` or `UI`)
-  /// as a suffix.
-  private func diagnoseStaticMembers(_ members: MemberBlockItemListSyntax, endingWith typeName: String) {
-    for member in members {
-      guard
-        let varDecl = member.decl.as(VariableDeclSyntax.self),
-        varDecl.modifiers.contains(anyOf: [.class, .static])
-      else { continue }
-
-      let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
-
-      for binding in varDecl.bindings {
-        guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
-          continue
-        }
+    let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
+    for binding in node.bindings {
+      guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
+        continue
+      }
 
-        let varName = identifierPattern.identifier.text
-        if varName.contains(bareTypeName) {
-          diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
-        }
+      let varName = identifierPattern.identifier.text
+      if varName.contains(bareTypeName) {
+        diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
       }
     }
+
+    return .visitChildren
   }
 
   /// Returns the portion of the given string that excludes a possible Objective-C-style capitalized
@@ -110,3 +69,40 @@ extension Finding.Message {
     "remove the suffix '\(type)' from the name of the variable '\(name)'"
   }
 }
+
+extension Syntax {
+  /// Returns the name of the immediately enclosing type of this decl if there is one,
+  /// otherwise nil.
+  fileprivate var containingDeclName: String? {
+    switch Syntax(self).as(SyntaxEnum.self) {
+    case .actorDecl(let node):
+      return node.name.text
+    case .classDecl(let node):
+      return node.name.text
+    case .enumDecl(let node):
+      return node.name.text
+    case .protocolDecl(let node):
+      return node.name.text
+    case .structDecl(let node):
+      return node.name.text
+    case .extensionDecl(let node):
+      switch Syntax(node.extendedType).as(SyntaxEnum.self) {
+      case .identifierType(let simpleType):
+        return simpleType.name.text
+      case .memberType(let memberType):
+        // the final component of the top type `A.B.C.D` is what we want `D`.
+        return memberType.name.text
+      default:
+        // Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
+        // we'll need to update this if we need to support some subset of those.
+        return nil
+      }
+    default:
+      if let parent = self.parent {
+        return parent.containingDeclName
+      }
+
+      return nil
+    }
+  }
+}

Sources/SwiftFormat/Rules/NeverForceUnwrap.swift

@@ -16,6 +16,7 @@ import SwiftSyntax
 ///
 /// This rule does not apply to test code, defined as code which:
 ///   * Contains the line `import XCTest`
+///   * The function is marked with `@Test` attribute
 ///
 /// Lint: If a force unwrap is used, a lint warning is raised.
 @_spi(Rules)

Sources/SwiftFormat/Rules/NeverUseForceTry.swift

@@ -16,6 +16,7 @@ import SwiftSyntax
 ///
 /// This rule does not apply to test code, defined as code which:
 ///   * Contains the line `import XCTest`
+///   * The function is marked with `@Test` attribute
 ///
 /// Lint: Using `try!` results in a lint error.
 ///

Sources/SwiftFormat/Rules/NeverUseImplicitlyUnwrappedOptionals.swift

@@ -18,6 +18,7 @@ import SwiftSyntax
 ///
 /// This rule does not apply to test code, defined as code which:
 ///   * Contains the line `import XCTest`
+///   * The function is marked with `@Test` attribute
 ///
 /// TODO: Create exceptions for other UI elements (ex: viewDidLoad)
 ///

Sources/generate-swift-format/PipelineGenerator.swift

@@ -93,6 +93,7 @@ final class PipelineGenerator: FileGenerator {
         handle.write(
           """
             override func visitPost(_ node: \(nodeType)) {
+          
           """
         )
         for ruleName in lintRules.sorted() {

Tests/SwiftFormatTests/Rules/DontRepeatTypeInStaticPropertiesTests.swift

@@ -32,6 +32,9 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
       extension URLSession {
         class var 8️⃣sharedSession: URLSession
       }
+      public actor Cookie {
+        static let 9️⃣chocolateChipCookie: Cookie
+      }
       """,
       findings: [
         FindingSpec("1️⃣", message: "remove the suffix 'Color' from the name of the variable 'redColor'"),
@@ -42,6 +45,7 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
         FindingSpec("6️⃣", message: "remove the suffix 'Game' from the name of the variable 'basketballGame'"),
         FindingSpec("7️⃣", message: "remove the suffix 'Game' from the name of the variable 'baseballGame'"),
         FindingSpec("8️⃣", message: "remove the suffix 'Session' from the name of the variable 'sharedSession'"),
+        FindingSpec("9️⃣", message: "remove the suffix 'Cookie' from the name of the variable 'chocolateChipCookie'"),
       ]
     )
   }