From 1a481fad708af5d6a9e9f29127ce211f31e459a7 Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Fri, 2 Dec 2016 10:40:36 -0800 Subject: [PATCH] adding support for arbitrary paths with ... --- .gitignore | 2 + filelist.go | 68 +++--- filelist_test.go | 245 +++++++++++++++++++++ main.go | 139 ++++++++---- main_test.go | 45 ++++ vendor.conf | 2 + vendor/github.com/ryanuber/go-glob/LICENSE | 21 ++ vendor/github.com/ryanuber/go-glob/glob.go | 51 +++++ 8 files changed, 490 insertions(+), 83 deletions(-) create mode 100644 filelist_test.go create mode 100644 main_test.go create mode 100644 vendor.conf create mode 100644 vendor/github.com/ryanuber/go-glob/LICENSE create mode 100644 vendor/github.com/ryanuber/go-glob/glob.go diff --git a/.gitignore b/.gitignore index 6f7e30869..b0326bdbf 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,5 @@ _testmain.go *.prof .DS_Store + +.vscode diff --git a/filelist.go b/filelist.go index 86e144cfc..3280edd89 100644 --- a/filelist.go +++ b/filelist.go @@ -15,60 +15,58 @@ package main import ( - "fmt" - "os" - "path/filepath" + "log" "strings" + + "github.com/ryanuber/go-glob" ) -type filelist struct { - paths map[string]bool - globs []string +// fileList uses a map for patterns to ensure each pattern only +// appears once +type fileList struct { + patterns map[string]struct{} } -func newFileList(paths ...string) *filelist { - - f := &filelist{ - make(map[string]bool), - make([]string, 0), +func newFileList(paths ...string) *fileList { + f := &fileList{ + patterns: make(map[string]struct{}), } - - for _, path := range paths { - if e := f.Set(path); e != nil { - // #nosec - fmt.Fprintf(os.Stderr, "Unable to add %s to filelist: %s\n", path, e) - } + for _, p := range paths { + f.patterns[p] = struct{}{} } return f } -func (f *filelist) String() string { - return strings.Join(f.globs, ", ") +func (f *fileList) String() string { + ps := make([]string, 0, len(f.patterns)) + for p := range f.patterns { + ps = append(ps, p) + } + return strings.Join(ps, ", ") } -func (f *filelist) Set(path string) error { - f.globs = append(f.globs, path) - matches, e := filepath.Glob(path) - if e != nil { - return e - } - for _, each := range matches { - abs, e := filepath.Abs(each) - if e != nil { - return e - } - f.paths[abs] = true +func (f *fileList) Set(path string) error { + if path == "" { + // don't bother adding the empty path + return nil } + f.patterns[path] = struct{}{} return nil } -func (f filelist) Contains(path string) bool { - _, present := f.paths[path] - return present +func (f fileList) Contains(path string) bool { + for p := range f.patterns { + if glob.Glob(p, path) { + log.Printf("excluding: %s\n", path) + return true + } + } + log.Printf("including: %s\n", path) + return false } /* -func (f filelist) Dump() { +func (f fileList) Dump() { for k, _ := range f.paths { println(k) } diff --git a/filelist_test.go b/filelist_test.go new file mode 100644 index 000000000..61421fa85 --- /dev/null +++ b/filelist_test.go @@ -0,0 +1,245 @@ +package main + +import ( + "reflect" + "testing" +) + +func Test_newFileList(t *testing.T) { + type args struct { + paths []string + } + tests := []struct { + name string + args args + want *fileList + }{ + { + name: "nil paths", + args: args{paths: nil}, + want: &fileList{patterns: map[string]struct{}{}}, + }, + { + name: "empty paths", + args: args{paths: []string{}}, + want: &fileList{patterns: map[string]struct{}{}}, + }, + { + name: "have paths", + args: args{paths: []string{"*_test.go"}}, + want: &fileList{patterns: map[string]struct{}{ + "*_test.go": struct{}{}, + }}, + }, + } + for _, tt := range tests { + if got := newFileList(tt.args.paths...); !reflect.DeepEqual(got, tt.want) { + t.Errorf("%q. newFileList() = %v, want %v", tt.name, got, tt.want) + } + } +} + +func Test_fileList_String(t *testing.T) { + type fields struct { + patterns []string + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "nil patterns", + fields: fields{patterns: nil}, + want: "", + }, + { + name: "empty patterns", + fields: fields{patterns: []string{}}, + want: "", + }, + { + name: "one pattern", + fields: fields{patterns: []string{"foo"}}, + want: "foo", + }, + { + name: "two patterns", + fields: fields{patterns: []string{"foo", "bar"}}, + want: "foo, bar", + }, + } + for _, tt := range tests { + f := newFileList(tt.fields.patterns...) + if got := f.String(); got != tt.want { + t.Errorf("%q. fileList.String() = %v, want %v", tt.name, got, tt.want) + } + } +} + +func Test_fileList_Set(t *testing.T) { + type fields struct { + patterns []string + } + type args struct { + path string + } + tests := []struct { + name string + fields fields + args args + want map[string]struct{} + wantErr bool + }{ + { + name: "add empty path", + fields: fields{patterns: nil}, + args: args{path: ""}, + want: map[string]struct{}{}, + wantErr: false, + }, + { + name: "add path to nil patterns", + fields: fields{patterns: nil}, + args: args{path: "foo"}, + want: map[string]struct{}{ + "foo": struct{}{}, + }, + wantErr: false, + }, + { + name: "add path to empty patterns", + fields: fields{patterns: []string{}}, + args: args{path: "foo"}, + want: map[string]struct{}{ + "foo": struct{}{}, + }, + wantErr: false, + }, + { + name: "add path to populated patterns", + fields: fields{patterns: []string{"foo"}}, + args: args{path: "bar"}, + want: map[string]struct{}{ + "foo": struct{}{}, + "bar": struct{}{}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + f := newFileList(tt.fields.patterns...) + if err := f.Set(tt.args.path); (err != nil) != tt.wantErr { + t.Errorf("%q. fileList.Set() error = %v, wantErr %v", tt.name, err, tt.wantErr) + } + if !reflect.DeepEqual(f.patterns, tt.want) { + t.Errorf("%q. got state fileList.patterns = %v, want state %v", tt.name, f.patterns, tt.want) + } + } +} + +func Test_fileList_Contains(t *testing.T) { + type fields struct { + patterns []string + } + type args struct { + path string + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "nil patterns", + fields: fields{patterns: nil}, + args: args{path: "foo"}, + want: false, + }, + { + name: "empty patterns", + fields: fields{patterns: nil}, + args: args{path: "foo"}, + want: false, + }, + { + name: "one pattern, no wildcard, no match", + fields: fields{patterns: []string{"foo"}}, + args: args{path: "bar"}, + want: false, + }, + { + name: "one pattern, no wildcard, match", + fields: fields{patterns: []string{"foo"}}, + args: args{path: "foo"}, + want: true, + }, + { + name: "one pattern, wildcard prefix, match", + fields: fields{patterns: []string{"*foo"}}, + args: args{path: "foo"}, + want: true, + }, + { + name: "one pattern, wildcard suffix, match", + fields: fields{patterns: []string{"foo*"}}, + args: args{path: "foo"}, + want: true, + }, + { + name: "one pattern, wildcard both ends, match", + fields: fields{patterns: []string{"*foo*"}}, + args: args{path: "foo"}, + want: true, + }, + { + name: "default test match 1", + fields: fields{patterns: []string{"*_test.go"}}, + args: args{path: "foo_test.go"}, + want: true, + }, + { + name: "default test match 2", + fields: fields{patterns: []string{"*_test.go"}}, + args: args{path: "bar/foo_test.go"}, + want: true, + }, + { + name: "default test match 3", + fields: fields{patterns: []string{"*_test.go"}}, + args: args{path: "/bar/foo_test.go"}, + want: true, + }, + { + name: "default test match 4", + fields: fields{patterns: []string{"*_test.go"}}, + args: args{path: "baz/bar/foo_test.go"}, + want: true, + }, + { + name: "default test match 5", + fields: fields{patterns: []string{"*_test.go"}}, + args: args{path: "/baz/bar/foo_test.go"}, + want: true, + }, + { + name: "many patterns, no match", + fields: fields{patterns: []string{"*_one.go", "*_two.go"}}, + args: args{path: "/baz/bar/foo_test.go"}, + want: false, + }, + { + name: "many patterns, match", + fields: fields{patterns: []string{"*_one.go", "*_two.go", "*_test.go"}}, + args: args{path: "/baz/bar/foo_test.go"}, + want: true, + }, + } + for _, tt := range tests { + f := newFileList(tt.fields.patterns...) + if got := f.Contains(tt.args.path); got != tt.want { + t.Errorf("%q. fileList.Contains() = %v, want %v", tt.name, got, tt.want) + } + } +} diff --git a/main.go b/main.go index 248f548c7..86860602e 100644 --- a/main.go +++ b/main.go @@ -29,22 +29,30 @@ import ( "github.com/GoASTScanner/gas/output" ) -// #nosec flag -var flagIgnoreNoSec = flag.Bool("nosec", false, "Ignores #nosec comments when set") +type recursion bool -// format output -var flagFormat = flag.String("fmt", "text", "Set output format. Valid options are: json, csv, html, or text") +const ( + recurse recursion = true + noRecurse recursion = false +) + +var ( + // #nosec flag + flagIgnoreNoSec = flag.Bool("nosec", false, "Ignores #nosec comments when set") + + // format output + flagFormat = flag.String("fmt", "text", "Set output format. Valid options are: json, csv, html, or text") -// output file -var flagOutput = flag.String("out", "", "Set output file for results") + // output file + flagOutput = flag.String("out", "", "Set output file for results") -// config file -var flagConfig = flag.String("conf", "", "Path to optional config file") + // config file + flagConfig = flag.String("conf", "", "Path to optional config file") -// quiet -var flagQuiet = flag.Bool("quiet", false, "Only show output when errors are found") + // quiet + flagQuiet = flag.Bool("quiet", false, "Only show output when errors are found") -var usageText = ` + usageText = ` GAS - Go AST Scanner Gas analyzes Go source code to look for common programming mistakes that @@ -67,7 +75,8 @@ USAGE: ` -var logger *log.Logger + logger *log.Logger +) func extendConfList(conf map[string]interface{}, name string, inputStr string) { if inputStr == "" { @@ -145,7 +154,7 @@ func main() { flag.Usage = usage // Exclude files - excluded := newFileList("**/*_test.go") + excluded := newFileList("*_test.go") flag.Var(excluded, "skip", "File pattern to exclude from scan") incRules := "" @@ -183,42 +192,11 @@ func main() { analyzer := gas.NewAnalyzer(config, logger) AddRules(&analyzer, config) - // Traverse directory structure if './...' - if flag.NArg() == 1 && flag.Arg(0) == "./..." { + toAnalyze := getFilesToAnalyze(flag.Args(), excluded) - cwd, err := os.Getwd() - if err != nil { - logger.Fatalf("Unable to traverse path %s, reason - %s", flag.Arg(0), err) - } - filepath.Walk(cwd, func(path string, info os.FileInfo, err error) error { - if excluded.Contains(path) && info.IsDir() { - logger.Printf("Skipping %s\n", path) - return filepath.SkipDir - } - if !info.IsDir() && !excluded.Contains(path) && - strings.HasSuffix(path, ".go") { - err = analyzer.Process(path) - if err != nil { - logger.Fatal(err) - } - } - return nil - }) - - } else { - - // Process each file individually - for _, filename := range flag.Args() { - if finfo, err := os.Stat(filename); err == nil { - if !finfo.IsDir() && !excluded.Contains(filename) && - strings.HasSuffix(filename, ".go") { - if err = analyzer.Process(filename); err != nil { - logger.Fatal(err) - } - } - } else { - logger.Fatal(err) - } + for _, file := range toAnalyze { + if err := analyzer.Process(file); err != nil { + logger.Fatal(err) } } @@ -245,3 +223,68 @@ func main() { os.Exit(1) } } + +// getFilesToAnalyze lists all files +func getFilesToAnalyze(paths []string, excluded *fileList) []string { + log.Println("getFilesToAnalyze: start") + var toAnalyze []string + for _, path := range paths { + log.Printf("getFilesToAnalyze: processing \"%s\"\n", path) + // get the absolute path before doing anything else + path, err := filepath.Abs(path) + if err != nil { + log.Fatal(err) + } + if filepath.Base(path) == "..." { + toAnalyze = append( + toAnalyze, + listFiles(filepath.Dir(path), recurse, excluded)..., + ) + } else { + var ( + finfo os.FileInfo + err error + ) + if finfo, err = os.Stat(path); err != nil { + logger.Fatal(err) + } + if !finfo.IsDir() { + if shouldInclude(path, excluded) { + toAnalyze = append(toAnalyze, path) + } + } else { + toAnalyze = listFiles(path, noRecurse, excluded) + } + } + } + log.Println("getFilesToAnalyze: end") + return toAnalyze +} + +// listFiles returns a list of all files found that pass the shouldInclude check. +// If doRecursiveWalk it true, it will walk the tree rooted at absPath, otherwise it +// will only include files directly within the dir referenced by absPath. +func listFiles(absPath string, doRecursiveWalk recursion, excluded *fileList) []string { + var files []string + + walk := func(path string, info os.FileInfo, err error) error { + if info.IsDir() && doRecursiveWalk == noRecurse { + return filepath.SkipDir + } + if shouldInclude(path, excluded) { + files = append(files, path) + } + return nil + } + + if err := filepath.Walk(absPath, walk); err != nil { + log.Fatal(err) + } + return files +} + +// shouldInclude checks if a specific path which is expected to reference +// a regular file should be included +func shouldInclude(path string, excluded *fileList) bool { + return filepath.Ext(path) == ".go" && !excluded.Contains(path) +} diff --git a/main_test.go b/main_test.go new file mode 100644 index 000000000..b47a4b518 --- /dev/null +++ b/main_test.go @@ -0,0 +1,45 @@ +package main + +import "testing" + +func Test_shouldInclude(t *testing.T) { + type args struct { + path string + excluded *fileList + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "non .go file", + args: args{ + path: "thing.txt", + excluded: newFileList(), + }, + want: false, + }, + { + name: ".go file, not excluded", + args: args{ + path: "thing.go", + excluded: newFileList(), + }, + want: true, + }, + { + name: ".go file, excluded", + args: args{ + path: "thing.go", + excluded: newFileList("thing.go"), + }, + want: false, + }, + } + for _, tt := range tests { + if got := shouldInclude(tt.args.path, tt.args.excluded); got != tt.want { + t.Errorf("%q. shouldInclude() = %v, want %v", tt.name, got, tt.want) + } + } +} diff --git a/vendor.conf b/vendor.conf new file mode 100644 index 000000000..323514faa --- /dev/null +++ b/vendor.conf @@ -0,0 +1,2 @@ +# Import path | revision | Repository(optional) +github.com/ryanuber/go-glob 572520ed46dbddaed19ea3d9541bdd0494163693 diff --git a/vendor/github.com/ryanuber/go-glob/LICENSE b/vendor/github.com/ryanuber/go-glob/LICENSE new file mode 100644 index 000000000..bdfbd9514 --- /dev/null +++ b/vendor/github.com/ryanuber/go-glob/LICENSE @@ -0,0 +1,21 @@ +The MIT License (MIT) + +Copyright (c) 2014 Ryan Uber + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/vendor/github.com/ryanuber/go-glob/glob.go b/vendor/github.com/ryanuber/go-glob/glob.go new file mode 100644 index 000000000..d9d46379a --- /dev/null +++ b/vendor/github.com/ryanuber/go-glob/glob.go @@ -0,0 +1,51 @@ +package glob + +import "strings" + +// The character which is treated like a glob +const GLOB = "*" + +// Glob will test a string pattern, potentially containing globs, against a +// subject string. The result is a simple true/false, determining whether or +// not the glob pattern matched the subject text. +func Glob(pattern, subj string) bool { + // Empty pattern can only match empty subject + if pattern == "" { + return subj == pattern + } + + // If the pattern _is_ a glob, it matches everything + if pattern == GLOB { + return true + } + + parts := strings.Split(pattern, GLOB) + + if len(parts) == 1 { + // No globs in pattern, so test for equality + return subj == pattern + } + + leadingGlob := strings.HasPrefix(pattern, GLOB) + trailingGlob := strings.HasSuffix(pattern, GLOB) + end := len(parts) - 1 + + // Check the first section. Requires special handling. + if !leadingGlob && !strings.HasPrefix(subj, parts[0]) { + return false + } + + // Go over the middle parts and ensure they match. + for i := 1; i < end; i++ { + if !strings.Contains(subj, parts[i]) { + return false + } + + // Trim evaluated text from subj as we loop over the pattern. + idx := strings.Index(subj, parts[i]) + len(parts[i]) + subj = subj[idx:] + } + + // Reached the last section. Requires special handling. + return trailingGlob || strings.HasSuffix(subj, parts[end]) +}