From 9f332683af871ae407446a3fd2c527977e9ace85 Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Mon, 30 Mar 2026 20:45:08 +0100 Subject: [PATCH 01/15] Adds Variable Shadowing Rule --- CHANGELOG.md | 4 + .../Models/BuiltInRules.swift | 1 + .../Rules/Lint/VariableShadowingRule.swift | 148 ++++++++++++++++++ Tests/GeneratedTests/GeneratedTests_10.swift | 6 + .../Resources/default_rule_configurations.yml | 5 + 5 files changed, 164 insertions(+) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index f6455ddce4..aedbcccc98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,10 @@ and FEFF formatting character (U+FEFF) in string literals, which can cause hard-to-debug issues. [kapitoshka438](https://github.com/kapitoshka438) [#6045](https://github.com/realm/SwiftLint/issues/6045) +* Add `variable_shadowing` rule that flags when a variable declaration shadows + an identifier from an outer scope. + [nadeemnali](https://github.com/nadeemnali) + [#6228](https://github.com/realm/SwiftLint/issues/6228) ### Bug Fixes diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index b6341fe12e..b30f1fbf89 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -240,6 +240,7 @@ public let builtInRules: [any Rule.Type] = [ UnusedParameterRule.self, UnusedSetterValueRule.self, ValidIBInspectableRule.self, + VariableShadowingRule.self, VerticalParameterAlignmentOnCallRule.self, VerticalParameterAlignmentRule.self, VerticalWhitespaceBetweenCasesRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift new file mode 100644 index 0000000000..f92c137732 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -0,0 +1,148 @@ +import SwiftLintCore +import SwiftSyntax + +@SwiftSyntaxRule +struct VariableShadowingRule: Rule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "variable_shadowing", + name: "Variable Shadowing", + description: "Prefer not to shadow variables declared in outer scopes", + kind: .lint, + nonTriggeringExamples: [ + Example(""" + var a: String? + func test(a: String?) { + print(a) + } + """), + Example(""" + var a: String = "hello" + if let b = a { + print(b) + } + """), + Example(""" + var a: String? + func test() { + if let b = a { + print(b) + } + } + """), + Example(""" + for i in 1...10 { + print(i) + } + for j in 1...10 { + print(j) + } + """), + Example(""" + func test() { + var a: String = "hello" + func nested() { + var b: String = "world" + print(a, b) + } + } + """), + Example(""" + class Test { + var a: String? + func test(a: String?) { + print(a) + } + } + """), + ], + triggeringExamples: [ + Example(""" + var outer: String = "hello" + func test() { + let ↓outer = "world" + print(outer) + } + """), + ] + ) +} + +private extension VariableShadowingRule { + final class Visitor: DeclaredIdentifiersTrackingVisitor { + override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind { + // Early exit for member blocks (class/struct properties) + if node.parent?.is(MemberBlockItemSyntax.self) != true { + // Check for shadowing BEFORE adding to scope + node.bindings.forEach { binding in + checkForShadowing(in: binding.pattern) + } + } + return super.visit(node) + } + + override func visit(_ node: CodeBlockItemListSyntax) -> SyntaxVisitorContinueKind { + guard let parent = node.parent else { + return super.visit(node) + } + + // Call super first so the base visitor opens the child scope and collects identifiers + let kind = super.visit(node) + + // Check conditions if this is an if/while body AFTER identifiers from conditions were collected + if let ifStmt = parent.as(IfExprSyntax.self) { + checkForShadowingInConditions(ifStmt.conditions) + } else if let whileStmt = parent.as(WhileStmtSyntax.self) { + checkForShadowingInConditions(whileStmt.conditions) + } + + return kind + } + + override func visitPost(_ node: GuardStmtSyntax) { + // Let the base visitor collect identifiers first (it does so in its visitPost) + super.visitPost(node) + // Check for shadowing in guard conditions after collection + checkForShadowingInConditions(node.conditions) + } + + private func checkForShadowingInConditions(_ conditions: ConditionElementListSyntax) { + for element in conditions { + if let binding = element.condition.as(OptionalBindingConditionSyntax.self) { + checkForShadowing(in: binding.pattern) + } + } + } + + private func checkForShadowing(in pattern: PatternSyntax) { + if let identifier = pattern.as(IdentifierPatternSyntax.self) { + let identifierText = identifier.identifier.text + if isShadowingOuterScope(identifierText) { + violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia) + } + return + } + + if let tuple = pattern.as(TuplePatternSyntax.self) { + tuple.elements.forEach { element in + checkForShadowing(in: element.pattern) + } + return + } + + // Other pattern kinds are not relevant for shadowing checks here; no action needed. + } + + private func isShadowingOuterScope(_ identifier: String) -> Bool { + guard !scope.isEmpty, scope.count > 1 else { return false } + + // Use early exit and lazy evaluation for better performance + for scopeDeclarations in scope.dropLast() where + scopeDeclarations.lazy.contains(where: { $0.declares(id: identifier) }) { + return true + } + return false + } + } +} diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index 46bb104908..30647bbe07 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -85,6 +85,12 @@ final class ValidIBInspectableRuleGeneratedTests: SwiftLintTestCase { } } +final class VariableShadowingRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(VariableShadowingRule.description) + } +} + final class VerticalParameterAlignmentOnCallRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(VerticalParameterAlignmentOnCallRule.description) diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index fb30c62bd2..a5c27b6bfb 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -1383,6 +1383,11 @@ valid_ibinspectable: meta: opt-in: false correctable: false +variable_shadowing: + severity: warning + meta: + opt-in: false + correctable: false vertical_parameter_alignment: severity: warning meta: From 9b413a2286bd2c2f3bb4a1186f4968d7681ecd16 Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Mon, 30 Mar 2026 21:36:16 +0100 Subject: [PATCH 02/15] Updates failing test cases --- .swiftlint.yml | 2 + .../Rules/Lint/VariableShadowingRule.swift | 41 ++++--------------- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 3f27461534..1c83037798 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -51,6 +51,8 @@ disabled_rules: - trailing_closure - type_contents_order - vertical_whitespace_between_cases + - variable_shadowing + # Configurations attributes: diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index f92c137732..e32bd54fd2 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -82,40 +82,8 @@ private extension VariableShadowingRule { return super.visit(node) } - override func visit(_ node: CodeBlockItemListSyntax) -> SyntaxVisitorContinueKind { - guard let parent = node.parent else { - return super.visit(node) - } - - // Call super first so the base visitor opens the child scope and collects identifiers - let kind = super.visit(node) - - // Check conditions if this is an if/while body AFTER identifiers from conditions were collected - if let ifStmt = parent.as(IfExprSyntax.self) { - checkForShadowingInConditions(ifStmt.conditions) - } else if let whileStmt = parent.as(WhileStmtSyntax.self) { - checkForShadowingInConditions(whileStmt.conditions) - } - - return kind - } - - override func visitPost(_ node: GuardStmtSyntax) { - // Let the base visitor collect identifiers first (it does so in its visitPost) - super.visitPost(node) - // Check for shadowing in guard conditions after collection - checkForShadowingInConditions(node.conditions) - } - - private func checkForShadowingInConditions(_ conditions: ConditionElementListSyntax) { - for element in conditions { - if let binding = element.condition.as(OptionalBindingConditionSyntax.self) { - checkForShadowing(in: binding.pattern) - } - } - } - private func checkForShadowing(in pattern: PatternSyntax) { + // Handle direct identifier patterns if let identifier = pattern.as(IdentifierPatternSyntax.self) { let identifierText = identifier.identifier.text if isShadowingOuterScope(identifierText) { @@ -124,6 +92,7 @@ private extension VariableShadowingRule { return } + // Recurse into tuple patterns: e.g., (a, b) if let tuple = pattern.as(TuplePatternSyntax.self) { tuple.elements.forEach { element in checkForShadowing(in: element.pattern) @@ -131,6 +100,12 @@ private extension VariableShadowingRule { return } + // Recurse into value binding patterns: e.g., `let a`, `var (a, b)` + if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) { + checkForShadowing(in: valueBinding.pattern) + return + } + // Other pattern kinds are not relevant for shadowing checks here; no action needed. } From de5c6726197f0df827aa0e2c2f0b8d9168c383f8 Mon Sep 17 00:00:00 2001 From: Nadeem Ali <44068397+nadeemnali@users.noreply.github.com> Date: Wed, 1 Apr 2026 20:17:17 +0100 Subject: [PATCH 03/15] Update Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .../Rules/Lint/VariableShadowingRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index e32bd54fd2..01f328728b 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -8,7 +8,7 @@ struct VariableShadowingRule: Rule { static let description = RuleDescription( identifier: "variable_shadowing", name: "Variable Shadowing", - description: "Prefer not to shadow variables declared in outer scopes", + description: "Do not shadow variables declared in outer scopes", kind: .lint, nonTriggeringExamples: [ Example(""" From 993154a492736972d9fd61ed527ef7fb77ec3258 Mon Sep 17 00:00:00 2001 From: Nadeem Ali <44068397+nadeemnali@users.noreply.github.com> Date: Wed, 1 Apr 2026 20:17:35 +0100 Subject: [PATCH 04/15] Update Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .../Rules/Lint/VariableShadowingRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 01f328728b..8565f134e2 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -73,7 +73,7 @@ private extension VariableShadowingRule { final class Visitor: DeclaredIdentifiersTrackingVisitor { override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind { // Early exit for member blocks (class/struct properties) - if node.parent?.is(MemberBlockItemSyntax.self) != true { + if node.parent?.is(MemberBlockItemSyntax.self) == false { // Check for shadowing BEFORE adding to scope node.bindings.forEach { binding in checkForShadowing(in: binding.pattern) From 08af13d0eb05247511b7f45d5334d465529f060f Mon Sep 17 00:00:00 2001 From: Nadeem Ali <44068397+nadeemnali@users.noreply.github.com> Date: Wed, 1 Apr 2026 20:17:58 +0100 Subject: [PATCH 05/15] Update Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .../Rules/Lint/VariableShadowingRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 8565f134e2..8b98c87bcf 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -114,7 +114,7 @@ private extension VariableShadowingRule { // Use early exit and lazy evaluation for better performance for scopeDeclarations in scope.dropLast() where - scopeDeclarations.lazy.contains(where: { $0.declares(id: identifier) }) { + scopeDeclarations.contains(where: { $0.declares(id: identifier) }) { return true } return false From df937a6f985655f400d09b00ddf22b9ab5ccd3df Mon Sep 17 00:00:00 2001 From: Nadeem Ali <44068397+nadeemnali@users.noreply.github.com> Date: Wed, 1 Apr 2026 20:18:10 +0100 Subject: [PATCH 06/15] Update Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .../Rules/Lint/VariableShadowingRule.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 8b98c87bcf..d8a61bdfe3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -110,7 +110,7 @@ private extension VariableShadowingRule { } private func isShadowingOuterScope(_ identifier: String) -> Bool { - guard !scope.isEmpty, scope.count > 1 else { return false } + guard scope.count > 1 else { return false } // Use early exit and lazy evaluation for better performance for scopeDeclarations in scope.dropLast() where From 76d9b8fbf442f378c0de3b5ebd382456d32ac5d1 Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Wed, 1 Apr 2026 20:57:25 +0100 Subject: [PATCH 07/15] Updated Examples --- .../Rules/Lint/VariableShadowingRule.swift | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index d8a61bdfe3..ca4afb9090 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -56,6 +56,52 @@ struct VariableShadowingRule: Rule { } } """), + Example(""" + var outer: String = "hello" + if let inner = Optional(outer) { + print(inner) + } + """), + Example(""" + var a: String = "outer" + let (b, c) = ("first", "second") + print(a, b, c) + """), + Example(""" + class Test { + var property: String = "class property" + func test() { + var localVar = "local" + print(property, localVar) + } + } + """), + Example(""" + func outer() { + func inner() { + print("no shadowing") + } + } + """), + Example(""" + var result: String? + if let unwrappedResult = result { + print(unwrappedResult) + } + """), + Example(""" + var value: Int? = 10 + guard let safeValue = value else { + return + } + print(safeValue) + """), + Example(""" + var data: [Int?] = [1, nil, 3] + for case let item? in data { + print(item) + } + """), ], triggeringExamples: [ Example(""" @@ -65,6 +111,38 @@ struct VariableShadowingRule: Rule { print(outer) } """), + Example(""" + var x = 1 + do { + let ↓x = 2 + print(x) + } + """), + Example(""" + var counter = 0 + func incrementCounter() { + var ↓counter = 1 + counter += 1 + } + """), + Example(""" + func outer() { + var value = 10 + do { + let ↓value = 20 + print(value) + } + } + """), + Example(""" + var globalName = "global" + func test() { + for item in [1, 2, 3] { + var ↓globalName = "local" + print(globalName) + } + } + """), ] ) } From b8978b70707303b3b7cbd25cc47a0bbfc888e61b Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Wed, 1 Apr 2026 20:57:51 +0100 Subject: [PATCH 08/15] Refactored the conditions --- .../Rules/Lint/VariableShadowingRule.swift | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index ca4afb9090..78b773f897 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -167,30 +167,20 @@ private extension VariableShadowingRule { if isShadowingOuterScope(identifierText) { violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia) } - return - } - - // Recurse into tuple patterns: e.g., (a, b) - if let tuple = pattern.as(TuplePatternSyntax.self) { + } else if let tuple = pattern.as(TuplePatternSyntax.self) { + // Recurse into tuple patterns: e.g., (a, b) tuple.elements.forEach { element in checkForShadowing(in: element.pattern) } - return - } - - // Recurse into value binding patterns: e.g., `let a`, `var (a, b)` - if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) { + } else if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) { + // Recurse into optional binding patterns: e.g., `if let a`, `while var (a, b)` checkForShadowing(in: valueBinding.pattern) - return } - - // Other pattern kinds are not relevant for shadowing checks here; no action needed. } private func isShadowingOuterScope(_ identifier: String) -> Bool { guard scope.count > 1 else { return false } - // Use early exit and lazy evaluation for better performance for scopeDeclarations in scope.dropLast() where scopeDeclarations.contains(where: { $0.declares(id: identifier) }) { return true From 93401210124d5db47e95f5904e3b00b7227b62f2 Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Tue, 7 Apr 2026 23:56:29 +0100 Subject: [PATCH 09/15] Updated Triggering examples with additional use cases --- .../Rules/Lint/VariableShadowingRule.swift | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 78b773f897..40dac8f764 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -143,6 +143,68 @@ struct VariableShadowingRule: Rule { } } """), + Example(""" + var foo = 1 + do { + let ↓foo = 2 + } + """), + Example(""" + var bar = 1 + func test() { + let ↓bar = 2 + } + """), + Example(""" + var a = 1 + if let a = Optional(2) { + let ↓a = 3 + print(a) + } + """), + Example(""" + var i = 1 + for i in 1...3 { + let ↓i = 2 + print(i) + } + """), + Example(""" + func test() { + var a = 1 + do { + var ↓a = 2 + print(a) + } + } + """), + Example(""" + func test() { + var a = 1 + if true { + var ↓a = 2 + print(a) + } + } + """), + Example(""" + func test() { + var a = 1 + for _ in 0..<1 { + var ↓a = 2 + print(a) + } + } + """), + Example(""" + func test() { + var a = 1 + while true { + var ↓a = 2 + break + } + } + """), ] ) } From ac850e1db01cc580da8425d0bb9599eb575d2114 Mon Sep 17 00:00:00 2001 From: Nadeem Ali <44068397+nadeemnali@users.noreply.github.com> Date: Wed, 8 Apr 2026 11:38:33 +0100 Subject: [PATCH 10/15] Apply suggestion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .swiftlint.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 1c83037798..cba38f4e75 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -53,7 +53,6 @@ disabled_rules: - vertical_whitespace_between_cases - variable_shadowing - # Configurations attributes: always_on_line_above: From dd8c79ae853215df33c7dc01e73c812628faaf4a Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Wed, 8 Apr 2026 12:05:25 +0100 Subject: [PATCH 11/15] Fixed failing registration issue --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aedbcccc98..33c047b76d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ and FEFF formatting character (U+FEFF) in string literals, which can cause hard-to-debug issues. [kapitoshka438](https://github.com/kapitoshka438) [#6045](https://github.com/realm/SwiftLint/issues/6045) + * Add `variable_shadowing` rule that flags when a variable declaration shadows an identifier from an outer scope. [nadeemnali](https://github.com/nadeemnali) From b6be605e6559497d6444b99c1d46c98943b0b5c4 Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Wed, 8 Apr 2026 16:27:40 +0100 Subject: [PATCH 12/15] Added Configuration for the function shadowing --- .../Rules/Lint/VariableShadowingRule.swift | 80 ++++++++++++++++--- .../VariableShadowingConfiguration.swift | 9 +++ .../VariableShadowingRuleTests.swift | 15 ++++ .../Resources/default_rule_configurations.yml | 1 + 4 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/VariableShadowingConfiguration.swift create mode 100644 Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 40dac8f764..e6e047db6a 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -3,7 +3,7 @@ import SwiftSyntax @SwiftSyntaxRule struct VariableShadowingRule: Rule { - var configuration = SeverityConfiguration(.warning) + var configuration = VariableShadowingConfiguration() static let description = RuleDescription( identifier: "variable_shadowing", @@ -16,7 +16,7 @@ struct VariableShadowingRule: Rule { func test(a: String?) { print(a) } - """), + """, configuration: ["ignore_parameters": true]), Example(""" var a: String = "hello" if let b = a { @@ -157,14 +157,14 @@ struct VariableShadowingRule: Rule { """), Example(""" var a = 1 - if let a = Optional(2) { + if let ↓a = Optional(2) { let ↓a = 3 print(a) } """), Example(""" var i = 1 - for i in 1...3 { + for ↓i in 1...3 { let ↓i = 2 print(i) } @@ -205,16 +205,28 @@ struct VariableShadowingRule: Rule { } } """), + Example(""" + var a = 1 + if let ↓a = Optional(2) {} + """), + Example(""" + var i = 1 + for ↓i in 1...3 {} + """), + Example(""" + var a: String? + func test(↓a: String?) { + print(a) + } + """, configuration: ["ignore_parameters": false]), ] ) } private extension VariableShadowingRule { - final class Visitor: DeclaredIdentifiersTrackingVisitor { + final class Visitor: DeclaredIdentifiersTrackingVisitor { override func visit(_ node: VariableDeclSyntax) -> SyntaxVisitorContinueKind { - // Early exit for member blocks (class/struct properties) if node.parent?.is(MemberBlockItemSyntax.self) == false { - // Check for shadowing BEFORE adding to scope node.bindings.forEach { binding in checkForShadowing(in: binding.pattern) } @@ -222,24 +234,66 @@ private extension VariableShadowingRule { return super.visit(node) } + override func visit(_ node: FunctionDeclSyntax) -> SyntaxVisitorContinueKind { + if !configuration.ignoreParameters { + for param in node.signature.parameterClause.parameters { + let nameToken = param.secondName ?? param.firstName + if nameToken.text != "_", isShadowingAnyScope(nameToken.text) { + violations.append(nameToken.positionAfterSkippingLeadingTrivia) + } + } + } + return super.visit(node) + } + + override func visit(_ node: ForStmtSyntax) -> SyntaxVisitorContinueKind { + checkForBindingShadowing(in: node.pattern) + return super.visit(node) + } + + override func visit(_ node: IfExprSyntax) -> SyntaxVisitorContinueKind { + for condition in node.conditions { + if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) { + checkForBindingShadowing(in: optBinding.pattern) + } + } + return super.visit(node) + } + + // Used for VariableDecl: the new identifier is added to the *current* scope, + // so we only check ancestor scopes (dropLast). private func checkForShadowing(in pattern: PatternSyntax) { - // Handle direct identifier patterns if let identifier = pattern.as(IdentifierPatternSyntax.self) { let identifierText = identifier.identifier.text if isShadowingOuterScope(identifierText) { violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia) } } else if let tuple = pattern.as(TuplePatternSyntax.self) { - // Recurse into tuple patterns: e.g., (a, b) tuple.elements.forEach { element in checkForShadowing(in: element.pattern) } } else if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) { - // Recurse into optional binding patterns: e.g., `if let a`, `while var (a, b)` checkForShadowing(in: valueBinding.pattern) } } + // Used for if-let / for-loop bindings: the new identifier is added to a *child* scope, + // so we check all current scopes. + private func checkForBindingShadowing(in pattern: PatternSyntax) { + if let identifier = pattern.as(IdentifierPatternSyntax.self) { + let identifierText = identifier.identifier.text + if isShadowingAnyScope(identifierText) { + violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia) + } + } else if let tuple = pattern.as(TuplePatternSyntax.self) { + tuple.elements.forEach { element in + checkForBindingShadowing(in: element.pattern) + } + } else if let valueBinding = pattern.as(ValueBindingPatternSyntax.self) { + checkForBindingShadowing(in: valueBinding.pattern) + } + } + private func isShadowingOuterScope(_ identifier: String) -> Bool { guard scope.count > 1 else { return false } @@ -249,5 +303,11 @@ private extension VariableShadowingRule { } return false } + + /// Checks all scope levels including the current one. Used for parameter checking + /// since parameters are declared into a child scope, not the current one. + private func isShadowingAnyScope(_ identifier: String) -> Bool { + scope.contains { $0.contains { $0.declares(id: identifier) } } + } } } diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/VariableShadowingConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/VariableShadowingConfiguration.swift new file mode 100644 index 0000000000..d77c73628f --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/VariableShadowingConfiguration.swift @@ -0,0 +1,9 @@ +import SwiftLintCore + +@AutoConfigParser +struct VariableShadowingConfiguration: SeverityBasedRuleConfiguration { + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "ignore_parameters") + private(set) var ignoreParameters = true +} diff --git a/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift b/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift new file mode 100644 index 0000000000..4b01bb5ce1 --- /dev/null +++ b/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift @@ -0,0 +1,15 @@ +@testable import SwiftLintBuiltInRules +import TestHelpers +import XCTest + +final class VariableShadowingRuleTests: SwiftLintTestCase { + func testWithIgnoreParametersTrue() { + let configuration = ["ignore_parameters": true] + verifyRule(VariableShadowingRule.description, ruleConfiguration: configuration) + } + + func testWithIgnoreParametersFalse() { + let configuration = ["ignore_parameters": false] + verifyRule(VariableShadowingRule.description, ruleConfiguration: configuration) + } +} diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index a5c27b6bfb..f6d2689c85 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -1385,6 +1385,7 @@ valid_ibinspectable: correctable: false variable_shadowing: severity: warning + ignore_parameters: true meta: opt-in: false correctable: false From fd51bd2fa6247e00c72df650a6a09ced4f893e9f Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Wed, 8 Apr 2026 21:51:52 +0100 Subject: [PATCH 13/15] Updated generated tests --- Tests/GeneratedTests/GeneratedTests_10.swift | 6 ------ Tests/GeneratedTests/GeneratedTests_11.swift | 14 ++++++++++++++ Tests/generated_tests.bzl | 4 +++- 3 files changed, 17 insertions(+), 7 deletions(-) create mode 100644 Tests/GeneratedTests/GeneratedTests_11.swift diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index 30647bbe07..3794c0e5d5 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -156,9 +156,3 @@ final class XCTSpecificMatcherRuleGeneratedTests: SwiftLintTestCase { verifyRule(XCTSpecificMatcherRule.description) } } - -final class YodaConditionRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(YodaConditionRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_11.swift b/Tests/GeneratedTests/GeneratedTests_11.swift new file mode 100644 index 0000000000..758f00504c --- /dev/null +++ b/Tests/GeneratedTests/GeneratedTests_11.swift @@ -0,0 +1,14 @@ +// GENERATED FILE. DO NOT EDIT! + +// swiftlint:disable:next blanket_disable_command superfluous_disable_command +// swiftlint:disable single_test_class type_name + +@testable import SwiftLintBuiltInRules +@testable import SwiftLintCore +import TestHelpers + +final class YodaConditionRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(YodaConditionRule.description) + } +} diff --git a/Tests/generated_tests.bzl b/Tests/generated_tests.bzl index 689f1a5310..e82cc1832d 100644 --- a/Tests/generated_tests.bzl +++ b/Tests/generated_tests.bzl @@ -14,7 +14,8 @@ GENERATED_TEST_TARGETS = [ "//Tests:GeneratedTests_07", "//Tests:GeneratedTests_08", "//Tests:GeneratedTests_09", - "//Tests:GeneratedTests_10" + "//Tests:GeneratedTests_10", + "//Tests:GeneratedTests_11" ] def generated_tests(): @@ -29,6 +30,7 @@ def generated_tests(): generated_test_shard("08") generated_test_shard("09") generated_test_shard("10") + generated_test_shard("11") native.test_suite( name = "GeneratedTests", From c841f4d8b5ff70745e5f71e9be117877d020b53f Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Fri, 10 Apr 2026 12:14:38 +0100 Subject: [PATCH 14/15] Updated additional examples, change rule to opt In --- .../Rules/Lint/VariableShadowingRule.swift | 67 +++++++++++++++++-- .../VariableShadowingRuleTests.swift | 15 ----- .../Resources/default_rule_configurations.yml | 2 +- 3 files changed, 62 insertions(+), 22 deletions(-) delete mode 100644 Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index e6e047db6a..3adeeea7a6 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -1,7 +1,7 @@ import SwiftLintCore import SwiftSyntax -@SwiftSyntaxRule +@SwiftSyntaxRule(optIn: true) struct VariableShadowingRule: Rule { var configuration = VariableShadowingConfiguration() @@ -219,6 +219,23 @@ struct VariableShadowingRule: Rule { print(a) } """, configuration: ["ignore_parameters": false]), + Example(""" + struct S { + var a = 1 + var b: Int { + let ↓a = 2 + return a + } + } + """), + Example(""" + var a: String? + while let ↓a = Optional("hello") {} + """), + Example(""" + var a: String? + guard let ↓a = Optional("hello") else { return } + """), ] ) } @@ -260,8 +277,46 @@ private extension VariableShadowingRule { return super.visit(node) } - // Used for VariableDecl: the new identifier is added to the *current* scope, - // so we only check ancestor scopes (dropLast). + override func visit(_ node: WhileStmtSyntax) -> SyntaxVisitorContinueKind { + for condition in node.conditions { + if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) { + checkForBindingShadowing(in: optBinding.pattern) + } + } + return super.visit(node) + } + + override func visit(_ node: GuardStmtSyntax) -> SyntaxVisitorContinueKind { + for condition in node.conditions { + if let optBinding = condition.condition.as(OptionalBindingConditionSyntax.self) { + checkForBindingShadowing(in: optBinding.pattern) + } + } + return super.visit(node) + } + + override func visit(_ node: MemberBlockSyntax) -> SyntaxVisitorContinueKind { + let result = super.visit(node) + if let parent = node.parent, + [.actorDecl, .classDecl, .enumDecl, .structDecl].contains(parent.kind) { + for member in node.members { + if let varDecl = member.decl.as(VariableDeclSyntax.self) { + for binding in varDecl.bindings { + if let id = binding.pattern.as(IdentifierPatternSyntax.self)?.identifier { + scope.modifyLast { $0.append(.localVariable(name: id)) } + } + } + } + } + } + return result + } + + // Used for VariableDecl: checks only ancestor scopes (dropLast), not the current scope. + // This avoids false positives for same-scope redeclarations, which are compile errors in + // Swift and are not shadowing. Using isShadowingAnyScope here would incorrectly flag code + // where the same name appears twice at the same scope level (e.g. inside a disabled lint + // region followed by an enabled one). private func checkForShadowing(in pattern: PatternSyntax) { if let identifier = pattern.as(IdentifierPatternSyntax.self) { let identifierText = identifier.identifier.text @@ -277,8 +332,9 @@ private extension VariableShadowingRule { } } - // Used for if-let / for-loop bindings: the new identifier is added to a *child* scope, - // so we check all current scopes. + // Used for if-let / for-loop / while-let / guard-let bindings: the binding is added to a + // child scope, so checking all current scopes (including the one where the outer variable + // lives) is correct and necessary. private func checkForBindingShadowing(in pattern: PatternSyntax) { if let identifier = pattern.as(IdentifierPatternSyntax.self) { let identifierText = identifier.identifier.text @@ -296,7 +352,6 @@ private extension VariableShadowingRule { private func isShadowingOuterScope(_ identifier: String) -> Bool { guard scope.count > 1 else { return false } - for scopeDeclarations in scope.dropLast() where scopeDeclarations.contains(where: { $0.declares(id: identifier) }) { return true diff --git a/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift b/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift deleted file mode 100644 index 4b01bb5ce1..0000000000 --- a/Tests/BuiltInRulesTests/VariableShadowingRuleTests.swift +++ /dev/null @@ -1,15 +0,0 @@ -@testable import SwiftLintBuiltInRules -import TestHelpers -import XCTest - -final class VariableShadowingRuleTests: SwiftLintTestCase { - func testWithIgnoreParametersTrue() { - let configuration = ["ignore_parameters": true] - verifyRule(VariableShadowingRule.description, ruleConfiguration: configuration) - } - - func testWithIgnoreParametersFalse() { - let configuration = ["ignore_parameters": false] - verifyRule(VariableShadowingRule.description, ruleConfiguration: configuration) - } -} diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index f6d2689c85..1d58598127 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -1387,7 +1387,7 @@ variable_shadowing: severity: warning ignore_parameters: true meta: - opt-in: false + opt-in: true correctable: false vertical_parameter_alignment: severity: warning From bcd25ddf1e8d35f24e9c7f287504c43816eb9b1f Mon Sep 17 00:00:00 2001 From: Nadeem Ali Date: Fri, 10 Apr 2026 13:02:30 +0100 Subject: [PATCH 15/15] Updated ShadowingAnyScope to use base function --- .../Rules/Lint/VariableShadowingRule.swift | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift index 3adeeea7a6..69c736bb8e 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/VariableShadowingRule.swift @@ -255,7 +255,7 @@ private extension VariableShadowingRule { if !configuration.ignoreParameters { for param in node.signature.parameterClause.parameters { let nameToken = param.secondName ?? param.firstName - if nameToken.text != "_", isShadowingAnyScope(nameToken.text) { + if nameToken.text != "_", hasSeenDeclaration(for: nameToken.text) { violations.append(nameToken.positionAfterSkippingLeadingTrivia) } } @@ -338,7 +338,7 @@ private extension VariableShadowingRule { private func checkForBindingShadowing(in pattern: PatternSyntax) { if let identifier = pattern.as(IdentifierPatternSyntax.self) { let identifierText = identifier.identifier.text - if isShadowingAnyScope(identifierText) { + if hasSeenDeclaration(for: identifierText) { violations.append(identifier.identifier.positionAfterSkippingLeadingTrivia) } } else if let tuple = pattern.as(TuplePatternSyntax.self) { @@ -358,11 +358,5 @@ private extension VariableShadowingRule { } return false } - - /// Checks all scope levels including the current one. Used for parameter checking - /// since parameters are declared into a child scope, not the current one. - private func isShadowingAnyScope(_ identifier: String) -> Bool { - scope.contains { $0.contains { $0.declares(id: identifier) } } - } } }