Skip to content

Commit f40f799

Browse files
committed
Make file-by-file version decisions, differentiate language and stdlib versions
1 parent 7e003f3 commit f40f799

File tree

7 files changed

+222
-32
lines changed

7 files changed

+222
-32
lines changed

analysis/code/code.go

Lines changed: 131 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@ import (
88
"go/constant"
99
"go/token"
1010
"go/types"
11+
"strconv"
1112
"strings"
1213

1314
"honnef.co/go/tools/analysis/facts/generated"
1415
"honnef.co/go/tools/analysis/facts/purity"
1516
"honnef.co/go/tools/analysis/facts/tokenfile"
17+
"honnef.co/go/tools/analysis/lint"
1618
"honnef.co/go/tools/go/ast/astutil"
1719
"honnef.co/go/tools/go/types/typeutil"
1820
"honnef.co/go/tools/pattern"
@@ -315,13 +317,136 @@ func MayHaveSideEffects(pass *analysis.Pass, expr ast.Expr, purity purity.Result
315317
}
316318
}
317319

318-
func IsGoVersion(pass *analysis.Pass, minor int) bool {
319-
f, ok := pass.Analyzer.Flags.Lookup("go").Value.(flag.Getter)
320-
if !ok {
321-
panic("requested Go version, but analyzer has no version flag")
320+
func LanguageVersion(pass *analysis.Pass, node Positioner) int {
321+
// As of Go 1.21, two places can specify the minimum Go version:
322+
// - 'go' directives in go.mod and go.work files
323+
// - individual files by using '//go:build'
324+
//
325+
// Individual files can upgrade to a higher version than the module version. Individual files
326+
// can also downgrade to a lower version, but only if the module version is at least Go 1.21.
327+
//
328+
// The restriction on downgrading doesn't matter to us. All language changes before Go 1.22 will
329+
// not type-check on versions that are too old, and thus never reach our analyzes. In practice,
330+
// such ineffective downgrading will always be useless, as the compiler will not restrict the
331+
// language features used, and doesn't ever rely on minimum versions to restrict the use of the
332+
// standard library. However, for us, both choices (respecting or ignoring ineffective
333+
// downgrading) have equal complexity, but only respecting it has a non-zero chance of reducing
334+
// noisy positives.
335+
//
336+
// The minimum Go versions are exposed via go/ast.File.GoVersion and go/types.Package.GoVersion.
337+
// ast.File's version is populated by the parser, whereas types.Package's version is populated
338+
// from the Go version specified in the types.Config, which is set by our package loader, based
339+
// on the module information provided by go/packages, via 'go list -json'.
340+
//
341+
// As of Go 1.21, standard library packages do not present themselves as modules, and thus do
342+
// not have a version set on their types.Package. In this case, we fall back to the version
343+
// provided by our '-go' flag. In most cases, '-go' defaults to 'module', which falls back to
344+
// the Go version that Staticcheck was built with when no module information exists. In the
345+
// future, the standard library will hopefully be a proper module (see
346+
// https://github.com/golang/go/issues/61174#issuecomment-1622471317). In that case, the version
347+
// of standard library packages will match that of the used Go version. At that point,
348+
// Staticcheck will refuse to work with Go versions that are too new, to avoid misinterpreting
349+
// code due to language changes.
350+
//
351+
// We also lack module information when building in GOPATH mode. In this case, the implied
352+
// language version is at most Go 1.21, as per https://github.com/golang/go/issues/60915. We
353+
// don't handle this yet, and it will not matter until Go 1.22.
354+
//
355+
// It is not clear how per-file downgrading behaves in GOPATH mode. On the one hand, no module
356+
// version at all is provided, which should preclude per-file downgrading. On the other hand,
357+
// https://github.com/golang/go/issues/60915 suggests that the language version is at most 1.21
358+
// in GOPATH mode, which would allow per-file downgrading. Again it doesn't affect us, as all
359+
// relevant language changes before Go 1.22 will lead to type-checking failures and never reach
360+
// us.
361+
//
362+
// It is not clear if per-file upgrading is possible in GOPATH mode. This needs clarification.
363+
364+
f := File(pass, node)
365+
var n int
366+
if v := fileGoVersion(f); v != "" {
367+
var ok bool
368+
n, ok = lint.ParseGoVersion(v)
369+
if !ok {
370+
panic(fmt.Sprintf("unexpected failure parsing version %q", v))
371+
}
372+
} else if v := packageGoVersion(pass.Pkg); v != "" {
373+
var ok bool
374+
n, ok = lint.ParseGoVersion(v)
375+
if !ok {
376+
panic(fmt.Sprintf("unexpected failure parsing version %q", v))
377+
}
378+
} else {
379+
v, ok := pass.Analyzer.Flags.Lookup("go").Value.(flag.Getter)
380+
if !ok {
381+
panic("requested Go version, but analyzer has no version flag")
382+
}
383+
n = v.Get().(int)
384+
}
385+
386+
return n
387+
}
388+
389+
func StdlibVersion(pass *analysis.Pass, node Positioner) int {
390+
var n int
391+
if v := packageGoVersion(pass.Pkg); v != "" {
392+
var ok bool
393+
n, ok = lint.ParseGoVersion(v)
394+
if !ok {
395+
panic(fmt.Sprintf("unexpected failure parsing version %q", v))
396+
}
397+
} else {
398+
v, ok := pass.Analyzer.Flags.Lookup("go").Value.(flag.Getter)
399+
if !ok {
400+
panic("requested Go version, but analyzer has no version flag")
401+
}
402+
n = v.Get().(int)
403+
}
404+
405+
f := File(pass, node)
406+
if f == nil {
407+
panic(fmt.Sprintf("no file found for node with position %s", pass.Fset.PositionFor(node.Pos(), false)))
408+
}
409+
410+
if v := fileGoVersion(f); v != "" {
411+
nf, err := strconv.Atoi(strings.TrimPrefix(v, "go1."))
412+
if err != nil {
413+
panic(fmt.Sprintf("unexpected error: %s", err))
414+
}
415+
416+
if n < 21 {
417+
// Before Go 1.21, the Go version set in go.mod specified the maximum language version
418+
// available to the module. It wasn't uncommon to set the version to Go 1.20 but only
419+
// use 1.20 functionality (both language and stdlib) in files tagged for 1.20, and
420+
// supporting a lower version overall. As such, a file tagged lower than the module
421+
// version couldn't expect to have access to the standard library of the version set in
422+
// go.mod.
423+
//
424+
// While Go 1.21's behavior has been backported to 1.19.11 and 1.20.6, users'
425+
// expectations have not.
426+
n = nf
427+
} else {
428+
// Go 1.21 and newer refuse to build modules that depend on versions newer than the Go
429+
// version. This means that in a 1.22 module with a file tagged as 1.17, the file can
430+
// expect to have access to 1.22's standard library.
431+
//
432+
// Do note that strictly speaking we're conflating the Go version and the module version in
433+
// our check. Nothing is stopping a user from using Go 1.17 to build a Go 1.22 module, in
434+
// which case the 1.17 file will not have acces to the 1.22 standard library. However, we
435+
// believe that if a module requires 1.21 or newer, then the author clearly expects the new
436+
// behavior, and doesn't care for the old one. Otherwise they would've specified an older
437+
// version.
438+
//
439+
// In other words, the module version also specifies what it itself actually means, with
440+
// >=1.21 being a minimum version for the toolchain, and <1.21 being a maximum version for
441+
// the language.
442+
443+
if nf > n {
444+
n = nf
445+
}
446+
}
322447
}
323-
version := f.Get().(int)
324-
return version >= minor
448+
449+
return n
325450
}
326451

327452
var integerLiteralQ = pattern.MustParse(`(IntegerLiteral tv)`)

analysis/code/version.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//go:build go1.21
2+
3+
package code
4+
5+
import (
6+
"go/ast"
7+
"go/types"
8+
)
9+
10+
func fileGoVersion(f *ast.File) string {
11+
return f.GoVersion
12+
}
13+
14+
func packageGoVersion(pkg *types.Package) string {
15+
return pkg.GoVersion()
16+
}

analysis/code/version_go120.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//go:build !go1.21
2+
3+
package code
4+
5+
import (
6+
"go/ast"
7+
"go/types"
8+
)
9+
10+
func fileGoVersion(f *ast.File) string {
11+
return ""
12+
}
13+
14+
func packageGoVersion(pkg *types.Package) string {
15+
return ""
16+
}

analysis/report/report.go

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,22 @@ import (
1010
"strconv"
1111
"strings"
1212

13+
"honnef.co/go/tools/analysis/code"
1314
"honnef.co/go/tools/analysis/facts/generated"
1415
"honnef.co/go/tools/go/ast/astutil"
1516

1617
"golang.org/x/tools/go/analysis"
1718
)
1819

1920
type Options struct {
20-
ShortRange bool
21-
FilterGenerated bool
22-
Fixes []analysis.SuggestedFix
23-
Related []analysis.RelatedInformation
21+
ShortRange bool
22+
FilterGenerated bool
23+
Fixes []analysis.SuggestedFix
24+
Related []analysis.RelatedInformation
25+
MinimumLanguageVersion int
26+
MaximumLanguageVersion int
27+
MinimumStdlibVersion int
28+
MaximumStdlibVersion int
2429
}
2530

2631
type Option func(*Options)
@@ -58,6 +63,19 @@ func Related(node Positioner, message string) Option {
5863
}
5964
}
6065

66+
func MinimumLanguageVersion(vers int) Option {
67+
return func(opts *Options) { opts.MinimumLanguageVersion = vers }
68+
}
69+
func MaximumLanguageVersion(vers int) Option {
70+
return func(opts *Options) { opts.MinimumLanguageVersion = vers }
71+
}
72+
func MinimumStdlibVersion(vers int) Option {
73+
return func(opts *Options) { opts.MinimumStdlibVersion = vers }
74+
}
75+
func MaximumStdlibVersion(vers int) Option {
76+
return func(opts *Options) { opts.MaximumStdlibVersion = vers }
77+
}
78+
6179
type Positioner interface {
6280
Pos() token.Pos
6381
}
@@ -170,6 +188,21 @@ func Report(pass *analysis.Pass, node Positioner, message string, opts ...Option
170188
opt(cfg)
171189
}
172190

191+
langVersion := code.LanguageVersion(pass, node)
192+
stdlibVersion := code.StdlibVersion(pass, node)
193+
if n := cfg.MaximumLanguageVersion; n != 0 && n < langVersion {
194+
return
195+
}
196+
if n := cfg.MaximumStdlibVersion; n != 0 && n < stdlibVersion {
197+
return
198+
}
199+
if n := cfg.MinimumLanguageVersion; n != 0 && n > langVersion {
200+
return
201+
}
202+
if n := cfg.MinimumStdlibVersion; n != 0 && n > stdlibVersion {
203+
return
204+
}
205+
173206
file := DisplayPosition(pass.Fset, node.Pos()).Filename
174207
if cfg.FilterGenerated {
175208
m := pass.ResultOf[generated.Analyzer].(map[string]generated.Generator)

simple/lint.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -890,18 +890,18 @@ var (
890890
)
891891

892892
func CheckTimeUntil(pass *analysis.Pass) (interface{}, error) {
893-
if !code.IsGoVersion(pass, 8) {
894-
return nil, nil
895-
}
896893
fn := func(node ast.Node) {
897894
if _, ok := code.Match(pass, checkTimeUntilQ, node); ok {
898895
if sel, ok := node.(*ast.CallExpr).Fun.(*ast.SelectorExpr); ok {
899896
r := pattern.NodeToAST(checkTimeUntilR.Root, map[string]interface{}{"arg": sel.X}).(ast.Node)
900897
report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())",
901898
report.FilterGenerated(),
899+
report.MinimumStdlibVersion(8),
902900
report.Fixes(edit.Fix("replace with call to time.Until", edit.ReplaceWithNode(pass.Fset, node, r))))
903901
} else {
904-
report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())", report.FilterGenerated())
902+
report.Report(pass, node, "should use time.Until instead of t.Sub(time.Now())",
903+
report.MinimumStdlibVersion(8),
904+
report.FilterGenerated())
905905
}
906906
}
907907
}
@@ -944,6 +944,7 @@ func CheckUnnecessaryBlank(pass *analysis.Pass) (interface{}, error) {
944944
if rs.Value == nil && astutil.IsBlank(rs.Key) {
945945
report.Report(pass, rs.Key, "unnecessary assignment to the blank identifier",
946946
report.FilterGenerated(),
947+
report.MinimumLanguageVersion(4),
947948
report.Fixes(edit.Fix("remove assignment to blank identifier", edit.Delete(edit.Range{rs.Key.Pos(), rs.TokPos + 1}))))
948949
}
949950

@@ -952,21 +953,21 @@ func CheckUnnecessaryBlank(pass *analysis.Pass) (interface{}, error) {
952953
// FIXME we should mark both key and value
953954
report.Report(pass, rs.Key, "unnecessary assignment to the blank identifier",
954955
report.FilterGenerated(),
956+
report.MinimumLanguageVersion(4),
955957
report.Fixes(edit.Fix("remove assignment to blank identifier", edit.Delete(edit.Range{rs.Key.Pos(), rs.TokPos + 1}))))
956958
}
957959

958960
// for x, _
959961
if !astutil.IsBlank(rs.Key) && astutil.IsBlank(rs.Value) {
960962
report.Report(pass, rs.Value, "unnecessary assignment to the blank identifier",
961963
report.FilterGenerated(),
964+
report.MinimumLanguageVersion(4),
962965
report.Fixes(edit.Fix("remove assignment to blank identifier", edit.Delete(edit.Range{rs.Key.End(), rs.Value.End()}))))
963966
}
964967
}
965968

