From 7a7ff4a91ea24c06be2ffd016373119c1e282968 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 30 Sep 2024 16:33:26 +0100 Subject: [PATCH] Apply review comments --- .../go.dbscheme | 0 .../old.dbscheme | 2 +- .../upgrade.properties | 0 go/extractor/dbscheme/tables.go | 6 ++-- go/extractor/extractor.go | 11 +++---- .../2024-09-03-tags-and-interface-ids.md | 4 +-- go/ql/lib/go.dbscheme | 2 +- go/ql/lib/semmle/go/Types.qll | 31 ++++++++++++------- .../go.dbscheme | 2 +- ...s.expected => InterfaceMethodIds.expected} | 0 ...{InterfaceIds.ql => InterfaceMethodIds.ql} | 2 +- 11 files changed, 34 insertions(+), 26 deletions(-) rename go/downgrades/{e47462df302b3e58a60d2e21f99aba63f973326f => d2066c23fe3de023d76d7e5a52dec6771d308534}/go.dbscheme (100%) rename go/downgrades/{e47462df302b3e58a60d2e21f99aba63f973326f => d2066c23fe3de023d76d7e5a52dec6771d308534}/old.dbscheme (99%) rename go/downgrades/{e47462df302b3e58a60d2e21f99aba63f973326f => d2066c23fe3de023d76d7e5a52dec6771d308534}/upgrade.properties (100%) rename go/ql/test/library-tests/semmle/go/Types/{InterfaceIds.expected => InterfaceMethodIds.expected} (100%) rename go/ql/test/library-tests/semmle/go/Types/{InterfaceIds.ql => InterfaceMethodIds.ql} (79%) diff --git a/go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/go.dbscheme b/go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/go.dbscheme similarity index 100% rename from go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/go.dbscheme rename to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/go.dbscheme diff --git a/go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/old.dbscheme b/go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/old.dbscheme similarity index 99% rename from go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/old.dbscheme rename to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/old.dbscheme index e47462df302..d2066c23fe3 100644 --- a/go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/old.dbscheme +++ b/go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/old.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); diff --git a/go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/upgrade.properties b/go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/upgrade.properties similarity index 100% rename from go/downgrades/e47462df302b3e58a60d2e21f99aba63f973326f/upgrade.properties rename to go/downgrades/d2066c23fe3de023d76d7e5a52dec6771d308534/upgrade.properties diff --git a/go/extractor/dbscheme/tables.go b/go/extractor/dbscheme/tables.go index 89cc7ffeadb..ec74c7a2c0d 100644 --- a/go/extractor/dbscheme/tables.go +++ b/go/extractor/dbscheme/tables.go @@ -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"), diff --git a/go/extractor/extractor.go b/go/extractor/extractor.go index 33186a9380f..8a399eb2b90 100644 --- a/go/extractor/extractor.go +++ b/go/extractor/extractor.go @@ -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++ { diff --git a/go/ql/lib/change-notes/2024-09-03-tags-and-interface-ids.md b/go/ql/lib/change-notes/2024-09-03-tags-and-interface-ids.md index 2baa8cd6da2..c8c6e88ab4c 100644 --- a/go/ql/lib/change-notes/2024-09-03-tags-and-interface-ids.md +++ b/go/ql/lib/change-notes/2024-09-03-tags-and-interface-ids.md @@ -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. diff --git a/go/ql/lib/go.dbscheme b/go/ql/lib/go.dbscheme index e47462df302..d2066c23fe3 100644 --- a/go/ql/lib/go.dbscheme +++ b/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); diff --git a/go/ql/lib/semmle/go/Types.qll b/go/ql/lib/semmle/go/Types.qll index 16e2c528ef4..51e9a953920 100644 --- a/go/ql/lib/semmle/go/Types.qll +++ b/go/ql/lib/semmle/go/Types.qll @@ -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,10 +768,10 @@ 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 @@ -772,15 +779,17 @@ class InterfaceType extends @interfacetype, CompositeType { * * 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, _) ) ) } diff --git a/go/ql/lib/upgrades/a58b81b1b4c4cccc8ca11731c1db86622f33af57/go.dbscheme b/go/ql/lib/upgrades/a58b81b1b4c4cccc8ca11731c1db86622f33af57/go.dbscheme index e47462df302..d2066c23fe3 100644 --- a/go/ql/lib/upgrades/a58b81b1b4c4cccc8ca11731c1db86622f33af57/go.dbscheme +++ b/go/ql/lib/upgrades/a58b81b1b4c4cccc8ca11731c1db86622f33af57/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); diff --git a/go/ql/test/library-tests/semmle/go/Types/InterfaceIds.expected b/go/ql/test/library-tests/semmle/go/Types/InterfaceMethodIds.expected similarity index 100% rename from go/ql/test/library-tests/semmle/go/Types/InterfaceIds.expected rename to go/ql/test/library-tests/semmle/go/Types/InterfaceMethodIds.expected diff --git a/go/ql/test/library-tests/semmle/go/Types/InterfaceIds.ql b/go/ql/test/library-tests/semmle/go/Types/InterfaceMethodIds.ql similarity index 79% rename from go/ql/test/library-tests/semmle/go/Types/InterfaceIds.ql rename to go/ql/test/library-tests/semmle/go/Types/InterfaceMethodIds.ql index 5b04b068e6b..b6246cf683a 100644 --- a/go/ql/test/library-tests/semmle/go/Types/InterfaceIds.ql +++ b/go/ql/test/library-tests/semmle/go/Types/InterfaceMethodIds.ql @@ -4,5 +4,5 @@ from NamedType nt, InterfaceType it, Type methodType, string id where nt.getName() = "MixedExportedAndNot" and it = nt.getUnderlyingType() and - methodType = it.getMethodTypeById(id) + methodType = it.getMethodTypeByQualifiedName(id) select it.pp(), methodType.pp(), id