Skip to content
New issue

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

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

Already on GitHub? # to your account

cmd/vet: GoVersion=="" on standard library packages #64293

Closed
timothy-king opened this issue Nov 20, 2023 · 2 comments
Closed

cmd/vet: GoVersion=="" on standard library packages #64293

timothy-king opened this issue Nov 20, 2023 · 2 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@timothy-king
Copy link
Contributor

Within standard library packages the GoVersion sent to unitchecker is "" for standard library packages. This can be recreated from gotip vet -x ./... from $GOROOT/src (which will be in a module).

This GoVersion is passed along to go/types in types.Config. When this is empty (which is an invalid version), (*types.Info).FileVersions is not populated. The loopclosure vet check uses FileVersions to decide what is the semantics of a given for loop is. So there will now be a divergence in behavior for cmd/vet when running on the standard libraries.

The relevant code to set the field is in buildVetConfig:

	if a.Package.Module != nil {
		v := a.Package.Module.GoVersion
		if v == "" {
			v = gover.DefaultGoModVersion
		}
		vcfg.GoVersion = "go" + v
	}

A hunch for why a.Package.Module is nil is that it comes from PackageModuleInfo which documents that nil is expected:

// PackageModuleInfo returns information about the module that provides
// a given package. If modules are not enabled or if the package is in the
// standard library or if the package was not successfully loaded with
// LoadPackages or ImportFromFiles, nil is returned.
func PackageModuleInfo(ctx context.Context, pkgpath string) *modinfo.ModulePublic {

cc @bcmills

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 21, 2023
@mknyszek mknyszek added this to the Backlog milestone Nov 21, 2023
@bcmills bcmills added the GoCommand cmd/go label Nov 21, 2023
@bcmills
Copy link
Contributor

bcmills commented Nov 21, 2023

@timothy-king, if you can write a regression test, I think I see the fix. Probably something like:

	if a.Package.Module != nil {
		v := a.Package.Module.GoVersion
		if v == "" {
			v = gover.DefaultGoModVersion
		}
		vcfg.GoVersion = "go" + v
	} else if a.Package.Standard {
		vcfg.GoVersion = gover.LocalToolchain()
	}

@samthanawalla
Copy link
Contributor

Addressed in #65612
Should be fixed now :)

GoVersion will be set to the local go version in this case.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants