Skip to content
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

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

Open
timothy-king opened this issue Nov 20, 2023 · 1 comment
Open

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

timothy-king opened this issue Nov 20, 2023 · 1 comment
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()
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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

3 participants