Skip to content

Go: Fix missing promoted fields due to name clash #18001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 2, 2024
Merged

Go: Fix missing promoted fields due to name clash #18001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
593896b
Add test showing promoted field bug
owen-mc Nov 8, 2024
8dc0688
Fix bug
owen-mc Nov 17, 2024
4990f16
Refactor struct field predicate to remove redundancy
owen-mc Nov 8, 2024
1bc1472
Add change note
owen-mc Nov 17, 2024
2cba97e
Small stylistic improvement
owen-mc Nov 22, 2024
0e94ee8
Don't getUnderlyingType before looking through pointer type
owen-mc Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...b/change-notes/2024-11-17-fix-missing-promoted-fields-and-methods-name-clash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: fix
---
* Fixed a bug which meant that promoted fields and methods were missing when the embedded parent was not promoted due to a name clash.
41 changes: 19 additions & 22 deletions go/ql/lib/semmle/go/Types.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,15 @@ class StructType extends @structtype, CompositeType {
Field getFieldOfEmbedded(Field embeddedParent, string name, int depth, boolean isEmbedded) {
// embeddedParent is a field of 'this' at depth 'depth - 1'
this.hasFieldCand(_, embeddedParent, depth - 1, true) and
// embeddedParent's type has the result field
exists(StructType embeddedType, Type fieldType |
fieldType = embeddedParent.getType().getUnderlyingType() and
pragma[only_bind_into](embeddedType) =
[fieldType, fieldType.(PointerType).getBaseType().getUnderlyingType()]
|
result = embeddedType.getOwnField(name, isEmbedded)
)
// embeddedParent's type has the result field. Note that it is invalid Go
// to have an embedded field with a named type whose underlying type is a
// pointer, so we don't have to have
// `lookThroughPointerType(embeddedParent.getType().getUnderlyingType())`.
result =
lookThroughPointerType(embeddedParent.getType())
.getUnderlyingType()
.(StructType)
.getOwnField(name, isEmbedded)
}

/**
Expand All @@ -523,8 +524,12 @@ class StructType extends @structtype, CompositeType {
private predicate hasFieldCand(string name, Field f, int depth, boolean isEmbedded) {
f = this.getOwnField(name, isEmbedded) and depth = 0
or
not this.hasOwnField(_, name, _, _) and
f = this.getFieldOfEmbedded(_, name, depth, isEmbedded)
f = this.getFieldOfEmbedded(_, name, depth, isEmbedded) and
// If this is a cyclic field and this is not the first time we see this embedded field
// then don't include it as a field candidate to avoid non-termination.
not exists(Type t | lookThroughPointerType(t) = lookThroughPointerType(f.getType()) |
this.hasOwnField(_, name, t, _)
)
}

private predicate hasMethodCand(string name, Method m, int depth) {
Expand All @@ -541,15 +546,7 @@ class StructType extends @structtype, CompositeType {
predicate hasField(string name, Type tp) {
exists(int mindepth |
mindepth = min(int depth | this.hasFieldCand(name, _, depth, _)) and
tp = unique(Field f | f = this.getFieldCand(name, mindepth, _)).getType()
)
}

private Field getFieldCand(string name, int depth, boolean isEmbedded) {
result = this.getOwnField(name, isEmbedded) and depth = 0
or
exists(Type embedded | this.hasEmbeddedField(embedded, depth - 1) |
result = embedded.getUnderlyingType().(StructType).getOwnField(name, isEmbedded)
tp = unique(Field f | this.hasFieldCand(name, f, mindepth, _)).getType()
)
}

Expand All @@ -564,9 +561,9 @@ class StructType extends @structtype, CompositeType {
* The depth of a field `f` declared in this type is zero.
*/
Field getFieldAtDepth(string name, int depth) {
depth = min(int depthCand | exists(this.getFieldCand(name, depthCand, _))) and
result = this.getFieldCand(name, depth, _) and
strictcount(this.getFieldCand(name, depth, _)) = 1
depth = min(int depthCand | this.hasFieldCand(name, _, depthCand, _)) and
this.hasFieldCand(name, result, depth, _) and
strictcount(Field f | this.hasFieldCand(name, f, depth, _)) = 1
}

Method getMethodAtDepth(string name, int depth) {
Expand Down
31 changes: 17 additions & 14 deletions go/ql/test/library-tests/semmle/go/Types/Field_getPackage.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
| depth.go:19:2:19:2 | f | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:4:2:4:2 | A | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:8:3:8:5 | Baz | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:13:2:13:4 | Qux | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:14:2:14:4 | Baz | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:12:2:12:4 | Qux | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| embedded.go:13:2:13:4 | Baz | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| generic.go:4:2:4:11 | valueField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| generic.go:5:2:5:13 | pointerField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| generic.go:6:2:6:11 | arrayField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
Expand All @@ -26,6 +26,7 @@
| generic.go:21:2:21:9 | mapField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| generic.go:25:2:25:12 | structField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| generic.go:29:2:29:13 | pointerField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| main.go:18:7:18:15 | NameClash | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| pkg1/embedding.go:19:23:19:26 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/embedding.go:22:27:22:30 | base | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/embedding.go:25:24:25:31 | embedder | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
Expand All @@ -36,20 +37,22 @@
| pkg1/promotedStructs.go:14:2:14:7 | PField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/promotedStructs.go:22:22:22:22 | S | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/promotedStructs.go:25:22:25:22 | P | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:4:2:4:2 | f | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:5:2:5:4 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:6:2:6:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:10:2:10:4 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:11:2:11:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:15:3:15:5 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:16:3:16:5 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:20:3:20:5 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:21:2:21:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:25:2:25:4 | val | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:26:2:26:5 | flag | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:30:2:30:5 | flag | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:6:2:6:2 | f | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:7:2:7:4 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:8:2:8:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:12:2:12:4 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:13:2:13:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:17:3:17:5 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:18:3:18:5 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:22:3:22:5 | Foo | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:23:2:23:4 | Bar | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:27:2:27:4 | val | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:28:2:28:5 | flag | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:32:2:32:5 | flag | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg1/tst.go:62:7:62:15 | NameClash | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1 |
| pkg2/tst.go:4:2:4:2 | g | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2 |
| pkg2/tst.go:8:2:8:2 | g | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2 |
| pkg2/tst.go:17:2:17:8 | NCField | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2 |
| struct_tags.go:4:2:4:7 | field1 | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| struct_tags.go:5:2:5:7 | field2 | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
| struct_tags.go:9:2:9:7 | field1 | package github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types |
Expand Down
44 changes: 25 additions & 19 deletions go/ql/test/library-tests/semmle/go/Types/Field_hasQualifiedName2.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
| depth.go:19:2:19:2 | f | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.c | f |
| depth.go:19:2:19:2 | f | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.d | f |
| embedded.go:4:2:4:2 | A | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.Baz | A |
| embedded.go:4:2:4:2 | A | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsBaz | A |
| embedded.go:4:2:4:2 | A | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.Qux | A |
| embedded.go:8:3:8:5 | Baz | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.Qux | Baz |
| embedded.go:13:2:13:4 | Qux | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsBaz | Qux |
| embedded.go:14:2:14:4 | Baz | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsBaz | Baz |
| embedded.go:12:2:12:4 | Qux | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsBaz | Qux |
| embedded.go:13:2:13:4 | Baz | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsBaz | Baz |
| generic.go:4:2:4:11 | valueField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.GenericStruct1 | valueField |
| generic.go:5:2:5:13 | pointerField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.GenericStruct1 | pointerField |
| generic.go:6:2:6:11 | arrayField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.GenericStruct1 | arrayField |
Expand All @@ -33,6 +34,7 @@
| generic.go:21:2:21:9 | mapField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.GenericStruct2 | mapField |
| generic.go:25:2:25:12 | structField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.GenericStruct2b | structField |
| generic.go:29:2:29:13 | pointerField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.CircularGenericStruct2 | pointerField |
| main.go:18:7:18:15 | NameClash | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsNameClash | NameClash |
| pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder | base |
| pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder2 | base |
| pkg1/embedding.go:19:23:19:26 | base | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.embedder3 | base |
Expand All @@ -49,27 +51,31 @@
| pkg1/promotedStructs.go:14:2:14:7 | PField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.SEmbedP | PField |
| pkg1/promotedStructs.go:22:22:22:22 | S | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.SEmbedS | S |
| pkg1/promotedStructs.go:25:22:25:22 | P | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.SEmbedP | P |
| pkg1/tst.go:4:2:4:2 | f | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | f |
| pkg1/tst.go:5:2:5:4 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | Foo |
| pkg1/tst.go:6:2:6:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | Bar |
| pkg1/tst.go:10:2:10:4 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | Foo |
| pkg1/tst.go:11:2:11:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | Bar |
| pkg1/tst.go:15:3:15:5 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | Foo |
| pkg1/tst.go:16:3:16:5 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | Bar |
| pkg1/tst.go:20:3:20:5 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | Foo |
| pkg1/tst.go:21:2:21:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | Bar |
| pkg1/tst.go:25:2:25:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Foo | val |
| pkg1/tst.go:25:2:25:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | val |
| pkg1/tst.go:25:2:25:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | val |
| pkg1/tst.go:25:2:25:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | val |
| pkg1/tst.go:26:2:26:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Foo | flag |
| pkg1/tst.go:26:2:26:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | flag |
| pkg1/tst.go:30:2:30:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Bar | flag |
| pkg1/tst.go:30:2:30:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | flag |
| pkg1/tst.go:6:2:6:2 | f | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | f |
| pkg1/tst.go:7:2:7:4 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | Foo |
| pkg1/tst.go:8:2:8:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | Bar |
| pkg1/tst.go:12:2:12:4 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | Foo |
| pkg1/tst.go:13:2:13:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | Bar |
| pkg1/tst.go:17:3:17:5 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | Foo |
| pkg1/tst.go:18:3:18:5 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | Bar |
| pkg1/tst.go:22:3:22:5 | Foo | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | Foo |
| pkg1/tst.go:23:2:23:4 | Bar | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | Bar |
| pkg1/tst.go:27:2:27:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Foo | val |
| pkg1/tst.go:27:2:27:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T | val |
| pkg1/tst.go:27:2:27:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T3 | val |
| pkg1/tst.go:27:2:27:4 | val | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | val |
| pkg1/tst.go:28:2:28:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Foo | flag |
| pkg1/tst.go:28:2:28:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T4 | flag |
| pkg1/tst.go:32:2:32:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.Bar | flag |
| pkg1/tst.go:32:2:32:5 | flag | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.T2 | flag |
| pkg1/tst.go:62:7:62:15 | NameClash | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.NameClash | NameClash |
| pkg2/tst.go:4:2:4:2 | g | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2.G | g |
| pkg2/tst.go:4:2:4:2 | g | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2.T | g |
| pkg2/tst.go:8:2:8:2 | g | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2.G | g |
| pkg2/tst.go:8:2:8:2 | g | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2.T | g |
| pkg2/tst.go:17:2:17:8 | NCField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.EmbedsNameClash | NCField |
| pkg2/tst.go:17:2:17:8 | NCField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg1.NameClash | NCField |
| pkg2/tst.go:17:2:17:8 | NCField | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types/pkg2.NameClash | NCField |
| struct_tags.go:4:2:4:7 | field1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.S1 | field1 |
| struct_tags.go:5:2:5:7 | field2 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.S1 | field2 |
| struct_tags.go:9:2:9:7 | field1 | github.com/github/codeql-go/ql/test/library-tests/semmle/go/Types.S2 | field1 |
Expand Down
Loading