Repository: bombsimon/wsl
Pull request #135 (2023-06-16 21:57:12)

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.


Patch:
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))
 }


Test patch:
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))
 		})
 	}