unified: Fix spurious resolution by refining isPrivateToLocalScope

Author: asgerf

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

@@ -1,5 +1,6 @@
 private import unified
 private import codeql.util.Unit
+private import codeql.unified.internal.LocalNameBinding
 private import codeql.unified.internal.NameBindingPluginSwift // ensure overrides are seen
 
 /** Extension point for language-specific inputs to name binding. */
@@ -14,13 +15,16 @@ class NameBindingPlugin extends Unit {
   predicate isInstanceMember(ClassLikeDeclaration cls, Member member) { none() }
 
   /**
-   * Holds if `member` is only visible in its local scope, and can thus be entirely resolved
+   * Holds if `binding`, declared by `member` is only visible in its local scope, and can thus be entirely resolved
    * by local name-binding, suppressing any store-steps that would otherwise be induced from the member.
    *
    * Need only be implemented for members that occur in the context of class or top-level, as other
    * contexts are considered local already.
+   *
+   * `binding` refers to an `Identifier` or `BulkImportingPattern` bound by the member.
    */
-  predicate isPrivateToLocalScope(Stmt member) { none() }
+  bindingset[member, binding]
+  predicate isPrivateToLocalScope(Stmt member, AstNode binding) { none() }
 }
 
 /** Holds if `member` is an instance member. */
@@ -31,9 +35,18 @@ predicate isInstanceMember(Member member) {
   )
 }
 
-/** Holds if `member` is only visible in its local scope. */
-predicate isPrivateToLocalScope(Stmt member) {
-  any(NameBindingPlugin p).isPrivateToLocalScope(member)
+/** Holds if `binding` is only visible in its local scope. */
+pragma[nomagic]
+predicate isPrivateToLocalScope(AstNode binding) {
+  exists(Stmt member |
+    bindingContext(binding, _, member) and
+    (
+      member = any(ClassLikeDeclaration cls).getAMember() or
+      member = any(TopLevel t).getBody().getAStmt()
+    ) and
+    (binding instanceof NameDeclaration or binding instanceof BulkImportingPattern) and
+    any(NameBindingPlugin p).isPrivateToLocalScope(member, binding)
+  )
 }
 
 /**

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

@@ -13,14 +13,18 @@ class NameBindingPluginSwift extends NameBindingPlugin {
     not member.hasModifier(["static", "class", "enum_case"])
   }
 
-  override predicate isPrivateToLocalScope(Stmt member) {
+  bindingset[member, binding]
+  override predicate isPrivateToLocalScope(Stmt member, AstNode binding) {
     // Private top-level members
     member = any(TopLevel top).getBody().getAStmt() and
     member.hasModifier(["private", "fileprivate"])
     or
     // Imports are always file-local, except `@_exported` import which re-export everything
     member instanceof ImportDeclaration and
-    not member.hasModifier("@_exported")
+    not (
+      member.hasModifier("@_exported") and
+      binding instanceof BulkImportingPattern
+    )
     //
     // Note: Private class members can be seen within type-extensions in the same file,
     // so we can't declare those private to their local scope.

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

@@ -152,7 +152,7 @@ predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) {
   exists(ClassLikeDeclaration cls, Member member, NameDeclaration nameDecl |
     member = cls.getAMember() and
     not isInstanceMember(member) and
-    not isPrivateToLocalScope(member) and
+    not isPrivateToLocalScope(nameDecl) and
     nameDecl.getDeclaration() = member and
     node1.isIdentifier(nameDecl) and
     name = nameDecl.getName() and
@@ -161,7 +161,7 @@ predicate storeStep(NameBindingNode node1, string name, NameBindingNode node2) {
   or
   exists(TopLevel top, Stmt stmt, NameDeclaration nameDecl |
     stmt = top.getBody().getAStmt() and
-    not isPrivateToLocalScope(stmt) and
+    not isPrivateToLocalScope(nameDecl) and
     nameDecl.getDeclaration() = stmt and
     node1.isIdentifier(nameDecl) and
     name = nameDecl.getName() and
@@ -215,7 +215,7 @@ predicate valueStep(NameBindingNode node1, NameBindingNode node2) {
     // Bulk re-exporting declarations
     exists(TopLevel top |
       declaration = top.getBody().getAStmt() and
-      not isPrivateToLocalScope(declaration) and
+      not isPrivateToLocalScope(p) and
       node2 = getModuleNodeFromFile(top.getFile())
     )
   )

unified/ql/test/library-tests/static-name-binding/package1/Sources/Target4/UseReExport.swift

@@ -10,5 +10,5 @@ private protocol P {
     let x6: Target3.C; // $ access=Target3.C
 
     // `@_exported import Target2` should not re-export the local 'Target2' name, only its contents
-    let x7: Target3.Target2.A; // $ SPURIOUS: access=Target2.A
+    let x7: Target3.Target2.A; // should not resolve
 }