Apply review comments

Author: smowton

go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/go.dbscheme renamed to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/go.dbscheme

@@ -208,7 +208,7 @@ underlying_type(unique int named: @namedtype ref, int tp: @type ref);
 component_types(int parent: @compositetype ref, int index: int ref, string name: string ref, int tp: @type ref);
 
 #keyset[parent, index]
-component_tags(int parent: @compositetype ref, int index: int ref, string tag: string ref);
+component_tags(int parent: @structtype ref, int index: int ref, string tag: string ref);
 
 #keyset[interface, index]
 interface_private_method_ids(int interface: @interfacetype ref, int index: int ref, string id: string ref);

go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/old.dbscheme renamed to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/old.dbscheme

@@ -1150,14 +1150,14 @@ var ComponentTypesTable = NewTable("component_types",
 	EntityColumn(TypeType, "tp"),
 ).KeySet("parent", "index")
 
-// ComponentTagsTable is the table associating composite types with their component types' tags
+// ComponentTagsTable is the table associating struct types with their component types' tags
 var ComponentTagsTable = NewTable("component_tags",
-	EntityColumn(CompositeType, "parent"),
+	EntityColumn(StructType, "parent"),
 	IntColumn("index"),
 	StringColumn("tag"),
 ).KeySet("parent", "index")
 
-// InterfacePrivateMethodIdsTable is the table associating interface types with their private method ids
+// InterfacePrivateMethodIdsTable is the table associating interface types with the indices and ids of their private methods.
 var InterfacePrivateMethodIdsTable = NewTable("interface_private_method_ids",
 	EntityColumn(InterfaceType, "interface"),
 	IntColumn("index"),

go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/upgrade.properties renamed to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/upgrade.properties

@@ -1624,7 +1624,9 @@ func extractType(tw *trap.Writer, tp types.Type) trap.Label {
 					name = ""
 				}
 				extractComponentType(tw, lbl, i, name, field.Type())
-				dbscheme.ComponentTagsTable.Emit(tw, lbl, i, tp.Tag(i))
+				if tp.Tag(i) != "" {
+					dbscheme.ComponentTagsTable.Emit(tw, lbl, i, tp.Tag(i))
+				}
 			}
 		case *types.Pointer:
 			kind = dbscheme.PointerType.Index()
