-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Go: fix DefinedType.getBaseType
#19654
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR changes DefinedType.getBaseType
to return the alias’s right‐hand side (the declared type) rather than its underlying type, updates related methods to maintain correct semantics, and refreshes library tests accordingly.
- Reimplement
getBaseType
to read from the declaration AST so aliases return their RHS. - Update
getMethod
andgetUnderlyingType
to use the new separation between base and underlying types. - Revise existing method‐resolution tests and add new tests for
getBaseType
.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
go/ql/test/library-tests/semmle/go/Types/MethodTypes.ql | Use Type.hasMethod and add a declaration existence check |
go/ql/test/library-tests/semmle/go/Types/MethodTypes.expected | Adjust expected methods list after alias semantics change |
go/ql/test/library-tests/semmle/go/Types/DefinedType_getBaseType.ql | New test querying DefinedType.getBaseType() |
go/ql/test/library-tests/semmle/go/Types/DefinedType_getBaseType.expected | Expected output for the new getBaseType test |
go/ql/lib/semmle/go/Types.qll | Change getBaseType , getUnderlyingType , and related logic |
Comments suppressed due to low confidence (1)
go/ql/test/library-tests/semmle/go/Types/DefinedType_getBaseType.ql:4
- Consider adding a test case to verify that
getBaseType
returns no result for types declared in external packages, ensuring the new behavior is validated against external definitions.
where tp = dt.getBaseType()
* Gets the type which this type is defined to be, if available. | ||
* | ||
* Note that this is only defined for types declared in the project being | ||
* analyzed. It will not be defined for type declared in external packages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the plural form here: change "type declared in external packages" to "types declared in external packages" for grammatical consistency.
* analyzed. It will not be defined for type declared in external packages. | |
* analyzed. It will not be defined for types declared in external packages. |
Copilot uses AI. Check for mistakes.
9535baf
to
b2f310c
Compare
Consider this situation:
Previously,
DefinedType.getBaseType
just gave the underlying type, so calling it onT1
would givestruct {}
. This PR fixes it so thatDefinedType.getBaseType
gives the right hand side of the type declaration, and hence calling it onT1
givesT2
.Note that this requires us to have extracted the declaration of the type, so it is only available for types declared in the project being analyzed. It is not defined for types defined in external packages. Since this information is not stored in the information that we already extract for external packages, I cannot see any way to improve this situation except for expanding what we look at from external packages.
I have also fixed a test which was the only use of
DefinedType.getBaseType
outside of the definition ofDefinedType
, and added a proper test forDefinedType.getBaseType
.