Skip to content

Improve the DontRepeatTypeInStaticProperties rule to better align with its original intent #918

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
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,23 @@ public final class DontRepeatTypeInStaticProperties: SyntaxLintRule {
/// type name (excluding possible namespace prefixes, like `NS` or `UI`) as a suffix.
public override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind {
guard node.modifiers.contains(anyOf: [.class, .static]),
let typeName = Syntax(node).containingDeclName
let typeName = Syntax(node).containingDeclName,
let variableTypeName = node.typeName,
typeName.hasSuffix(variableTypeName) || variableTypeName == "Self"
else {
return .visitChildren
}

let bareTypeName = removingPossibleNamespacePrefix(from: typeName)
// the final component of the top type `A.B.C.D` is what we want `D`.
let lastTypeName = typeName.components(separatedBy: ".").last!
let bareTypeName = removingPossibleNamespacePrefix(from: lastTypeName)
for binding in node.bindings {
guard let identifierPattern = binding.pattern.as(IdentifierPatternSyntax.self) else {
continue
}

let varName = identifierPattern.identifier.text
if varName.contains(bareTypeName) {
if varName.hasSuffix(bareTypeName) {
diagnose(.removeTypeFromName(name: varName, type: bareTypeName), on: identifierPattern)
}
}
Expand Down Expand Up @@ -90,8 +94,7 @@ extension Syntax {
case .identifierType(let simpleType):
return simpleType.name.text
case .memberType(let memberType):
// the final component of the top type `A.B.C.D` is what we want `D`.
return memberType.name.text
return memberType.description.trimmingCharacters(in: .whitespacesAndNewlines)
default:
// Do nothing for non-nominal types. If Swift adds support for extensions on non-nominals,
// we'll need to update this if we need to support some subset of those.
Expand All @@ -106,3 +109,23 @@ extension Syntax {
}
}
}

extension VariableDeclSyntax {
fileprivate var typeName: String? {
if let typeAnnotation = bindings.first?.typeAnnotation {
return typeAnnotation.type.description
} else if let initializerCalledExpression = bindings.first?.initializer?.value.as(FunctionCallExprSyntax.self)?
.calledExpression
{
if let memberAccessExprSyntax = initializerCalledExpression.as(MemberAccessExprSyntax.self),
memberAccessExprSyntax.declName.baseName.tokenKind == .keyword(.`init`)
{
return memberAccessExprSyntax.base?.description
} else {
return initializerCalledExpression.description
}
} else {
return nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
extension A {
static let b = C()
}
""",
findings: []
extension UIImage {
static let fooImage: Int
}
"""
)
}

Expand All @@ -75,20 +77,89 @@ final class DontRepeatTypeInStaticPropertiesTests: LintOrFormatRuleTestCase {
)
}

func testDottedExtendedTypeWithNamespacePrefix() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
extension Dotted.RANDThing {
static let 1️⃣defaultThing: Dotted.RANDThing
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
]
)
}

func testSelfType() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
extension Dotted.Thing {
static let 1️⃣defaultThing: Self
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
]
)
}

func testDottedExtendedTypeInitializer() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
extension Dotted.Thing {
static let 1️⃣defaultThing = Dotted.Thing()
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Thing' from the name of the variable 'defaultThing'")
]
)
}

func testExplicitInitializer() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
struct Foo {
static let 1️⃣defaultFoo = Foo.init()
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'defaultFoo'")
]
)
}

func testSelfTypeInitializer() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
struct Foo {
static let 1️⃣defaultFoo = Self()
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'defaultFoo'")
]
)
}

func testIgnoreSingleDecl() {
assertLint(
DontRepeatTypeInStaticProperties.self,
"""
struct Foo {
// swift-format-ignore: DontRepeatTypeInStaticProperties
static let defaultFoo: Int
static let 1️⃣alternateFoo: Int
static let defaultFoo: Foo
static let 1️⃣alternateFoo: Foo
}
""",
findings: [
FindingSpec("1️⃣", message: "remove the suffix 'Foo' from the name of the variable 'alternateFoo'")
]
)
}

}