Surface violations of prefer_self_in_static_references inside extensions#6639
Open
itsybitsybootsy wants to merge 2 commits into
Open
Surface violations of prefer_self_in_static_references inside extensions#6639itsybitsybootsy wants to merge 2 commits into
prefer_self_in_static_references inside extensions#6639itsybitsybootsy wants to merge 2 commits into
Conversation
Generated by 🚫 Danger |
SimplyDanny
requested changes
May 21, 2026
Collaborator
SimplyDanny
left a comment
SimplyDanny
left a comment
There was a problem hiding this comment.
Thanks! That's the right direction with a few remarks ...
| } | ||
| } | ||
| extension Outer.Inner { | ||
| func f() -> Int { ↓Inner.i } |
Collaborator
There was a problem hiding this comment.
Inner.i doesn't compile. It should be Outer.Inner.i.
Comment on lines
+15
to
+19
| * Surface violations of `prefer_self_in_static_references` inside | ||
| extensions of types whose name is a plain identifier or a member type | ||
| (e.g. `extension Foo` or `extension Foo.Bar`). Stored-property | ||
| initializers and function signatures remain exempt — matching the | ||
| rule's existing behaviour for top-level class declarations. |
Collaborator
There was a problem hiding this comment.
Could be shorter. Something like "Treat extensions like classes in ... rule.".
| extension C { | ||
| func f() -> Int { Self.i } | ||
| } | ||
| """), |
Collaborator
There was a problem hiding this comment.
Please add an Outer.Inner.i correction example as well.
…sions The rule previously pushed `.skipReferences` for every `ExtensionDeclSyntax`, which silenced the rule inside the entire extension body. Per the maintainer's note on realm#3993, treating an extension as a class scope is a reasonable first step that surfaces violations in member-access contexts (method bodies, computed properties, closure-body references on the right-hand side) without introducing false positives. When the extended type is a plain identifier (`extension Foo`) or a member type (`extension Foo.Bar`), push `.likeClass(name: extendedTypeName)` so that references to that name inside the extension body are flagged and corrected to `Self`. Complex extended-type shapes (generic specialisations, opaque types, function types) continue to fall through to the existing `.skipReferences` behaviour. The extension declaration header itself (the extended-type identifier) is excluded from violations to avoid rewriting `extension Foo` to `extension Self`. Resolves realm#3993. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
itsybitsybootsy
force-pushed
the
fix/prefer-self-in-extensions
branch
from
d89e068 to
95f0f1b
Compare
Author
|
comment #1 wasn't just a rename. the rule never matched member types on extensions, only single identifiers, so |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
prefer_self_in_static_referencescurrently pushes.skipReferencesfor everyExtensionDeclSyntax, which silences the rule inside the entire extension body. The rule misses violations like:Per @SimplyDanny's comment on #3993:
Fix
When the extended type is a plain identifier (
extension Foo) or a member type (extension Foo.Bar), push.likeClass(name: extendedTypeName)instead of.skipReferences. Complex extended-type shapes (generic specialisations, opaque types, function types) continue to fall through to the previous.skipReferencesbehaviour.The class scope is the most conservative reuse of existing logic — it already correctly handles method bodies, computed properties, and closure invocations on the right-hand side, while still skipping the things class mode skips (stored-property initializers, function signatures) to avoid false positives. This matches the maintainer's "first step" framing: the reporter's exact case (
extension Foo { static let otherValue = Foo.value }— a stored-property initializer) remains exempt under this change, matching how the rule already treats top-level class declarations.The
IdentifierTypeSyntaxvisitor also gains a guard so that the extension declaration header itself (the extended-type identifier) is excluded from violations — without it, the new scope would rewriteextension Footoextension Self.Tests
Outer.Inner) confirmingMemberTypeSyntaxhandling.↓C.i→Self.i.extension T { static let j = S.i + T.i; static let k = { T.j }() }) continues to pass: class-modeMemberBlockSyntaxalready skips stored-property initializer expressions, so neitherT.inor the closure body'sT.jis reachable.Self-corrections
The integration test (
testSwiftLintAutoCorrects) lints SwiftLint's own source against all rules. With this change, four pre-existing violations in SwiftLint's own code are surfaced and auto-corrected — they're equivalent rewrites of the same shape (String.operators→Self.operatorsinside aprivate extension String, etc.) and are bundled here so the integration test passes:Source/SwiftLintBuiltInRules/Rules/Idiomatic/PreferKeyPathRule.swift:as ExprSyntax→as SelfSource/SwiftLintBuiltInRules/Rules/Performance/EmptyCountRule.swift:String.operators→Self.operatorsSource/SwiftLintBuiltInRules/Rules/Style/ContrastedOpeningBraceRule.swift:\IfExprSyntax.elseBody→\Self.elseBodySource/SwiftLintFramework/Reporters/SummaryReporter.swift:Int.numberFormatter→Self.numberFormatterVerified locally with
swift run swiftlint-dev rules register && swift test. All tests pass.Resolves #3993.
This contribution was developed with the help of Claude Code. The fix, regression tests, edge-case analysis, and the local test suite were reviewed manually before submitting.