Merge pull request #221 from dylansturg/test_name_freedom

Add an exception to AlwaysUseLowerCamelCase for test names.

Author: allevato

Sources/SwiftFormatRules/AlwaysUseLowerCamelCase.swift

@@ -19,38 +19,84 @@ import SwiftSyntax
 /// Lint: If an identifier contains underscores or begins with a capital letter, a lint error is
 ///       raised.
 public final class AlwaysUseLowerCamelCase: SyntaxLintRule {
+  /// Stores function decls that are test cases.
+  private var testCaseFuncs = Set<FunctionDeclSyntax>()
+
+  public override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
+    // Tracks whether "XCTest" is imported in the source file before processing individual nodes.
+    setImportsXCTest(context: context, sourceFile: node)
+    return .visitChildren
+  }
+
+  public override func visit(_ node: ClassDeclSyntax) -> SyntaxVisitorContinueKind {
+    // Check if this class is an `XCTestCase`, otherwise it cannot contain any test cases.
+    guard context.importsXCTest == .importsXCTest else { return .visitChildren }
+
+    // Identify and store all of the function decls that are test cases.
+    let testCases = node.members.members.compactMap {
+      $0.decl.as(FunctionDeclSyntax.self)
+    }.filter {
+      // Filter out non-test methods using the same heuristics as XCTest to identify tests.
+      // Test methods are methods that start with "test", have no arguments, and void return type.
+      $0.identifier.text.starts(with: "test")
+        && $0.signature.input.parameterList.isEmpty
+        && $0.signature.output.map { $0.isVoid } ?? true
+    }
+    testCaseFuncs.formUnion(testCases)
+    return .visitChildren
+  }
+
+  public override func visitPost(_ node: ClassDeclSyntax) {
+    testCaseFuncs.removeAll()
+  }
 
   public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
     for binding in node.bindings {
       guard let pat = binding.pattern.as(IdentifierPatternSyntax.self) else {
         continue
       }
-      diagnoseLowerCamelCaseViolations(pat.identifier)
+      diagnoseLowerCamelCaseViolations(pat.identifier, allowUnderscores: false)
     }
     return .skipChildren
   }
 
   public override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseLowerCamelCaseViolations(node.identifier)
+    // 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)
+    diagnoseLowerCamelCaseViolations(node.identifier, allowUnderscores: allowUnderscores)
     return .skipChildren
   }
 
   public override func visit(_ node: EnumCaseElementSyntax) -> SyntaxVisitorContinueKind {
-    diagnoseLowerCamelCaseViolations(node.identifier)
+    diagnoseLowerCamelCaseViolations(node.identifier, allowUnderscores: false)
     return .skipChildren
   }
 
