Support GODEBUG=gotypesalias=1 #248
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b7d0b65
to
e286b83
Compare
Currently, the go/types.Type for a declared type alias is the same type as the that which it is aliased too. In Go 1.23, when GODEBUG=gotypesalias=1 is enabled by default, it is a go/types.Alias instead, which is a unique type node for aliases that points to the type it is aliased to via go/types.Unalias(). (see golang/go#63223 for more details). errcheck's tests do not currently fail with GODEBUG=gotypesalias=1, but that is because the tests themselves do not exercise the problematic situations, such as the following: ```go type t struct{} func (x t) a() error { /* ... */ } type embedt struct { t } type embedtalias = embedt func foo() { embedtalias.a() } ``` With GODEBUG=gotypesalias=1, errcheck will panic when inspecting `embedtalias.a()` to see if it should be excluded from checking: ``` panic: cannot get Field of a type that is not a struct, got _/home/user/go/src/github.com/kisielk/errcheck/errcheck/testdata/src/a.embedtalias (*types.Alias) goroutine 2962 [running]: github.com/kisielk/errcheck/errcheck.getTypeAtFieldIndex({0x7779c0?, 0xc002eceab0?}, 0x4c?) /home/user/go/src/github.com/kisielk/errcheck/errcheck/embedded_walker.go:96 +0xf9 github.com/kisielk/errcheck/errcheck.walkThroughEmbeddedInterfaces(0x6b83a0?) /home/user/go/src/github.com/kisielk/errcheck/errcheck/embedded_walker.go:53 +0x8e github.com/kisielk/errcheck/errcheck.(*visitor).namesForExcludeCheck(0xc0025eeff0, 0xc002596140?) /home/user/go/src/github.com/kisielk/errcheck/errcheck/errcheck.go:364 +0x165 github.com/kisielk/errcheck/errcheck.(*visitor).excludeCall(0xc0025eeff0, 0x4d?) /home/user/go/src/github.com/kisielk/errcheck/errcheck/errcheck.go:403 +0x72 github.com/kisielk/errcheck/errcheck.(*visitor).ignoreCall(0xc0025eeff0, 0xc002596140) /home/user/go/src/github.com/kisielk/errcheck/errcheck/errcheck.go:415 +0x2c github.com/kisielk/errcheck/errcheck.(*visitor).Visit(0xc0025eeff0, {0x776d48?, 0xc0066e08a0}) /home/user/go/src/github.com/kisielk/errcheck/errcheck/errcheck.go:564 +0x137 ``` This change uses `types.Unalias` to follow through this extra level of indirection, which fixes the panic and ensures the same behavior with both settings. This change also adds these cases to the test suite, and runs the tests using both settings of gotypesalias. Without the calls to `types.Unalias`, the tests fail.
e286b83
to
ec57966
Compare
sywhang
reviewed
Jun 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the go/types.Type for a declared type alias is the same type as the that which it is aliased too.
In Go 1.23, when GODEBUG=gotypesalias=1 is enabled by default, it is a go/types.Alias instead, which is a unique type node for aliases that points to the type it is aliased to via go/types.Unalias().
See golang/go#63223 for more details.
errcheck's tests do not currently fail with GODEBUG=gotypesalias=1, but that is because the tests themselves do not exercise the problematic situations, such as the following:
With GODEBUG=gotypesalias=1, errcheck will panic when inspecting
embedtalias.a()to see if it should be excluded from checking:This change uses
types.Unaliasto follow through this extra level of indirection, which fixes the panic and ensures the same behavior with both settings.This change also adds these cases to the test suite, and runs the tests using both settings of gotypesalias. Without the calls to
types.Unalias, the tests fail.