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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1c9d743
Add check to allow underscores in test functions marked with @Test at…
hamtiko Jul 13, 2024
5a63aee
Add Swift Testing checks
hamtiko Jul 14, 2024
77779ac
Move attribute lookup to separate extension
hamtiko Jul 15, 2024
dca8bec
Merge branch 'adding-swift-testing-checks' into allow-underscore-for-…
hamtiko Jul 15, 2024
c7d8018
Use new extension for attribute lookup
hamtiko Jul 15, 2024
8019514
Update hasAttribute extension signature to check module name as well.
hamtiko Jul 18, 2024
d2d5dc3
updating the doc for hasAttribute
hamtiko Jul 18, 2024
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
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/Core/SyntaxProtocol+Convenience.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ extension SyntaxProtocol {
var hasTestAncestor: Bool {
var parent = self.parent
while let existingParent = parent {
if let functionDecl = existingParent.as(FunctionDeclSyntax.self), functionDecl.hasAttribute("Test") {
if let functionDecl = existingParent.as(FunctionDeclSyntax.self),
functionDecl.hasAttribute("Test", inModule: "Testing") {
return true
}
parent = existingParent.parent
Expand Down
29 changes: 18 additions & 11 deletions Sources/SwiftFormat/Core/WithAttributesSyntax+Convenience.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,23 @@
import SwiftSyntax

extension WithAttributesSyntax {
/// Indicates whether the node has attribute with the given `name`.
///
/// - Parameter name: The name of the attribute to lookup.
/// - Returns: True if the node has an attribute with the given `name`, otherwise false.
func hasAttribute(_ name: String) -> Bool {
attributes.contains { attribute in
let attributeName = attribute.as(AttributeSyntax.self)?.attributeName
return attributeName?.as(IdentifierTypeSyntax.self)?.name.text == name
// support @Module.Attribute syntax as well
|| attributeName?.as(MemberTypeSyntax.self)?.name.text == name
}
/// Indicates whether the node has attribute with the given `name`.
Copy link
Member

@allevato allevato Jul 18, 2024

Choose a reason for hiding this comment

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

Can you mention in the doc comment that the module name is only considered if present if the attribute is written as @Module.Name, and that that's intentional? Otherwise it might look like a bug to someone reading it in the future.

Copy link
Contributor Author

@hamtiko hamtiko Jul 18, 2024

Choose a reason for hiding this comment

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

good point, done. 👍

///
/// - 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) {
return identifier.name.text == name
}
// support @Module.Attribute syntax as well
if let memberType = attributeName?.as(MemberTypeSyntax.self) {
return memberType.name.text == name
&& memberType.baseType.as(IdentifierTypeSyntax.self)?.name.text == module
}
return false
}
}
}
2 changes: 1 addition & 1 deletion Sources/SwiftFormat/Rules/AlwaysUseLowerCamelCase.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ 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) || node.hasAttribute("Test")
let allowUnderscores = testCaseFuncs.contains(node) || node.hasAttribute("Test", inModule: "Testing")

diagnoseLowerCamelCaseViolations(
node.name, allowUnderscores: allowUnderscores,
Expand Down
3 changes: 3 additions & 0 deletions Tests/SwiftFormatTests/Rules/AlwaysUseLowerCamelCaseTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,13 @@ final class AlwaysUseLowerCamelCaseTests: LintOrFormatRuleTestCase {
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"),
]
)
}
Expand Down