Skip to content

Commit c5ce990

Browse files
committed
Use module's Go version as the default version
In the past, the '-go' flag defaulted to the version of Go that Staticcheck was built with. This wasn't a great default as it was usually too aggressive, suggesting new language features right away, despite most people wanting to support the last N versions of Go. Go modules themselves specify a Go version to support. This version restricts the set of Go language features that can be used in a module. This maps quite well to our '-go' flag, which similarly suppresses suggestions that would depend on a newer version of Go. The mapping isn't perfect, in particular because we conflate language features and the version of the standard library (a Go module targeting Go 1.0 can't use newer language features, but it can still use parts of the stdlib that were introduced in later versions), but it is better than our previous default. The user is still able to override the default if needed. Due to the design of flags in go/analysis, we can't easily specify different versions for different packages. In theory, this should not be a problem, as the Go build system only allows operating on one module at a time. We may still see a mix of packages where some have a module and others don't – this is because the synthetic "foo.test" packages are not part of the module. We can skip those. Should we ever see more than one module, we fall back to the old default and emit a warning. If the user isn't operating in module mode, we fall back to the old default. Closes gh-853
1 parent b878418 commit c5ce990

File tree

4 files changed

+63
-27
lines changed

4 files changed

+63
-27
lines changed

analysis/lint/lint.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
package lint
33

44
import (
5-
"errors"
65
"flag"
76
"fmt"
87
"go/ast"
@@ -66,13 +65,13 @@ func (v *VersionFlag) String() string {
6665

6766
func (v *VersionFlag) Set(s string) error {
6867
if len(s) < 3 {
69-
return errors.New("invalid Go version")
68+
return fmt.Errorf("invalid Go version: %q", s)
7069
}
7170
if s[0] != '1' {
72-
return errors.New("invalid Go version")
71+
return fmt.Errorf("invalid Go version: %q", s)
7372
}
7473
if s[1] != '.' {
75-
return errors.New("invalid Go version")
74+
return fmt.Errorf("invalid Go version: %q", s)
7675
}
7776
i, err := strconv.Atoi(s[2:])
7877
*v = VersionFlag(i)

go/loader/loader.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ type PackageSpec struct {
3737
Imports map[string]*PackageSpec
3838
TypesSizes types.Sizes
3939
Hash cache.ActionID
40+
Module *packages.Module
4041

4142
Config config.Config
4243
}
@@ -74,7 +75,8 @@ func Graph(cfg *packages.Config, patterns ...string) ([]*PackageSpec, error) {
7475
packages.NeedExportsFile |
7576
packages.NeedFiles |
7677
packages.NeedCompiledGoFiles |
77-
packages.NeedTypesSizes
78+
packages.NeedTypesSizes |
79+
packages.NeedModule
7880
pkgs, err := packages.Load(&dcfg, patterns...)
7981
if err != nil {
8082
return nil, err
@@ -93,6 +95,7 @@ func Graph(cfg *packages.Config, patterns ...string) ([]*PackageSpec, error) {
9395
ExportFile: pkg.ExportFile,
9496
Imports: map[string]*PackageSpec{},
9597
TypesSizes: pkg.TypesSizes,
98+
Module: pkg.Module,
9699
}
97100
for path, imp := range pkg.Imports {
98101
spec.Imports[path] = m[imp]

lintcmd/cmd.go

+11-8
Original file line numberDiff line numberDiff line change
@@ -268,21 +268,24 @@ func filterIgnored(problems []problem, res runner.ResultData, allowedAnalyzers m
268268
return append(problems, moreProblems...), nil
269269
}
270270

271+
func defaultGoVersion() string {
272+
tags := build.Default.ReleaseTags
273+
v := tags[len(tags)-1][2:]
274+
return v
275+
}
276+
271277
func newLinter(cfg config.Config) (*linter, error) {
272278
r, err := runner.New(cfg)
273279
if err != nil {
274280
return nil, err
275281
}
282+
r.FallbackGoVersion = defaultGoVersion()
276283
return &linter{
277284
Config: cfg,
278285
Runner: r,
279286
}, nil
280287
}
281288

282-
func (l *linter) SetGoVersion(n int) {
283-
l.Runner.GoVersion = n
284-
}
285-
286289
func (l *linter) Lint(cfg *packages.Config, patterns []string) (problems []problem, warnings []string, err error) {
287290
results, err := l.Runner.Run(cfg, l.Checkers, patterns)
288291
if err != nil {
@@ -528,7 +531,7 @@ func FlagSet(name string) *flag.FlagSet {
528531
panic(fmt.Sprintf("internal error: %s", err))
529532
}
530533

531-
flags.Var(version, "go", "Target Go `version` in the format '1.x'")
534+
flags.String("go", "module", "Target Go `version` in the format '1.x', or the literal 'module' to use the module's Go version")
532535
return flags
533536
}
534537

@@ -544,7 +547,7 @@ func findCheck(cs []*analysis.Analyzer, check string) (*analysis.Analyzer, bool)
544547
func ProcessFlagSet(cs []*analysis.Analyzer, fs *flag.FlagSet) {
545548
tags := fs.Lookup("tags").Value.(flag.Getter).Get().(string)
546549
tests := fs.Lookup("tests").Value.(flag.Getter).Get().(bool)
547-
goVersion := fs.Lookup("go").Value.(flag.Getter).Get().(int)
550+
goVersion := fs.Lookup("go").Value.(flag.Getter).Get().(string)
548551
theFormatter := fs.Lookup("f").Value.(flag.Getter).Get().(string)
549552
printVersion := fs.Lookup("version").Value.(flag.Getter).Get().(bool)
550553
showIgnored := fs.Lookup("show-ignored").Value.(flag.Getter).Get().(bool)
@@ -721,7 +724,7 @@ type options struct {
721724

722725
Tags string
723726
LintTests bool
724-
GoVersion int
727+
GoVersion string
725728
PrintAnalyzerMeasurement func(analysis *analysis.Analyzer, pkg *loader.PackageSpec, d time.Duration)
726729
}
727730

@@ -761,7 +764,7 @@ func doLint(cs []*analysis.Analyzer, paths []string, opt *options) ([]problem, [
761764
return nil, nil, err
762765
}
763766
l.Checkers = cs
764-
l.SetGoVersion(opt.GoVersion)
767+
l.Runner.GoVersion = opt.GoVersion
765768
l.Runner.Stats.PrintAnalyzerMeasurement = opt.PrintAnalyzerMeasurement
766769

767770
cfg := &packages.Config{}

lintcmd/runner/runner.go

+45-14
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,13 @@ func (act *analyzerAction) String() string {
335335
// A Runner executes analyzers on packages.
336336
type Runner struct {
337337
Stats Stats
338-
GoVersion int
338+
GoVersion string
339+
// if GoVersion == "module", and we couldn't determine the
340+
// module's Go version, use this as the fallback
341+
FallbackGoVersion string
342+
343+
// GoVersion might be "module"; actualGoVersion contains the resolved version
344+
actualGoVersion string
339345

340346
// Config that gets merged with per-package configs
341347
cfg config.Config
@@ -499,7 +505,7 @@ func (r *subrunner) do(act action) error {
499505
fmt.Fprintf(h, "cfg %#v\n", hashCfg)
500506
fmt.Fprintf(h, "pkg %x\n", a.Package.Hash)
501507
fmt.Fprintf(h, "analyzers %s\n", r.analyzerNames)
502-
fmt.Fprintf(h, "go 1.%d\n", r.GoVersion)
508+
fmt.Fprintf(h, "go %s\n", r.actualGoVersion)
503509

504510
// OPT(dh): do we actually need to hash vetx? can we not assume
505511
// that for identical inputs, staticcheck will produce identical
@@ -1097,22 +1103,10 @@ func allAnalyzers(analyzers []*analysis.Analyzer) []*analysis.Analyzer {
10971103
//
10981104
// If cfg is nil, a default config will be used. Otherwise, cfg will
10991105
// be used, with the exception of the Mode field.
1100-
//
1101-
// Run can be called multiple times on the same Runner and it is safe
1102-
// for concurrent use. All runs will share the same semaphore.
11031106
func (r *Runner) Run(cfg *packages.Config, analyzers []*analysis.Analyzer, patterns []string) ([]Result, error) {
11041107
analyzers = allAnalyzers(analyzers)
11051108
registerGobTypes(analyzers)
11061109

1107-
for _, a := range analyzers {
1108-
flag := a.Flags.Lookup("go")
1109-
if flag == nil {
1110-
continue
1111-
}
1112-
// OPT(dh): this is terrible
1113-
flag.Value.Set(fmt.Sprintf("1.%d", r.GoVersion))
1114-
}
1115-
11161110
r.Stats.setState(StateLoadPackageGraph)
11171111
lpkgs, err := loader.Graph(cfg, patterns...)
11181112
if err != nil {
@@ -1124,6 +1118,43 @@ func (r *Runner) Run(cfg *packages.Config, analyzers []*analysis.Analyzer, patte
11241118
return nil, nil
11251119
}
11261120

1121+
var goVersion string
1122+
if r.GoVersion == "module" {
1123+
for _, lpkg := range lpkgs {
1124+
if m := lpkg.Module; m != nil {
1125+
if goVersion == "" {
1126+
goVersion = m.GoVersion
1127+
} else if goVersion != m.GoVersion {
1128+
// Theoretically, we should only ever see a single Go
1129+
// module. At least that's currently (as of Go 1.15)
1130+
// true when using 'go list'.
1131+
fmt.Fprintln(os.Stderr, "warning: encountered multiple modules and could not deduce targeted Go version")
1132+
goVersion = ""
1133+
break
1134+
}
1135+
}
1136+
}
1137+
} else {
1138+
goVersion = r.GoVersion
1139+
}
1140+
1141+
if goVersion == "" {
1142+
if r.FallbackGoVersion == "" {
1143+
panic("could not determine Go version of module, and fallback version hasn't been set")
1144+
}
1145+
goVersion = r.FallbackGoVersion
1146+
}
1147+
r.actualGoVersion = goVersion
1148+
for _, a := range analyzers {
1149+
flag := a.Flags.Lookup("go")
1150+
if flag == nil {
1151+
continue
1152+
}
1153+
if err := flag.Value.Set(goVersion); err != nil {
1154+
return nil, err
1155+
}
1156+
}
1157+
11271158
r.Stats.setState(StateBuildActionGraph)
11281159
all := map[*loader.PackageSpec]*packageAction{}
11291160
root := &packageAction{}

0 commit comments

Comments
 (0)