From a0ac9b7e0b980a30242af748f78954fb7137a531 Mon Sep 17 00:00:00 2001 From: golangci Date: Sat, 5 May 2018 11:08:14 +0300 Subject: [PATCH] prepare testing framework --- Gopkg.lock | 103 +---------------------------- cmd/golangci-lint/main.go | 3 + internal/commands/run.go | 16 +++-- pkg/config/config.go | 5 +- pkg/golinters/errcheck_test.go | 60 ----------------- pkg/golinters/gofmt.go | 4 +- pkg/golinters/linter.go | 34 ++++++---- pkg/golinters/linter_test.go | 50 ++++++++++++++ pkg/golinters/supported_linters.go | 2 +- pkg/golinters/test.go | 2 +- pkg/golinters/testdata/errcheck.go | 34 ++++++++++ pkg/linter.go | 3 +- pkg/runner.go | 2 +- 13 files changed, 133 insertions(+), 185 deletions(-) delete mode 100644 pkg/golinters/errcheck_test.go create mode 100644 pkg/golinters/linter_test.go create mode 100644 pkg/golinters/testdata/errcheck.go diff --git a/Gopkg.lock b/Gopkg.lock index 2dfbec21..fd08f78f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -31,12 +31,6 @@ revision = "507f6050b8568533fb3f5504de8e5205fa62a114" version = "v1.6.0" -[[projects]] - name = "github.com/fsnotify/fsnotify" - packages = ["."] - revision = "c2828203cd70a50dcccfb2761f8b1f8ceef9a8e9" - version = "v1.4.7" - [[projects]] name = "github.com/go-ole/go-ole" packages = [ @@ -67,24 +61,7 @@ "pkg/runmode", "pkg/timeutils" ] - revision = "044f3332f2e8c38cfbb56bab29f65b4245bbe76b" - -[[projects]] - branch = "master" - name = "github.com/hashicorp/hcl" - packages = [ - ".", - "hcl/ast", - "hcl/parser", - "hcl/printer", - "hcl/scanner", - "hcl/strconv", - "hcl/token", - "json/parser", - "json/scanner", - "json/token" - ] - revision = "ef8a98b0bbce4a65b5aa4c368430a80ddc533168" + revision = "841f39765a89eddef59ce611612775291b83594b" [[projects]] name = "github.com/inconshreveable/mousetrap" @@ -92,12 +69,6 @@ revision = "76626ae9c91c4f2a10f34cad8ce83ea42c93bb75" version = "v1.0" -[[projects]] - name = "github.com/magiconair/properties" - packages = ["."] - revision = "c3beff4c2358b44d0493c7dda585e7db7ff28ae6" - version = "v1.7.6" - [[projects]] name = "github.com/mattn/go-colorable" packages = ["."] @@ -110,24 +81,6 @@ revision = "0360b2af4f38e8d38c7fce2a9f4e702702d73a39" version = "v0.0.3" -[[projects]] - branch = "master" - name = "github.com/mitchellh/go-homedir" - packages = ["."] - revision = "b8bc1bf767474819792c23f32d8286a45736f1c6" - -[[projects]] - branch = "master" - name = "github.com/mitchellh/mapstructure" - packages = ["."] - revision = "00c29f56e2386353d58c599509e8dc3801b0d716" - -[[projects]] - name = "github.com/pelletier/go-toml" - packages = ["."] - revision = "acdc4509485b587f5e675510c4f2c63e90ff68a8" - version = "v1.1.0" - [[projects]] name = "github.com/pmezard/go-difflib" packages = ["difflib"] @@ -165,49 +118,18 @@ revision = "c155da19408a8799da419ed3eeb0cb5db0ad5dbc" version = "v1.0.5" -[[projects]] - name = "github.com/spf13/afero" - packages = [ - ".", - "mem" - ] - revision = "63644898a8da0bc22138abf860edaf5277b6102e" - version = "v1.1.0" - -[[projects]] - name = "github.com/spf13/cast" - packages = ["."] - revision = "8965335b8c7107321228e3e3702cab9832751bac" - version = "v1.2.0" - [[projects]] name = "github.com/spf13/cobra" - packages = [ - ".", - "cobra", - "cobra/cmd" - ] + packages = ["."] revision = "a1f051bc3eba734da4772d60e2d677f47cf93ef4" version = "v0.0.2" -[[projects]] - branch = "master" - name = "github.com/spf13/jwalterweatherman" - packages = ["."] - revision = "7c0cea34c8ece3fbeb2b27ab9b59511d360fb394" - [[projects]] name = "github.com/spf13/pflag" packages = ["."] revision = "583c0c0531f06d5278b7d917446061adc344b5cd" version = "v1.0.1" -[[projects]] - name = "github.com/spf13/viper" - packages = ["."] - revision = "b5e8006cbee93ec955a89ab31e0e3ce3204f3736" - version = "v1.0.2" - [[projects]] name = "github.com/stretchr/testify" packages = ["assert"] @@ -244,25 +166,6 @@ ] revision = "6f686a352de66814cdd080d970febae7767857a3" -[[projects]] - name = "golang.org/x/text" - packages = [ - "internal/gen", - "internal/triegen", - "internal/ucd", - "transform", - "unicode/cldr", - "unicode/norm" - ] - revision = "f21a4dfb5e38f5895301dc265a8def02365cc3d0" - version = "v0.3.0" - -[[projects]] - name = "gopkg.in/yaml.v2" - packages = ["."] - revision = "5420a8b6744d3b0345ab293f6fcba19c978f1183" - version = "v2.2.1" - [[projects]] branch = "master" name = "sourcegraph.com/sourcegraph/go-diff" @@ -278,6 +181,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "e734f3b5571fe0e9119d20d9e9a7f8ee2b2f316686cf194307a3e5fb3327c85b" + inputs-digest = "e924e6f11d7b225c9680fc50df9661941727487bae2c0b819733d10d757e606b" solver-name = "gps-cdcl" solver-version = 1 diff --git a/cmd/golangci-lint/main.go b/cmd/golangci-lint/main.go index 2dac78a9..0f26afcb 100644 --- a/cmd/golangci-lint/main.go +++ b/cmd/golangci-lint/main.go @@ -4,10 +4,13 @@ import ( "log" "github.com/golangci/golangci-lint/internal/commands" + "github.com/golangci/golangci-shared/pkg/analytics" + "github.com/sirupsen/logrus" ) func main() { log.SetFlags(0) // don't print time + analytics.SetLogLevel(logrus.WarnLevel) e := commands.NewExecutor() if err := e.Execute(); err != nil { diff --git a/internal/commands/run.go b/internal/commands/run.go index 532359ea..ca72fed2 100644 --- a/internal/commands/run.go +++ b/internal/commands/run.go @@ -6,7 +6,6 @@ import ( "fmt" "log" "os" - "path/filepath" "strings" "github.com/fatih/color" @@ -28,6 +27,8 @@ func (e *Executor) initRun() { runCmd.Flags().StringVarP(&e.cfg.Run.OutFormat, "out-format", "", config.OutFormatColoredLineNumber, fmt.Sprintf("Format of output: %s", strings.Join(config.OutFormats, "|"))) + runCmd.Flags().IntVarP(&e.cfg.Run.ExitCodeIfIssuesFound, "issues-exit-code", "", + 1, "Exit code when issues were found") } func (e Executor) executeRun(cmd *cobra.Command, args []string) { @@ -35,16 +36,17 @@ func (e Executor) executeRun(cmd *cobra.Command, args []string) { linters := golinters.GetSupportedLinters() ctx := context.Background() - ex, err := os.Executable() + pwd, err := os.Getwd() if err != nil { return err } - exPath := filepath.Dir(ex) - exec := executors.NewShell(exPath) + exec := executors.NewShell(pwd) + + e.cfg.Run.Paths = args issues := []result.Issue{} for _, linter := range linters { - res, err := linter.Run(ctx, exec) + res, err := linter.Run(ctx, exec, &e.cfg.Run) if err != nil { return err } @@ -55,6 +57,10 @@ func (e Executor) executeRun(cmd *cobra.Command, args []string) { return fmt.Errorf("can't output %d issues: %s", len(issues), err) } + if len(issues) != 0 { + os.Exit(e.cfg.Run.ExitCodeIfIssuesFound) + } + return nil } diff --git a/pkg/config/config.go b/pkg/config/config.go index 99a18262..72ef71bb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -15,8 +15,9 @@ type Common struct { } type Run struct { - Paths []string - OutFormat string + Paths []string + OutFormat string + ExitCodeIfIssuesFound int } type Config struct { diff --git a/pkg/golinters/errcheck_test.go b/pkg/golinters/errcheck_test.go deleted file mode 100644 index 5afa444d..00000000 --- a/pkg/golinters/errcheck_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package golinters - -import ( - "testing" - - "github.com/golangci/golangci-lint/pkg/result" -) - -func TestErrcheckSimple(t *testing.T) { - const source = `package p - - func retErr() error { - return nil - } - - func missedErrorCheck() { - retErr() - } -` - - ExpectIssues(t, errCheck, source, []result.Issue{NewIssue("errcheck", "Error return value is not checked", 8)}) -} - -func TestErrcheckIgnoreClose(t *testing.T) { - sources := []string{`package p - - import "os" - - func ok() error { - f, err := os.Open("t.go") - if err != nil { - return err - } - - f.Close() - return nil - } -`, - `package p - -import "net/http" - -func f() { - resp, err := http.Get("http://example.com/") - if err != nil { - panic(err) - } - defer resp.Body.Close() - - panic(resp) -} -`} - - for _, source := range sources { - ExpectIssues(t, errCheck, source, []result.Issue{}) - } -} - -// TODO: add cases of non-compiling code -// TODO: don't report issues if got more than 20 issues diff --git a/pkg/golinters/gofmt.go b/pkg/golinters/gofmt.go index 283974a1..01a3f97f 100644 --- a/pkg/golinters/gofmt.go +++ b/pkg/golinters/gofmt.go @@ -5,6 +5,7 @@ import ( "context" "fmt" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-shared/pkg/analytics" "github.com/golangci/golangci-shared/pkg/executors" @@ -81,7 +82,8 @@ func (g gofmt) extractIssuesFromPatch(patch string) ([]result.Issue, error) { return issues, nil } -func (g gofmt) Run(ctx context.Context, exec executors.Executor) (*result.Result, error) { +func (g gofmt) Run(ctx context.Context, exec executors.Executor, cfg *config.Run) (*result.Result, error) { + // TODO: cfg support paths, err := getPathsForGoProject(exec.WorkDir()) if err != nil { return nil, fmt.Errorf("can't get files to analyze: %s", err) diff --git a/pkg/golinters/linter.go b/pkg/golinters/linter.go index ee49faa5..650126b0 100644 --- a/pkg/golinters/linter.go +++ b/pkg/golinters/linter.go @@ -12,6 +12,7 @@ import ( "syscall" "text/template" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-shared/pkg/analytics" "github.com/golangci/golangci-shared/pkg/executors" @@ -85,25 +86,32 @@ func getOutTail(out string, linesCount int) string { return strings.Join(lines[len(lines)-linesCount:], "\n") } -func (lint linter) Run(ctx context.Context, exec executors.Executor) (*result.Result, error) { - paths, err := getPathsForGoProject(exec.WorkDir()) - if err != nil { - return nil, fmt.Errorf("can't get files to analyze: %s", err) +func (lint linter) Run(ctx context.Context, exec executors.Executor, cfg *config.Run) (*result.Result, error) { + paths := &ProjectPaths{ + files: cfg.Paths, + dirs: []string{}, // TODO + } + var err error + if len(cfg.Paths) == 0 { + paths, err = getPathsForGoProject(exec.WorkDir()) + if err != nil { + return nil, fmt.Errorf("can't get files to analyze: %s", err) + } } retIssues := []result.Issue{} - const maxDirsPerRun = 100 // run one linter multiple times with groups of dirs: limit memory usage in the cost of higher CPU usage + const maxFilesPerRun = 100 // run one linter multiple times with groups of files: limit memory usage in the cost of higher CPU usage - for len(paths.dirs) != 0 { + for len(paths.files) != 0 { args := append([]string{}, lint.args...) - dirsCount := len(paths.dirs) - if dirsCount > maxDirsPerRun { - dirsCount = maxDirsPerRun + filesCount := len(paths.files) + if filesCount > maxFilesPerRun { + filesCount = maxFilesPerRun } - dirs := paths.dirs[:dirsCount] - args = append(args, dirs...) + files := paths.files[:filesCount] + args = append(args, files...) out, err := exec.Run(ctx, lint.name, args...) if err != nil && !lint.doesExitCodeMeansIssuesWereFound(err) { @@ -114,7 +122,7 @@ func (lint linter) Run(ctx context.Context, exec executors.Executor) (*result.Re issues := lint.parseLinterOut(out) retIssues = append(retIssues, issues...) - paths.dirs = paths.dirs[dirsCount:] + paths.files = paths.files[filesCount:] } return &result.Result{ @@ -176,7 +184,7 @@ func (lint linter) parseLinterOut(out string) []result.Issue { for scanner.Scan() { vars, err := lint.parseLinterOutLine(scanner.Text()) if err != nil { - analytics.Log(context.TODO()).Warnf("Can't parse linter out line: %s", err) + analytics.Log(context.TODO()).Warnf("Can't parse linter %s out line: %s", lint.Name(), err) continue } diff --git a/pkg/golinters/linter_test.go b/pkg/golinters/linter_test.go new file mode 100644 index 00000000..d395f853 --- /dev/null +++ b/pkg/golinters/linter_test.go @@ -0,0 +1,50 @@ +package golinters + +import ( + "bytes" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" +) + +func runGoErrchk(c *exec.Cmd, t *testing.T) { + output, err := c.CombinedOutput() + assert.NoError(t, err, "Output:\n%s", output) + + // Can't check exit code: tool only prints to output + assert.False(t, bytes.Contains(output, []byte("BUG")), "Output:\n%s", output) +} + +const testdataDir = "testdata" +const binName = "golangci-lint" + +func TestSourcesFromTestdataDir(t *testing.T) { + t.Log(filepath.Join(testdataDir, "*.go")) + sources, err := filepath.Glob(filepath.Join(testdataDir, "*.go")) + assert.NoError(t, err) + assert.NotEmpty(t, sources) + + installBinary(t) + + for _, s := range sources { + s := s + t.Run(s, func(t *testing.T) { + t.Parallel() + testOneSource(t, s) + }) + } +} + +func installBinary(t *testing.T) { + cmd := exec.Command("go", "install", filepath.Join("..", "..", "cmd", binName)) + assert.NoError(t, cmd.Run()) +} + +func testOneSource(t *testing.T, sourcePath string) { + goErrchkBin := filepath.Join(runtime.GOROOT(), "test", "errchk") + cmd := exec.Command(goErrchkBin, binName, "run", sourcePath) + runGoErrchk(cmd, t) +} diff --git a/pkg/golinters/supported_linters.go b/pkg/golinters/supported_linters.go index 2429b530..ca408108 100644 --- a/pkg/golinters/supported_linters.go +++ b/pkg/golinters/supported_linters.go @@ -17,5 +17,5 @@ var golint = newLinter("golint", newLinterConfig("", pathLineColMessage, "")) var govet = newLinter("govet", newLinterConfig("", pathLineMessage, "", "--no-recurse")) func GetSupportedLinters() []linters.Linter { - return []linters.Linter{gofmt{}, gofmt{useGoimports: true}, golint, govet} + return []linters.Linter{errCheck} } diff --git a/pkg/golinters/test.go b/pkg/golinters/test.go index 4c8358be..e45eefdf 100644 --- a/pkg/golinters/test.go +++ b/pkg/golinters/test.go @@ -32,7 +32,7 @@ func ExpectIssues(t *testing.T, linter linters.Linter, source string, issues []r err = ioutil.WriteFile(path.Join(subDir, "f.go"), []byte(source), os.ModePerm) assert.NoError(t, err) - res, err := linter.Run(context.Background(), exec) + res, err := linter.Run(context.Background(), exec, nil) assert.NoError(t, err) assert.Equal(t, issues, res.Issues) diff --git a/pkg/golinters/testdata/errcheck.go b/pkg/golinters/testdata/errcheck.go new file mode 100644 index 00000000..b10d07d1 --- /dev/null +++ b/pkg/golinters/testdata/errcheck.go @@ -0,0 +1,34 @@ +package p + +import ( + "net/http" + "os" +) + +func retErr() error { + return nil +} + +func missedErrorCheck() { + retErr() // ERROR "Error return value is not checked" +} + +func ignoreCloseMissingErrHandling() error { + f, err := os.Open("t.go") + if err != nil { + return err + } + + f.Close() + return nil +} + +func ignoreCloseInDeferMissingErrHandling() { + resp, err := http.Get("http://example.com/") + if err != nil { + panic(err) + } + defer resp.Body.Close() + + panic(resp) +} diff --git a/pkg/linter.go b/pkg/linter.go index 44a0c0b4..be42bf30 100644 --- a/pkg/linter.go +++ b/pkg/linter.go @@ -3,11 +3,12 @@ package linters import ( "context" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/result" "github.com/golangci/golangci-shared/pkg/executors" ) type Linter interface { - Run(ctx context.Context, exec executors.Executor) (*result.Result, error) + Run(ctx context.Context, exec executors.Executor, cfg *config.Run) (*result.Result, error) Name() string } diff --git a/pkg/runner.go b/pkg/runner.go index 4ef7f865..a001cae4 100644 --- a/pkg/runner.go +++ b/pkg/runner.go @@ -21,7 +21,7 @@ type SimpleRunner struct { func (r SimpleRunner) Run(ctx context.Context, linters []Linter, exec executors.Executor) ([]result.Issue, error) { results := []result.Result{} for _, linter := range linters { - res, err := linter.Run(ctx, exec) + res, err := linter.Run(ctx, exec, nil) if err != nil { analytics.Log(ctx).Warnf("Can't run linter %+v: %s", linter, err) continue