Skip to content

Commit 7349f2a

Browse files
committed
Address review comments
1 parent 590903d commit 7349f2a

4 files changed

Lines changed: 25 additions & 42 deletions

File tree

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

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ module Unified {
4040

4141
/**
4242
* Gets the nearest syntactically enclosing class declaration, if any.
43-
*
44-
* For the semantically enclosing class, use `getParentClass()` instead.
4543
*/
4644
ClassLikeDeclaration getEnclosingClass() {
4745
exists(AstNode parent | parent = this.getParent() |
@@ -52,33 +50,6 @@ module Unified {
5250
)
5351
}
5452

55-
/**
56-
* Gets the class that this node belongs to, if any.
57-
*
58-
* Unlike `getEnclosingClass`, this takes extensions into account:
59-
*
60-
* ```swift
61-
* class A {
62-
* let x : Int // belongs to class A
63-
* }
64-
*
65-
* extension A {
66-
* func foo() {} // also belongs to the class A
67-
* }
68-
* ```
69-
*/
70-
ClassLikeDeclaration getParentClass() {
71-
exists(ClassLikeDeclaration encl | encl = this.getEnclosingClass() |
72-
not exists(encl.getExtensionTarget()) and
73-
result = encl
74-
or
75-
exists(NameBinding::NamespaceNode ns |
76-
ns.isStaticMemberNamespace(result) and
77-
ns.getAnExtension().isStaticMemberNamespace(encl)
78-
)
79-
)
80-
}
81-
8253
private AstNode overrideEnclosingCallableParent() {
8354
exists(FunctionExpr func |
8455
// Capture declarations are evaluated as part of the outer context, and
@@ -244,13 +215,10 @@ module Unified {
244215

245216
private module QualifiedName<getDeclaringClassSig/1 getDeclaringClass> {
246217
string getQualifiedName(Member m) {
247-
not m instanceof U::EnumCaseConstructor and
248218
result = getQualifiedName(getDeclaringClass(m)) + "." + m.getNameOrFallback()
249219
or
250220
not exists(getDeclaringClass(m)) and
251221
result = m.getNameOrFallback()
252-
or
253-
result = getQualifiedName(m.(U::EnumCaseConstructor).getEnumCaseClass())
254222
}
255223
}
256224

@@ -277,17 +245,27 @@ module Unified {
277245
NameBinding::getStaticBindingTargetFromRef(this.getABaseType().getType())
278246
}
279247

248+
/**
249+
* Gets the class that this class resolves to. Either itself, when it
250+
* is not extension, otherwise the class that it extends.
251+
*/
252+
ClassLikeDeclaration resolve() {
253+
not exists(this.getExtensionTarget()) and
254+
result = this
255+
or
256+
exists(NameBinding::NamespaceNode ns |
257+
ns.isStaticMemberNamespace(result) and
258+
ns.getAnExtension().isStaticMemberNamespace(this)
259+
)
260+
}
261+
280262
/**
281263
* Gets a member of this class, including members from extensions.
282264
*/
283265
Member getAMemberIncludingExtensions() {
284-
result = this.getAMember() and
285-
not exists(this.getExtensionTarget())
286-
or
287-
exists(ClassLikeDeclaration ext, NameBinding::NamespaceNode ns |
288-
ns.isStaticMemberNamespace(this) and
289-
ns.getAnExtension().isStaticMemberNamespace(ext) and
290-
result = ext.getAMember()
266+
exists(ClassLikeDeclaration cls |
267+
this = cls.resolve() and
268+
result = cls.getAMember()
291269
)
292270
}
293271

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -784,7 +784,7 @@ module Input3 implements InputSig3 {
784784
path.isEmpty()
785785
or
786786
exists(ClassLikeDeclaration c |
787-
c = n.(SuperExpr).getParentClass() and
787+
c = n.(SuperExpr).getEnclosingClass().resolve() and
788788
result = c.getBaseType(0).getType().(TypeMention).getTypeAt(path)
789789
)
790790
or

‎unified/ql/lib/utils/test/TestUtils.qll‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ predicate callableName(Callable c, string alias) {
4848
keyValueCommentAt(filepath, line, "name", alias)
4949
or
5050
not keyValueCommentAt(filepath, line, "name", _) and
51-
alias = c.(Member).getQualifiedName()
51+
(
52+
not c instanceof EnumCaseConstructor and
53+
alias = c.(Member).getQualifiedName()
54+
or
55+
alias = c.(EnumCaseConstructor).getEnumCaseClass().getQualifiedName()
56+
)
5257
)
5358
}

‎unified/ql/test/library-tests/controlflow/basicblock-slices.expected‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
| 8 | cfg.swift:8:1:8:6 | Double | 'Double -> Argument -V topLevelDecl -^ Double(...)' |
88
| 10 | cfg.swift:10:1:13:1 | enum MyError | 'enum MyError -V MyError -> BaseType -V Error' |
99
| 11 | cfg.swift:11:10:11:15 | MyError.error1 | 'MyError.error1 -V error1 -> MyError.error2 -V error2' |
10-
| 12 | cfg.swift:12:10:12:31 | MyError.error3 | 'MyError.error3 -V error3 -^ MyError.error3' |
10+
| 12 | cfg.swift:12:10:12:31 | MyError.error3 | 'MyError.error3 -V error3 -^ MyError.error3.(constructor)' |
1111
| 12 | cfg.swift:12:17:12:25 | withParam | 'withParam -? Block' |
1212
| 15 | cfg.swift:15:1:17:1 | func isZero | 'func isZero -V isZero' |
1313
| 15 | cfg.swift:15:13:15:13 | x | 'x -> Block' |

0 commit comments

Comments
 (0)