966969
code.Preorder(pass, fn1, (*ast.AssignStmt)(nil))
967-
if code.IsGoVersion(pass, 4) {
968-
code.Preorder(pass, fn3, (*ast.RangeStmt)(nil))
969-
}
970+
code.Preorder(pass, fn3, (*ast.RangeStmt)(nil))
970971
return nil, nil
971972
}
972973

@@ -1067,7 +1068,7 @@ func CheckSimplerStructConversion(pass *analysis.Pass) (interface{}, error) {
10671068
if typ1 == typ2 {
10681069
return
10691070
}
1070-
if code.IsGoVersion(pass, 8) {
1071+
if code.LanguageVersion(pass, node) >= 8 {
10711072
if !types.IdenticalIgnoreTags(s1, s2) {
10721073
return
10731074
}

staticcheck/lint.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ var (
139139
checkEncodingBinaryRules = map[string]CallCheck{
140140
"encoding/binary.Write": func(call *Call) {
141141
arg := call.Args[knowledge.Arg("encoding/binary.Write.data")]
142-
if !CanBinaryMarshal(call.Pass, arg.Value) {
142+
if !CanBinaryMarshal(call.Pass, call.Parent, arg.Value) {
143143
arg.Invalid(fmt.Sprintf("value of type %s cannot be used with binary.Write", arg.Value.Value.Type()))
144144
}
145145
},
@@ -1175,12 +1175,6 @@ func CheckDubiousDeferInChannelRangeLoop(pass *analysis.Pass) (interface{}, erro
11751175
}
11761176

11771177
func CheckTestMainExit(pass *analysis.Pass) (interface{}, error) {
1178-
if code.IsGoVersion(pass, 15) {
1179-
// Beginning with Go 1.15, the test framework will call
1180-
// os.Exit for us.
1181-
return nil, nil
1182-
}
1183-
11841178
var (
11851179
fnmain ast.Node
11861180
callsExit bool
@@ -1209,6 +1203,11 @@ func CheckTestMainExit(pass *analysis.Pass) (interface{}, error) {
12091203
if !isTestMain(pass, node) {
12101204
return false
12111205
}
1206+
if code.StdlibVersion(pass, node) >= 15 {
1207+
// Beginning with Go 1.15, the test framework will call
1208+
// os.Exit for us.
1209+
return false
1210+
}
12121211
fnmain = node
12131212
arg = pass.TypesInfo.ObjectOf(node.Type.Params.List[0].Names[0])
12141213
return true
@@ -3126,7 +3125,7 @@ func CheckDeprecated(pass *analysis.Pass) (interface{}, error) {
31263125
// warnings.
31273126
//
31283127
// See also https://staticcheck.dev/issues/1318.
3129-
if !code.IsGoVersion(pass, std.DeprecatedSince) {
3128+
if code.StdlibVersion(pass, node) < std.DeprecatedSince {
31303129
return
31313130
}
31323131
}

0 commit comments

Comments
 (0)