Visitar URL original
Unified: Improve static name binding involving shadowing · github/codeql@cb9d071 · GitHub
Skip to content

Commit cb9d071

Browse files
hvitvedhvitved
committed
Unified: Improve static name binding involving shadowing
1 parent 16f2595 commit cb9d071

4 files changed

Lines changed: 132 additions & 27 deletions

File tree

‎unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
private import unified
22
private import codeql.util.Unit
3+
private import codeql.util.Option
4+
private import StaticNameBinding
35

46
private module Plugins {
57
private import codeql.unified.internal.NameBindingPluginSwift
@@ -43,6 +45,23 @@ class NameBindingPlugin extends Unit {
4345
bindingset[cls, member]
4446
predicate isInheritableMember(ClassLikeDeclaration cls, Member member) { none() }
4547

48+
/**
49+
* Holds if `member` is considered invalid within the namespace `n`.
50+
*/
51+
bindingset[n, member]
52+
predicate isInvalidMember(NamespaceNode n, Member member) { none() }
53+
54+
/**
55+
* Gets the key used to determine if `n` is shadowed by another declaration
56+
* with the same name and key.
57+
*
58+
* Nodes without shadowing keys will not shadow inherited declarations.
59+
*
60+
* This means that shadowing can be completely disabled by not implementing this
61+
* predicate, and completely enabled by assigning the same key to all members.
62+
*/
63+
string getShadowingKey(Member m) { none() }
64+
4665
/** Gets the name of the implicit receiver parameter in `callable`, if it has one. */
4766
string getImplicitReceiverParameterName(Callable callable) { none() }
4867

@@ -89,6 +108,28 @@ predicate isInheritableMember(Member member) {
89108
)
90109
}
91110

111+
bindingset[n, member]
112+
predicate isInvalidMember(NamespaceNode n, Member member) {
113+
any(NameBindingPlugin p).isInvalidMember(n, member)
114+
}
115+
116+
private string getShadowingKey0(NameBindingNode n) {
117+
result = any(NameBindingPlugin p).getShadowingKey(any(Member m | n.isMember(m)))
118+
}
119+
120+
private class ShadowingKey extends string {
121+
ShadowingKey() { this = getShadowingKey0(_) }
122+
}
123+
124+
class ShadowingKeyOpt = Option<ShadowingKey>::Option;
125+
126+
ShadowingKeyOpt getShadowingKey(NameBindingNode n) {
127+
result.asSome() = getShadowingKey0(n)
128+
or
129+
not exists(getShadowingKey0(n)) and
130+
result.isNone()
131+
}
132+
92133
/**
93134
* A representative for a module scope.
94135
*

‎unified/ql/lib/codeql/unified/internal/NameBindingPluginSwift.qll‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
*/
44

55
private import unified
6+
private import codeql.unified.internal.StaticNameBinding
67
private import codeql.unified.internal.NameBindingPlugin
78

9+
private predicate isGeneratedConstructor(ConstructorDeclaration c) { c.hasModifier("generated") }
10+
811
class NameBindingPluginSwift extends NameBindingPlugin {
912
bindingset[e]
1013
override predicate isNonPattern(Expr e) { isUnboundPattern(e.(Identifier)) }
@@ -39,6 +42,35 @@ class NameBindingPluginSwift extends NameBindingPlugin {
3942
not member.hasModifier("private")
4043
}
4144

45+
bindingset[n, member]
46+
override predicate isInvalidMember(NamespaceNode n, Member member) {
47+
isGeneratedConstructor(member) and
48+
(
49+
// Remove generated constructors when there are inherited constructors available
50+
// Note: If the base class only has private constructors, this class must have an
51+
// explicit constructor, in which case there is no generated constructor
52+
n.getOwnMember(_).isMember(member) and
53+
n.getAnInheritanceParent().getMemberFull(_, _).isMember(any(ConstructorDeclaration inherited))
54+
or
55+
// Remove inherited generated constructors when there are explicit constructors available
56+
n.getAnInheritanceParent().getMemberFull(_, _).isMember(member) and
57+
n.getOwnMember(_)
58+
.isMember(any(ConstructorDeclaration override | not isGeneratedConstructor(override)))
59+
)
60+
}
61+
62+
override string getShadowingKey(Member m) {
63+
not m instanceof Callable and result = ""
64+
or
65+
not isGeneratedConstructor(m) and
66+
result =
67+
concat(int i, Parameter p |
68+
p = m.(Callable).getParameter(i)
69+
|
70+
p.getExternalNameNode().getValue(), "," order by i
71+
)
72+
}
73+
4274
override string getImplicitReceiverParameterName(Callable callable) {
4375
// TODO: field initializers can also reference 'self' but are not currently wrapped in a Callable
4476
callable = any(ClassLikeDeclaration cls).getAMember() and

‎unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ class NameBindingNode extends TNameBindingNode {
2929

3030
Identifier asIdentifier() { this.isIdentifier(result) }
3131

32+
predicate isMember(Member m) { this.asIdentifier() = m.getNameNode() }
33+
3234
predicate isBulkImport(BulkImportingPattern p) { this = TBulkImport(p) }
3335

3436
predicate isLocalName(LocalName local) { this = TLocalName(local) }
@@ -312,6 +314,10 @@ signature module TrackInputSig {
312314
/** Holds if the forward-flow of `node` should be tracked. */
313315
predicate shouldTrack(NameBindingNode node);
314316

317+
default NameBindingNode getMember(NamespaceNode namespace, string name) {
318+
result = namespace.getMember(name)
319+
}
320+
315321
default predicate additionalValueStep(NameBindingNode node1, NameBindingNode node2) { none() }
316322
}
317323

@@ -327,6 +333,25 @@ module Track<TrackInputSig Input> {
327333
exists(NameBindingNode prev | prev = track(node) | valueStepEx(prev, result))
328334
}
329335

336+
pragma[nomagic]
337+
private predicate derivedStoreReadStep0(
338+
NamespaceNode namespace, string name, NameBindingNode node2
339+
) {
340+
readStep(namespace.ref(), name, node2)
341+
}
342+
343+
/**
344+
* Holds if `node1 -> node2` is derived by combining a store and a read step, with zero or more value steps and inheritance steps in-between.
345+
*/
346+
pragma[nomagic]
347+
private predicate derivedStoreReadStep(NameBindingNode node1, NameBindingNode node2) {
348+
exists(NamespaceNode namespace, string name |
349+
node1 = getMember(namespace, name) and // getMember() combines a store step with subsequent inheritance steps
350+
derivedStoreReadStep0(namespace, name, node2) and
351+
node1 != node2
352+
)
353+
}
354+
330355
/** Holds if there is an effective value step `node1 -> node2`. */
331356
pragma[inline]
332357
private predicate valueStepEx(NameBindingNode node1, NameBindingNode node2) {
@@ -338,23 +363,6 @@ module Track<TrackInputSig Input> {
338363
}
339364
}
340365

341-
pragma[nomagic]
342-
private predicate derivedStoreReadStep0(NamespaceNode namespace, string name, NameBindingNode node2) {
343-
readStep(namespace.ref(), pragma[only_bind_into](name), node2)
344-
}
345-
346-
/**
347-
* Holds if `node1 -> node2` is derived by combining a store and a read step, with zero or more value steps and inheritance steps in-between.
348-
*/
349-
pragma[nomagic]
350-
private predicate derivedStoreReadStep(NameBindingNode node1, NameBindingNode node2) {
351-
exists(NamespaceNode namespace, string name |
352-
node1 = namespace.getMember(name) and // getMember() combines a store step with subsequent inheritance steps
353-
derivedStoreReadStep0(namespace, name, node2) and
354-
node1 != node2
355-
)
356-
}
357-
358366
/** Holds if the member represented by `node` can be inherited. */
359367
pragma[nomagic]
360368
private predicate isInheritableMemberNode(NameBindingNode node) {
@@ -423,23 +431,47 @@ class NamespaceNode extends NameBindingNode {
423431
/** Gets a namespace that directly inherits from this one. */
424432
NamespaceNode getAnInheritanceChild() { result.getAnInheritanceParent() = this }
425433

434+
pragma[nomagic]
435+
private predicate hasOwnMemberWithShadowingKey(string name, string shadowingKey) {
436+
shadowingKey = getShadowingKey(this.getOwnMember(name)).asSome()
437+
}
438+
439+
/**
440+
* Gets a potential member of this namespace of the given name.
441+
*
442+
* Unlike `getMember`, this predicate does not filter away invalid members.
443+
*/
444+
pragma[nomagic]
445+
NameBindingNode getMemberFull(string name, ShadowingKeyOpt shadowingKey) {
446+
result = this.getOwnMember(name) and
447+
shadowingKey = getShadowingKey(result)
448+
or
449+
not this.hasOwnMemberWithShadowingKey(name, shadowingKey.asSome()) and
450+
result = this.getAnInheritanceParent().getMemberFull(name, shadowingKey) and
451+
isInheritableMemberNode(result)
452+
or
453+
result = this.getAnExtension().getMemberFull(name, shadowingKey)
454+
}
455+
426456
/** Gets a member of this namespace of the given name. */
427457
cached
428458
NameBindingNode getMember(string name) {
429459
CachedStage::ref() and
430-
result = this.getOwnMember(name)
431-
or
432-
not this.hasOwnMember(name) and
433-
result = this.getAnInheritanceParent().getMember(name) and
434-
isInheritableMemberNode(result)
435-
or
436-
result = this.getAnExtension().getMember(name)
460+
result = this.getMemberFull(name, _) and
461+
not exists(Member m |
462+
result.isMember(m) and
463+
isInvalidMember(this, m)
464+
)
437465
}
438466
}
439467

440468
private module TrackNamespaceInput implements TrackInputSig {
441469
predicate shouldTrack(NameBindingNode node) { node instanceof NamespaceNode }
442470

471+
NameBindingNode getMember(NamespaceNode namespace, string name) {
472+
result = namespace.getMemberFull(name, _)
473+
}
474+
443475
predicate additionalValueStep(NameBindingNode node1, NameBindingNode node2) {
444476
// Namespace-tracking goes through aliases, but declaration-tracking does not
445477
exists(TypeAliasDeclaration decl |

‎unified/ql/test/library-tests/static-name-binding/inheritance.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ func t1() {
3232
let x5: E.C = nil // $ access=E access=A.B.C
3333

3434
let x6 = A.init() // $ access=A access=A.init
35-
let x7 = D.init() // $ access=D $ MISSING: access=A.init $ SPURIOUS: access=D.init
35+
let x7 = D.init() // $ access=D access=A.init
3636

3737
let x8 = A.f() // $ access=A access=A.f
38-
let x9 = D.f() // $ access=D access=D.f -- type inference should filter away the `D.f` target $ MISSING: access=A.f
39-
let x10 = D.f(x: 0) // $ access=D access=D.f $ MISSING: access=A.f -- type inference should filter away the `A.f` target
38+
let x9 = D.f() // $ access=D access=A.f access=D.f -- type inference should filter away the `D.f` target
39+
let x10 = D.f(x: 0) // $ access=D access=A.f access=D.f -- type inference should filter away the `A.f` target
4040

4141
let x11 = E.init() // $ access=E access=E.init
4242

0 commit comments

Comments
 (0)