Skip to content

Improve XCTest import detection logic. #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions Sources/SwiftFormatRules/ImportsXCTestVisitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions Tests/SwiftFormatRulesTests/AlwaysUseLowerCamelCaseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
"""
Expand Down
57 changes: 57 additions & 0 deletions Tests/SwiftFormatRulesTests/ImportsXCTestVisitorTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import SwiftFormatCore
import SwiftFormatRules
import SwiftFormatTestSupport
import SwiftParser
import XCTest

class ImportsXCTestVisitorTests: DiagnosingTestCase {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would attributes like @_implementationOnly effect the decl? Might add a test case that has an attribute on the XCTest import.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They won't make a difference since we visit all ImportDecls and ignore the attributes.

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
}
}