unified: Let bindingContext dictate resolution for name bindings

Author: asgerf

shared/namebinding/codeql/namebinding/LocalNameBinding.qll

@@ -449,6 +449,12 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
 
     /** Gets a textual representation of this local entity. */
     string toString() { result = this.getName() }
+
+    /** Holds if this variable has the given name and scope. */
+    pragma[nomagic]
+    predicate hasNameAndScope(string name, AstNode scope) {
+      this = TExplicitLocal(_, name, scope) or this = TImplicitLocal(name, scope)
+    }
   }
 
   final class Local = LocalImpl;
@@ -484,10 +490,6 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
     override string getName() { result = name }
 
     override Location getLocation() { result = scope.getLocation() }
-
-    /** 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. */

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

@@ -347,6 +347,17 @@ predicate bindingContext = LocalNameBindingInput::bindingContext/3;
 
 module LocalNameBindingOutput = LocalNameBinding<Location, LocalNameBindingInput>;
 
+private LocalNameBindingOutput::Local resolveId(Identifier id) {
+  // For names in binding position, always resolve to the scope set by the bindingContext predicate
+  exists(AstNode scope |
+    bindingContext(id, scope, _) and
+    result.hasNameAndScope(id.getValue(), scope)
+  )
+  or
+  not bindingContext(id, _, _) and
+  result = id.(LocalNameBindingOutput::LocalAccess).getLocal()
+}
+
 module Public {
   /**
    * A representative for a lexically scoped entity, such as a local variable, type name, or module name.
@@ -379,7 +390,7 @@ module Public {
     string getName() { result = this.getValue() }
 
     /** Gets the representative for the local name introduced by this declaration. */
-    LocalName getLocalName() { result = this.(LocalNameBindingOutput::LocalAccess).getLocal() }
+    LocalName getLocalName() { result = resolveId(this) }
   }
 
   final class LocalVariable = LocalVariableImpl;
@@ -495,7 +506,7 @@ module Public {
  */
 class PotentialLocalNameAccess extends IdentifierExpr {
   /** Gets the representative for the local name being accessed. */
-  LocalName getLocalName() { result = this.(LocalNameBindingOutput::LocalAccess).getLocal() }
+  LocalName getLocalName() { result = resolveId(this) }
 
   /** Gets the name being accessed. */
   string getName() { result = this.getValue() }

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

@@ -385,23 +385,23 @@ func t39(value: Int) {
 
 func t40() {
     // This function and its parameter have the same name
-    func foo(  // $ SPURIOUS: access=foo2 // name=foo1
-        _ foo: String  // $ SPURIOUS: access=foo2 captured=foo2 // name=foo2
+    func foo(  // name=foo1
+        _ foo: String  // name=foo2
     ) {
         foo  // $ access=foo2
     }
-    foo()  // $ MISSING: access=foo1
+    foo()  // $ access=foo1
 }
 
 func t41() {
     // This function and its parameter have the same name
     // The parameter default value must resolve to the outer name (the function)
     // swift-format-ignore
-    func foo(  // $ SPURIOUS: access=foo2 // name=foo1
-        _ foo: Any = // $ SPURIOUS: access=foo2 captured=foo2 // name=foo2
+    func foo(  // name=foo1
+        _ foo: Any = // name=foo2
             foo  // $ MISSING: access=foo1 SPURIOUS: access=foo2
     ) {
         foo  // $ access=foo2
     }
-    foo()  // $ MISSING: access=foo1
+    foo()  // $ access=foo1
 }