-  private func diagnoseLowerCamelCaseViolations(_ identifier: TokenSyntax) {
+  private func diagnoseLowerCamelCaseViolations(_ identifier: TokenSyntax, allowUnderscores: Bool) {
     guard case .identifier(let text) = identifier.tokenKind else { return }
     if text.isEmpty { return }
-    if text.dropFirst().contains("_") || ("A"..."Z").contains(text.first!) {
+    if (text.dropFirst().contains("_") && !allowUnderscores) || ("A"..."Z").contains(text.first!) {
       diagnose(.variableNameMustBeLowerCamelCase(text), on: identifier) {
         $0.highlight(identifier.sourceRange(converter: self.context.sourceLocationConverter))
       }
     }
   }
 }
 
+extension ReturnClauseSyntax {
+  /// Whether this return clause specifies an explicit `Void` return type.
+  fileprivate var isVoid: Bool {
+    if let returnTypeIdentifier = returnType.as(SimpleTypeIdentifierSyntax.self) {
+      return returnTypeIdentifier.name.text == "Void"
+    }
+    if let returnTypeTuple = returnType.as(TupleTypeSyntax.self) {
+      return returnTypeTuple.elements.isEmpty
+    }
+    return false
+  }
+}
+
 extension Diagnostic.Message {
   public static func variableNameMustBeLowerCamelCase(_ name: String) -> Diagnostic.Message {
     return .init(.warning, "rename variable '\(name)' using lower-camel-case")

Sources/SwiftFormatRules/NeverForceUnwrap.swift

@@ -19,7 +19,7 @@ import SwiftSyntax
 public final class NeverForceUnwrap: SyntaxLintRule {
 
   public override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
-    // Tracks whether "XCTest" is imported in the source file before processing the individual
+    // Tracks whether "XCTest" is imported in the source file before processing individual nodes.
     setImportsXCTest(context: context, sourceFile: node)
     return .visitChildren
   }

Tests/SwiftFormatRulesTests/AlwaysUseLowerCamelCaseTests.swift

@@ -1,6 +1,11 @@
 import SwiftFormatRules
 
 final class AlwaysUseLowerCamelCaseTests: LintOrFormatRuleTestCase {
+  override func setUp() {
+    super.setUp()
+    shouldCheckForUnassertedDiagnostics = true
+  }
+
   func testInvalidVariableCasing() {
     let input =
       """
@@ -11,13 +16,55 @@ final class AlwaysUseLowerCamelCaseTests: LintOrFormatRuleTestCase {
       struct Foo {
         func FooFunc() {}
       }
+      class UnitTests: XCTestCase {
+        func test_HappyPath_Through_GoodCode() {}
+      }
       """
     performLint(AlwaysUseLowerCamelCase.self, input: input)
-    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("Test"))
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("Test"), line: 1, column: 5)
     XCTAssertNotDiagnosed(.variableNameMustBeLowerCamelCase("foo"))
-    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("bad_name"))
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("bad_name"), line: 3, column: 5)
     XCTAssertNotDiagnosed(.variableNameMustBeLowerCamelCase("_okayName"))
     XCTAssertNotDiagnosed(.variableNameMustBeLowerCamelCase("Foo"))
-    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("FooFunc"))
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("FooFunc"), line: 6, column: 8)
+    XCTAssertDiagnosed(
+      .variableNameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode"), line: 9, column: 8)
+  }
+
+  func testIgnoresUnderscoresInTestNames() {
+    let input =
+      """
+      import XCTest
+
+      let Test = 1
+      class UnitTests: XCTestCase {
+        static let My_Constant_Value = 0
+        func test_HappyPath_Through_GoodCode() {}
+        private func FooFunc() {}
+        private func helperFunc_For_HappyPath_Setup() {}
+        private func testLikeMethod_With_Underscores(_ arg1: ParamType) {}
+        private func testLikeMethod_With_Underscores2() -> ReturnType {}
+        func test_HappyPath_Through_GoodCode_ReturnsVoid() -> Void {}
+        func test_HappyPath_Through_GoodCode_ReturnsShortVoid() -> () {}
+        func test_HappyPath_Through_GoodCode_Throws() throws {}
+      }
+      """
+    performLint(AlwaysUseLowerCamelCase.self, input: input)
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("Test"), line: 3, column: 5)
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("My_Constant_Value"), line: 5, column: 14)
+    XCTAssertNotDiagnosed(.variableNameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode"))
+    XCTAssertDiagnosed(.variableNameMustBeLowerCamelCase("FooFunc"), line: 7, column: 16)
+    XCTAssertDiagnosed(
+      .variableNameMustBeLowerCamelCase("helperFunc_For_HappyPath_Setup"), line: 8, column: 16)
+    XCTAssertDiagnosed(
+      .variableNameMustBeLowerCamelCase("testLikeMethod_With_Underscores"), line: 9, column: 16)
+    XCTAssertDiagnosed(
+      .variableNameMustBeLowerCamelCase("testLikeMethod_With_Underscores2"), line: 10, column: 16)
+    XCTAssertNotDiagnosed(
+      .variableNameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode_ReturnsVoid"))
+    XCTAssertNotDiagnosed(
+      .variableNameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode_ReturnsShortVoid"))
+    XCTAssertNotDiagnosed(
+      .variableNameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode_Throws"))
   }
 }