From 76451b0353fee40149ded5cf70b0631308d600ba Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 18 Feb 2021 19:33:52 +0100 Subject: [PATCH] revive: fix types and default configuration. --- go.mod | 1 + pkg/golinters/revive.go | 134 +++++++++++++++++++++++++------ test/testdata/configs/revive.yml | 12 +++ 3 files changed, 123 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index 0c1ca09cdb8a..399026052237 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( 4d63.com/gochecknoglobals v0.0.0-20201008074935-acfc0b28355a + github.com/BurntSushi/toml v0.3.1 github.com/Djarvur/go-err113 v0.0.0-20200511133814-5174e21577d5 github.com/OpenPeeDeeP/depguard v1.0.1 github.com/alexkohler/prealloc v0.0.0-20210204145425-77a5b5dd9799 diff --git a/pkg/golinters/revive.go b/pkg/golinters/revive.go index 377ad946b07d..c1d4bbae9d24 100644 --- a/pkg/golinters/revive.go +++ b/pkg/golinters/revive.go @@ -1,14 +1,17 @@ package golinters import ( + "bytes" "encoding/json" "fmt" "go/token" "io/ioutil" + "github.com/BurntSushi/toml" "github.com/mgechev/dots" reviveConfig "github.com/mgechev/revive/config" "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/config" @@ -47,7 +50,7 @@ func NewRevive(cfg *config.ReviveSettings) *goanalysis.Linter { files = append(files, pass.Fset.PositionFor(file.Pos(), false).Filename) } - conf, err := setReviveConfig(cfg) + conf, err := getReviveConfig(cfg) if err != nil { return nil, err } @@ -128,46 +131,129 @@ func NewRevive(cfg *config.ReviveSettings) *goanalysis.Linter { }).WithLoadMode(goanalysis.LoadModeSyntax) } -func setReviveConfig(cfg *config.ReviveSettings) (*lint.Config, error) { - // Get revive default configuration - conf, err := reviveConfig.GetConfig("") +// This function mimics the GetConfig function of revive. +// This allow to get default values and right types. +// https://github.com/golangci/golangci-lint/issues/1745 +// https://github.com/mgechev/revive/blob/389ba853b0b3587f0c3b71b5f0c61ea4e23928ec/config/config.go#L155 +func getReviveConfig(cfg *config.ReviveSettings) (*lint.Config, error) { + rawRoot := createConfigMap(cfg) + + buf := bytes.NewBuffer(nil) + + err := toml.NewEncoder(buf).Encode(rawRoot) if err != nil { return nil, err } - // Default is false - conf.IgnoreGeneratedHeader = cfg.IgnoreGeneratedHeader + conf := defaultConfig() - if cfg.Severity != "" { - conf.Severity = lint.Severity(cfg.Severity) + _, err = toml.DecodeReader(buf, conf) + if err != nil { + return nil, err } - if cfg.Confidence != 0 { - conf.Confidence = cfg.Confidence - } + normalizeConfig(conf) // By default golangci-lint ignores missing doc comments, follow same convention by removing this default rule // Relevant issue: https://github.com/golangci/golangci-lint/issues/456 + delete(conf.Rules, "package-comments") delete(conf.Rules, "exported") - if len(cfg.Rules) != 0 { - // Clear default rules, only use rules defined in config - conf.Rules = make(map[string]lint.RuleConfig, len(cfg.Rules)) + return conf, nil +} + +func createConfigMap(cfg *config.ReviveSettings) map[string]interface{} { + rawRoot := map[string]interface{}{ + "ignoreGeneratedHeader": cfg.IgnoreGeneratedHeader, + "confidence": cfg.Confidence, + "severity": cfg.Severity, + "errorCode": cfg.ErrorCode, + "warningCode": cfg.WarningCode, } - for _, r := range cfg.Rules { - conf.Rules[r.Name] = lint.RuleConfig{Arguments: r.Arguments, Severity: lint.Severity(r.Severity)} + + rawDirectives := map[string]map[string]interface{}{} + for _, directive := range cfg.Directives { + rawDirectives[directive.Name] = map[string]interface{}{ + "severity": directive.Severity, + } } - conf.ErrorCode = cfg.ErrorCode - conf.WarningCode = cfg.WarningCode + if len(rawDirectives) > 0 { + rawRoot["directive"] = rawDirectives + } - if len(cfg.Directives) != 0 { - // Clear default Directives, only use Directives defined in config - conf.Directives = make(map[string]lint.DirectiveConfig, len(cfg.Directives)) + rawRules := map[string]map[string]interface{}{} + for _, s := range cfg.Rules { + rawRules[s.Name] = map[string]interface{}{ + "severity": s.Severity, + "arguments": s.Arguments, + } } - for _, d := range cfg.Directives { - conf.Directives[d.Name] = lint.DirectiveConfig{Severity: lint.Severity(d.Severity)} + + if len(rawRules) > 0 { + rawRoot["rule"] = rawRules } - return conf, nil + return rawRoot +} + +// This element is not exported by revive, so we need copy the code. +// Extracted from https://github.com/mgechev/revive/blob/389ba853b0b3587f0c3b71b5f0c61ea4e23928ec/config/config.go#L15 +var defaultRules = []lint.Rule{ + &rule.VarDeclarationsRule{}, + &rule.PackageCommentsRule{}, + &rule.DotImportsRule{}, + &rule.BlankImportsRule{}, + &rule.ExportedRule{}, + &rule.VarNamingRule{}, + &rule.IndentErrorFlowRule{}, + &rule.IfReturnRule{}, + &rule.RangeRule{}, + &rule.ErrorfRule{}, + &rule.ErrorNamingRule{}, + &rule.ErrorStringsRule{}, + &rule.ReceiverNamingRule{}, + &rule.IncrementDecrementRule{}, + &rule.ErrorReturnRule{}, + &rule.UnexportedReturnRule{}, + &rule.TimeNamingRule{}, + &rule.ContextKeysType{}, + &rule.ContextAsArgumentRule{}, +} + +// This element is not exported by revive, so we need copy the code. +// Extracted from https://github.com/mgechev/revive/blob/389ba853b0b3587f0c3b71b5f0c61ea4e23928ec/config/config.go#L133 +func normalizeConfig(cfg *lint.Config) { + if cfg.Confidence == 0 { + cfg.Confidence = 0.8 + } + severity := cfg.Severity + if severity != "" { + for k, v := range cfg.Rules { + if v.Severity == "" { + v.Severity = severity + } + cfg.Rules[k] = v + } + for k, v := range cfg.Directives { + if v.Severity == "" { + v.Severity = severity + } + cfg.Directives[k] = v + } + } +} + +// This element is not exported by revive, so we need copy the code. +// Extracted from https://github.com/mgechev/revive/blob/389ba853b0b3587f0c3b71b5f0c61ea4e23928ec/config/config.go#L182 +func defaultConfig() *lint.Config { + defaultConfig := lint.Config{ + Confidence: 0.0, + Severity: lint.SeverityWarning, + Rules: map[string]lint.RuleConfig{}, + } + for _, r := range defaultRules { + defaultConfig.Rules[r.Name()] = lint.RuleConfig{} + } + return &defaultConfig } diff --git a/test/testdata/configs/revive.yml b/test/testdata/configs/revive.yml index 7f7ac0b3a346..84ee90e9fe4e 100644 --- a/test/testdata/configs/revive.yml +++ b/test/testdata/configs/revive.yml @@ -5,3 +5,15 @@ linters-settings: rules: - name: indent-error-flow severity: warning + - name: cognitive-complexity + arguments: [ 7 ] + - name: line-length-limit + arguments: [ 110 ] + - name: function-result-limit + arguments: [ 3 ] + - name: argument-limit + arguments: [ 4 ] + - name: cyclomatic + arguments: [ 10 ] + - name: max-public-structs + arguments: [ 3 ]