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: report use of too-new standard library symbols #46136

Open
jayconrod opened this issue May 12, 2021 · 55 comments
Open

cmd/vet: report use of too-new standard library symbols #46136

jayconrod opened this issue May 12, 2021 · 55 comments
Assignees
Labels
Analysis Issues related to static analysis (vet, x/tools/go/analysis) Proposal Proposal-Accepted
Milestone

Comments

@jayconrod
Copy link
Contributor

jayconrod commented May 12, 2021

Author(s): Jay Conrod based on discussion with Daniel Martí, Paul Jolly, Roger
Peppe, Bryan Mills, and others.

Last updated: 2021-05-12
Proposal: https://go.googlesource.com/proposal/+/refs/heads/master/design/46136-vet-std-references.md

Abstract

With this proposal, go vet (and go test) would report an error if a package
imports a standard library package or references an exported standard library
definition that was introduced in a higher version of Go than the version
declared in the containing module's go.mod file.

This makes the meaning of the go directive clearer and more consistent. As
part of this proposal, we'll clarify the reference documentation and make a
recommendation to module authors about how the go directive should be set.
Specifically, the go directive should indicate the minimum version of Go that
a module supports. Authors should set their go version to the minimum version
of Go they're willing to support. Clients may or may not see errors when using a
lower version of Go, for example, when importing a module package that imports a
new standard library package or uses a new language feature.

Background

The go directive was introduced in Go 1.12, shortly after modules were
introduced in Go 1.11.

At the time, there were several proposed language changes that seemed like they
might be backward incompatible (collectively, "Go 2"). To avoid an incompatible
split (like Python 2 and 3), we needed a way to declare the language version
used by a set of packages so that Go 1 and Go 2 packages could be mixed together
in the same build, compiled with different syntax and semantics.

We haven't yet made incompatible changes to the language, but we have made some
small compatible changes (binary literals added in 1.13). If a developer using
Go 1.12 or older attempts to build a package with a binary literal (or any other
unknown syntax), and the module containing the package declares Go 1.13 or
higher, the compiler reports an error explaining the problem. The developer also
sees an error in their own package if their go.mod file declares go 1.12 or
lower.

In addition to language changes, the go directive has been used to opt in to
new, potentially incompatible module behavior. In Go 1.14, the go version was
used to enable automatic vendoring. In 1.17, the go version will control lazy
module loading.

One major complication is that access to standard library packages and features
has not been consistently limited. For example, a module author might use
errors.Is (added in 1.13) or io/fs (added in 1.16) while believing their
module is compatible with a lower version of Go. The author shouldn't be
expected to know this history, but they can't easily determine the lowest
version of Go their module is compatible with.

This complication has made the meaning of the go directive very murky.

Proposal

We propose adding a new go vet analysis to report errors in packages that
reference standard library packages and definitions that aren't available
in the version of Go declared in the containing module's go.mod file. The
analysis will cover imports, references to exported top-level definitions
(functions, constants, etc.), and references to other exported symbols (fields,
methods).

The analysis should evaluate build constraints in source files (// +build
and //go:build comments) as if the go version in the containing module's
go.mod were the actual version of Go used to compile the package. The
analysis should not consider imports and references in files that would only
be built for higher versions of Go.

This analysis should have no false positives, so it may be enabled by default
in go test.

Note that both go vet and go test report findings for packages named on
the command line, but not their dependencies. go vet all may be used to check
packages in the main module and everything needed to build them.

The analysis would not report findings for standard library packages.

The analysis would not be enabled in GOPATH mode.

For the purpose of this proposal, modules lacking a go directive (including
projects without a go.mod file) are assumed to declare Go 1.16.

Rationale

When writing this proposal, we also considered restrictions in the go command
and in the compiler.

The go command parses imports and applies build constraints, so it can report
an error if a package in the standard library should not be imported. However,
this may break currently non-compliant builds in a way that module authors
can't easily fix: the error may be in one of their dependencies. We could
disable errors in packages outside the main module, but we still can't easily
re-evaluate build constraints for a lower release version of Go. The go
command doesn't type check packages, so it can't easily detect references
to new definitions in standard library packages.

The compiler does perform type checking, but it does not evaluate build
constraints. The go command provides the compiler with a list of files to
compile, so the compiler doesn't even know about files excluded by build
constraints.

For these reasons, a vet analysis seems like a better, consistent way to
find these problems.

Compatibility

The analysis in this proposal may introduce new errors in go vet and go test
for packages that reference parts of the standard library that aren't available
in the declared go version. Module authors can fix these errors by increasing
the go version, changing usage (for example, using a polyfill), or guarding
usage with build constraints.

Errors should only be reported in packages named on the command line. Developers
should not see errors in packages outside their control unless they test with
go test all or something similar. For those tests, authors may use -vet=off
or a narrower set of analyses.

We may want to add this analysis to go vet without immediately enabling it by
default in go test. While it should be safe to enable in go test (no false
positives), we'll need to verify this is actually the case, and we'll need
to understand how common these errors will be.

Implementation

This proposal is targeted for Go 1.18. Ideally, it should be implemented
at the same time or before generics, since there will be a lot of language
and library changes around that time.

The Go distribution includes files in the api/ directory that track when
packages and definitions were added to the standard library. These are used to
guard against unintended changes. They're also used in pkgsite documentation.
These files are the source of truth for this proposal. cmd/vet will access
these files from GOROOT.

The analysis can determine the go version for each package by walking up
the file tree and reading the go.mod file for the containing module. If the
package is in the module cache, the analysis will use the .mod file for the
module version. This file is generated by the go command if no go.mod
file was present in the original tree.

Each analysis receives a set of parsed and type checked files from cmd/vet.
If the proposed analysis detects that one or more source files (including
ignored files) contain build constraints with release tags (like go1.18),
the analysis will parse and type check the package again, applying a corrected
set of release tags. The analysis can then look for inappropriate imports
and references.

@jayconrod jayconrod added this to the Go1.18 milestone May 12, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) May 12, 2021
@myitcv
Copy link
Member

myitcv commented May 24, 2021

@jayconrod thanks very much for writing this up, and apologies for being slow at reading it through.

I'm a little confused regarding the following conclusion:

Specifically, the go directive should indicate the minimum version of Go that a module supports.

AFAIU, one of the roles of the go directive is to enable features like go:embed:

-- blah.txt --
test
-- go.mod --
module blah.com

go 1.15
-- hello.txt --
Test
-- main.go --
package main

import (
        _ "embed"
        "fmt"
)

//go:embed blah.txt
var s string

func main() {
        fmt.Printf("%v\n", s)
}

Gives (using tip):

> go version
[stdout]
go version devel go1.17-5e191f8f48 Tue May 18 02:39:04 2021 +0000 linux/amd64

> go run main.go
[stderr]
# command-line-arguments
./main.go:8:3: go:embed requires go1.16 or later (-lang was set to go1.15; check go.mod)

That is, my module needs to set go 1.16 to enable go:embed. Whether my module supports go1.15 is a function of whether my use of go1.16 features is build constrained or not, provides alternatives etc (something that is more concretely ensured by running go test using go1.15). So it's entirely possible for my module to set go 1.16 but still fully support go1.15. Therefore, as it stands today, the go directive is enabling later features rather than giving any sort of accurate indication on the minimum version of Go supported.

This also means that modules are not forced to bump their go directive version periodically for no real reason. If a module does not require later features, the go directive version presumably does not need to change. Because if someone has a problem with my ancient module that declares go 1.13, I'm really not going to provide support unless it's a problem they are having with the latest two Go versions (my policy in any case) - and I don't want to periodically release new versions of my module just to indicate that.

Another conclusion were the go directive to become an indicator of "minimum version of Go supported" is that modules cannot use later features, like go:embed, until the version that introduces such a feature becomes the minimum.

My understanding regarding this proposal is that it only sought to introduce a new vet rule, a vet rule that would help answer the question "do I need to bump the go directive version in order to use new feature X?" where X could be go:embed, a new standard library package, .... Because the answer would then always be "yes". However, I'm unclear therefore that the conclusion I quoted is, without changing the behaviour of the go directive, accurate.

Unless I missed the point, and instead this proposal is looking to change the advice regarding the go directive to "don't use features in newer Go versions if you want to support old Go versions". That would certainly be a valid position, but I don't think it's one that can be enforced, nor does it seem necessary given that people can use new features and support old versions via build constraints.

My feeling is that the Go version policy for a module is something sufficiently nuanced as to require specific CI testing of those versions, as well as human commentary, and not something we should try and communicate via the go directive.

But as I said, I'm late at replying to this and so might well be missing context 😄 In which case I would very much appreciate your thoughts on where my thinking has gone off the rails!

Thanks

Critical edit: s/sort/sought/ 😄 - thanks @rogpeppe. My grasp of the English language is frighteningly poor.

@rogpeppe
Copy link
Contributor

That is, my module needs to set go 1.16 to enable go:embed.

Thinking about this behaviour now in the context of this proposal, I think that this behaviour should be changed. In particular, I think that if the go.mod file declares go 1.14, but a build constraint on a file declares go 1.16, then go:embed (and potentially other new language/toolchain features such as generics) should be enabled.

For example, I think this example should run OK:

go run .
-- blah.txt --
test
-- go.mod --
module blah.com

go 1.15
-- hello.txt --
Test
-- main.go --
package main

import "fmt"

func main() {
        fmt.Println(s)
}
-- embed_pre1.16.go --
//+build !go1.16

package main

var s string

-- embed.go --
//+build go1.16

package main
import _ "embed"

//go:embed blah.txt
var s string

Currently it produces the error:

./embed.go:6:3: go:embed requires go1.16 or later (-lang was set to go1.15; check go.mod)

Then, as has always been the case, you can support older versions while using new features in Go versions that provide them and, as this proposal suggests, the go.mod version is the minimum supported.

I'd also suggest that non-stdlib changes such as embed or generics should be checked by the compiler rather than vet - the main reason that this current proposal is for vet is to avoid breaking existing code, and that's not a problem with new changes, or for embed that already has a check that's stricter than necessary.

@jayconrod
Copy link
Contributor Author

@myitcv @rogpeppe You both bring up some excellent questions. I'll take some more time to think and read through the go:embed history before suggesting anything though.

One thought that occurs to me: in Android, there's a separate concept of minimum and target SDK versions. The minimum version is the lowest version of Android an app works with. The target version is the version an app was compiled against. An app may use APIs from the target version that aren't in the minimum version, but it must do so conditionally.

Because of release tags in build constraints, we have a similar split here, but we haven't acknowledged or defined it. The go directive in go.mod effectively sets the target version, but the compiler treats -lang as a minimum version. The actual minimum version is only implied by build constraints.

@myitcv
Copy link
Member

myitcv commented May 24, 2021

... the go.mod version is the minimum supported.

In further discussions offline with @rogpeppe and @mvdan we concluded the phrase "the minimum version of Go supported" is ambiguous at best, because the word "support" has connotations as far as users of a module are concerned. So I think we need to be careful with the wording, because "minimum" is not meant to signal anything about good support to end users.

I think we therefore also concluded that it should be documented as best practice that module authors declare a policy for Go version support separately from the go directive version. In that context, @mvdan also raised the question of whether a new module version is required when support for a Go version is dropped: @mvdan suggested a minor version bump would be appropriate in such cases (all else equal).

@bcmills
Copy link
Contributor

bcmills commented May 27, 2021

Lately I've tended to think of the go version in the go.mod file as “the minimum Go version which supports full-fidelity builds”. Lower versions may work, but may be missing something — such as functionality (or test coverage) guarded by a go:build constraint, or reproducibility due to some change in module semantics or cmd/go behavior.

@jayconrod
Copy link
Contributor Author

I wonder if we want some machine-readable way to distinguish the minimum and target Go versions though? It would be useful for this proposal. It could be displayed by pkgsite and any other generated documentation. It would also reduce the need for the go mod tidy -compat flag in Go 1.17.

@bcmills
Copy link
Contributor

bcmills commented Aug 12, 2021

As a real-world example, in https://stackoverflow.com/q/68752103 a user was confused about build errors stemming from references to embed and io/fs in their project. I don't know for certain, but I suspect that they would have gotten a clearer failure mode if original author of those package imports had gotten a vet warning indicating that the embed and io/fs packages needed a different Go version.

@dmitshur
Copy link
Contributor

dmitshur commented Nov 10, 2021

This proposal was targeting Go 1.18, but it is still in Incoming column, and there's no assignee, so it doesn't seem that it can make it to 1.18 given the release timing. I'll move this to Proposal milestone (according to https://github.com/golang/proposal#accepted, proposals are moved to a release milestone after being accepted). Please feel free to undo if needed.

@dmitshur dmitshur modified the milestones: Go1.18, Proposal Nov 10, 2021
@soypat
Copy link

soypat commented Nov 29, 2021

I'll voice my support for this proposal and share an issue that seems to be covered by the proposal:
I was working on a module with go 1.16 version and was able to add reflect.VisibleFields (added go 1.17) function to my code without being warned the module would be broken. I pushed to code to production and only realized my module was broken when a colleague opened the project with his local go 1.16 installation.

@findleyr
Copy link
Contributor

findleyr commented Jan 19, 2022

Thanks @bcmills for pointing me here from #50689.

Gopls will need the analyzer described here in any case, as we want to better support using gopls built with a recent Go to develop for older versions of Go. In #50689, I mention that we'd probably need explicit configuration for gopls, precisely because we want to be consistent with the go command by default.

If this proposal were accepted, then gopls could simply read the go.mod go version and do the right thing for our users, without requiring external configuration.

Of course, if we implement this for gopls (initially in x/tools/internal/lsp/analysis), then it would be trivial to expose for vet by moving the analyzer to x/tools/go/analysis/passes.

@flwyd
Copy link

flwyd commented May 16, 2023

In the absence of this proposal, is there a way to check if "this module can be built by Go 1.X" short of setting up a CI environment that uses a Go 1.X toolchain?

@thepudds
Copy link
Contributor

I think this proposal is mostly overtaken by the forwards compatibility proposal #57001?

That proposal is accepted and mostly implemented.

@zephyrtronium
Copy link
Contributor

If #57001 was supposed to cover what this proposal addresses, it does not. On multiple occasions, I've pushed code that compiled, tested, and ran correctly for me but used stdlib APIs which were newer than the go.mod version and so failed to build for a teammate who was on that version exactly. I've talked to several other people who've had similar experiences.

I imagined a vet check against api/*.txt as an effective solution and found this proposal. The advantage we have now is that #57001 formalized the machinery for module- and constraint-determined language versions as mentioned in the proposal.

@thepudds
Copy link
Contributor

Hi @zephyrtronium, during the proposal discussion for #57001, my personal expectation was that it would handle the standard library as well, but it turns out my expectation was incorrect. 😅

As far as I understand it, this issue here is still considered a valid candidate solution.

@rsc rsc changed the title proposal: cmd/vet: check references to standard library packages inconsistent with go.mod go version proposal: cmd/vet: report references to too-new standard library symbols Feb 28, 2024
@gopherbot
Copy link

Change https://go.dev/cl/567837 mentions this issue: internal/stdlib: manifest of all std symbols since go1.0

@adonovan
Copy link
Member

Here's a subtle problem I discovered while prototyping.

Go 1.22 introduces types.Alias, breaking every type switch over types.Types. To ease the transition in x/tools, we introduced aliases.Alias a cycle before, which is itself defined as an alias to either the real types.Alias in go1.22 or a nonce type in older Go releases.

When the proposed new vet check analyzes code such as this:

go 1.18
 case *aliases.Alias:
    return t.Underlying()

while using version 1.22 of go list, it will select the go1.22-tagged files in package aliases, so the type checker will judge t to be a *types.Alias. This will cause the new vet check to spuriously report that the reference to Alias.Underlying is unacceptable in a project using a go.mod file that specifies go 1.18.

How we can we avoid this?

@gopherbot
Copy link

Change https://go.dev/cl/568237 mentions this issue: gopls/internal/analysis/apicheck: report refs to too-new std symbols

@timothy-king
Copy link
Contributor

I misunderstood the suggestion. Apologies for the noise.

@gopherbot
Copy link

Change https://go.dev/cl/569796 mentions this issue: gopls/internal/golang/completion: honor std symbol versions (imported)

gopherbot pushed a commit to golang/tools that referenced this issue Mar 7, 2024
This CL adds to gopls a new analyzer that reports references to
standard library symbols that are too new for the version of
Go in force in the referring file.

The test is duplicated within the gopls marker framework to
ensure we have coverage of per-file Go versions within gopls.

Updates golang/go#46136

Change-Id: Ic7e04e0ae75a586dc0ac5dfc86c6b4aa67d5d194
Reviewed-on: https://go-review.googlesource.com/c/tools/+/568237
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link

Change https://go.dev/cl/569835 mentions this issue: go/analysis/passes/stdversion: move out of gopls

@adonovan
Copy link
Member

adonovan commented Mar 7, 2024

I've implemented the proposed analyzer, now enabled in gopls; please install gopls at master to try it out. I've also updated gopls' completion feature not to offer completions of symbols that are too new for the current file, based on the same manifest of std symbols; the CLs for this are still pending but I hope to land them today.

Once this proposal is approved I will add the analyzer to cmd/vet as well.

I ran the analyzer across a sample (min imports=10) of the modproxy corpus and found 640 modules (about 1%) for which diagnostics are reported, with a mean of 9 diagnostics per module. Here's a random sample of 100 diagnostics:

https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L700: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/dial.go#L80: tls.HandshakeContext requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/hashicorp/memberlist@v0.5.0/label_test.go#L265: io.ReadAll requires go1.16 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L704: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L218: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L932: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/github.com/gravitational/trace@v1.3.1/udphook_test.go#L38: io.Discard requires go1.16 or later (module is go1.15)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/charset/charset.go#L95: slices.SortFunc requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L692: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L702: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L689: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/emersion/go-smtp@v0.20.2/server.go#L198: net.ErrClosed requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/jinzhu/copier@v0.4.0/copier_different_type_test.go#L65: sql.NullByte requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/emersion/go-smtp@v0.20.2/client.go#L73: tls.Dialer requires go1.15 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/filecoin-project/go-state-types@v0.13.0/abi/cbor_bytes_transparent.go#L20: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/dial.go#L80: tls.HandshakeContext requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/adapters_test.go#L72: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L937: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/goyacc/main.go#L397: slices.Sort requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/emersion/go-smtp@v0.20.2/client.go#L73: tls.Dialer requires go1.15 or later (module is go1.13)
https://go-mod-viewer.appspot.com/yunion.io/x/jsonutils@v1.0.0/marshal.go#L384: reflect.IsZero requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/go/common/util/cmdutil/term_test.go#L455: errors.Join requires go1.20 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L198: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/hashicorp/vault/sdk@v0.11.0/helper/backoff/backoff.go#L103: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/golang.org/x/image@v0.15.0/tiff/reader_test.go#L505: binary.AppendByteOrder requires go1.19 or later (module is go1.18)
https://go-mod-viewer.appspot.com/yunion.io/x/jsonutils@v1.0.0/marshal.go#L242: reflect.IsZero requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/emperror.dev/errors@v0.8.1/wrap_go1_13.go#L26: errors.As requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar.go#L410: io.Discard requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/andybalholm/brotli@v1.1.0/brotli_test.go#L499: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L683: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/yunion.io/x/jsonutils@v1.0.0/marshal.go#L384: reflect.IsZero requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/weaveworks/common@v0.0.0-20230728070032-dd9e68f319d5/server/server_test.go#L559: os.ReadFile requires go1.16 or later (module is go1.14)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/go/common/resource/config/repr_test.go#L144: strings.CutSuffix requires go1.20 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar.go#L456: io.Discard requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L197: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/emperror.dev/errors@v0.8.1/wrap_go1_13.go#L26: errors.As requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L207: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/emperror.dev/errors@v0.8.1/wrap_go1_13.go#L14: errors.Is requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/sourcegraph/jsonrpc2@v0.2.0/conn_opt_test.go#L23: log.Lmsgprefix requires go1.14 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/alecthomas/jsonschema@v0.0.0-20220216202328-9eeeec9d044b/comment_extractor.go#L30: fs.FileInfo requires go1.16 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L707: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/doug-martin/goqu/v9@v9.19.0/internal/util/reflect.go#L80: reflect.IsZero requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/emperror.dev/errors@v0.8.1/wrap_go1_13.go#L14: errors.Is requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L682: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/nodejs/npm/workspaces.go#L55: slices.Contains requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/golang/mock@v1.6.0/mockgen/version.1.12.go#L27: debug.ReadBuildInfo requires go1.12 or later (module is go1.11)
https://go-mod-viewer.appspot.com/yunion.io/x/jsonutils@v1.0.0/marshal.go#L242: reflect.IsZero requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar_test.go#L574: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/go/common/promise/promise.go#L37: atomic.Int32 requires go1.19 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar_test.go#L381: io.Discard requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/model/model.go#L1713: cmp.Compare requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/sourcegraph/jsonrpc2@v0.2.0/conn_test.go#L113: io.Discard requires go1.16 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L687: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L697: netip.Addr requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L136: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/gagliardetto/binary@v0.7.9/u128.go#L128: big.FillBytes requires go1.15 or later (module is go1.14)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L925: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L703: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L698: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/apex/log@v1.9.0/entry.go#L53: time.Milliseconds requires go1.13 or later (module is go1.12)
https://go-mod-viewer.appspot.com/github.com/weaveworks/common@v0.0.0-20230728070032-dd9e68f319d5/server/server_test.go#L581: io.ReadAll requires go1.16 or later (module is go1.14)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L690: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/emersion/go-smtp@v0.20.2/server.go#L198: net.ErrClosed requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L705: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/andybalholm/brotli@v1.1.0/brotli_test.go#L84: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar_test.go#L628: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L685: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/weaveworks/common@v0.0.0-20230728070032-dd9e68f319d5/server/server_test.go#L306: io.Discard requires go1.16 or later (module is go1.14)
https://go-mod-viewer.appspot.com/github.com/hashicorp/nomad/api@v0.0.0-20240306165712-3193ac204f65/error_unexpected_response.go#L109: slices.Clone requires go1.21 or later (module is go1.20)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L920: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/markers/markers_test.go#L295: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/urfave/cli@v1.22.14/fish_test.go#L72: os.ReadFile requires go1.16 or later (module is go1.11)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/charset/charset.go#L95: slices.SortFunc requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/marshal.go#L1390: time.UnixMilli requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L684: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L699: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar.go#L456: io.Discard requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/go/common/util/cmdutil/exit_test.go#L131: errors.Join requires go1.20 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar.go#L410: io.Discard requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/urfave/cli@v1.22.14/fish_test.go#L75: strings.ReplaceAll requires go1.12 or later (module is go1.11)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi/sdk/v3@v3.108.1/go/common/util/cmdutil/exit_test.go#L100: errors.Join requires go1.20 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/google/go-cmp@v0.6.0/cmp/cmpopts/util_test.go#L688: netip.AddrFrom4 requires go1.18 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar_test.go#L602: os.CreateTemp requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L258: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/k8s.io/gengo@v0.0.0-20240225000952-471f57d40f57/examples/defaulter-gen/generators/defaulter.go#L517: regexp.SubexpIndex requires go1.15 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errutil/as_test.go#L109: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/Azure/go-autorest/autorest/azure/auth@v0.5.12/auth_test.go#L272: testing.Setenv requires go1.17 or later (module is go1.15)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L234: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/marshal.go#L1400: time.UnixMilli requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/errbase/format_error_internal_test.go#L196: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/marshal.go#L1400: time.UnixMilli requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/weaveworks/common@v0.0.0-20230728070032-dd9e68f319d5/server/server_test.go#L329: io.ReadAll requires go1.16 or later (module is go1.14)
https://go-mod-viewer.appspot.com/github.com/schollz/progressbar/v3@v3.14.2/progressbar_test.go#L548: os.CreateTemp requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/gocql/gocql@v1.6.0/marshal.go#L1390: time.UnixMilli requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/golang/mock@v1.6.0/gomock/controller_114_test.go#L27: testing.Cleanup requires go1.14 or later (module is go1.11)
https://go-mod-viewer.appspot.com/github.com/jinzhu/copier@v0.4.0/copier_different_type_test.go#L67: sql.NullInt16 requires go1.17 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/cockroachdb/errors@v1.11.1/markers/markers_test.go#L338: errors.Join requires go1.20 or later (module is go1.19)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/charset/charset.go#L96: cmp.Compare requires go1.21 or later (module is go1.18)
https://go-mod-viewer.appspot.com/github.com/andybalholm/brotli@v1.1.0/brotli_test.go#L490: io.ReadAll requires go1.16 or later (module is go1.13)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/charset/charset.go#L96: cmp.Compare requires go1.21 or later (module is go1.18)

@rsc
Copy link
Contributor

rsc commented Mar 8, 2024

The checker should probably not bother with reporting uses of things from before Go 1.21. Those were mistakes years ago but stopped being mistakes when Go 1.22 was issued, since now Go 1.21 and Go 1.22 are the only supported Go versions. Let's avoid all the churn that will be caused by ancient reports that don't matter anymore by not emitting them.

@rsc
Copy link
Contributor

rsc commented Mar 8, 2024

I also don't see any reports for "file is go1.18" as opposed to "module is go1.18".
Was there really not a single mistake in a file that said "//go:build go1.N"?

@rsc
Copy link
Contributor

rsc commented Mar 8, 2024

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to add a new ‘stdversion’ analyzer that reports when the go line is too old for something used from the standard library. A sample error would be:

file.go:123: slices.Clone requires go1.21 or later (module is go1.20)

@adonovan
Copy link
Member

adonovan commented Mar 8, 2024

Below are a random 10 matching "file is". (From the fact that a single file shows up so often you can tell that there weren't many matches overall of this kind: a total of only 56 diagnostics in 9 files.)

https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L566: context.Background requires go1.7 or later (file is go1.2)
https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L501: context.Background requires go1.7 or later (file is go1.2)
https://go-mod-viewer.appspot.com/github.com/admpub/gopiper@v1.1.1/filter.go#L524: time.TimeOnly requires go1.20 or later (file is go1.18)
https://go-mod-viewer.appspot.com/github.com/aliyun/aliyun-oss-go-sdk@v3.0.2+incompatible/oss/transport_1_7.go#L34: http.MaxConnsPerHost requires go1.11 or later (file is go1.7)
https://go-mod-viewer.appspot.com/k8s.io/apimachinery@v0.29.2/pkg/util/net/http_test.go#L937: testing.Setenv requires go1.17 or later (file is go1.8)
https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L438: context.Context requires go1.7 or later (file is go1.2)
https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L545: context.Background requires go1.7 or later (file is go1.2)
https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L640: errors.Is requires go1.13 or later (file is go1.2)
https://go-mod-viewer.appspot.com/github.com/zitadel/oidc/v3@v3.14.0/pkg/oidc/authorization_test.go#L20: slog.Any requires go1.21 or later (file is go1.20)
https://go-mod-viewer.appspot.com/github.com/webx-top/com@v1.2.12/cmd.go#L626: errors.Is requires go1.13 or later (file is go1.2)

@gopherbot
Copy link

Change https://go.dev/cl/570138 mentions this issue: gopls/internal/analysis/stdversion: set RunDespiteErrors

@gopherbot
Copy link

Change https://go.dev/cl/570140 mentions this issue: gopls/internal/analysis/stdversion: suppress before go1.21

gopherbot pushed a commit to golang/tools that referenced this issue Mar 8, 2024
This change is the counterpart to CL 569435 for completions
in imported packages.

Updates golang/go#46136

Change-Id: I57011897c395d37a89a8e3a99e8c3511de017ad3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/569796
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 8, 2024
This change enables RunDespiteErrors, after auditing the code.
This should give more timely feedback while editing.

Also, it moves the vet/gopls common code (DisallowedSymbols)
to typesinternal.TooNewStdSymbols, out of the gopls module,
in anticipation of adding this analyzer to vet.

Updates golang/go#46136

Change-Id: I8d742bf543c9146376d43ae94f7adae3b453e471
Reviewed-on: https://go-review.googlesource.com/c/tools/+/570138
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 8, 2024
This change causes the stdversion checker not to report
any diagnostics in modules with go versons before go1.21,
because the semantics of the go declaration were not
clearly defined to correspond to toolchain requirements
at that point. Also, those Go versions are now unsupported,
so reporting diagnostics just creates unnecessary churn.

Updates golang/go#46136

Change-Id: I323f704c4d4f1f0fe5fc8b5680824bc07d3c4112
Reviewed-on: https://go-review.googlesource.com/c/tools/+/570140
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@adonovan
Copy link
Member

adonovan commented Mar 11, 2024

#46136 (comment):

[@rsc] The checker should probably not bother with reporting uses of things from before Go 1.21. Those were mistakes years ago but stopped being mistakes when Go 1.22 was issued, since now Go 1.21 and Go 1.22 are the only supported Go versions. Let's avoid all the churn that will be caused by ancient reports that don't matter anymore by not emitting them.

This is what I implemented. However, a counterpoint: this behavior nullifies any value this checker has for projects that strive to maintain compatibility with multiple older versions of Go. For example, golang.org/x/tools and starlark.go.net both currently support the 4 latest versions, so their go.mod files require go1.19, which disables the new checker. These are the projects that would benefit most.

Perhaps the analyzer should be modified so that, when run within gopls, it does not disable the check on pre-go1.21 modules. The behavior within vet would be unchanged.

@findleyr
Copy link
Contributor

@adonovan presumably for x/tools we're going to increase the go.mod go directive following #65917, so I don't expect this to be a problem for long.

It does not seem particularly worthwhile to me to take extra action to accommodate Go 1.19 and Go 1.20. Eventually, most projects will use a Go directive version that is 1.21 or later, at which point the current check will be sufficient.

@rsc
Copy link
Contributor

rsc commented Mar 15, 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 to add a new ‘stdversion’ analyzer that reports when the go line is too old for something used from the standard library. A sample error would be:

file.go:123: slices.Clone requires go1.21 or later (module is go1.20)

@rsc rsc changed the title proposal: cmd/vet: report use of too-new standard library symbols cmd/vet: report use of too-new standard library symbols Mar 15, 2024
@rsc rsc modified the milestones: Proposal, Backlog Mar 15, 2024
@gopherbot
Copy link

Change https://go.dev/cl/572035 mentions this issue: go/analysis/passes/stdversion: publish

gopherbot pushed a commit to golang/tools that referenced this issue Mar 18, 2024
This CL moves the stdversion analyzer out of gopls,
into the exported analyzer tree, whence it will be
vendored into cmd/vet in a follow-up.

Updates golang/go#46136

Change-Id: I039ef78ecdcfd6bc64d5b089017a9b8635cf9aa5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/572035
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@gopherbot
Copy link

Change https://go.dev/cl/572777 mentions this issue: internal/relui: update major release x/tools generate sequence

@gopherbot
Copy link

Change https://go.dev/cl/571276 mentions this issue: internal/imports: remove obsolete go:generate directive

gopherbot pushed a commit to golang/build that referenced this issue Mar 19, 2024
CL 567837 extracted the stdlib manifest from x/tools/internal/imports
to x/tools/internal/stdlib. That is the new package that will need to
be regenerated every 6 months after each major Go release.

Update the relui task that automated that recurring work accordingly.

For golang/go#46136.
For golang/go#38706.

Change-Id: I9705ac881a18a5e7b03188228eadfaccf816aaca
Reviewed-on: https://go-review.googlesource.com/c/build/+/572777
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopherbot pushed a commit to golang/tools that referenced this issue Mar 19, 2024
CL 567837 deleted mkstdlib.go (a generator) and zstdlib.go (its output).

For golang/go#46136.
For golang/go#38706.

Change-Id: If5623e3ab014c2fee63b1d058c726ea5acb06aa2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/571276
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
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
Projects
Status: Accepted
Development

No branches or pull requests