Skip to content

Commit d28ac00

Browse files
committed
Add idiomatic optional binding detection
1 parent 4a0ca31 commit d28ac00

1 file changed

Lines changed: 52 additions & 14 deletions

File tree

Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,30 @@ struct VariableShadowingRule: Rule {
102102
print(item)
103103
}
104104
"""),
105+
Example("""
106+
let a: Int?
107+
if let a { print(a) }
108+
"""),
109+
Example("""
110+
let a: Int?
111+
guard let a else { return }
112+
"""),
113+
Example("""
114+
let a: Int?
115+
while let a { print(a) }
116+
"""),
117+
Example("""
118+
var a = 1
119+
if let a = a {
120+
print(a)
121+
}
122+
"""),
123+
Example("""
124+
func test() {
125+
var a = 1
126+
var b = 2
127+
}
128+
"""),
105129
],
106130
triggeringExamples: [
107131
Example("""
@@ -271,7 +295,7 @@ private extension VariableShadowingRule {
271295
override func visit(_ node: IfExprSyntax) -> SyntaxVisitorContinueKind {
272296
for condition in node.conditions {
273297
if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) {
274-
checkForBindingShadowing(in: optBinding.pattern)
298+
checkForBindingShadowing(in: optBinding.pattern, binding: optBinding)
275299
}
276300
}
277301
return super.visit(node)
@@ -280,7 +304,7 @@ private extension VariableShadowingRule {
280304
override func visit(_ node: WhileStmtSyntax) -> SyntaxVisitorContinueKind {
281305
for condition in node.conditions {
282306
if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) {
283-
checkForBindingShadowing(in: optBinding.pattern)
307+
checkForBindingShadowing(in: optBinding.pattern, binding: optBinding)
284308
}
285309
}
286310
return super.visit(node)
@@ -289,7 +313,7 @@ private extension VariableShadowingRule {
289313
override func visit(_ node: GuardStmtSyntax) -> SyntaxVisitorContinueKind {
290314
for condition in node.conditions {
291315
if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) {
292-
checkForBindingShadowing(in: optBinding.pattern)
316+
checkForBindingShadowing(in: optBinding.pattern, binding: optBinding)
293317
}
294318
}
295319
return super.visit(node)
@@ -312,11 +336,8 @@ private extension VariableShadowingRule {
312336
return result
313337
}
314338

315-
// Used for VariableDecl: checks only ancestor scopes (dropLast), not the current scope.
316-
// This avoids false positives for same-scope redeclarations, which are compile errors in
317-
// Swift and are not shadowing. Using isShadowingAnyScope here would incorrectly flag code
318-
// where the same name appears twice at the same scope level (e.g. inside a disabled lint
319-
// region followed by an enabled one).
339+
// For VariableDecl: checks only ancestor scopes, not current scope.
340+
// Avoids false positives for same-scope redeclarations (compile errors in Swift).
320341
private func checkForShadowing(in pattern: PatternSyntax) {
321342
if let identifier = pattern.as(IdentifierPatternSyntax.self) {
322343
let identifierText = identifier.identifier.text
@@ -332,24 +353,41 @@ private extension VariableShadowingRule {
332353
}
333354
}
334355

335-
// Used for if-let / for-loop / while-let / guard-let bindings: the binding is added to a
336-
// child scope, so checking all current scopes (including the one where the outer variable
337-
// lives) is correct and necessary.
338-
private func checkForBindingShadowing(in pattern: PatternSyntax) {
356+
// For optional bindings - skips idiomatic patterns (if let a / if let a = a)
357+
private func checkForBindingShadowing(
358+
in pattern: PatternSyntax,
359+
binding: OptionalBindingConditionSyntax? = nil
360+
) {
339361
if let identifier = pattern.as(IdentifierPatternSyntax.self) {
340362
let identifierText = identifier.identifier.text
363+
if let binding, isIdiomatic(pattern: identifier, binding: binding) {
364+
return
365+
}
341366
if hasSeenDeclaration(for: identifierText) {
342367
violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia)
343368
}
344369
} else if let tuple = pattern.as(TuplePatternSyntax.self) {
345370
tuple.elements.forEach { element in
346-
checkForBindingShadowing(in: element.pattern)
371+
checkForBindingShadowing(in: element.pattern, binding: binding)
347372
}
348373
} else if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) {
349-
checkForBindingShadowing(in: valueBinding.pattern)
374+
checkForBindingShadowing(in: valueBinding.pattern, binding: binding)
350375
}
351376
}
352377

378+
// Idiomatic: shorthand (if let a) or identity (if let a = a)
379+
private func isIdiomatic(
380+
pattern: IdentifierPatternSyntax,
381+
binding: OptionalBindingConditionSyntax
382+
) -> Bool {
383+
let patternName = pattern.identifier.text
384+
guard let initializer = binding.initializer else { return true }
385+
if let identifierExpr = initializer.value.as(DeclReferenceExprSyntax.self) {
386+
return identifierExpr.baseName.text == patternName
387+
}
388+
return false
389+
}
390+
353391
private func isShadowingOuterScope(_ identifier: String) -> Bool {
354392
guard scope.count > 1 else { return false }
355393
for scopeDeclarations in scope.dropLast() where

0 commit comments

Comments
 (0)