From ecfb7997fe657d11f28cd58b498fa980836b6ff6 Mon Sep 17 00:00:00 2001 From: Cameron McAvoy Date: Thu, 6 Aug 2020 12:53:10 -0500 Subject: [PATCH 1/2] Update Makefile + Linter config --- .golangci.yml | 20 ++++++++------------ Makefile | 50 ++++++++++++++++++++++++++++++++++++-------------- gofmt.go | 6 +++--- gofmt_test.go | 2 +- main.go | 6 +++--- 5 files changed, 51 insertions(+), 33 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 92b00d1..5b7f1a4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -23,28 +23,21 @@ linters-settings: - unnecessaryBlock settings: rangeValCopy: - sizeThreshold: 256 + sizeThreshold: 512 hugeParam: - sizeThreshold: 256 + sizeThreshold: 512 gocyclo: min-complexity: 16 - goimports: - local-prefixes: github.com/golangci/golangci-lint golint: min-confidence: 0 govet: - check-shadowing: true + check-shadowing: false lll: line-length: 300 maligned: suggest-new: true misspell: locale: US - nolintlint: - allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) - allow-unused: false # report any unused nolint directives - require-explanation: false # don't require an explanation for nolint directives - require-specific: false # don't require nolint directives to be specific about which linter is being skipped linters: disable-all: true @@ -56,11 +49,13 @@ linters: - dogsled - dupl - errcheck + - exhaustive + - exportloopref - gochecknoinits - goconst - gocritic + - godot - gofmt - - goimports - golint - goprintffuncname - gosec @@ -76,6 +71,7 @@ linters: - prealloc - rowserrcheck - scopelint + - sqlclosecheck - staticcheck - structcheck - stylecheck @@ -84,4 +80,4 @@ linters: - unparam - unused - varcheck - - whitespace \ No newline at end of file + - whitespace diff --git a/Makefile b/Makefile index b64c632..5b92063 100644 --- a/Makefile +++ b/Makefile @@ -1,31 +1,53 @@ -BIN_DIR ?= $(shell go env GOPATH)/bin +MAKEFLAGS += --warn-undefined-variables +SHELL := bash +.SHELLFLAGS := -euo pipefail -c +.DEFAULT_GOAL := all -default: test +BIN_DIR ?= $(shell go env GOPATH)/bin +export PATH := $(PATH):$(BIN_DIR) +.PHONY: deps deps: ## download go modules go mod download -fmt: lint/install # ensure consistent code style +.PHONY: fmt +fmt: lint/check ## ensure consistent code style + go run oss.indeed.com/go/go-groups -w . gofmt -s -w . golangci-lint run --fix > /dev/null 2>&1 || true + go mod tidy -lint/install: +.PHONY: lint/check +lint/check: @if ! golangci-lint --version > /dev/null 2>&1; then \ - echo "Installing golangci-lint"; \ - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(BIN_DIR) v1.28.3; \ + echo -e "\033[0;33mgolangci-lint is not installed: run \`\033[0;32mmake lint-install\033[0m\033[0;33m\` or install it from https://golangci-lint.run\033[0m"; \ + exit 1; \ fi -lint: lint/install ## run golangci-lint - @if ! golangci-lint run; then \ - echo "\033[0;33mdetected fmt problems: run \`\033[0;32mmake fmt\033[0m\`"; \ - exit 1; \ - fi +.PHONY: lint-install +lint-install: ## installs golangci-lint to the go bin dir + @if ! golangci-lint --version > /dev/null 2>&1; then \ + echo "Installing golangci-lint"; \ + curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(BIN_DIR) v1.30.0; \ + fi +.PHONY: lint +lint: lint/check ## run golangci-lint + golangci-lint run + @if [ -n "$$(gofmt -s -l .)" ] || [ -n "$$(go run oss.indeed.com/go/go-groups -d .)" ]; then \ + echo -e "\033[0;33mdetected fmt problems: run \`\033[0;32mmake fmt\033[0m\033[0;33m\`\033[0m"; \ + exit 1; \ + fi + +.PHONY: test test: lint ## run go tests - go vet ./... go test ./... -race +.PHONY: all +all: test + +.PHONY: help help: ## displays this help message - @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_\/-]+:.*?## / {printf "\033[34m%-30s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | \ + @awk 'BEGIN {FS = ":.*?## "} /^[a-zA-Z_\/-]+:.*?## / {printf "\033[34m%-12s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | \ sort | \ - grep -v '#' + grep -v '#' \ No newline at end of file diff --git a/gofmt.go b/gofmt.go index 9ae8bd2..3bbec14 100644 --- a/gofmt.go +++ b/gofmt.go @@ -89,13 +89,13 @@ func processFile(filename string, in io.Reader, out io.Writer, fixFmt bool) erro if !bytes.Equal(src, res) && rewritten { // formatting has changed if *list { - if _, err := fmt.Fprintln(out, filename); err != nil { //nolint:govet + if _, err := fmt.Fprintln(out, filename); err != nil { return err } } if *write { // make a temporary backup before overwriting original - bakname, err := backupFile(filename+".", src, perm) //nolint:govet + bakname, err := backupFile(filename+".", src, perm) if err != nil { return err } @@ -110,7 +110,7 @@ func processFile(filename string, in io.Reader, out io.Writer, fixFmt bool) erro } } if *doDiff { - data, err := diff(src, res, filename) //nolint:govet + data, err := diff(src, res, filename) if err != nil { return fmt.Errorf("computing diff: %s", err) } diff --git a/gofmt_test.go b/gofmt_test.go index 8f5a9cd..82f85dc 100644 --- a/gofmt_test.go +++ b/gofmt_test.go @@ -41,7 +41,7 @@ import ( "testing" ) -// based on https://golang.org/src/cmd/gofmt/gofmt_test.go with a few modifications +// based on https://golang.org/src/cmd/gofmt/gofmt_test.go with a few modifications. func TestBackupFile(t *testing.T) { dir, err := ioutil.TempDir("", "gofmt_test") if err != nil { diff --git a/main.go b/main.go index f9b95e5..63d2d4d 100644 --- a/main.go +++ b/main.go @@ -23,7 +23,7 @@ var ( importStartRegex = regexp.MustCompile(`^\s*import\s*\(\s*$`) importEndRegex = regexp.MustCompile(`^\s*\)\s*$`) - // any whitespace + any unicode_letter_or_underscore + any unicode_letter_or_underscore_or_unicode number + any whitespace + quote + any + quote + any + // any whitespace + any unicode_letter_or_underscore + any unicode_letter_or_underscore_or_unicode number + any whitespace + quote + any + quote + any. groupedImportRegex = regexp.MustCompile(`^\s*[\p{L}_\\.]*[\s*[\p{L}_\p{N}]*\s*".*".*$`) externalImport = regexp.MustCompile(`"([a-zA-Z0-9_]{1}[a-zA-Z0-9_-]{0,62}){1}(\.[a-zA-Z0-9_]{1}[a-zA-Z0-9_-]{0,62})*[\._]?\/([\p{L}_\-\p{N}]*)\/?.*"`) @@ -113,7 +113,7 @@ func parse(src []byte) (result []byte, rewritten bool) { group.lineEnd = n groups = append(groups, group) } else if groupedImportRegex.MatchString(line) { - importLine := importLine{ //nolint:govet + importLine := importLine{ line: line, } var above, below int @@ -220,7 +220,7 @@ func fixupFile(contents map[int]string, numLines int, groups []importGroup) []by // standard library imports are grouped together and sorted alphabetically // each second-level external import is grouped together (e.g github.com/pkg.* is one group) // each of these second-level groups is discovered and sorted alphabetically -// then each import is matched with their group and the list of lines to be written is built up +// then each import is matched with their group and the list of lines to be written is built up. func regroupImportGroups(group importGroup) importGroup { standardImports := make(Imports, 0, len(group.lines)) From 04aec40fa3960c0bf13f9bcb7569a6e56b76fdde Mon Sep 17 00:00:00 2001 From: Cameron McAvoy Date: Thu, 6 Aug 2020 13:49:56 -0500 Subject: [PATCH 2/2] gh-17: update go-groups to ignore generated code by default --- .travis.yml | 5 ++++- CHANGELOG.md | 9 ++++++++- README.md | 20 ++++++++++++++------ go.mod | 4 +--- go.sum | 1 + gofmt.go | 25 +++++++++++++++++++++++-- main.go | 9 ++++++--- main_test.go | 26 ++++++++++++++++++++++++-- testdata/code_generated.txt | 22 ++++++++++++++++++++++ testdata/code_generated_valid.txt | 22 ++++++++++++++++++++++ 10 files changed, 125 insertions(+), 18 deletions(-) create mode 100644 testdata/code_generated.txt create mode 100644 testdata/code_generated_valid.txt diff --git a/.travis.yml b/.travis.yml index 55fe05c..9bce3fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,7 @@ language: go go: - - 1.14.x \ No newline at end of file + - 1.14.x + +before_script: + - make lint-install \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index c54c53e..0f7bb3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [1.1.0] - 07-15-20 +## [1.1.1] - 2020-08-06 +### Added +- Added -g flag to instruct go-groups to analyze and include generated code + +### Changed +- go-groups now detects generated code and ignores generated code by default + +## [1.1.0] - 2020-07-15 ### Added - Added golangci-lint linter, Makefile, and fixed linter warnings diff --git a/README.md b/README.md index 6db14c3..1400981 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,6 @@ go-groups [![NetflixOSS Lifecycle](https://img.shields.io/osslifecycle/indeedeng/go-groups.svg)](OSSMETADATA) [![GitHub](https://img.shields.io/github/license/indeedeng/go-groups.svg)](LICENSE) - # Project Overview Command `go-groups` is a CLI tool to parse go import blocks, sort, and re-group @@ -18,7 +17,6 @@ lexigraphically, it also separates import blocks at a per-project level. Like `goimports`, `go-groups` also runs gofmt and fixes any style/formatting issues. - # Getting Started The `go-groups` command can be installed by running: @@ -27,6 +25,20 @@ The `go-groups` command can be installed by running: $ go get oss.indeed.com/go/go-groups ``` +# Usage +``` +$ go-groups -h + usage: go-groups [flags] [path ...] + -d display diffs instead of rewriting files + -f disables the automatic gofmt style fixes + -g include generated code in analysis + -l list files whose formatting differs + -v display the version of go-groups + -w write result to (source) file instead of stdout +``` + +# Formatting Behavior + #### With this example source file input ``` import ( @@ -55,10 +67,6 @@ import ( Run `go-groups -w ./..` to rewrite and sort import groupings for go source files in a project. -#### More info - -`go-groups -h` to print usage help text and available flags - # Asking Questions For technical questions about `go-groups`, just file an issue in the GitHub tracker. diff --git a/go.mod b/go.mod index ce9b907..1961018 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,4 @@ module oss.indeed.com/go/go-groups go 1.14 -require ( - github.com/stretchr/testify v1.6.1 -) +require github.com/stretchr/testify v1.6.1 diff --git a/go.sum b/go.sum index 56d62e7..afe7890 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/gofmt.go b/gofmt.go index 3bbec14..6c11e39 100644 --- a/gofmt.go +++ b/gofmt.go @@ -31,6 +31,7 @@ package main */ import ( + "bufio" "bytes" "fmt" "go/scanner" @@ -43,6 +44,16 @@ import ( "strings" ) +func isGeneratedCode(src []byte) bool { + scanner := bufio.NewScanner(bytes.NewReader(src)) + for scanner.Scan() { + if generatedRegex.MatchString(scanner.Text()) { + return true + } + } + return false +} + // based on https://golang.org/src/cmd/gofmt/gofmt.go with a few modifications func isGoFile(f os.FileInfo) bool { @@ -52,7 +63,7 @@ func isGoFile(f os.FileInfo) bool { } // If in == nil, the source is the contents of the file with the given filename. -func processFile(filename string, in io.Reader, out io.Writer, fixFmt bool) error { +func processFile(filename string, in io.Reader, out io.Writer, fixFmt, generatedCode bool) error { var perm os.FileMode = 0644 if in == nil { f, err := os.Open(filename) @@ -73,6 +84,16 @@ func processFile(filename string, in io.Reader, out io.Writer, fixFmt bool) erro return err } + if !generatedCode && isGeneratedCode(src) { + if !*list { + _, err = out.Write(src) + if err != nil { + return err + } + } + return nil + } + if fixFmt { buffer := bytes.Buffer{} buffer.Write(src) @@ -131,7 +152,7 @@ func processFile(filename string, in io.Reader, out io.Writer, fixFmt bool) erro func visitFile(path string, f os.FileInfo, err error) error { if err == nil && isGoFile(f) { - err = processFile(path, nil, os.Stdout, !*noFormat) + err = processFile(path, nil, os.Stdout, !*noFormat, *genCode) } // Don't complain if a file was deleted in the meantime (i.e. // the directory changed concurrently while running gofmt). diff --git a/main.go b/main.go index 63d2d4d..005612e 100644 --- a/main.go +++ b/main.go @@ -16,7 +16,7 @@ const ( exitBadStdin = 2 exitInternalError = 3 - versionStr = "go-groups version 1.1.0 (2020-07-15)" + versionStr = "go-groups version 1.1.1 (2020-08-06)" ) var ( @@ -26,12 +26,15 @@ var ( // any whitespace + any unicode_letter_or_underscore + any unicode_letter_or_underscore_or_unicode number + any whitespace + quote + any + quote + any. groupedImportRegex = regexp.MustCompile(`^\s*[\p{L}_\\.]*[\s*[\p{L}_\p{N}]*\s*".*".*$`) externalImport = regexp.MustCompile(`"([a-zA-Z0-9_]{1}[a-zA-Z0-9_-]{0,62}){1}(\.[a-zA-Z0-9_]{1}[a-zA-Z0-9_-]{0,62})*[\._]?\/([\p{L}_\-\p{N}]*)\/?.*"`) + // see https://golang.org/pkg/cmd/go/internal/generate/ for details. + generatedRegex = regexp.MustCompile(`^// Code generated .* DO NOT EDIT.$`) list = flag.Bool("l", false, "list files whose formatting differs") write = flag.Bool("w", false, "write result to (source) file instead of stdout") doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") version = flag.Bool("v", false, "display the version of go-groups") noFormat = flag.Bool("f", false, "disables the automatic gofmt style fixes") + genCode = flag.Bool("g", false, "include generated code in analysis") ) func main() { @@ -49,7 +52,7 @@ func main() { _, _ = fmt.Fprintln(os.Stderr, "error: cannot use -w with standard input") os.Exit(exitBadStdin) } - if err := processFile("", os.Stdin, os.Stdout, !*noFormat); err != nil { + if err := processFile("", os.Stdin, os.Stdout, !*noFormat, *genCode); err != nil { _, _ = fmt.Fprintln(os.Stderr, "failed to parse stdin: "+err.Error()) os.Exit(exitBadFlags) } @@ -68,7 +71,7 @@ func main() { os.Exit(exitInternalError) } default: - _ = processFile(path, nil, os.Stdout, !*noFormat) + _ = processFile(path, nil, os.Stdout, !*noFormat, *genCode) } } } diff --git a/main_test.go b/main_test.go index e856378..fc96c14 100644 --- a/main_test.go +++ b/main_test.go @@ -86,7 +86,7 @@ func TestParse_gofmt(t *testing.T) { expected := testdata(t, "gofmt.txt") var buf bytes.Buffer - err := processFile("", strings.NewReader(string(b)), &buf, true) + err := processFile("", strings.NewReader(string(b)), &buf, true, false) require.NoError(t, err) require.Equal(t, string(expected), buf.String()) @@ -97,7 +97,29 @@ func TestParse_gofmt_disabled(t *testing.T) { expected := testdata(t, "gofmt_invalid.txt") var buf bytes.Buffer - err := processFile("", strings.NewReader(string(b)), &buf, false) + err := processFile("", strings.NewReader(string(b)), &buf, false, false) + require.NoError(t, err) + + require.Equal(t, string(expected), buf.String()) +} + +func TestParse_generated_code(t *testing.T) { + b := testdata(t, "code_generated.txt") + expected := testdata(t, "code_generated.txt") + + var buf bytes.Buffer + err := processFile("", strings.NewReader(string(b)), &buf, true, false) + require.NoError(t, err) + + require.Equal(t, string(expected), buf.String()) +} + +func TestParse_generated_code_enabled(t *testing.T) { + b := testdata(t, "code_generated.txt") + expected := testdata(t, "code_generated_valid.txt") + + var buf bytes.Buffer + err := processFile("", strings.NewReader(string(b)), &buf, true, true) require.NoError(t, err) require.Equal(t, string(expected), buf.String()) diff --git a/testdata/code_generated.txt b/testdata/code_generated.txt new file mode 100644 index 0000000..3f3df1a --- /dev/null +++ b/testdata/code_generated.txt @@ -0,0 +1,22 @@ +package main + +// Code generated by go-groups DO NOT EDIT. + +import ( + "io" + "strings" + "strconv" + "fmt" + "os" + "io/ioutil" + + "indeed.com/devops/foobar" + + "indeed.com/gophers/quz" + + "github.com/hashicorp/vault/x" +) + +func main() { + fmt.Println("Hello world") +} diff --git a/testdata/code_generated_valid.txt b/testdata/code_generated_valid.txt new file mode 100644 index 0000000..f076601 --- /dev/null +++ b/testdata/code_generated_valid.txt @@ -0,0 +1,22 @@ +package main + +// Code generated by go-groups DO NOT EDIT. + +import ( + "fmt" + "io" + "io/ioutil" + "os" + "strconv" + "strings" + + "github.com/hashicorp/vault/x" + + "indeed.com/devops/foobar" + + "indeed.com/gophers/quz" +) + +func main() { + fmt.Println("Hello world") +}