Skip to content

Commit dea08cb

Browse files
hvitvedhvitved
authored
Merge pull request #23010 from hvitved/unified/super-as-self
Unified: Treat `super` as an alias for `self` in local name binding
2 parents 26b5f26 + e508201 commit dea08cb

7 files changed

Lines changed: 72 additions & 3 deletions

File tree

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ module Public {
480480
}
481481
}
482482

483+
final class PotentialLocalNameAccess = PotentialLocalNameAccessImpl;
484+
483485
/**
484486
* A name node that is possibly a reference to a local name, but could also refer to a member
485487
* visible through imports or inheritance.
@@ -493,17 +495,27 @@ module Public {
493495
* }
494496
* ```
495497
*/
496-
class PotentialLocalNameAccess extends IdentifierExpr {
498+
abstract private class PotentialLocalNameAccessImpl extends AstNode {
497499
/** Gets the representative for the local name being accessed. */
498500
LocalName getLocalName() { result = this.(LocalNameBindingOutput::LocalAccess).getLocal() }
499501

500502
/** Gets the name being accessed. */
501-
string getName() { result = this.getValue() }
503+
abstract string getName();
502504

503505
/** Holds if this is one of the binding sites for a name, such as the `x` in `let x = 123`. */
504506
predicate isBindingSite() { this instanceof NameBinding }
505507
}
506508

509+
private class IdentifierPotentialLocalNameAccess extends PotentialLocalNameAccessImpl,
510+
IdentifierExpr
511+
{
512+
override string getName() { result = this.getValue() }
513+
}
514+
515+
private class SuperPotentialLocalNameAccess extends PotentialLocalNameAccessImpl, SuperExpr {
516+
override string getName() { result = any(NameBindingPlugin p).getSuperLocalName(this) }
517+
}
518+
507519
/** Gets the implicitly-declared variable through which the given callable refers to its receiver. */
508520
LocalVariable getImplicitReceiverVariable(Callable callable) {
509521
exists(string name |

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,12 @@ class NameBindingPlugin extends Unit {
6868
/** Gets the name of the implicit receiver parameter in `callable`, if it has one. */
6969
string getImplicitReceiverParameterName(Callable callable) { none() }
7070

71+
/**
72+
* Gets the name to be used when resolving `super`, for example `this` or `self`.
73+
*/
74+
bindingset[se]
75+
string getSuperLocalName(SuperExpr se) { none() }
76+
7177
/**
7278
* Gets the name through which static members of the enclosing class `cls` can be
7379
* accessed, for example `Self` in Swift.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ class NameBindingPluginSwift extends NameBindingPlugin {
107107
result = "self"
108108
}
109109

110+
bindingset[se]
111+
override string getSuperLocalName(SuperExpr se) { exists(se) and result = "self" }
112+
110113
bindingset[cls]
111114
override string getStaticSelfName(ClassLikeDeclaration cls) { exists(cls) and result = "Self" }
112115
}

‎unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,11 @@ module Input3 implements InputSig3 {
219219
}
220220

221221
class Variable extends LocalNameBindingOutput::Local {
222-
Expr getAnAccess() { result = super.getAnAccess() }
222+
Expr getAnAccess() {
223+
result = super.getAnAccess() and
224+
// for type inference, we want super expressions to have the type of the base class
225+
not result instanceof SuperExpr
226+
}
223227
}
224228

225229
final class VariableDeclaration = VariableDeclarationImpl;

‎unified/ql/test/library-tests/dataflow/accessors_instance.swift‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,21 @@ func t12() {
159159
let c = C()
160160
c.s.update()
161161
}
162+
163+
func t13() {
164+
class C1 {
165+
var x: String = "safe"
166+
}
167+
class C2: C1 {
168+
func updateX(newValue: String) {
169+
super.x = newValue
170+
}
171+
172+
func getX() -> String {
173+
return super.x
174+
}
175+
}
176+
var c2 = C2()
177+
c2.updateX(newValue: source("t13.1"))
178+
sink(c2.getX()) // $ hasValueFlow=t13.1
179+
}

‎unified/ql/test/library-tests/dataflow/test.expected‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ edges
4242
| accessors_instance.swift:151:42:151:56 | source(...) | accessors_instance.swift:151:34:151:38 | field | provenance | |
4343
| accessors_instance.swift:156:24:156:31 | newValue [field] | accessors_instance.swift:156:24:156:37 | ... .field | provenance | |
4444
| accessors_instance.swift:160:5:160:7 | [post] ... .s [field] | accessors_instance.swift:156:24:156:31 | newValue [field] | provenance | |
45+
| accessors_instance.swift:168:22:168:29 | newValue | accessors_instance.swift:169:23:169:30 | newValue | provenance | |
46+
| accessors_instance.swift:169:13:169:19 | ... .x | accessors_instance.swift:169:13:169:17 | [post] super [x] | provenance | |
47+
| accessors_instance.swift:169:23:169:30 | newValue | accessors_instance.swift:169:13:169:19 | ... .x | provenance | |
48+
| accessors_instance.swift:173:20:173:24 | super [x] | accessors_instance.swift:173:20:173:26 | ... .x | provenance | |
49+
| accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:168:22:168:29 | newValue | provenance | |
50+
| accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:178:10:178:11 | c2 [x] | provenance | |
51+
| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:173:20:173:24 | super [x] | provenance | |
52+
| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:178:10:178:18 | ... .getX(...) | provenance | |
4553
| accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | provenance | |
4654
| accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | provenance | |
4755
| accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | provenance | |
@@ -580,6 +588,15 @@ nodes
580588
| accessors_instance.swift:156:24:156:31 | newValue [field] | semmle.label | newValue [field] |
581589
| accessors_instance.swift:156:24:156:37 | ... .field | semmle.label | ... .field |
582590
| accessors_instance.swift:160:5:160:7 | [post] ... .s [field] | semmle.label | [post] ... .s [field] |
591+
| accessors_instance.swift:168:22:168:29 | newValue | semmle.label | newValue |
592+
| accessors_instance.swift:169:13:169:17 | [post] super [x] | semmle.label | [post] super [x] |
593+
| accessors_instance.swift:169:13:169:19 | ... .x | semmle.label | ... .x |
594+
| accessors_instance.swift:169:23:169:30 | newValue | semmle.label | newValue |
595+
| accessors_instance.swift:173:20:173:24 | super [x] | semmle.label | super [x] |
596+
| accessors_instance.swift:173:20:173:26 | ... .x | semmle.label | ... .x |
597+
| accessors_instance.swift:177:26:177:40 | source(...) | semmle.label | source(...) |
598+
| accessors_instance.swift:178:10:178:11 | c2 [x] | semmle.label | c2 [x] |
599+
| accessors_instance.swift:178:10:178:18 | ... .getX(...) | semmle.label | ... .getX(...) |
583600
| accessors_local.swift:7:21:7:34 | source(...) | semmle.label | source(...) |
584601
| accessors_local.swift:8:10:8:10 | x | semmle.label | x |
585602
| accessors_local.swift:15:15:15:28 | source(...) | semmle.label | source(...) |
@@ -1183,6 +1200,8 @@ nodes
11831200
subpaths
11841201
| accessors_instance.swift:84:10:84:10 | c [field] | accessors_instance.swift:79:25:79:28 | self [field] | accessors_instance.swift:79:25:79:34 | ... .field | accessors_instance.swift:84:10:84:12 | ... .x |
11851202
| accessors_instance.swift:98:5:98:5 | c [field] | accessors_instance.swift:93:32:93:36 | field | accessors_instance.swift:93:19:93:28 | otherField | accessors_instance.swift:99:10:99:10 | c [otherField] |
1203+
| accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:168:22:168:29 | newValue | accessors_instance.swift:169:13:169:17 | [post] super [x] | accessors_instance.swift:178:10:178:11 | c2 [x] |
1204+
| accessors_instance.swift:178:10:178:11 | c2 [x] | accessors_instance.swift:173:20:173:24 | super [x] | accessors_instance.swift:173:20:173:26 | ... .x | accessors_instance.swift:178:10:178:18 | ... .getX(...) |
11861205
| accessors_local.swift:45:9:45:9 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:46:21:46:21 | y | accessors_local.swift:47:10:47:10 | x |
11871206
| accessors_local.swift:54:5:54:5 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:52:21:52:21 | y | accessors_local.swift:55:10:55:10 | x |
11881207
| calls.swift:31:17:31:30 | source(...) | calls.swift:28:19:28:19 | x | calls.swift:29:16:29:24 | ... + ... | calls.swift:31:10:31:31 | target(...) |
@@ -1218,6 +1237,7 @@ testFailures
12181237
| accessors_instance.swift:134:24:134:33 | ... .0 | accessors_instance.swift:143:13:143:27 | source(...) | accessors_instance.swift:134:24:134:33 | ... .0 | $@ | accessors_instance.swift:143:13:143:27 | source(...) | source(...) |
12191238
| accessors_instance.swift:134:24:134:33 | ... .0 | accessors_instance.swift:145:14:145:28 | source(...) | accessors_instance.swift:134:24:134:33 | ... .0 | $@ | accessors_instance.swift:145:14:145:28 | source(...) | source(...) |
12201239
| accessors_instance.swift:156:24:156:37 | ... .field | accessors_instance.swift:151:42:151:56 | source(...) | accessors_instance.swift:156:24:156:37 | ... .field | $@ | accessors_instance.swift:151:42:151:56 | source(...) | source(...) |
1240+
| accessors_instance.swift:178:10:178:18 | ... .getX(...) | accessors_instance.swift:177:26:177:40 | source(...) | accessors_instance.swift:178:10:178:18 | ... .getX(...) | $@ | accessors_instance.swift:177:26:177:40 | source(...) | source(...) |
12211241
| accessors_local.swift:8:10:8:10 | x | accessors_local.swift:7:21:7:34 | source(...) | accessors_local.swift:8:10:8:10 | x | $@ | accessors_local.swift:7:21:7:34 | source(...) | source(...) |
12221242
| accessors_local.swift:17:10:17:10 | x | accessors_local.swift:15:15:15:28 | source(...) | accessors_local.swift:17:10:17:10 | x | $@ | accessors_local.swift:15:15:15:28 | source(...) | source(...) |
12231243
| accessors_local.swift:25:10:25:10 | x | accessors_local.swift:22:15:22:28 | source(...) | accessors_local.swift:25:10:25:10 | x | $@ | accessors_local.swift:22:15:22:28 | source(...) | source(...) |

‎unified/ql/test/library-tests/local-name-binding/self_access.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,9 @@ class C {
3838
}
3939
}
4040
}
41+
42+
class Sub: C { // $ access=C
43+
func t6() { // implicit-self=t6.self
44+
super.t1() // $ access=t6.self (we treat `super` as an alias for `self`)
45+
}
46+
}

0 commit comments

Comments
 (0)