Unified: Resolve Self in static name binding - #22548
Conversation
2dfc953 to
f86e1a4
Compare
hvitved
changed the title
self/SelfSelf in static name binding
f86e1a4 to
2d10f98
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Qualified type extensions still cannot resolve Self because their opaque names are not connected to type declarations.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll — Resolve Self in qualified type extensions |
What changed in this PR
Adds Swift Self resolution to unified static name binding.
Changes:
- Adds a language-plugin hook for static self names.
- Maps Swift
Selfto the enclosing type’s static namespace. - Adds class and inheritance tests.
| File | Description |
|---|---|
StaticNameBinding.qll |
Adds static-self namespace binding. |
NameBindingPlugin.qll |
Defines the plugin extension point. |
NameBindingPluginSwift.qll |
Configures Swift’s Self name. |
explicit-instance-field-access.swift |
Tests direct and inherited access. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| or | ||
| exists(ClassLikeDeclaration cls | | ||
| name = any(NameBindingPlugin p).getStaticSelfName() and | ||
| node1.isIdentifier(cls.getNameNode()) and |
| * Gets the name through which static members of the enclosing class can be | ||
| * accessed, for example `Self` in Swift. | ||
| */ | ||
| string getStaticSelfName() { none() } |
There was a problem hiding this comment.
Can we make this take a ClassLikeDeclaration?
I've tried to ensure that plugin predicates take an AST node or something, to ensure it works in a multi-language setting. That's why getImplicitReceiverParameterName is defined per Callable.
| exists(ClassLikeDeclaration cls | | ||
| name = any(NameBindingPlugin p).getStaticSelfName() and | ||
| node1.isIdentifier(cls.getNameNode()) and | ||
| node2.isStaticMemberNamespace(cls) |
There was a problem hiding this comment.
| node2.isStaticMemberNamespace(cls) | |
| node2.isLocalNamespace(cls) |
It's not inherited and not accessible as Foo.Self, so I'd just put it in the local namespace.
Come to think of it, it would be even better to introduce this in local name-binding so shadowing is handled more precisely. Otherwise Self in a nested class will in some cases resolve to an enclosing class.
Actually we should have a test for that: (not sure if it will pass or fail right now)
class A {
static let x = 1
class B {
static let x = 2
static let foo = Self.x // $ access=A.B.x
}
}
No description provided.