Merge pull request #417 from allevato/conditional-xctests

Improve `XCTest` import detection logic.

Author: allevato

Diff for: Sources/SwiftFormatRules/ImportsXCTestVisitor.swift

@@ -13,39 +13,47 @@
 import SwiftFormatCore
 import SwiftSyntax
 
-/// Visitor that determines if the target source file imports XCTest
-fileprivate class ImportsXCTestVisitor: SyntaxVisitor {
+/// A visitor that determines if the target source file imports `XCTest`.
+private class ImportsXCTestVisitor: SyntaxVisitor {
   private let context: Context
 
   init(context: Context) {
     self.context = context
     super.init(viewMode: .sourceAccurate)
   }
 
-  override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
-    for statement in node.statements {
-      guard let importDecl = statement.item.as(ImportDeclSyntax.self) else { continue }
-      for component in importDecl.path {
-        guard component.name.text == "XCTest" else { continue }
-        context.importsXCTest = .importsXCTest
-        return .skipChildren
-      }
+  override func visit(_ node: ImportDeclSyntax) -> SyntaxVisitorContinueKind {
+    // If we already know whether or not `XCTest` is imported, don't bother doing anything else.
+    guard context.importsXCTest == .notDetermined else { return .skipChildren }
+
+    // If the first import path component is the `XCTest` module, record that fact. Checking in this
+    // way lets us catch `import XCTest` but also specific decl imports like
+    // `import class XCTest.XCTestCase`, if someone wants to do that.
+    if node.path.first!.name.tokenKind == .identifier("XCTest") {
+      context.importsXCTest = .importsXCTest
     }
-    context.importsXCTest = .doesNotImportXCTest
+
     return .skipChildren
   }
+
+  override func visitPost(_ node: SourceFileSyntax) {
+    // If we visited the entire source file and didn't find an `XCTest` import, record that fact.
+    if context.importsXCTest == .notDetermined {
+      context.importsXCTest = .doesNotImportXCTest
+    }
+  }
 }
 
-/// Sets the appropriate value of the importsXCTest field in the Context class, which
-/// indicates whether the file contains test code or not.
+/// Sets the appropriate value of the `importsXCTest` field in the context, which approximates
+/// whether the file contains test code or not.
 ///
 /// This setter will only run the visitor if another rule hasn't already called this function to
-/// determine if the source file imports XCTest.
+/// determine if the source file imports `XCTest`.
 ///
 /// - Parameters:
 ///   - context: The context information of the target source file.
 ///   - sourceFile: The file to be visited.
-func setImportsXCTest(context: Context, sourceFile: SourceFileSyntax) {
+public func setImportsXCTest(context: Context, sourceFile: SourceFileSyntax) {
   guard context.importsXCTest == .notDetermined else { return }
   let visitor = ImportsXCTestVisitor(context: context)
   visitor.walk(sourceFile)

Diff for: Tests/SwiftFormatRulesTests/AlwaysUseLowerCamelCaseTests.swift

@@ -124,6 +124,54 @@ final class AlwaysUseLowerCamelCaseTests: LintOrFormatRuleTestCase {
       .nameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode_Throws", description: "function"))
   }
 
+  func testIgnoresUnderscoresInTestNamesWhenImportedConditionally() {
+    let input =
+      """
+      #if SOME_FEATURE_FLAG
+        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 {}
+        }
+      #endif
+      """
+    performLint(AlwaysUseLowerCamelCase.self, input: input)
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("Test", description: "constant"), line: 4, column: 7)
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("My_Constant_Value", description: "constant"), line: 6, column: 16)
+    XCTAssertNotDiagnosed(
+      .nameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode", description: "function"))
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("FooFunc", description: "function"), line: 8, column: 18)
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("helperFunc_For_HappyPath_Setup", description: "function"),
+      line: 9, column: 18)
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("testLikeMethod_With_Underscores", description: "function"),
+      line: 10, column: 18)
+    XCTAssertDiagnosed(
+      .nameMustBeLowerCamelCase("testLikeMethod_With_Underscores2", description: "function"),
+      line: 11, column: 18)
+    XCTAssertNotDiagnosed(
+      .nameMustBeLowerCamelCase(
+        "test_HappyPath_Through_GoodCode_ReturnsVoid", description: "function"))
+    XCTAssertNotDiagnosed(
+      .nameMustBeLowerCamelCase(
+        "test_HappyPath_Through_GoodCode_ReturnsShortVoid", description: "function"))
+    XCTAssertNotDiagnosed(
+      .nameMustBeLowerCamelCase("test_HappyPath_Through_GoodCode_Throws", description: "function"))
+  }
+
   func testIgnoresFunctionOverrides() {
     let input =
       """

Diff for: Tests/SwiftFormatRulesTests/ImportsXCTestVisitorTests.swift

@@ -0,0 +1,57 @@
+import SwiftFormatCore
+import SwiftFormatRules
+import SwiftFormatTestSupport
+import SwiftParser
+import XCTest
+
+class ImportsXCTestVisitorTests: DiagnosingTestCase {
+  func testDoesNotImportXCTest() throws {
+    XCTAssertEqual(
+      try makeContextAndSetImportsXCTest(source: """
+        import Foundation
+        """),
+      .doesNotImportXCTest
+    )
+  }
+
+  func testImportsXCTest() throws {
+    XCTAssertEqual(
+      try makeContextAndSetImportsXCTest(source: """
+        import Foundation
+        import XCTest
+        """),
+      .importsXCTest
+    )
+  }
+
+  func testImportsSpecificXCTestDecl() throws {
+    XCTAssertEqual(
+      try makeContextAndSetImportsXCTest(source: """
+        import Foundation
+        import class XCTest.XCTestCase
+        """),
+      .importsXCTest
+    )
+  }
+
+  func testImportsXCTestInsideConditional() throws {
+    XCTAssertEqual(
+      try makeContextAndSetImportsXCTest(source: """
+        import Foundation
+        #if SOME_FEATURE_FLAG
+          import XCTest
+        #endif
+        """),
+      .importsXCTest
+    )
+  }
+
+  /// Parses the given source, makes a new `Context`, then populates and returns its `XCTest`
+  /// import state.
+  private func makeContextAndSetImportsXCTest(source: String) throws -> Context.XCTestImportState {
+    let sourceFile = try Parser.parse(source: source)
+    let context = makeContext(sourceFileSyntax: sourceFile)
+    setImportsXCTest(context: context, sourceFile: sourceFile)
+    return context.importsXCTest
+  }
+}