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: consider reporting invalid/ignored Go versions in build tags #64127

Closed
griesemer opened this issue Nov 14, 2023 · 11 comments
Closed

cmd/vet: consider reporting invalid/ignored Go versions in build tags #64127

griesemer opened this issue Nov 14, 2023 · 11 comments
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Proposal Proposal-Accepted release-blocker
Milestone

Comments

@griesemer
Copy link
Contributor

Files may specify their own Go language version via a build tag. For instance:

//go:build go1.22

indicates that this file requires Go language version 1.22.

If the language version in the build tag contains dot-release information, say it's "go1.22.1", it is invalid.
The compiler simply ignores the version (it doesn't use, say "go1.22").

It seems (to me) that this shouldn't be silently ignored as it might be very confusing for users. go vet might be the right tool to report an error. Alternatively, the compiler could report an error, but generally, the compiler doesn't complain about errors in (pragma and build) comments.

@gopherbot gopherbot added this to the Proposal milestone Nov 14, 2023
@timothy-king
Copy link
Contributor

go/analysis/passes/buildtag would be a good home for this check.

@Goclipse27

This comment was marked as off-topic.

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Nov 15, 2023
@Goclipse27
Copy link
Contributor

@griesemer @ianlancetaylor - Do you consider handling tags including expression like go:build go1.21 && go1.22.3 or a single tag ?

@Goclipse27

This comment was marked as resolved.

@griesemer
Copy link
Contributor Author

It should report any Go version string that is silently dropped and ignored.
Also, before anybody starts working on this: note this is a proposal and needs to be discussed by the proposal review committee first. Thanks.

@adonovan adonovan added the Analysis Issues related to static analysis (vet, x/tools/go/analysis) label May 29, 2024
@rsc
Copy link
Contributor

rsc commented May 30, 2024

We already have a //go:build line checker. It should definitely report these.

@rsc rsc moved this from Incoming to Active in Proposals May 30, 2024
@rsc
Copy link
Contributor

rsc commented May 30, 2024

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Jun 5, 2024

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is for the ‘buildtag’ checker to report build tags of the form go1.X.Y since only go1.X is recognized in build tags.

@rsc rsc moved this from Active to Accepted in Proposals Jun 5, 2024
@rsc rsc changed the title proposal: cmd/vet: consider reporting invalid/ignored Go versions in build tags cmd/vet: consider reporting invalid/ignored Go versions in build tags Jun 5, 2024
@rsc rsc modified the milestones: Proposal, Backlog Jun 5, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/597576 mentions this issue: go/analysis/passes/buildtag: report invalid go versions in build tags.

@dmitshur
Copy link
Contributor

A new check added to vet seems like something that needs to be mentioned in Go 1.24 release notes, and it's not mentioned there yet. Reopening as a release blocker for tracking that.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/632355 mentions this issue: doc/next: document buildtag changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Proposal Proposal-Accepted release-blocker
Projects
Status: Accepted
Development

No branches or pull requests

8 participants