diff --git a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll index 4a9c5b61db92..2dd12bbc8abc 100644 --- a/shared/namebinding/codeql/namebinding/LocalNameBinding.qll +++ b/shared/namebinding/codeql/namebinding/LocalNameBinding.qll @@ -337,54 +337,91 @@ module LocalNameBinding ) } - private predicate accessCandInLookupScope(AstNode n, string name, Scope lookup) { - accessCand(n, name) and - ( - lookupStartsAt(n, lookup) - or - not lookupStartsAt(n, _) and - lookup = getEnclosingScope(n) - ) - } - - pragma[nomagic] - private predicate lookupInScope(string name, Scope lookup, Scope scope) { - accessCandInLookupScope(_, name, lookup) and - scope = lookup - or - exists(Scope mid | - lookupInScope(name, lookup, mid) and - not declInScope(name, mid) and - not isTopScope(mid) and - scope = getEnclosingScope(mid) - ) - } - private predicate declInScope(string name, AstNode scope) { declInScope(_, name, scope) or implicitDeclInScope(name, scope) } + signature predicate accessCandSig(AstNode n, string name); + /** - * Holds if `name`, when resolved from `lookup`, may resolve to one of the uncertain members of `scope`. + * Allows resolution of access candidates. + * + * This is instantiated once by the local name binding library itself in order to populate `LocalAccess`. + * It can be instantiated further by the client, to resolve additional lookups at a later evaluation stage. */ - pragma[nomagic] - private predicate lookupInUncertainScope(string name, Scope lookup, Scope scope) { - lookupInScope(name, lookup, scope) and - uncertainScope(scope) and - not declInScope(name, scope) + module ResolveAccesses { + private predicate accessCandInLookupScope(AstNode n, string name, Scope lookup) { + accessCandInput(n, name) and + ( + lookupStartsAt(n, lookup) + or + not lookupStartsAt(n, _) and + lookup = getEnclosingScope(n) + ) + } + + pragma[nomagic] + private predicate lookupInScope(string name, Scope lookup, Scope scope) { + accessCandInLookupScope(_, name, lookup) and + scope = lookup + or + exists(Scope mid | + lookupInScope(name, lookup, mid) and + not declInScope(name, mid) and + not isTopScope(mid) and + scope = getEnclosingScope(mid) + ) + } + + pragma[nomagic] + private predicate resolveInScope(string name, Scope lookup, Local l) { + exists(Scope scope | lookupInScope(name, lookup, scope) | + l = TExplicitLocal(_, name, scope) or + l = TImplicitLocal(name, scope) + ) + } + + /** Holds if `access` resolves to `l`. */ + predicate access(AstNode access, Local l) { + exists(Scope lookup, string name | + accessCandInLookupScope(access, name, lookup) and + resolveInScope(name, lookup, l) + ) + } + + /** + * Holds if `name`, when resolved from `lookup`, may resolve to one of the uncertain members of `scope`. + */ + pragma[nomagic] + private predicate lookupInUncertainScope(string name, Scope lookup, Scope scope) { + lookupInScope(name, lookup, scope) and + uncertainScope(scope) and + not declInScope(name, scope) + } + + /** + * Gets an uncertain scope in which the `accessCand` pair may resolve. + */ + AstNode getAnUncertainScope(AstNode access, string name) { + exists(Scope lookup | + accessCandInLookupScope(access, name, lookup) and + lookupInUncertainScope(name, lookup, result) + ) + } } - /** - * Gets an uncertain scope in which the `accessCand` pair may resolve. - */ - AstNode getAnUncertainScope(AstNode access, string name) { - exists(Scope lookup | - accessCandInLookupScope(access, name, lookup) and - lookupInUncertainScope(name, lookup, result) - ) + private module DefaultAccesses = ResolveAccesses; + + /** Holds if `access` resolves to `l`. */ + cached + private predicate access(AstNode access, Local l) { + CachedStage::ref() and + DefaultAccesses::access(access, l) } + predicate getAnUncertainScope = DefaultAccesses::getAnUncertainScope/2; + cached private newtype TLocal = TExplicitLocal(AstNode definingNode, string name, AstNode scope) { @@ -447,23 +484,10 @@ module LocalNameBinding override string getName() { result = name } override Location getLocation() { result = scope.getLocation() } - } - - pragma[nomagic] - private predicate resolveInScope(string name, Scope lookup, Local l) { - exists(Scope scope | lookupInScope(name, lookup, scope) | - l = TExplicitLocal(_, name, scope) or - l = TImplicitLocal(name, scope) - ) - } - cached - private predicate access(AstNode access, Local l) { - CachedStage::ref() and - exists(Scope lookup, string name | - accessCandInLookupScope(access, name, lookup) and - resolveInScope(name, lookup, l) - ) + /** Holds if this variable has the given name and scope. */ + pragma[nomagic] + predicate hasNameAndScope(string name_, AstNode scope_) { name = name_ and scope = scope_ } } /** A local access. */ diff --git a/unified/ql/lib/codeql/Definitions.qll b/unified/ql/lib/codeql/Definitions.qll index 3eb6ddcef102..1f6a1bbc9e81 100644 --- a/unified/ql/lib/codeql/Definitions.qll +++ b/unified/ql/lib/codeql/Definitions.qll @@ -3,14 +3,14 @@ */ private import unified -private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.NameBinding /** * Holds if `reference` refers to `definition`. */ cached -predicate definitionOf(Identifier reference, NameDeclaration definition, string kind) { +predicate definitionOf(Identifier reference, NameBinding definition, string kind) { definition = getStaticBindingTarget(reference) and - not reference instanceof NameDeclaration and + not reference instanceof NameBinding and kind = "name" } diff --git a/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll b/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll index d1568edfff80..d5eb5071176e 100644 --- a/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll +++ b/unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll @@ -1,8 +1,6 @@ private import unified private import codeql.util.ReportStats -private import codeql.unified.internal.StaticNameBinding -private import codeql.unified.internal.LocalNameBinding -private import codeql.unified.internal.NameBindingPlugin +private import codeql.unified.internal.NameBinding /** Stats about name nodes that static name binding could resolve. */ module StaticNameResolutionStats implements EntityStatsSig { @@ -44,15 +42,21 @@ module StaticNameResolutionStats implements EntityStatsSig { this = getIdentifierFromRef(ref) and not memberAccessDependsOnTypeInference(ref) ) and - not this instanceof NameDeclaration + not this instanceof NameBinding } NameBindingNode getTarget() { - ( - result.asIdentifier() = getStaticBindingTarget(this) - or - result.isModuleScopeNode(_) and - result.(NamespaceNode).ref().isIdentifier(this) + result.asIdentifier() = getStaticBindingTarget(this) + or + result.isModuleScopeNode(_) and + result.(NamespaceNode).ref().isIdentifier(this) + or + // Resolving to an implicitly-declared local such as "self" should count as + // as a successfully resolved name + exists(LocalName implicitLocal | + implicitLocal = this.(LocalNameAccess).getLocalName() and + not exists(implicitLocal.getABinding()) and + result.isLocalName(implicitLocal) ) } diff --git a/unified/ql/lib/codeql/unified/internal/AstExtra.qll b/unified/ql/lib/codeql/unified/internal/AstExtra.qll index fea1a2490363..5e8e54faa61a 100644 --- a/unified/ql/lib/codeql/unified/internal/AstExtra.qll +++ b/unified/ql/lib/codeql/unified/internal/AstExtra.qll @@ -3,6 +3,7 @@ */ private import unified +private import codeql.unified.internal.NameBindingPlugin module Public { /** A short-circuiting logical AND expression. */ @@ -29,24 +30,14 @@ module Public { * Declaration of a local or top-level variable. */ class LocalVariableDeclaration extends VariableDeclaration { - private Block block; - - LocalVariableDeclaration() { this = block.getStmt(_) } - - /** Gets the block in which this variable is declared. */ - Block getDeclaringBlock() { result = block } + LocalVariableDeclaration() { not isStaticMember(this) and not isInstanceMember(this) } } /** * Declaration of a local or top-level function. */ class LocalFunctionDeclaration extends FunctionDeclaration { - private Block block; - - LocalFunctionDeclaration() { this = block.getStmt(_) } - - /** Gets the block in which this function is declared. */ - Block getDeclaringBlock() { result = block } + LocalFunctionDeclaration() { not isStaticMember(this) and not isInstanceMember(this) } } /** diff --git a/unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll b/unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll index d545c589185f..2c60994c4cd5 100644 --- a/unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll +++ b/unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll @@ -39,14 +39,7 @@ private module Ast implements AstSig { not skipControlFlow(result) } - Callable getEnclosingCallable(AstNode node) { - exists(AstNode parent | parent = node.getParent() | - result = parent - or - not parent instanceof Callable and - result = getEnclosingCallable(parent) - ) - } + Callable getEnclosingCallable(AstNode node) { result = node.getEnclosingCallable() } class Callable = U::Callable; diff --git a/unified/ql/lib/codeql/unified/internal/FacadeAst.qll b/unified/ql/lib/codeql/unified/internal/FacadeAst.qll index dcec57f0a4ae..f5ad7e6b29a5 100644 --- a/unified/ql/lib/codeql/unified/internal/FacadeAst.qll +++ b/unified/ql/lib/codeql/unified/internal/FacadeAst.qll @@ -23,12 +23,47 @@ module Unified { ) } - /** Gets the nearest enclosing class declaration, possibly this node itself. */ + /** Gets the nearest enclosing class declaration, if any. */ ClassLikeDeclaration getEnclosingClass() { - result = this - or - not this instanceof ClassLikeDeclaration and - result = this.getParent().getEnclosingClass() + exists(AstNode parent | parent = this.getParent() | + result = parent + or + not parent instanceof ClassLikeDeclaration and + result = parent.getEnclosingClass() + ) + } + + private AstNode overrideEnclosingCallableParent() { + exists(FunctionExpr func | + // Capture declarations are evaluated as part of the outer context, and + // considered to be captured by the function expression. + this = func.getACaptureDeclaration() and + result = func.getParent() + ) + } + + /** + * Gets the nearest callable containing this AST node. + * + * If this node is itself a callable, this gets the outer callable, not the node itself. + * + * Note that the `TopLevel` is callable, so all nodes other than the `TopLevel` itself has an enclosing callable. + * + * In some cases this predicate skips overs the syntactically-enclosing callable in order to get the callable in which + * the AST is actually evaluated (such as for capture declarations in a function expression). + */ + Callable getEnclosingCallable() { + exists(AstNode parent | + parent = this.overrideEnclosingCallableParent() + or + not exists(this.overrideEnclosingCallableParent()) and + parent = this.getParent() + | + result = parent + or + not parent instanceof Callable and + result = parent.getEnclosingCallable() + ) } /** Gets the depth of this node in the AST. The root node has a depth of 0. */ diff --git a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll index 6fe8ace83fcf..a944db724c5a 100644 --- a/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll +++ b/unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll @@ -6,6 +6,7 @@ private import unified private import unified as U private import codeql.namebinding.LocalNameBinding private import codeql.unified.internal.NameBindingPlugin +private import codeql.unified.internal.StaticNameBinding private module LocalNameBindingInput implements LocalNameBindingInputSig { class AstNode = U::AstNode; @@ -167,7 +168,10 @@ private module LocalNameBindingInput implements LocalNameBindingInputSig; +private module TrackNameBinding = Track; /** Gets a name-binding node that may refer to the given declaration. */ -NameBindingNode trackNameDeclaration(NameDeclaration decl) { +NameBindingNode trackNameBinding(NameBinding decl) { exists(NameBindingNode start | start.isIdentifier(decl) and - result = TrackNameDeclaration::track(start) + result = TrackNameBinding::track(start) ) } @@ -490,7 +490,7 @@ module DebugGraph { */ private module FolderHeuristic { private predicate topLevelNameDef(File file, string name, NameBindingNode node) { - exists(TopLevel top, Stmt stmt, NameDeclaration nameDecl | + exists(TopLevel top, Stmt stmt, NameBinding nameDecl | top.getFile() = file and stmt = top.getBody().getAStmt() and not isPrivateToLocalScope(nameDecl) and @@ -583,10 +583,10 @@ private module FolderHeuristic { * or as a static member. */ private predicate unqualifiedMemberAccessCand( - PotentialLocalNameAccess access, boolean instanceAccess, NameDeclaration target, + PotentialLocalNameAccess access, boolean instanceAccess, NameBinding target, ClassLikeDeclaration accessingClass ) { - not access instanceof NameDeclaration and + not access instanceof NameBinding and ( // Resolved by local scoping exists(LocalName local | @@ -628,41 +628,69 @@ private int unqualifiedMemberAccessDepth(PotentialLocalNameAccess access) { * `instanceAccess` indicates if it is an instance member or static member. */ predicate unqualifiedMemberAccess( - PotentialLocalNameAccess access, boolean instanceAccess, NameDeclaration target, + PotentialLocalNameAccess access, boolean instanceAccess, NameBinding target, ClassLikeDeclaration accessingClass ) { unqualifiedMemberAccessCand(access, instanceAccess, target, accessingClass) and accessingClass.getDepth() = unqualifiedMemberAccessDepth(access) } -/** - * A name node appearing in an unqualified position, referring to a member of an enclosing class. - */ -class UnqualifiedMemberAccess extends Identifier { - private boolean instanceAccess; - private NameDeclaration target; - private ClassLikeDeclaration accessingClass; +module Public { + /** + * A name node appearing in an unqualified position, referring to a member of an enclosing class. + */ + class UnqualifiedMemberAccess extends Identifier { + private boolean instanceAccess; + private NameBinding target; + private ClassLikeDeclaration accessingClass; - UnqualifiedMemberAccess() { - unqualifiedMemberAccess(this, instanceAccess, target, accessingClass) - } + UnqualifiedMemberAccess() { + unqualifiedMemberAccess(this, instanceAccess, target, accessingClass) + } + + /** Gets the name binding of the member being accessed. */ + NameBinding getTarget() { result = target } - /** Gets the name declaration of the member being accessed. */ - NameDeclaration getTarget() { result = target } + /** Gets the enclosing class whose (possibly inherited) member is being accessed. */ + ClassLikeDeclaration getAccessingClass() { result = accessingClass } - /** Gets the enclosing class whose (possibly inherited) member is being accessed. */ - ClassLikeDeclaration getAccessingClass() { result = accessingClass } + /** Holds if this is an instance access on the accessing class. */ + predicate isInstanceAccess() { instanceAccess = true } - /** Holds if this is an instance access on the accessing class. */ - predicate isInstanceAccess() { instanceAccess = true } + /** Gets the local variable implicitly referenced as the base of this access. */ + LocalVariable getImplicitQualifierVariable() { + ResolveImplicitReceiverAccess::access(this, result) + } + } } /** Gets the declaration being accessed by `access`, as determined by static name binding. */ -NameDeclaration getStaticBindingTarget(Identifier access) { +NameBinding getStaticBindingTarget(Identifier access) { // For unqualified accesses, use the shadowing-aware lookup result = access.(UnqualifiedMemberAccess).getTarget() or // For others, just follow the name binding graph not access instanceof UnqualifiedMemberAccess and - trackNameDeclaration(result).asIdentifier() = access + trackNameBinding(result).asIdentifier() = access +} + +/** + * Gets the name of the implicit receiver parameter in scope at `callable` (possibly declared by an outer callable). + * + * Note that we only propagate the name, not the LocalVariable, since capture-declarations and Swift's `guard let self` statements + * may re-introduce a new binding for `self`, which becomes the one referenced by subsequent unqualified member accesses. + */ +private string getEnclosingReceiverParameterName(Callable callable) { + result = any(NameBindingPlugin p).getImplicitReceiverParameterName(callable) + or + not exists(any(NameBindingPlugin p).getImplicitReceiverParameterName(callable)) and + result = getEnclosingReceiverParameterName(callable.getEnclosingCallable()) +} + +/** Holds if `access` contains a reference to the implicit receiver parameter `name`. */ +private predicate implicitReceiverAccess(AstNode access, string name) { + name = getEnclosingReceiverParameterName(access.(UnqualifiedMemberAccess).getEnclosingCallable()) } + +private module ResolveImplicitReceiverAccess = + LocalNameBindingOutput::ResolveAccesses; diff --git a/unified/ql/lib/codeql/unified/internal/dev/debugLocalNameBindingGraph.ql b/unified/ql/lib/codeql/unified/internal/dev/debugLocalNameBindingGraph.ql index 71887f476f16..4c0db3a049df 100644 --- a/unified/ql/lib/codeql/unified/internal/dev/debugLocalNameBindingGraph.ql +++ b/unified/ql/lib/codeql/unified/internal/dev/debugLocalNameBindingGraph.ql @@ -6,7 +6,7 @@ */ private import unified -private import codeql.unified.internal.LocalNameBinding +private import codeql.unified.internal.NameBinding /** * Holds if `node` should be shown in the graph. diff --git a/unified/ql/lib/codeql/unified/internal/dev/debugStaticNameBindingGraph.ql b/unified/ql/lib/codeql/unified/internal/dev/debugStaticNameBindingGraph.ql index f365d3915f89..e8c97ce41852 100644 --- a/unified/ql/lib/codeql/unified/internal/dev/debugStaticNameBindingGraph.ql +++ b/unified/ql/lib/codeql/unified/internal/dev/debugStaticNameBindingGraph.ql @@ -6,7 +6,7 @@ */ private import unified -private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.NameBinding /** * Holds if `node` should be shown in the graph. diff --git a/unified/ql/lib/ide-contextual-queries/definitions.ql b/unified/ql/lib/ide-contextual-queries/definitions.ql index f72c91bcd9dd..0d2477a83b57 100644 --- a/unified/ql/lib/ide-contextual-queries/definitions.ql +++ b/unified/ql/lib/ide-contextual-queries/definitions.ql @@ -13,7 +13,7 @@ import unified external string selectedSourceFile(); -from Identifier reference, NameDeclaration definition, string kind +from Identifier reference, NameBinding definition, string kind where definitionOf(reference, definition, kind) and reference.getLocation().getFile() = getFileBySourceArchiveName(selectedSourceFile()) diff --git a/unified/ql/lib/unified.qll b/unified/ql/lib/unified.qll index 033d00aa6def..500de80b64f3 100644 --- a/unified/ql/lib/unified.qll +++ b/unified/ql/lib/unified.qll @@ -7,4 +7,4 @@ import codeql.files.FileSystem import codeql.unified.internal.Ast::UnifiedFinal import codeql.unified.internal.AstExtra::Public import codeql.unified.internal.ControlFlowGraph -import codeql.unified.internal.LocalNameBinding::Public +import codeql.unified.internal.NameBinding::Public diff --git a/unified/ql/lib/utils/test/CommentUtil.qll b/unified/ql/lib/utils/test/CommentUtil.qll index bd6f887a4010..932be8671a86 100644 --- a/unified/ql/lib/utils/test/CommentUtil.qll +++ b/unified/ql/lib/utils/test/CommentUtil.qll @@ -12,7 +12,7 @@ predicate plainCommentAt(string filepath, int line, string text) { predicate keyValueCommentAt(string filepath, int line, string key, string value) { exists(string text, string regexp, string match | plainCommentAt(filepath, line, text) and - regexp = "(\\w+)=([\\w.0-9]+)" and + regexp = "([\\w.-]+)=([\\w.0-9]+)" and match = text.regexpFind(regexp, _, _) and key = match.regexpCapture(regexp, 1) and value = match.regexpCapture(regexp, 2) diff --git a/unified/ql/lib/utils/test/TestUtils.qll b/unified/ql/lib/utils/test/TestUtils.qll index 6a7fbae3d592..4a232e984e78 100644 --- a/unified/ql/lib/utils/test/TestUtils.qll +++ b/unified/ql/lib/utils/test/TestUtils.qll @@ -1,15 +1,15 @@ private import unified private import CommentUtil -private import codeql.unified.internal.StaticNameBinding +private import codeql.unified.internal.NameBinding private string deriveClassName(ClassLikeDeclaration cls) { - not exists(cls.getParent().getEnclosingClass()) and + not exists(cls.getEnclosingClass()) and result = cls.getName() or - result = deriveClassName(cls.getParent().getEnclosingClass()) + "." + cls.getName() + result = deriveClassName(cls.getEnclosingClass()) + "." + cls.getName() } -private string defaultName(NameDeclaration decl) { +private string defaultName(NameBinding decl) { exists(ClassLikeDeclaration cls | decl.getDeclaration() = cls.getAMember() and result = deriveClassName(cls) + "." + decl.getName() @@ -19,11 +19,12 @@ private string defaultName(NameDeclaration decl) { result = decl.getName() } -private predicate declAt(NameDeclaration v, string filepath, int line) { +private predicate declAt(NameBinding v, string filepath, int line) { v.getLocation().hasLocationInfo(filepath, line, _, _, _) } -predicate nameDeclaration(NameDeclaration v, string alias) { +/** Holds if the name-binding `v` has been assigned the given `alias` by a comment in the test code. */ +predicate nameBinding(NameBinding v, string alias) { exists(string filepath, int line | declAt(v, filepath, line) | keyValueCommentAt(filepath, line, "name", alias) or diff --git a/unified/ql/src/diagnostic/FilesCoveredByModuleManifest.ql b/unified/ql/src/diagnostic/FilesCoveredByModuleManifest.ql index 6ac4cec641bd..ca226bedc005 100644 --- a/unified/ql/src/diagnostic/FilesCoveredByModuleManifest.ql +++ b/unified/ql/src/diagnostic/FilesCoveredByModuleManifest.ql @@ -9,7 +9,7 @@ */ import unified -import codeql.unified.internal.StaticNameBinding +import codeql.unified.internal.NameBinding import codeql.unified.internal.NameBindingPlugin import codeql.unified.internal.AnalysisQuality diff --git a/unified/ql/src/diagnostic/StaticNameResolution.ql b/unified/ql/src/diagnostic/StaticNameResolution.ql index 501c39126e54..b15516a3b7b8 100644 --- a/unified/ql/src/diagnostic/StaticNameResolution.ql +++ b/unified/ql/src/diagnostic/StaticNameResolution.ql @@ -9,7 +9,7 @@ */ import unified -import codeql.unified.internal.StaticNameBinding +import codeql.unified.internal.NameBinding import codeql.unified.internal.AnalysisQuality from StaticNameResolutionStats::Candidate c, NameBindingNode target diff --git a/unified/ql/test/library-tests/definitions/test.ql b/unified/ql/test/library-tests/definitions/test.ql index 361a9fd6a03c..9479e756bc94 100644 --- a/unified/ql/test/library-tests/definitions/test.ql +++ b/unified/ql/test/library-tests/definitions/test.ql @@ -7,11 +7,11 @@ module DefinitionsTest implements TestSig { string getARelevantTag() { result = "definition" } predicate hasActualResult(Location location, string element, string tag, string value) { - exists(Identifier reference, NameDeclaration definition | + exists(Identifier reference, NameBinding definition | definitionOf(reference, definition, "name") and location = reference.getLocation() and element = reference.toString() and - nameDeclaration(definition, value) and + nameBinding(definition, value) and tag = "definition" ) } diff --git a/unified/ql/test/library-tests/local-name-binding/class_scope.swift b/unified/ql/test/library-tests/local-name-binding/class_scope.swift index 5e251b8e6a0d..5c20bc19cbc1 100644 --- a/unified/ql/test/library-tests/local-name-binding/class_scope.swift +++ b/unified/ql/test/library-tests/local-name-binding/class_scope.swift @@ -5,8 +5,8 @@ class A { let b: B = nil // $ access=A.B let c: C = nil // $ access=A.C } - func instance_before() { - print(instanceVar) // $ access=instanceVar + func instance_before() { // implicit-self=instance_before.self + print(instanceVar) // $ access=instanceVar implicit-qualifier=instance_before.self B(); // $ access=A.B let b: B = nil // $ access=A.B let c: C = nil // $ access=A.C @@ -25,8 +25,8 @@ class A { let c: C = nil // $ access=A.C } - func instance_after() { - print(instanceVar) // $ access=instanceVar + func instance_after() { // implicit-self=instance_after.self + print(instanceVar) // $ access=instanceVar implicit-qualifier=instance_after.self B(); // $ access=A.B let b: B = nil // $ access=A.B let c: C = nil // $ access=A.C diff --git a/unified/ql/test/library-tests/local-name-binding/self_access.swift b/unified/ql/test/library-tests/local-name-binding/self_access.swift new file mode 100644 index 000000000000..795d7b8bac7a --- /dev/null +++ b/unified/ql/test/library-tests/local-name-binding/self_access.swift @@ -0,0 +1,33 @@ +class C { + func t1() { // implicit-self=t1.self + print(self) // $ access=t1.self + } + + var instanceField = 123; + + func t2() { // implicit-self=t2.self + print(instanceField) // $ access=instanceField implicit-qualifier=t2.self + } + + func t3() { // implicit-self=t3.self + foo(123) { [self] in // $ captured=closure.self // name=closure.self + print(self) // $ access=closure.self + print(instanceField) // $ access=instanceField implicit-qualifier=closure.self + } + } + + func t4() { // implicit-self=t4.self + foo(123) { [weak self] in // $ captured=weak.self // name=weak.self + // Here, 'self' is an Option referring to .some() if it + // has not been GC'ed yet. Swift does not allow unqualified self access here. + + print(self) // $ access=weak.self + + // Unwrap the 'self' optional to get a strong reference. + guard let self else { return } // $ access=weak.self // name=guarded.self + + print(self) // $ access=guarded.self + print(instanceField) // $ access=instanceField implicit-qualifier=guarded.self + } + } +} diff --git a/unified/ql/test/library-tests/local-name-binding/test.ql b/unified/ql/test/library-tests/local-name-binding/test.ql index e430f16ba48d..974322851944 100644 --- a/unified/ql/test/library-tests/local-name-binding/test.ql +++ b/unified/ql/test/library-tests/local-name-binding/test.ql @@ -1,27 +1,36 @@ import unified import utils.test.InlineExpectationsTest import utils.test.CommentUtil -import codeql.unified.internal.LocalNameBinding +import codeql.unified.internal.NameBinding module VariableAccessTest implements TestSig { - string getARelevantTag() { result = "access" } + string getARelevantTag() { result = ["access", "implicit-qualifier", "captured"] } additional predicate declAt(LocalName v, string filepath, int line) { v.getLocation().hasLocationInfo(filepath, line, _, _, _) } private predicate decl(LocalName v, string alias) { - exists(string filepath, int line | declAt(v, filepath, line) | - keyValueCommentAt(filepath, line, "name", alias) + exists(string filepath, int line, string tag | + declAt(v, filepath, line) and + if exists(v.getABinding()) + then + // explicit declarations must be annotated with 'name' + tag = "name" + else ( + // implicit declarations have their own tags + v.getName() = "self" and tag = "implicit-self" + ) + | + keyValueCommentAt(filepath, line, tag, alias) or - not keyValueCommentAt(filepath, line, "name", _) and + not keyValueCommentAt(filepath, line, tag, _) and alias = v.getName() ) } private PotentialLocalNameAccess getUniqueDeclarationSite(LocalName name) { - result = - unique(PotentialLocalNameAccess ac | ac.isDeclarationSite() and ac.getLocalName() = name) + result = unique(PotentialLocalNameAccess ac | ac.isBindingSite() and ac.getLocalName() = name) } predicate hasActualResult(Location location, string element, string tag, string value) { @@ -33,6 +42,23 @@ module VariableAccessTest implements TestSig { decl(v, value) and tag = "access" ) + or + exists(UnqualifiedMemberAccess access, LocalName v | + v = access.getImplicitQualifierVariable() and + location = access.getLocation() and + element = access.toString() and + decl(v, value) and + access.isInstanceAccess() and // For now, don't annotate receiver access in static methods. It technically exists, it's just not important yet. + tag = "implicit-qualifier" + ) + or + exists(LocalVariable v | + v.isCaptured() and + location = v.getLocation() and + element = v.toString() and + decl(v, value) and + tag = "captured" + ) } } diff --git a/unified/ql/test/library-tests/local-name-binding/test.swift b/unified/ql/test/library-tests/local-name-binding/test.swift index 23d71d8f6dcd..02558d808d4e 100644 --- a/unified/ql/test/library-tests/local-name-binding/test.swift +++ b/unified/ql/test/library-tests/local-name-binding/test.swift @@ -135,7 +135,7 @@ func t16() throws { // Closure captures func t17() { - let x = 1 // name=x1 + let x = 1 // $ captured=x1 // name=x1 let closure = { // name=closure1 print(x) // $ access=x1 } @@ -181,7 +181,7 @@ func t21() { // Nested functions func t22() { let x = 1 // name=x1 - func inner() { // name=inner1 + func inner() { // $ captured=inner1 // name=inner1 let x = 2 // name=x2 print(x) // $ access=x2 } diff --git a/unified/ql/test/library-tests/static-name-binding/test.ql b/unified/ql/test/library-tests/static-name-binding/test.ql index e6dd7e64f6b3..c39202fe5a41 100644 --- a/unified/ql/test/library-tests/static-name-binding/test.ql +++ b/unified/ql/test/library-tests/static-name-binding/test.ql @@ -1,18 +1,18 @@ import unified import utils.test.InlineExpectationsTest import utils.test.TestUtils -import codeql.unified.internal.StaticNameBinding +import codeql.unified.internal.NameBinding module StaticDeclAccess implements TestSig { string getARelevantTag() { result = "access" } predicate hasActualResult(Location location, string element, string tag, string value) { - exists(NameDeclaration decl, Identifier access | + exists(NameBinding decl, Identifier access | decl = getStaticBindingTarget(access) and - not access instanceof NameDeclaration and + not access instanceof NameBinding and location = access.getLocation() and element = access.toString() and - nameDeclaration(decl, value) and + nameBinding(decl, value) and tag = "access" ) }