diff --git a/README.md b/README.md index 801df8b..9035a78 100644 --- a/README.md +++ b/README.md @@ -40,15 +40,17 @@ model/DTO structs and mapping code between them, reducing boilerplate and mainte ```yaml settings: propro: - entity-list-file: path/to/entity_list.go + entity-list-files: + - path/to/entity_list.go + - path/to/another_entity_list.go structs: - User - Order ``` -- **`entity-list-file`** may contain path to a go file containing **`EntityList`** variable with the list of empty pointers to - the protected structs. - - Such a list is required for database migration purposes by ORM tools. +- **`entity-list-files`** may contain a list of paths to go files each containing an **`EntityList`** variable with the list of + empty pointers to the protected structs. + - Such a list is required for database migration purposes by ORM tools. - Example content of such a go file: ```go var EntityList = []any{ @@ -56,13 +58,14 @@ settings: &users.Order{}, } ``` - - The file may be empty or not present, and then this configuration option is ignored. + - Any listed file may be empty or not present, in which case it contributes no entries. The union of the `EntityList` + entries across all listed files is used. - **`structs`**: may contain a list of struct names that should be protected. May be empty or not present. -If both `entity-list-file` and `structs` are specified, the union of the two sets is used. If neither is specified, +If both `entity-list-files` and `structs` are specified, the union of the two sets is used. If neither is specified, the linter **protects ALL STRUCTS** in the analyzed packages. If you don't want any structs to be protected, just disable the linter. @@ -93,11 +96,11 @@ go get github.com/digitalstraw/propro/v2/ git clone git@github.com:digitalstraw/propro.git go build -o propro cmd/propro/main.go mv propro $GOPATH/bin/ -propro -test=false -entityListFile=./some/path/entity_config.go -structs=Entity1,Entity2 ./... +propro -test=false -entityListFiles=./some/path/entity_config.go,./other/path/entity_config.go -structs=Entity1,Entity2 ./... ``` Available CLI parameters: -- `-entityListFile string` - path to a go file containing `EntityList` variable with the list of protected structs. +- `-entityListFiles string` - comma-separated list of paths to go files each containing an `EntityList` variable with the list of protected structs. - `-structs string` - comma-separated list of struct names to be protected. - `-test bool` - whether to run on test files. This flag is provided by the driver, not the analyzer. Default is `true` and it is recommended to turn it off. diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 3088c82..e64e828 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -9,6 +9,7 @@ import ( "go/token" "go/types" "strings" + "sync" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -22,14 +23,15 @@ const ( entityListVarName = "EntityList" // These must be identical to golangci-lint repo config keys. - entityListFileArg = "entityListFile" - structsArg = "structs" + entityListFilesArg = "entityListFiles" + structsArg = "structs" ) var ( - StructsArgValue string - EntityFile string - Structs []string + StructsArgValue string + EntityFilesArgValue string + EntityFiles []string + Structs []string ProtectedStructsMap map[string]bool protectAllStructs bool @@ -39,10 +41,12 @@ var ( ErrNotInspectAnalyzer = errors.New("inspect analyzer result is not *inspector.Inspector") flagSet flag.FlagSet + + initOnce sync.Once ) func init() { - flagSet.StringVar(&EntityFile, entityListFileArg, "", "Path to file listing protected structs") + flagSet.StringVar(&EntityFilesArgValue, entityListFilesArg, "", "Comma-separated list of paths to files listing protected structs") flagSet.StringVar(&StructsArgValue, structsArg, "", "Comma-separated list of protected structs") } @@ -90,29 +94,45 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } -// setUpFromInput initializes EntityFile and Structs from cfg or CLI, then builds ProtectedStructsMap. +// setUpFromInput initializes EntityFiles and Structs from cfg or CLI, then builds ProtectedStructsMap. +// run() may be invoked concurrently per package (e.g. under golangci-lint); sync.Once keeps +// initialization race-free and single-shot so the global slices don't grow across packages. func setUpFromInput() { - tryInitFromCfg() - if EntityFile == "" && len(Structs) == 0 { - tryInitFromCLI() - } - buildProtectedStructMap() + initOnce.Do(func() { + tryInitFromCfg() + if len(EntityFiles) == 0 && len(Structs) == 0 { + tryInitFromCLI() + } + buildProtectedStructMap() + }) } func tryInitFromCfg() { - if v, ok := cfg[entityListFileArg].(string); ok && v != "" { - EntityFile = v + switch v := cfg[entityListFilesArg].(type) { + case []string: + EntityFiles = append(EntityFiles, v...) + case []any: + for _, item := range v { + if s, ok := item.(string); ok && s != "" { + EntityFiles = append(EntityFiles, s) + } + } } if v, ok := cfg[structsArg].([]string); ok && len(v) > 0 { Structs = append(Structs, v...) } } -// tryInitFromCLI initializes EntityFile and Structs from CLI flags. +// tryInitFromCLI initializes EntityFiles and Structs from CLI flags. func tryInitFromCLI() { - entityListNameFlag := flagSet.Lookup(entityListFileArg) - if entityListNameFlag != nil && entityListNameFlag.Value.String() != "" { - EntityFile = strings.TrimSpace(entityListNameFlag.Value.String()) + entityListFilesFlag := flagSet.Lookup(entityListFilesArg) + if entityListFilesFlag != nil && entityListFilesFlag.Value.String() != "" { + for _, part := range strings.Split(entityListFilesFlag.Value.String(), ",") { + part = strings.TrimSpace(part) + if part != "" { + EntityFiles = append(EntityFiles, part) + } + } } structsFlag := flagSet.Lookup(structsArg) @@ -129,15 +149,14 @@ func tryInitFromCLI() { // buildProtectedStructMap populates ProtectedStructsMap and sets protectAllStructs when empty. func buildProtectedStructMap() { - if len(ProtectedStructsMap) > 0 || protectAllStructs { - // Concurrency expected: already built - return - } - ProtectedStructsMap = make(map[string]bool) - if EntityFile != "" { - for k := range loadEntityList(EntityFile) { + for _, file := range EntityFiles { + file = strings.TrimSpace(file) + if file == "" { + continue + } + for k := range loadEntityList(file) { ProtectedStructsMap[k] = true } } diff --git a/pkg/analyzer/analyzer_test.go b/pkg/analyzer/analyzer_test.go index 4d02b78..d93289c 100644 --- a/pkg/analyzer/analyzer_test.go +++ b/pkg/analyzer/analyzer_test.go @@ -4,6 +4,7 @@ import ( "flag" "os" "path/filepath" + "sync" "testing" "golang.org/x/tools/go/analysis/analysistest" @@ -11,8 +12,10 @@ import ( func setUp() string { ProtectedStructsMap = make(map[string]bool) - EntityFile = "" + protectAllStructs = false + EntityFiles = nil Structs = []string{} + initOnce = sync.Once{} path, _ := os.Getwd() testdata := filepath.Join(filepath.Dir(filepath.Dir(path)), "testdata") @@ -24,7 +27,7 @@ func TestWithEntityFileParameter(t *testing.T) { testdata := setUp() cfg := map[string]any{ - entityListFileArg: filepath.Join(testdata, "src/config/entities.go"), + entityListFilesArg: []string{filepath.Join(testdata, "src/config/entities.go")}, } analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") @@ -44,8 +47,8 @@ func TestWithEntityFileAndStructsWithOverlap(t *testing.T) { testdata := setUp() cfg := map[string]any{ // contains UnProtectedEntity to test that only specified structs are protected - entityListFileArg: filepath.Join(testdata, "src/config/entities.go"), - structsArg: []string{"Entity", "SubEntity"}, + entityListFilesArg: []string{filepath.Join(testdata, "src/config/entities.go")}, + structsArg: []string{"Entity", "SubEntity"}, } analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") @@ -55,8 +58,8 @@ func TestWithEntityFileAndStructsComposed(t *testing.T) { testdata := setUp() cfg := map[string]any{ // contains UnProtectedEntity to test that only specified structs are protected - entityListFileArg: filepath.Join(testdata, "src/config2/entities.go"), // Entity - structsArg: []string{"SubEntity"}, + entityListFilesArg: []string{filepath.Join(testdata, "src/config2/entities.go")}, // Entity + structsArg: []string{"SubEntity"}, } // Test twice to simulate concurrent runs which reuse already set up configuration. @@ -64,10 +67,61 @@ func TestWithEntityFileAndStructsComposed(t *testing.T) { analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") } +func TestWithMultipleEntityFiles(t *testing.T) { + testdata := setUp() + cfg := map[string]any{ + entityListFilesArg: []string{ + filepath.Join(testdata, "src/config/entities.go"), // Entity, SubEntity from protectall + filepath.Join(testdata, "src/config2/entities.go"), // Entity from protectselected + }, + } + + analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") +} + +func TestWithNonExistentEntityFile(t *testing.T) { + testdata := setUp() + cfg := map[string]any{ + entityListFilesArg: []string{ + filepath.Join(testdata, "src/does_not_exist.go"), + filepath.Join(testdata, "src/config/entities.go"), + }, + } + + analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") +} + +func TestWithEmptyAndWhitespaceEntityFilePaths_fallsBackToAllStructs(t *testing.T) { + testdata := setUp() + + // An entity-list-files entry that is empty or whitespace must not be treated as "provided"; + // with no real paths and no structs, the linter falls back to protecting all structs. + cfg := map[string]any{ + entityListFilesArg: []string{"", " "}, + } + + analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectall") +} + +func TestTryInitFromCfg_WithAnySliceOfEntityFiles(t *testing.T) { + testdata := setUp() + + // golangci-lint may decode YAML list values as []any rather than []string. + cfg := map[string]any{ + entityListFilesArg: []any{ + filepath.Join(testdata, "src/config/entities.go"), + "", + 123, // non-string entries are ignored + }, + } + + analysistest.Run(t, testdata, NewAnalyzer(cfg), "protectselected") +} + func TestWithEntityFileWhichDoesNotCompile(t *testing.T) { testdata := setUp() cfg := map[string]any{ - entityListFileArg: filepath.Join(testdata, "src/config3/entities.go.txt"), + entityListFilesArg: []string{filepath.Join(testdata, "src/config3/entities.go.txt")}, structsArg: []string{ "UnProtectedEntity", "Entity", "SubEntity", "Entity2", "SubEntity2", "SubSubEntity2", "Entity3", "SubEntity3", "SubSubEntity3", "Entity4", "SubEntity4", "SubSubEntity4", @@ -89,11 +143,11 @@ func TestTryInitFromCLI(t *testing.T) { _ = setUp() fs := flag.NewFlagSet("test", flag.ContinueOnError) - fs.String(entityListFileArg, "", "path to file containing list of entities") + fs.String(entityListFilesArg, "", "comma-separated list of paths to files containing list of entities") fs.String(structsArg, "", "comma separated list of structs to protect") _ = fs.Set(structsArg, " Entity , Entity2") - _ = fs.Set(entityListFileArg, " /path/to/file.go ") + _ = fs.Set(entityListFilesArg, " /path/to/file1.go , /path/to/file2.go ") flagSet = *fs tryInitFromCLI() @@ -101,7 +155,7 @@ func TestTryInitFromCLI(t *testing.T) { if len(Structs) != 2 || Structs[0] != "Entity" || Structs[1] != "Entity2" { t.Errorf("tryInitFromCLI did not set Structs correctly, got: %v", Structs) } - if EntityFile != "/path/to/file.go" { - t.Errorf("tryInitFromCLI did not set EntityFile correctly, got: %s", EntityFile) + if len(EntityFiles) != 2 || EntityFiles[0] != "/path/to/file1.go" || EntityFiles[1] != "/path/to/file2.go" { + t.Errorf("tryInitFromCLI did not set EntityFiles correctly, got: %v", EntityFiles) } }