Skip to content

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

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 7 commits into from
Jul 19, 2024
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
4 changes: 4 additions & 0 deletions Documentation/RuleDocumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand All @@ -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.

Expand All @@ -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)

Expand Down
1 change: 1 addition & 0 deletions Sources/SwiftFormat/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions Sources/SwiftFormat/Core/SyntaxProtocol+Convenience.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
37 changes: 37 additions & 0 deletions Sources/SwiftFormat/Core/WithAttributesSyntax+Convenience.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
}
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/Rules/AlwaysUseLowerCamelCase.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions Sources/SwiftFormat/Rules/NeverForceUnwrap.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions Sources/SwiftFormat/Rules/NeverUseForceTry.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down
23 changes: 23 additions & 0 deletions Tests/SwiftFormatTests/Rules/AlwaysUseLowerCamelCaseTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
]
)
}
}
19 changes: 19 additions & 0 deletions Tests/SwiftFormatTests/Rules/NeverForceUnwrapTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
)
}
}
16 changes: 16 additions & 0 deletions Tests/SwiftFormatTests/Rules/NeverUseForceTryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
)
}
}