@@ -1643,11 +1645,8 @@ func extractType(tw *trap.Writer, tp types.Type) trap.Label {
 
 				extractComponentType(tw, lbl, i, meth.Name(), meth.Type())
 
-				// meth.Id() will be equal to meth.Name() for an exported method, or
-				// packge-qualified otherwise.
-				privateMethodId := meth.Id()
-				if privateMethodId != meth.Name() {
-					dbscheme.InterfacePrivateMethodIdsTable.Emit(tw, lbl, i, privateMethodId)
+				if !meth.Exported() {
+					dbscheme.InterfacePrivateMethodIdsTable.Emit(tw, lbl, i, meth.Id())
 				}
 			}
 			for i := 0; i < tp.NumEmbeddeds(); i++ {

go/extractor/dbscheme/tables.go

@@ -1,5 +1,5 @@
 ---
 category: minorAnalysis
 ---
-* Added methods `StructTag.hasOwnFieldWithTag` and `Field.getTag`, which enable CodeQL queries to examine struct field tags.
-* Added method `InterfaceType.getMethodTypeById`, which enables CodeQL queries to distinguish interfaces with matching non-exported method names that are declared in different packages, and are therefore incompatible.
+* Added member predicates `StructTag.hasOwnFieldWithTag` and `Field.getTag`, which enable CodeQL queries to examine struct field tags.
+* Added member predicate `InterfaceType.getMethodTypeByQualifiedName`, which enables CodeQL queries to distinguish interfaces with matching non-exported method names that are declared in different packages, and are therefore incompatible.

go/extractor/extractor.go

@@ -208,7 +208,7 @@ underlying_type(unique int named: @namedtype ref, int tp: @type ref);
 component_types(int parent: @compositetype ref, int index: int ref, string name: string ref, int tp: @type ref);
 
 #keyset[parent, index]
-component_tags(int parent: @compositetype ref, int index: int ref, string tag: string ref);
+component_tags(int parent: @structtype ref, int index: int ref, string tag: string ref);
 
 #keyset[interface, index]
 interface_private_method_ids(int interface: @interfacetype ref, int index: int ref, string id: string ref);

go/ql/lib/change-notes/2024-09-03-tags-and-interface-ids.md

@@ -465,7 +465,11 @@ class StructType extends @structtype, CompositeType {
    */
   predicate hasOwnFieldWithTag(int i, string name, Type tp, boolean isEmbedded, string tag) {
     this.hasOwnField(i, name, tp, isEmbedded) and
-    component_tags(this, i, tag)
+    (
+      component_tags(this, i, tag)
+      or
+      not component_tags(this, i, _) and tag = ""
+    )
   }
 
   /**
@@ -586,10 +590,13 @@ class StructType extends @structtype, CompositeType {
   override string pp() {
     result =
       "struct { " +
-        concat(int i, string name, Type tp, string tag, string tagToPrint |
+        concat(int i, string name, Type tp, string tagToPrint |
           component_types(this, i, name, tp) and
-          component_tags(this, i, tag) and
-          (if tag = "" then tagToPrint = "" else tagToPrint = " `" + tag + "`")
+          (
+            tagToPrint = " `" + any(string tag | component_tags(this, i, tag)) + "`"
+            or
+            tagToPrint = "" and not component_tags(this, i, _)
+          )
         |
           name + " " + tp.pp() + tagToPrint, "; " order by i
         ) + " }"
@@ -761,26 +768,28 @@ class InterfaceType extends @interfacetype, CompositeType {
   }
 
   /**
-   * Gets the type of method `id` of this interface type.
+   * Gets the type of method `qname` of this interface type.
    *
-   * This differs from `getMethodType` in that if the method is not exported, the `id`
-   * will be package-qualified. This means that the set of `id`s` together with any
+   * This differs from `getMethodType` in that if the method is not exported, the `qname`
+   * will be package-qualified. This means that the set of `qname`s` together with any
    * embedded types fully distinguishes the interface from any other, whereas the set
    * of names matched by `getMethodName` may be ambiguous between interfaces with matching
    * exported methods and unexported methods that have matching names but belong to
    * different packages.
    *
    * For example, `interface { Exported() int; notExported() int }` declared in two
    * different packages defines two distinct types, but they appear identical according to
-   * `getMethodType`.
+   * `getMethodType`. If the packages were named `a` and `b`, `getMethodType` would yield
+   * `notExported -> int` for both, whereas this method would yield `a.notExported -> int`
+   * and `b.notExported -> int` respectively.
    */
-  Type getMethodTypeById(string id) {
+  Type getMethodTypeByQualifiedName(string qname) {
     exists(int i, string name | i >= 0 |
       component_types(this, i, name, result) and
       (
-        interface_private_method_ids(this, i, id)
+        interface_private_method_ids(this, i, qname)
         or
-        name = id and not interface_private_method_ids(this, i, _)
+        name = qname and not interface_private_method_ids(this, i, _)
       )
     )
   }

go/ql/lib/go.dbscheme

@@ -208,7 +208,7 @@ underlying_type(unique int named: @namedtype ref, int tp: @type ref);
 component_types(int parent: @compositetype ref, int index: int ref, string name: string ref, int tp: @type ref);
 
 #keyset[parent, index]
-component_tags(int parent: @compositetype ref, int index: int ref, string tag: string ref);
+component_tags(int parent: @structtype ref, int index: int ref, string tag: string ref);
 
 #keyset[interface, index]
 interface_private_method_ids(int interface: @interfacetype ref, int index: int ref, string id: string ref);