Merge pull request #767 from hamtiko/allow-underscore-for-testing-funcs

Add checks for test functions marked with @test attribute in relevant Rules

Author: allevato

Documentation/RuleDocumentation.md

@@ -79,6 +79,7 @@ Underscores (except at the beginning of an identifier) are disallowed.
 
 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.
@@ -188,6 +189,7 @@ Force-unwraps are strongly discouraged and must be documented.
 
 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.
 
@@ -199,6 +201,7 @@ Force-try (`try!`) is forbidden.
 
 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.
 
@@ -214,6 +217,7 @@ Certain properties (e.g. `@IBOutlet`) tied to the UI lifecycle are ignored.
 
 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/SwiftFormat/CMakeLists.txt

@@ -42,6 +42,7 @@ add_library(SwiftFormat
   Core/SyntaxLintRule.swift
   Core/SyntaxProtocol+Convenience.swift
   Core/Trivia+Convenience.swift
+  Core/WithAttributesSyntax+Convenience.swift
   Core/WithSemicolonSyntax.swift
   PrettyPrint/Comment.swift
   PrettyPrint/Indent+Length.swift

Sources/SwiftFormat/Core/SyntaxProtocol+Convenience.swift

@@ -149,6 +149,19 @@ extension SyntaxProtocol {
     }
     return leadingTrivia.hasAnyComments
   }
