From bf921fe4731b92f5ba9bea9b6b5a671ac74f9da4 Mon Sep 17 00:00:00 2001 From: Loong Dai Date: Mon, 30 May 2022 15:21:38 +0800 Subject: [PATCH] Update codes and re-enable in golangci-lint (#64) * fix test hung Signed-off-by: Loong Dai * add GH for PR Signed-off-by: Loong Dai * support analyzer Signed-off-by: Loong Dai --- .github/workflows/gci.yml | 82 +++++++++++++++++++++++ .golangci.yml | 85 ++++++++++++++++++++++++ README.md | 6 +- cmd/gci/completion.go | 1 + main.go | 4 +- pkg/analyzer/analyzer.go | 47 ++++++++----- pkg/gci/configuration.go | 4 +- pkg/gci/errors.go | 10 +-- pkg/gci/errors_test.go | 4 +- pkg/gci/format.go | 3 +- pkg/gci/gci.go | 18 ++++- pkg/gci/gci_test.go | 14 ++-- pkg/gci/parse.go | 2 +- pkg/gci/sections/newline_test.go | 1 + pkg/gci/sections/section.go | 2 - pkg/gci/sections/standardpackage_list.go | 2 +- pkg/gci/specificity/default.go | 4 +- pkg/gci/specificity/mismatch.go | 4 +- pkg/gci/specificity/specificity.go | 2 - pkg/gci/specificity/standard.go | 3 +- pkg/io/stdin.go | 3 +- 21 files changed, 250 insertions(+), 51 deletions(-) create mode 100644 .github/workflows/gci.yml create mode 100644 .golangci.yml diff --git a/.github/workflows/gci.yml b/.github/workflows/gci.yml new file mode 100644 index 0000000..9e4c1a4 --- /dev/null +++ b/.github/workflows/gci.yml @@ -0,0 +1,82 @@ +name: gci + +on: + pull_request: + +jobs: + build: + name: Build ${{ matrix.target_os }}_${{ matrix.target_arch }} binaries + runs-on: ${{ matrix.os }} + env: + GOVER: 1.18 + GOOS: ${{ matrix.target_os }} + GOARCH: ${{ matrix.target_arch }} + GOPROXY: https://proxy.golang.org + ARCHIVE_OUTDIR: dist/archives + TEST_OUTPUT_FILE_PREFIX: test_report + strategy: + matrix: + os: [ubuntu-latest, windows-2019, macOS-latest] + target_arch: [arm, arm64, amd64] + include: + - os: ubuntu-latest + target_os: linux + - os: windows-2019 + target_os: windows + - os: macOS-latest + target_os: darwin + exclude: + - os: windows-2019 + target_arch: arm + - os: windows-2019 + target_arch: arm64 + - os: macOS-latest + target_arch: arm + steps: + - name: Set up Go ${{ env.GOVER }} + uses: actions/setup-go@v2 + with: + go-version: ${{ env.GOVER }} + - name: Check out code into the Go module directory + uses: actions/checkout@v2 + - name: Cache Go modules (Linux) + if: matrix.target_os == 'linux' + uses: actions/cache@v3 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build- + - name: Cache Go modules (Windows) + if: matrix.target_os == 'windows' + uses: actions/cache@v3 + with: + path: | + ~\AppData\Local\go-build + ~\go\pkg\mod + key: ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build- + - name: Cache Go modules (macOS) + if: matrix.target_os == 'darwin' + uses: actions/cache@v3 + with: + path: | + ~/Library/Caches/go-build + ~/go/pkg/mod + key: ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ matrix.target_os }}-${{ matrix.target_arch }}-go-${{ env.GOVER }}-build- + - name: golangci-lint + if: matrix.target_arch == 'amd64' && matrix.target_os == 'linux' + uses: golangci/golangci-lint-action@v3.1.0 + with: + version: ${{ env.GOLANGCILINT_VER }} + - name: Run make test + env: + COVERAGE_OPTS: "-coverprofile=coverage.txt -covermode=atomic" + if: matrix.target_arch == 'amd64' + run: make test + diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..2e72bba --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,85 @@ +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + deadline: 10m + + # exit code when at least one issue was found, default is 1 + issues-exit-code: 1 + + # include test files or not, default is true + tests: true + + # list of build tags, all linters use it. Default is empty list. + build-tags: + + # which dirs to skip: they won't be analyzed; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but next dirs are always skipped independently + # from this option's value: + # third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs: + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + skip-files: + +# output configuration options +output: + # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number" + format: tab + + # print lines of code with issue, default is true + print-issued-lines: true + + # print linter name in the end of issue text, default is true + print-linter-name: true + + +# all available settings of specific linters +linters-settings: + gci: + # Checks that no inline Comments are present. + # Default: false + no-inline-comments: false + + # Checks that no prefix Comments(comment lines above an import) are present. + # Default: false + no-prefix-comments: false + + # Section configuration to compare against. + # Section names are case-insensitive and may contain parameters in (). + # Default: ["standard", "default"] + sections: + - standard # Captures all standard packages if they do not match another section. + - default # Contains all imports that could not be matched to another section type. + - prefix(github.com/daixiang0/gci) # Groups all imports with the specified Prefix. + + # Separators that should be present between sections. + # Default: ["newLine"] + section-separators: + - newLine + + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + goimports: + # put imports beginning with prefix after 3rd-party packages; + # it's a comma-separated list of prefixes + local-prefixes: github.com/daixiang0/gci + +linters: + fast: false + enable: + - gofmt + - gofumpt + - goimports + - gci + disable-all: true + +issues: + exclude: diff --git a/README.md b/README.md index 1ff1095..085a843 100644 --- a/README.md +++ b/README.md @@ -102,9 +102,13 @@ Flags: ``` +**Note**:: + +The old style is only for local tests, `golangci-lint` uses new style. + ## Examples -Run `gci write --Section Standard --Section Default --Section "Prefix(github.com/daixiang0/gci)" main.go` and you will handle following cases: +Run `gci write --Section Standard --Section Default --Section "Prefix(github.com/daixiang0/gci)" main.go` and you will handle following cases: ### simple case diff --git a/cmd/gci/completion.go b/cmd/gci/completion.go index 5992105..8880f3b 100644 --- a/cmd/gci/completion.go +++ b/cmd/gci/completion.go @@ -21,6 +21,7 @@ func subCommandOrGoFileCompletion(cmd *cobra.Command, args []string, toComplete } return goFileCompletion(cmd, args, toComplete) } + func goFileCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { return []string{"go"}, cobra.ShellCompDirectiveFilterFileExt } diff --git a/main.go b/main.go index 5ef3c31..9496b2e 100644 --- a/main.go +++ b/main.go @@ -6,9 +6,7 @@ import ( "github.com/daixiang0/gci/cmd/gci" ) -var ( - version = "0.3" -) +var version = "0.3" func main() { e := gci.NewExecutor(version) diff --git a/pkg/analyzer/analyzer.go b/pkg/analyzer/analyzer.go index 10818a7..6ca93e1 100644 --- a/pkg/analyzer/analyzer.go +++ b/pkg/analyzer/analyzer.go @@ -6,12 +6,13 @@ import ( "go/token" "strings" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "github.com/daixiang0/gci/pkg/configuration" "github.com/daixiang0/gci/pkg/gci" "github.com/daixiang0/gci/pkg/io" - - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/inspect" + "github.com/daixiang0/gci/pkg/log" ) const ( @@ -19,7 +20,7 @@ const ( NoPrefixCommentsFlag = "noPrefixComments" SectionsFlag = "Sections" SectionSeparatorsFlag = "SectionSeparators" - SectionDelimiter = ";" + SectionDelimiter = "," ) var ( @@ -34,6 +35,9 @@ func init() { Analyzer.Flags.BoolVar(&noPrefixComments, NoPrefixCommentsFlag, false, "If comments above an input should be present") Analyzer.Flags.StringVar(§ionsStr, SectionsFlag, "", "Specify the Sections format that should be used to check the file formatting") Analyzer.Flags.StringVar(§ionSeparatorsStr, SectionSeparatorsFlag, "", "Specify the Sections that are inserted as Separators between Sections") + + log.InitLogger() + defer log.L().Sync() } var Analyzer = &analysis.Analyzer{ @@ -44,8 +48,6 @@ var Analyzer = &analysis.Analyzer{ } func runAnalysis(pass *analysis.Pass) (interface{}, error) { - // TODO input validation - var fileReferences []*token.File // extract file references for all files in the analyzer pass for _, pkgFile := range pass.Files { @@ -80,17 +82,7 @@ func runAnalysis(pass *analysis.Pass) (interface{}, error) { case -1: // no difference default: - diffPos := file.Position(file.Pos(diffIdx)) - // prevent invalid access to array - fileRune := "nil" - formattedRune := "nil" - if len(fileRunes)-1 >= diffIdx { - fileRune = fmt.Sprintf("%q", fileRunes[diffIdx]) - } - if len(formattedRunes)-1 >= diffIdx { - formattedRune = fmt.Sprintf("%q", formattedRunes[diffIdx]) - } - pass.Reportf(file.Pos(diffIdx), "Expected %s, Found %s at %s[line %d,col %d]", formattedRune, fileRune, filePath, diffPos.Line, diffPos.Column) + pass.Reportf(file.Pos(diffIdx), "fix by `%s %s`", generateCmdLine(*gciCfg), filePath) } } return nil, nil @@ -126,6 +118,27 @@ func parseGciConfiguration() (*gci.GciConfiguration, error) { var sectionSeparatorStrings []string if sectionSeparatorsStr != "" { sectionSeparatorStrings = strings.Split(sectionSeparatorsStr, SectionDelimiter) + fmt.Println(sectionSeparatorsStr) } return gci.GciStringConfiguration{fmtCfg, sectionStrings, sectionSeparatorStrings}.Parse() } + +func generateCmdLine(cfg gci.GciConfiguration) string { + result := "gci write" + + if cfg.FormatterConfiguration.NoInlineComments { + result += " --NoInlineComments " + } + + if cfg.FormatterConfiguration.NoPrefixComments { + result += " --NoPrefixComments " + } + + for _, s := range cfg.Sections.String() { + result += fmt.Sprintf(" --Section \"%s\" ", s) + } + for _, s := range cfg.SectionSeparators.String() { + result += fmt.Sprintf(" --SectionSeparator %s ", s) + } + return result +} diff --git a/pkg/gci/configuration.go b/pkg/gci/configuration.go index f1ef7ed..5825238 100644 --- a/pkg/gci/configuration.go +++ b/pkg/gci/configuration.go @@ -3,10 +3,10 @@ package gci import ( "io/ioutil" + "gopkg.in/yaml.v3" + "github.com/daixiang0/gci/pkg/configuration" sectionsPkg "github.com/daixiang0/gci/pkg/gci/sections" - - "gopkg.in/yaml.v3" ) type GciConfiguration struct { diff --git a/pkg/gci/errors.go b/pkg/gci/errors.go index 6fde84f..90cb7ee 100644 --- a/pkg/gci/errors.go +++ b/pkg/gci/errors.go @@ -40,7 +40,7 @@ type InvalidImportSplitError struct { } func (i InvalidImportSplitError) Error() string { - return fmt.Sprintf("seperating the inline comment from the import yielded an invalid number of segments: %v", i.segments) + return fmt.Sprintf("separating the inline comment from the import yielded an invalid number of segments: %v", i.segments) } func (i InvalidImportSplitError) Is(err error) bool { @@ -53,7 +53,7 @@ type InvalidAliasSplitError struct { } func (i InvalidAliasSplitError) Error() string { - return fmt.Sprintf("seperating the alias from the path yielded an invalid number of segments: %v", i.segments) + return fmt.Sprintf("separating the alias from the path yielded an invalid number of segments: %v", i.segments) } func (i InvalidAliasSplitError) Is(err error) bool { @@ -61,8 +61,10 @@ func (i InvalidAliasSplitError) Is(err error) bool { return ok } -var MissingImportStatementError = FileParsingError{errors.New("no import statement present in File")} -var ImportStatementNotClosedError = FileParsingError{errors.New("import statement not closed")} +var ( + MissingImportStatementError = FileParsingError{errors.New("no import statement present in File")} + ImportStatementNotClosedError = FileParsingError{errors.New("import statement not closed")} +) type FileParsingError struct { error diff --git a/pkg/gci/errors_test.go b/pkg/gci/errors_test.go index b67bddb..7e3583e 100644 --- a/pkg/gci/errors_test.go +++ b/pkg/gci/errors_test.go @@ -4,10 +4,10 @@ import ( "errors" "testing" + "github.com/stretchr/testify/assert" + importPkg "github.com/daixiang0/gci/pkg/gci/imports" sectionsPkg "github.com/daixiang0/gci/pkg/gci/sections" - - "github.com/stretchr/testify/assert" ) func TestErrorMatching(t *testing.T) { diff --git a/pkg/gci/format.go b/pkg/gci/format.go index 08c43bb..2ac9fa1 100644 --- a/pkg/gci/format.go +++ b/pkg/gci/format.go @@ -39,11 +39,10 @@ func formatGoFile(input []byte, cfg GciConfiguration) ([]byte, error) { // Takes unsorted imports as byte array and formats them according to the specified sections func formatImportBlock(input []byte, cfg GciConfiguration) ([]byte, error) { - //strings.ReplaceAll(input, "\r\n", linebreak) lines := strings.Split(string(input), constants.Linebreak) imports, err := parseToImportDefinitions(lines) if err != nil { - return nil, fmt.Errorf("an error occured while trying to parse imports: %w", err) + return nil, fmt.Errorf("an error occurred while trying to parse imports: %w", err) } log.L().Debug(fmt.Sprintf("Parsed imports in file: %v", imports)) diff --git a/pkg/gci/gci.go b/pkg/gci/gci.go index 23152f7..eb30c11 100644 --- a/pkg/gci/gci.go +++ b/pkg/gci/gci.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "sync" "github.com/hexops/gotextdiff" "github.com/hexops/gotextdiff/myers" @@ -33,6 +34,7 @@ func DefaultSections() SectionList { func DefaultSectionSeparators() SectionList { return SectionList{sectionsPkg.NewLine{}} } + func LocalFlagsToSections(localFlags []string) SectionList { sections := DefaultSections() // Add all local arguments as ImportPrefix sections @@ -56,7 +58,7 @@ func WriteFormattedFiles(paths []string, cfg GciConfiguration) error { return nil } log.L().Info(fmt.Sprintf("Writing formatted File: %s", filePath)) - return os.WriteFile(filePath, formattedFile, 0644) + return os.WriteFile(filePath, formattedFile, 0o644) }) } @@ -70,6 +72,20 @@ func DiffFormattedFiles(paths []string, cfg GciConfiguration) error { }) } +func DiffFormattedFilesToArray(paths []string, cfg GciConfiguration, diffs *[]string, lock *sync.Mutex) error { + log.InitLogger() + defer log.L().Sync() + return processStdInAndGoFilesInPaths(paths, cfg, func(filePath string, unmodifiedFile, formattedFile []byte) error { + fileURI := span.URIFromPath(filePath) + edits := myers.ComputeEdits(fileURI, string(unmodifiedFile), string(formattedFile)) + unifiedEdits := gotextdiff.ToUnified(filePath, filePath, string(unmodifiedFile), edits) + lock.Lock() + *diffs = append(*diffs, fmt.Sprint(unifiedEdits)) + lock.Unlock() + return nil + }) +} + type fileFormattingFunc func(filePath string, unmodifiedFile, formattedFile []byte) error func processStdInAndGoFilesInPaths(paths []string, cfg GciConfiguration, fileFunc fileFormattingFunc) error { diff --git a/pkg/gci/gci_test.go b/pkg/gci/gci_test.go index 3fb34f9..15362ac 100644 --- a/pkg/gci/gci_test.go +++ b/pkg/gci/gci_test.go @@ -7,12 +7,18 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/daixiang0/gci/pkg/gci/sections" "github.com/daixiang0/gci/pkg/io" - - "github.com/stretchr/testify/assert" + "github.com/daixiang0/gci/pkg/log" ) +func init() { + log.InitLogger() + defer log.L().Sync() +} + var testFilesPath = "internal/testdata" func isTestInputFile(file os.FileInfo) bool { @@ -73,7 +79,6 @@ func TestInitGciConfigFromYAML(t *testing.T) { func TestSkippingOverIncorrectlyFormattedFiles(t *testing.T) { cfg, err := GciStringConfiguration{}.Parse() assert.NoError(t, err) - validFileProcessedChan := make(chan bool, 1) var importUnclosedCtr, noImportCtr, validCtr int var files []io.FileObj @@ -81,11 +86,12 @@ func TestSkippingOverIncorrectlyFormattedFiles(t *testing.T) { files = append(files, TestFile{io.File{"internal/skipTest/no-import.testgo"}, &noImportCtr}) files = append(files, TestFile{io.File{"internal/skipTest/valid.testgo"}, &validCtr}) + validFileProcessedChan := make(chan bool, len(files)) + generatorFunc := func() ([]io.FileObj, error) { return files, nil } fileAccessTestFunc := func(filePath string, unmodifiedFile, formattedFile []byte) error { - assert.Equal(t, "internal/skipTest/valid.testgo", filePath, "file should not have been processed") validFileProcessedChan <- true return nil } diff --git a/pkg/gci/parse.go b/pkg/gci/parse.go index df31005..9ed3821 100644 --- a/pkg/gci/parse.go +++ b/pkg/gci/parse.go @@ -14,7 +14,7 @@ func parseToImportDefinitions(unformattedLines []string) ([]importPkg.ImportDef, for index, unformattedLine := range unformattedLines { line := strings.TrimSpace(unformattedLine) if line == "" { - //empty line --> starts a new import + // empty line --> starts a new import return parseToImportDefinitions(unformattedLines[index+1:]) } if strings.HasPrefix(line, constants.LineCommentFlag) { diff --git a/pkg/gci/sections/newline_test.go b/pkg/gci/sections/newline_test.go index a13b4cc..7fca8f7 100644 --- a/pkg/gci/sections/newline_test.go +++ b/pkg/gci/sections/newline_test.go @@ -14,6 +14,7 @@ func TestNewLineSpecificity(t *testing.T) { } testSpecificity(t, testCases) } + func TestNewLineParsing(t *testing.T) { testCases := []sectionTestData{ {"nl", NewLine{}, nil}, diff --git a/pkg/gci/sections/section.go b/pkg/gci/sections/section.go index cbfd467..56dd4b6 100644 --- a/pkg/gci/sections/section.go +++ b/pkg/gci/sections/section.go @@ -22,8 +22,6 @@ type Section interface { String() string } -//Unbound methods that are required until interface methods are supported - // Default method for formatting a section func inorderSectionFormat(section Section, imports []importPkg.ImportDef, cfg configuration.FormatterConfiguration) string { imports = importPkg.SortImportsByPath(imports) diff --git a/pkg/gci/sections/standardpackage_list.go b/pkg/gci/sections/standardpackage_list.go index 645233e..d11dee2 100644 --- a/pkg/gci/sections/standardpackage_list.go +++ b/pkg/gci/sections/standardpackage_list.go @@ -1,6 +1,6 @@ package sections -// Code generated based on go1.18. DO NOT EDIT. +// Code generated based on go1.18.2. DO NOT EDIT. var standardPackages = map[string]struct{}{ "archive/tar": {}, diff --git a/pkg/gci/specificity/default.go b/pkg/gci/specificity/default.go index 2d91bd8..f7ae4b8 100644 --- a/pkg/gci/specificity/default.go +++ b/pkg/gci/specificity/default.go @@ -1,11 +1,11 @@ package specificity -type Default struct { -} +type Default struct{} func (d Default) IsMoreSpecific(than MatchSpecificity) bool { return isMoreSpecific(d, than) } + func (d Default) Equal(to MatchSpecificity) bool { return equalSpecificity(d, to) } diff --git a/pkg/gci/specificity/mismatch.go b/pkg/gci/specificity/mismatch.go index 78013e3..8e87111 100644 --- a/pkg/gci/specificity/mismatch.go +++ b/pkg/gci/specificity/mismatch.go @@ -1,7 +1,6 @@ package specificity -type MisMatch struct { -} +type MisMatch struct{} func (m MisMatch) IsMoreSpecific(than MatchSpecificity) bool { return isMoreSpecific(m, than) @@ -17,5 +16,4 @@ func (m MisMatch) class() specificityClass { func (m MisMatch) String() string { return "Mismatch" - } diff --git a/pkg/gci/specificity/specificity.go b/pkg/gci/specificity/specificity.go index 32b6e28..0a7c9f8 100644 --- a/pkg/gci/specificity/specificity.go +++ b/pkg/gci/specificity/specificity.go @@ -16,8 +16,6 @@ type MatchSpecificity interface { class() specificityClass } -//Unbound methods that are required until interface methods are supported - func isMoreSpecific(this, than MatchSpecificity) bool { return this.class() > than.class() } diff --git a/pkg/gci/specificity/standard.go b/pkg/gci/specificity/standard.go index 30e8f8f..8b11d04 100644 --- a/pkg/gci/specificity/standard.go +++ b/pkg/gci/specificity/standard.go @@ -1,7 +1,6 @@ package specificity -type StandardPackageMatch struct { -} +type StandardPackageMatch struct{} func (s StandardPackageMatch) IsMoreSpecific(than MatchSpecificity) bool { return isMoreSpecific(s, than) diff --git a/pkg/io/stdin.go b/pkg/io/stdin.go index 5d92768..ccab284 100644 --- a/pkg/io/stdin.go +++ b/pkg/io/stdin.go @@ -5,8 +5,7 @@ import ( "os" ) -type stdInFile struct { -} +type stdInFile struct{} func (s stdInFile) Load() ([]byte, error) { return ioutil.ReadAll(os.Stdin)