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

feat: automatic Go version detection #2669

Merged
merged 5 commits into from Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .golangci.example.yml
Expand Up @@ -66,8 +66,8 @@ run:

# Define the Go version limit.
# Mainly related to generics support in go1.18.
# Default: 1.17
go: 1.18
# Default: use Go version from the go.mod file, fallback on the env var `GOVERSION`, fallback on 1.17
ldez marked this conversation as resolved.
Show resolved Hide resolved
go: '1.18'


# output configuration options
Expand Down
1 change: 1 addition & 0 deletions .golangci.yml
Expand Up @@ -130,6 +130,7 @@ issues:

run:
timeout: 5m
go: '1.17' # TODO(ldez): we force to use an old version of Go for the CI and the tests.
skip-dirs:
- test/testdata_etc
- internal/cache
Expand Down
4 changes: 4 additions & 0 deletions pkg/commands/executor.go
Expand Up @@ -110,6 +110,10 @@ func NewExecutor(version, commit, date string) *Executor {
e.log.Fatalf("Can't read config: %s", err)
}

if commandLineCfg.Run.Go == "" && e.cfg.Run.Go == "" {
e.cfg.Run.Go = config.DetectGoVersion()
}

// recreate after getting config
e.DBManager = lintersdb.NewManager(e.cfg, e.log).WithCustomLinters()

Expand Down
2 changes: 1 addition & 1 deletion pkg/commands/run.go
Expand Up @@ -95,7 +95,7 @@ func initFlagSet(fs *pflag.FlagSet, cfg *config.Config, m *lintersdb.Manager, is
"Modules download mode. If not empty, passed as -mod=<mode> to go tools")
fs.IntVar(&rc.ExitCodeIfIssuesFound, "issues-exit-code",
exitcodes.IssuesFound, wh("Exit code when issues were found"))
fs.StringVar(&rc.Go, "go", "1.17", wh("Targeted Go version"))
fs.StringVar(&rc.Go, "go", "", wh("Targeted Go version"))
fs.StringSliceVar(&rc.BuildTags, "build-tags", nil, wh("Build tags"))

fs.DurationVar(&rc.Timeout, "deadline", defaultTimeout, wh("Deadline for total work"))
Expand Down
37 changes: 37 additions & 0 deletions pkg/config/config.go
@@ -1,5 +1,13 @@
package config

import (
"os"
"strings"

hcversion "github.com/hashicorp/go-version"
"github.com/ldez/gomoddirectives"
)

