From 943c010b59dfd9446fd4f85b33c5ddb628fc02c3 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Sun, 11 Jun 2023 23:51:00 +0200 Subject: [PATCH] Rework config and setup Avoid global variables by adding a constructor for the analyzer. At the same time, accept configuration as an input to not mutate a global var, or pass nil to set the configuration through flags. --- analyzer.go | 146 ++++++++++++++++++++++++++++-------------------- cmd/wsl/main.go | 2 +- wsl_test.go | 40 ++++++------- 3 files changed, 103 insertions(+), 85 deletions(-) diff --git a/analyzer.go b/analyzer.go index 16dfaf2..90e9dd4 100644 --- a/analyzer.go +++ b/analyzer.go @@ -7,84 +7,80 @@ import ( "golang.org/x/tools/go/analysis" ) -// Analyzer is the wsl analyzer. -// -//nolint:gochecknoglobals // Analyzers tend to be global. -var Analyzer = &analysis.Analyzer{ - Name: "wsl", - Doc: "add or remove empty lines", - Flags: flags(), - Run: run, - RunDespiteErrors: true, +func NewAnalyzer(config *Configuration) *analysis.Analyzer { + wa := &wslAnalyzer{config: config} + + return &analysis.Analyzer{ + Name: "wsl", + Doc: "add or remove empty lines", + Flags: wa.flags(), + Run: wa.run, + RunDespiteErrors: true, + } } -//nolint:gochecknoglobals // Config needs to be overwritten with flags. -var config = Configuration{ - AllowAssignAndAnythingCuddle: false, - AllowAssignAndCallCuddle: true, - AllowCuddleDeclaration: false, - AllowMultiLineAssignCuddle: true, - AllowSeparatedLeadingComment: false, - AllowTrailingComment: false, - ForceCuddleErrCheckAndAssign: false, - ForceExclusiveShortDeclarations: false, - StrictAppend: true, - AllowCuddleWithCalls: []string{"Lock", "RLock"}, - AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, - ErrorVariableNames: []string{"err"}, - ForceCaseTrailingWhitespaceLimit: 0, +func defaultConfig() *Configuration { + return &Configuration{ + AllowAssignAndAnythingCuddle: false, + AllowAssignAndCallCuddle: true, + AllowCuddleDeclaration: false, + AllowMultiLineAssignCuddle: true, + AllowSeparatedLeadingComment: false, + AllowTrailingComment: false, + ForceCuddleErrCheckAndAssign: false, + ForceExclusiveShortDeclarations: false, + StrictAppend: true, + AllowCuddleWithCalls: []string{"Lock", "RLock"}, + AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, + ErrorVariableNames: []string{"err"}, + ForceCaseTrailingWhitespaceLimit: 0, + } } -func flags() flag.FlagSet { - flags := flag.NewFlagSet("", flag.ExitOnError) - - flags.BoolVar(&config.AllowAssignAndAnythingCuddle, "allow-assign-and-anything", false, "Allow assignments and anything to be cuddled") - flags.BoolVar(&config.AllowAssignAndCallCuddle, "allow-assign-and-call", true, "Allow assignments and calls to be cuddled (if using same variable/type)") - flags.BoolVar(&config.AllowCuddleDeclaration, "allow-cuddle-declarations", false, "Allow declarations to be cuddled") - flags.BoolVar(&config.AllowMultiLineAssignCuddle, "allow-multi-line-assign", true, "Allow cuddling with multi line assignments") - flags.BoolVar(&config.AllowSeparatedLeadingComment, "allow-separated-leading-comment", false, "Allow empty newlines in leading comments") - flags.BoolVar(&config.AllowTrailingComment, "allow-trailing-comment", false, "Allow blocks to end with a comment") - flags.BoolVar(&config.ForceCuddleErrCheckAndAssign, "force-err-cuddling", false, "Force cuddling of error checks with error var assignment") - flags.BoolVar(&config.ForceExclusiveShortDeclarations, "force-short-decl-cuddling", false, "Force short declarations to cuddle by themselves") - flags.BoolVar(&config.StrictAppend, "strict-append", true, "Strict rules for append") - flags.IntVar(&config.ForceCaseTrailingWhitespaceLimit, "force-case-trailing-whitespace", 0, "Force newlines for case blocks > this number.") - - flags.String("allow-cuddle-with-calls", "Lock,RLock", "Comma separated list of idents that can have cuddles after") - flags.String("allow-cuddle-with-rhs", "Unlock,RUnlock", "Comma separated list of idents that can have cuddles before") - flags.String("error-variable-names", "err", "Comma separated list of error variable names") - - return *flags +// wslAnalyzer is a wrapper around the configuration which is used to be able to +// set the configuration when creating the analyzer and later be able to update +// flags and running method. +type wslAnalyzer struct { + config *Configuration } -func lookupAndSplit(pass *analysis.Pass, flagName string) []string { - flagVal := pass.Analyzer.Flags.Lookup(flagName) - if flagVal == nil { - return nil +func (wa *wslAnalyzer) flags() flag.FlagSet { + flags := flag.NewFlagSet("", flag.ExitOnError) + + // If we have a configuration set we're not running from the command line so + // we don't use any flags. + if wa.config != nil { + return *flags } - return strings.Split(flagVal.Value.String(), ",") -} + wa.config = defaultConfig() -func run(pass *analysis.Pass) (interface{}, error) { - if v := lookupAndSplit(pass, "allow-cuddle-with-calls"); v != nil { - config.AllowCuddleWithCalls = v - } + flags.BoolVar(&wa.config.AllowAssignAndAnythingCuddle, "allow-assign-and-anything", false, "Allow assignments and anything to be cuddled") + flags.BoolVar(&wa.config.AllowAssignAndCallCuddle, "allow-assign-and-call", true, "Allow assignments and calls to be cuddled (if using same variable/type)") + flags.BoolVar(&wa.config.AllowCuddleDeclaration, "allow-cuddle-declarations", false, "Allow declarations to be cuddled") + flags.BoolVar(&wa.config.AllowMultiLineAssignCuddle, "allow-multi-line-assign", true, "Allow cuddling with multi line assignments") + flags.BoolVar(&wa.config.AllowSeparatedLeadingComment, "allow-separated-leading-comment", false, "Allow empty newlines in leading comments") + flags.BoolVar(&wa.config.AllowTrailingComment, "allow-trailing-comment", false, "Allow blocks to end with a comment") + flags.BoolVar(&wa.config.ForceCuddleErrCheckAndAssign, "force-err-cuddling", false, "Force cuddling of error checks with error var assignment") + flags.BoolVar(&wa.config.ForceExclusiveShortDeclarations, "force-short-decl-cuddling", false, "Force short declarations to cuddle by themselves") + flags.BoolVar(&wa.config.StrictAppend, "strict-append", true, "Strict rules for append") + flags.IntVar(&wa.config.ForceCaseTrailingWhitespaceLimit, "force-case-trailing-whitespace", 0, "Force newlines for case blocks > this number.") - if v := lookupAndSplit(pass, "allow-cuddle-with-rhs"); v != nil { - config.AllowCuddleWithRHS = v - } + flags.Var(&multiStringValue{slicePtr: &wa.config.AllowCuddleWithCalls}, "allow-cuddle-with-calls", "Comma separated list of idents that can have cuddles after") + flags.Var(&multiStringValue{slicePtr: &wa.config.AllowCuddleWithRHS}, "allow-cuddle-with-rhs", "Comma separated list of idents that can have cuddles before") + flags.Var(&multiStringValue{slicePtr: &wa.config.ErrorVariableNames}, "error-variable-names", "Comma separated list of error variable names") - if v := lookupAndSplit(pass, "error-variable-names"); v != nil { - config.ErrorVariableNames = v - } + return *flags +} +func (wa *wslAnalyzer) run(pass *analysis.Pass) (interface{}, error) { for _, file := range pass.Files { filename := pass.Fset.Position(file.Pos()).Filename if !strings.HasSuffix(filename, ".go") { continue } - processor := newProcessorWithConfig(file, pass.Fset, &config) + processor := newProcessorWithConfig(file, pass.Fset, wa.config) processor.parseAST() for pos, fix := range processor.result { @@ -113,3 +109,33 @@ func run(pass *analysis.Pass) (interface{}, error) { //nolint:nilnil // A pass don't need to return anything. return nil, nil } + +// multiStringValue is a flag that supports multiple values. It's implemented to +// contain a pointer to a string slice that will be overwritten when the flag's +// `Set` method is called. +type multiStringValue struct { + slicePtr *[]string +} + +// Set implements the flag.Value interface and will overwrite the pointer to the +// slice with a new pointer after splitting the flag by comma. +func (m *multiStringValue) Set(value string) error { + s := []string{} + + for _, v := range strings.Split(value, ",") { + s = append(s, strings.TrimSpace(v)) + } + + *m.slicePtr = s + + return nil +} + +// Set implements the flag.Value interface. +func (m *multiStringValue) String() string { + if m.slicePtr == nil { + return "" + } + + return strings.Join(*m.slicePtr, ", ") +} diff --git a/cmd/wsl/main.go b/cmd/wsl/main.go index 42baec2..92d1e8a 100644 --- a/cmd/wsl/main.go +++ b/cmd/wsl/main.go @@ -6,5 +6,5 @@ import ( ) func main() { - singlechecker.Main(wsl.Analyzer) + singlechecker.Main(wsl.NewAnalyzer(nil)) } diff --git a/wsl_test.go b/wsl_test.go index ff56bc2..4847e0a 100644 --- a/wsl_test.go +++ b/wsl_test.go @@ -1,7 +1,6 @@ package wsl import ( - "flag" "path/filepath" "testing" @@ -10,98 +9,91 @@ import ( func TestWIP(t *testing.T) { testdata := analysistest.TestData() - analyzer := Analyzer - analyzer.Flags = flags() + analyzer := NewAnalyzer(nil) analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "wip") } func TestDefaultConfig(t *testing.T) { testdata := analysistest.TestData() - analyzer := Analyzer - analyzer.Flags = flags() + analyzer := NewAnalyzer(nil) analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "default_config") } func TestWithConfig(t *testing.T) { - defaultConfig := config - - flags := flag.NewFlagSet("", flag.ExitOnError) - testdata := analysistest.TestData() - analyzer := Analyzer - analyzer.Flags = *flags for _, tc := range []struct { subdir string - configFn func() + configFn func(*Configuration) }{ { subdir: "case_blocks", - configFn: func() { + configFn: func(config *Configuration) { config.ForceCaseTrailingWhitespaceLimit = 3 }, }, { subdir: "multi_line_assign", - configFn: func() { + configFn: func(config *Configuration) { config.AllowMultiLineAssignCuddle = false }, }, { subdir: "assign_and_call", - configFn: func() { + configFn: func(config *Configuration) { config.AllowAssignAndCallCuddle = false }, }, { subdir: "trailing_comments", - configFn: func() { + configFn: func(config *Configuration) { config.AllowTrailingComment = true }, }, { subdir: "separate_leading_whitespace", - configFn: func() { + configFn: func(config *Configuration) { config.AllowSeparatedLeadingComment = true }, }, { subdir: "error_check", - configFn: func() { + configFn: func(config *Configuration) { config.ForceCuddleErrCheckAndAssign = true }, }, { subdir: "short_decl", - configFn: func() { + configFn: func(config *Configuration) { config.ForceExclusiveShortDeclarations = true }, }, { subdir: "strict_append", - configFn: func() { + configFn: func(config *Configuration) { config.StrictAppend = false }, }, { subdir: "assign_and_anything", - configFn: func() { + configFn: func(config *Configuration) { config.AllowAssignAndAnythingCuddle = true }, }, { subdir: "decl", - configFn: func() { + configFn: func(config *Configuration) { config.AllowCuddleDeclaration = true }, }, } { t.Run(tc.subdir, func(t *testing.T) { - config = defaultConfig - tc.configFn() + config := defaultConfig() + tc.configFn(config) + analyzer := NewAnalyzer(config) analysistest.RunWithSuggestedFixes(t, testdata, analyzer, filepath.Join("with_config", tc.subdir)) }) }