+
+  /// Indicates whether the node has any function ancestor marked with `@Test` attribute.
+  var hasTestAncestor: Bool {
+    var parent = self.parent
+    while let existingParent = parent {
+      if let functionDecl = existingParent.as(FunctionDeclSyntax.self),
+          functionDecl.hasAttribute("Test", inModule: "Testing") {
+        return true
+      }
+      parent = existingParent.parent
+    }
+    return false
+  }
 }
 
 extension SyntaxCollection {

Sources/SwiftFormat/Core/WithAttributesSyntax+Convenience.swift

@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftSyntax
+
+extension WithAttributesSyntax {
+  /// Indicates whether the node has attribute with the given `name` and `module`.
+  /// The `module` is only considered if the attribute is written as `@Module.Attribute`.
+  ///
+  /// - Parameter name: The name of the attribute to lookup.
+  /// - Parameter module: The module name to lookup the attribute in.
+  /// - Returns: True if the node has an attribute with the given `name`, otherwise false.
+  func hasAttribute(_ name: String, inModule module: String) -> Bool {
+    attributes.contains { attribute in
+      let attributeName = attribute.as(AttributeSyntax.self)?.attributeName
+      if let identifier = attributeName?.as(IdentifierTypeSyntax.self) {
+        // @Attribute syntax
+        return identifier.name.text == name
+      }
+      if let memberType = attributeName?.as(MemberTypeSyntax.self) {
+        // @Module.Attribute syntax
+        return memberType.name.text == name
+          && memberType.baseType.as(IdentifierTypeSyntax.self)?.name.text == module
+      }
+      return false
+    }
+  }
+}

Sources/SwiftFormat/Rules/AlwaysUseLowerCamelCase.swift

@@ -100,7 +100,8 @@ public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
 
     // We allow underscores in test names, because there's an existing convention of using
     // underscores to separate phrases in very detailed test names.
-    let allowUnderscores = testCaseFuncs.contains(node)
+    let allowUnderscores = testCaseFuncs.contains(node) || node.hasAttribute("Test", inModule: "Testing")
+
     diagnoseLowerCamelCaseViolations(
       node.name, allowUnderscores: allowUnderscores,
       description: identifierDescription(for: node))

Sources/SwiftFormat/Rules/NeverForceUnwrap.swift

@@ -34,6 +34,8 @@ public final class NeverForceUnwrap: SyntaxLintRule {
 
   public override func visit(_ node: ForceUnwrapExprSyntax) -> SyntaxVisitorContinueKind {
     guard context.importsXCTest == .doesNotImportXCTest else { return .skipChildren }
+    // Allow force unwrapping if it is in a function marked with @Test attribute.
+    if node.hasTestAncestor { return .skipChildren }
     diagnose(.doNotForceUnwrap(name: node.expression.trimmedDescription), on: node)
     return .skipChildren
   }
@@ -44,6 +46,8 @@ public final class NeverForceUnwrap: SyntaxLintRule {
     guard context.importsXCTest == .doesNotImportXCTest else { return .skipChildren }
     guard let questionOrExclamation = node.questionOrExclamationMark else { return .skipChildren }
     guard questionOrExclamation.tokenKind == .exclamationMark else { return .skipChildren }
+    // Allow force cast if it is in a function marked with @Test attribute.
+    if node.hasTestAncestor { return .skipChildren }
     diagnose(.doNotForceCast(name: node.type.trimmedDescription), on: node)
     return .skipChildren
   }

Sources/SwiftFormat/Rules/NeverUseForceTry.swift

@@ -36,6 +36,8 @@ public final class NeverUseForceTry: SyntaxLintRule {
   public override func visit(_ node: TryExprSyntax) -> SyntaxVisitorContinueKind {
     guard context.importsXCTest == .doesNotImportXCTest else { return .skipChildren }
     guard let mark = node.questionOrExclamationMark else { return .visitChildren }
+    // Allow force try if it is in a function marked with @Test attribute.
+    if node.hasTestAncestor { return .skipChildren }
     if mark.tokenKind == .exclamationMark {
       diagnose(.doNotForceTry, on: node.tryKeyword)
     }

Sources/SwiftFormat/Rules/NeverUseImplicitlyUnwrappedOptionals.swift

@@ -39,6 +39,8 @@ public final class NeverUseImplicitlyUnwrappedOptionals: SyntaxLintRule {
 
   public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
     guard context.importsXCTest == .doesNotImportXCTest else { return .skipChildren }
+    // Allow implicitly unwrapping if it is in a function marked with @Test attribute.
+    if node.hasTestAncestor { return .skipChildren }
     // Ignores IBOutlet variables
     for attribute in node.attributes {
       if (attribute.as(AttributeSyntax.self))?.attributeName.as(IdentifierTypeSyntax.self)?.name.text == "IBOutlet" {

Tests/SwiftFormatTests/Rules/AlwaysUseLowerCamelCaseTests.swift

@@ -210,4 +210,27 @@ final class AlwaysUseLowerCamelCaseTests: LintOrFormatRuleTestCase {
       ]
     )
   }
+
+  func testIgnoresFunctionsWithTestAttributes() {
+    assertLint(
+      AlwaysUseLowerCamelCase.self,
+      """
+      @Test
+      func function_With_Test_Attribute() {}
+      @Testing.Test("Description for test functions",
+            .tags(.testTag))
+      func function_With_Test_Attribute_And_Args() {}
+      func 1️⃣function_Without_Test_Attribute() {}
+      @objc
+      func 2️⃣function_With_Non_Test_Attribute() {}
+      @Foo.Test
+      func 3️⃣function_With_Test_Attribute_From_Foo_Module() {}
+      """,
+      findings: [
+        FindingSpec("1️⃣", message: "rename the function 'function_Without_Test_Attribute' using lowerCamelCase"),
+        FindingSpec("2️⃣", message: "rename the function 'function_With_Non_Test_Attribute' using lowerCamelCase"),
+        FindingSpec("3️⃣", message: "rename the function 'function_With_Test_Attribute_From_Foo_Module' using lowerCamelCase"),
+      ]
+    )
+  }
 }

Tests/SwiftFormatTests/Rules/NeverForceUnwrapTests.swift

@@ -40,4 +40,23 @@ final class NeverForceUnwrapTests: LintOrFormatRuleTestCase {
       findings: []
     )
   }
+
+  func testIgnoreTestAttributeFunction() {
+    assertLint(
+      NeverForceUnwrap.self,
+      """
+      @Test
+      func testSomeFunc() {
+        var b = a as! Int
+      }
+      @Test
+      func testAnotherFunc() {
+        func nestedFunc() {
+          let c = someValue()!
+        }
+      }
+      """,
+      findings: []
+    )
+  }
 }

Tests/SwiftFormatTests/Rules/NeverUseForceTryTests.swift

@@ -38,4 +38,20 @@ final class NeverUseForceTryTests: LintOrFormatRuleTestCase {
       findings: []
     )
   }
+  
+  func testAllowForceTryInTestAttributeFunction() {
+    assertLint(
+      NeverUseForceTry.self,
+      """
+      @Test
+      func testSomeFunc() {
+        let document = try! Document(path: "important.data")
+        func nestedFunc() {
+          let x = try! someThrowingFunction()
+        }
+      }
+      """,
+      findings: []
+    )
+  }
 }

Tests/SwiftFormatTests/Rules/NeverUseImplicitlyUnwrappedOptionalsTests.swift

@@ -35,4 +35,20 @@ final class NeverUseImplicitlyUnwrappedOptionalsTests: LintOrFormatRuleTestCase
       findings: []
     )
   }
+
+  func testIgnoreTestAttrinuteFunction() {
+    assertLint(
+      NeverUseImplicitlyUnwrappedOptionals.self,
+      """
+      @Test
+      func testSomeFunc() {
+        var s: String!
+        func nestedFunc() {
+          var f: Foo!
+        }
+      }
+      """,
+      findings: []
+    )
+  }
 }