// Config encapsulates the config data specified in the golangci yaml config file.
type Config struct {
cfgDir string // The directory containing the golangci config file.
Expand Down Expand Up @@ -31,3 +39,32 @@ func NewDefault() *Config {
type Version struct {
Format string `mapstructure:"format"`
}

func IsGreaterThanOrEqualGo118(v string) bool {
v1, err := hcversion.NewVersion(strings.TrimPrefix(v, "go"))
if err != nil {
return false
}

limit, err := hcversion.NewVersion("1.18")
if err != nil {
return false
}

return v1.GreaterThanOrEqual(limit)
}

func DetectGoVersion() string {
ldez marked this conversation as resolved.
Show resolved Hide resolved
file, _ := gomoddirectives.GetModuleFile()

if file != nil && file.Go != nil && file.Go.Version != "" {
return file.Go.Version
}

v := os.Getenv("GOVERSION")
ldez marked this conversation as resolved.
Show resolved Hide resolved
if v != "" {
return v
}

return "1.17"
}
5 changes: 3 additions & 2 deletions pkg/config/linters_settings.go
Expand Up @@ -374,7 +374,8 @@ type GoSecSettings struct {
}

type GovetSettings struct {
CheckShadowing bool `mapstructure:"check-shadowing"`
Go string `mapstructure:"-"`
CheckShadowing bool `mapstructure:"check-shadowing"`
Settings map[string]map[string]interface{}

Enable []string
Expand All @@ -383,7 +384,7 @@ type GovetSettings struct {
DisableAll bool `mapstructure:"disable-all"`
}

func (cfg GovetSettings) Validate() error {
func (cfg *GovetSettings) Validate() error {
if cfg.EnableAll && cfg.DisableAll {
return errors.New("enable-all and disable-all can't be combined")
}
Expand Down
77 changes: 43 additions & 34 deletions pkg/golinters/govet.go
Expand Up @@ -119,7 +119,46 @@ var (
}
)

func NewGovet(cfg *config.GovetSettings) *goanalysis.Linter {
var settings map[string]map[string]interface{}
if cfg != nil {
settings = cfg.Settings
}
return goanalysis.NewLinter(
"govet",
"Vet examines Go source code and reports suspicious constructs, "+
"such as Printf calls whose arguments do not align with the format string",
analyzersFromConfig(cfg),
settings,
).WithLoadMode(goanalysis.LoadModeTypesInfo)
}

func analyzersFromConfig(cfg *config.GovetSettings) []*analysis.Analyzer {
if cfg == nil {
return defaultAnalyzers
}

if cfg.CheckShadowing {
// Keeping for backward compatibility.
cfg.Enable = append(cfg.Enable, shadow.Analyzer.Name)
}

var enabledAnalyzers []*analysis.Analyzer
for _, a := range allAnalyzers {
if isAnalyzerEnabled(a.Name, cfg, defaultAnalyzers) {
enabledAnalyzers = append(enabledAnalyzers, a)
}
}

return enabledAnalyzers
}

func isAnalyzerEnabled(name string, cfg *config.GovetSettings, defaultAnalyzers []*analysis.Analyzer) bool {
if (name == nilness.Analyzer.Name || name == unusedwrite.Analyzer.Name) &&
config.IsGreaterThanOrEqualGo118(cfg.Go) {
ldez marked this conversation as resolved.
Show resolved Hide resolved
return false
}

if cfg.EnableAll {
for _, n := range cfg.Disable {
if n == name {
Expand All @@ -128,58 +167,28 @@ func isAnalyzerEnabled(name string, cfg *config.GovetSettings, defaultAnalyzers
}
return true
}

// Raw for loops should be OK on small slice lengths.
for _, n := range cfg.Enable {
if n == name {
return true
}
}

for _, n := range cfg.Disable {
if n == name {
return false
}
}

if cfg.DisableAll {
return false
}

for _, a := range defaultAnalyzers {
if a.Name == name {
return true
}
}
return false
}

func analyzersFromConfig(cfg *config.GovetSettings) []*analysis.Analyzer {
if cfg == nil {
return defaultAnalyzers
}

if cfg.CheckShadowing {
// Keeping for backward compatibility.
cfg.Enable = append(cfg.Enable, shadow.Analyzer.Name)
}

var enabledAnalyzers []*analysis.Analyzer
for _, a := range allAnalyzers {
if isAnalyzerEnabled(a.Name, cfg, defaultAnalyzers) {
enabledAnalyzers = append(enabledAnalyzers, a)
}
}

return enabledAnalyzers
}

func NewGovet(cfg *config.GovetSettings) *goanalysis.Linter {
var settings map[string]map[string]interface{}
if cfg != nil {
settings = cfg.Settings
}
return goanalysis.NewLinter(
"govet",
"Vet examines Go source code and reports suspicious constructs, "+
"such as Printf calls whose arguments do not align with the format string",
analyzersFromConfig(cfg),
settings,
).WithLoadMode(goanalysis.LoadModeTypesInfo)
}
26 changes: 4 additions & 22 deletions pkg/lint/linter/config.go
@@ -1,9 +1,6 @@
package linter

import (
"strings"

hcversion "github.com/hashicorp/go-version"
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"

Expand Down Expand Up @@ -126,14 +123,17 @@ func (lc *Config) Name() string {
}

func (lc *Config) WithNoopFallback(cfg *config.Config) *Config {
if isGreaterThanOrEqualGo118(cfg) {
if cfg != nil && config.IsGreaterThanOrEqualGo118(cfg.Run.Go) {
lc.Linter = &Noop{
name: lc.Linter.Name(),
desc: lc.Linter.Desc(),
run: func(pass *analysis.Pass) (interface{}, error) {
return nil, nil
},
}

lc.LoadMode = 0
ldez marked this conversation as resolved.
Show resolved Hide resolved
return lc.WithLoadFiles()
}

return lc
Expand All @@ -145,21 +145,3 @@ func NewConfig(linter Linter) *Config {
}
return lc.WithLoadFiles()
}

func isGreaterThanOrEqualGo118(cfg *config.Config) bool {
if cfg == nil {
return false
}

v1, err := hcversion.NewVersion(strings.TrimPrefix(cfg.Run.Go, "go"))
if err != nil {
return false
}

limit, err := hcversion.NewVersion("1.18")
if err != nil {
return false
}

return v1.GreaterThanOrEqual(limit)
}
1 change: 0 additions & 1 deletion pkg/lint/linter/linter.go
Expand Up @@ -22,7 +22,6 @@ type Noop struct {

func (n Noop) Run(_ context.Context, lintCtx *Context) ([]result.Issue, error) {
lintCtx.Log.Warnf("%s is disabled because of go1.18."+
" If you are not using go1.18, you can set `go: go1.17` in the `run` section."+
" You can track the evolution of the go1.18 support by following the https://github.com/golangci/golangci-lint/issues/2649.", n.name)
return nil, nil
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/lint/lintersdb/manager.go
Expand Up @@ -164,6 +164,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
unusedCfg = &m.cfg.LintersSettings.Unused
varnamelenCfg = &m.cfg.LintersSettings.Varnamelen
wrapcheckCfg = &m.cfg.LintersSettings.Wrapcheck

if govetCfg != nil {
govetCfg.Go = m.cfg.Run.Go
}
}

const megacheckName = "megacheck"
Expand Down Expand Up @@ -446,7 +450,8 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
WithLoadForGoAnalysis().
WithPresets(linter.PresetStyle).
WithURL("https://github.com/mvdan/interfacer").
Deprecated("The repository of the linter has been archived by the owner.", "v1.38.0", ""),
Deprecated("The repository of the linter has been archived by the owner.", "v1.38.0", "").
WithNoopFallback(m.cfg),

linter.NewConfig(golinters.NewIreturn(ireturnCfg)).
WithSince("v1.43.0").
Expand Down
1 change: 1 addition & 0 deletions test/fix_test.go
Expand Up @@ -43,6 +43,7 @@ func TestFix(t *testing.T) {
t.Parallel()

args := []string{
"--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests.
"--disable-all", "--print-issued-lines=false", "--print-linter-name=false", "--out-format=line-number",
"--allow-parallel-runners", "--fix",
input,
Expand Down
1 change: 1 addition & 0 deletions test/linters_test.go
Expand Up @@ -179,6 +179,7 @@ func saveConfig(t *testing.T, cfg map[string]interface{}) (cfgPath string, finis
func testOneSource(t *testing.T, sourcePath string) {
args := []string{
"run",
"--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests.
"--allow-parallel-runners",
"--disable-all",
"--print-issued-lines=false",
Expand Down
5 changes: 4 additions & 1 deletion test/testshared/testshared.go
Expand Up @@ -98,7 +98,10 @@ func (r *LintRunner) Run(args ...string) *RunResult {
func (r *LintRunner) RunCommand(command string, args ...string) *RunResult {
r.Install()

runArgs := append([]string{command}, "--internal-cmd-test")
runArgs := append([]string{command},
"--go=1.17", // TODO(ldez): we force to use an old version of Go for the CI and the tests.
"--internal-cmd-test",
)
runArgs = append(runArgs, args...)

defer func(startedAt time.Time) {
Expand Down