diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 6a7e25e77..89ef81452 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -9,7 +9,7 @@ A clear and concise description of what the bug is. **To Reproduce** Steps to reproduce the behavior: -1. I updated revive `go get -u github.com/mgechev/revive` +1. I updated revive `go get -u github.com/deepsourcelabs/revive` 2. I run it with the following flags & configuration file: ```shell diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build.yaml similarity index 58% rename from .github/workflows/build-test.yaml rename to .github/workflows/build.yaml index b6ca02ae2..751d8ee20 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build.yaml @@ -1,5 +1,6 @@ -name: Build and Test +name: Build on: + workflow_dispatch: pull_request: types: [opened, edited, synchronize, reopened] @@ -11,10 +12,3 @@ jobs: - uses: actions/checkout@v2 - name: build run: make build - test: - name: Test - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - name: test - run: make test diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 7dc3def0b..b6b6d0b7c 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -1,5 +1,6 @@ name: Lint on: + workflow_dispatch: pull_request: types: [opened, edited, synchronize, reopened] diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml new file mode 100644 index 000000000..bea1ab18f --- /dev/null +++ b/.github/workflows/test.yaml @@ -0,0 +1,28 @@ +name: Test +on: + workflow_dispatch: + pull_request: + types: [opened, edited, synchronize, reopened] + +jobs: + test: + name: Test + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + go-version: + - 1.16.x + - 1.17.x + - 1.18.x + steps: + - name: Checkout code + uses: actions/checkout@v3.0.2 + - name: Set up Go + uses: actions/setup-go@v3.2.0 + with: + go-version: ${{ matrix.go-version }} + cache: true + cache-dependency-path: '**/go.sum' + - name: Run tests + run: go test -race ./... diff --git a/.goreleaser.yml b/.goreleaser.yml index 20a78a181..7ed9303f4 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -29,8 +29,8 @@ dockers: build_flag_templates: - --label=org.opencontainers.image.title={{ .ProjectName }} - --label=org.opencontainers.image.description={{ .ProjectName }} - - --label=org.opencontainers.image.url=https://github.com/mgechev/revive - - --label=org.opencontainers.image.source=https://github.com/mgechev/revive + - --label=org.opencontainers.image.url=https://github.com/deepsourcelabs/revive + - --label=org.opencontainers.image.source=https://github.com/deepsourcelabs/revive - --label=org.opencontainers.image.version={{ .Version }} - --label=org.opencontainers.image.revision={{ .FullCommit }} - --label=org.opencontainers.image.licenses=MIT diff --git a/.travis.yml b/.travis.yml index edb9ae89b..daae19873 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,7 @@ install: make install script: - make build - make test -- git clone https://github.com/mgechev/revive.run revive-docs && cd revive-docs +- git clone https://github.com/deepsourcelabs/revive.run revive-docs && cd revive-docs - npm i || true - npm run build || true - git config --global user.email "mgechev@gmail.com" @@ -18,6 +18,6 @@ script: - git push --force --quiet "https://${GH_TOKEN}@${GH_REF}" || true env: global: - - GH_REF: github.com/mgechev/revive.run.git + - GH_REF: github.com/deepsourcelabs/revive.run.git - secure: Bp2/fJOVEor5aj3rNwA3+4/wecCmX2mVQ2pQt1AJ1n+pT6MjKYywswTDT6kzK/Cu1bPcfEGh3a7XKieAhIWVKvchoyYVV8J80F76HGyqgSBuzFXvV0c32zFn2LtxQxvmtCNynjmGAV57dHtsxGmHxkX9u8JIJ4J06E2Eq9nuuflTCf5o5gHtaE7P7hQT2WL/JRJVABHUMa0XzsMuUdRNO0OBXGMm+SqiWEcZetf2Vq3tfo2LL4ula99oTKKspg0iRKiauCZZaRxyZG/V3QiR0Rl9nhTVnb6hx6/RFrxru63Pm1FUaK1gIqEq9EUMpZRTddGW77UPp9GSB81/GaUm+e0GNFjUAL2e59t72wMxCQEOT+835hVbeCjgdksg0IDbn7sR/S+rYbiCyxTuCA/4YNlDoEajl9RMxK4culsq35LnibE1x7L4Q/5blD7HwVSMhA33HSDCC5MINwTdWwsdHYiAvFo0RCi5B0GngMzE6/pJxvYhWV3YhKWrgSmhafV1QO3Qu9dCn6P+7KsEVDbUeA8Yxnugd60kQNh2vG7bdTYKaZ6GhfU12zAM15xd2SSrKl6szSAo64CYOTznNFMBMskpm05SubTW1w+xDQy8vGjIHqb6zntiqUhFhTDd326iRYfrQuhAK53XbU1NUFFOyZa8kCTlSsPWDoSOX68oH9Q= - secure: WhbUu6HA4S8quk2bLuU/cuxZPRBAi47iFYuPZOd2zKnPkd8j+FAApeNwdGcYReUJUmmYIJHbSAH8MmlaD2TwMpiGXHm83EXT5TtSL612T0fG8IZ3LNdQ2j+q736IDDt3Z024QixfB4uPnO+Q8no6/nAR4m9kdrhNVOQFIEMPtunrWLeD5v7snF3Nhzr01Djl9G7GJnSPenwpBdIVQwpcYTR9qIUWjeXI/Cosvbz/fzLHla1QNKilV2l7PiKWt6zYsTnDX7Sqfh7WGVpmDganG5LITOmKoJsg13rx52/4cNhOTIuvSikpAC3opfrMW7Q+Lif6GWkVFtgVHRTNY2xBjBGRImFvYhKz5vbv04YrbG5GhRoTjJoJsyoyKlQUg6fKIk19eNRpSirL5Z6xUldquQk9WpeSP9ODfYwReKcGx2g8vUEExcaawmGLvKRjxKjqDPbfB/7DHic198fJwJdeYeyF8TfikuT82o59ppaMiEPVtD2FsMQAccXHUi13pu2uLifqHXqVpWpKNr3SMT8zh6KG96QNr+Wvz7ZBm4cYLrTx2AUvpN/bIsHiwrgENVLHn4a9x+r7DJi3tKLl6EN+ndc8netH7qn7GgvJdI1m+enolQbpdhL/U0yVWhFfMLZAqkZqh5hi1cE9dmuBCJO9cq8R+6OxUWN3N9glY+wcxYk= diff --git a/DEVELOPING.md b/DEVELOPING.md index a8860333b..7c67dd2dd 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -7,8 +7,8 @@ This document explains how to build, test, and develop features for revive. In order to install all the dependencies run: ```bash -go get -u github.com/mgechev/revive -cd $GOPATH/src/github.com/mgechev/revive +go get -u github.com/deepsourcelabs/revive +cd $GOPATH/src/github.com/deepsourcelabs/revive ``` After that install the dependencies using go modules: @@ -29,7 +29,7 @@ The command will produce the `revive` binary in the root of the project. ## Development of rules -If you want to develop a new rule, follow as an example the already existing rules in the [rule package](https://github.com/mgechev/revive/tree/master/rule). +If you want to develop a new rule, follow as an example the already existing rules in the [rule package](https://github.com/deepsourcelabs/revive/tree/master/rule). All rules should implement the following interface: @@ -42,7 +42,7 @@ type Rule interface { ## Development of formatters -If you want to develop a new formatter, follow as an example the already existing formatters in the [formatter package](https://github.com/mgechev/revive/tree/master/formatter). +If you want to develop a new formatter, follow as an example the already existing formatters in the [formatter package](https://github.com/deepsourcelabs/revive/tree/master/formatter). All formatters should implement the following interface: diff --git a/Makefile b/Makefile index f85c21200..08d6c4d68 100644 --- a/Makefile +++ b/Makefile @@ -15,5 +15,5 @@ build: @go build -ldflags='$(VERSION_FLAGS)' test: - @go test -v ./... + @go test -v -race ./... diff --git a/README.md b/README.md index 254a5a16c..b6d24a2ac 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ Here's how `revive` is different from `golint`: - [`Beaver`](https://github.com/Clivern/Beaver) - A Real Time Messaging Server. - [`ggz`](https://github.com/go-ggz/ggz) - An URL shortener service written in Golang - [`Codeac.io`](https://www.codeac.io?ref=revive) - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt. +- [`DevLake`](https://github.com/apache/incubator-devlake) - Apache DevLake is an open-source dev data platform to ingest, analyze, and visualize the fragmented data from DevOps tools，which can distill insights to improve engineering productivity. *Open a PR to add your project*. @@ -103,14 +104,14 @@ Here's how `revive` is different from `golint`: ## Installation ```bash -go install github.com/mgechev/revive@latest +go install github.com/deepsourcelabs/revive@latest ``` -or get a released executable from the [Releases](https://github.com/mgechev/revive/releases) page. +or get a released executable from the [Releases](https://github.com/deepsourcelabs/revive/releases) page. You can install the main branch (including the last commit) with: ```bash -go install github.com/mgechev/revive@master +go install github.com/deepsourcelabs/revive@master ``` ## Usage @@ -194,7 +195,7 @@ Please notice that if no particular configuration is provided, `revive` will beh `revive` accepts the following command line parameters: - `-config [PATH]` - path to config file in TOML format, defaults to `$HOME/revive.toml` if present. -- `-exclude [PATTERN]` - pattern for files/directories/packages to be excluded for linting. You can specify the files you want to exclude for linting either as package name (i.e. `github.com/mgechev/revive`), list them as individual files (i.e. `file.go`), directories (i.e. `./foo/...`), or any combination of the three. +- `-exclude [PATTERN]` - pattern for files/directories/packages to be excluded for linting. You can specify the files you want to exclude for linting either as package name (i.e. `github.com/deepsourcelabs/revive`), list them as individual files (i.e. `file.go`), directories (i.e. `./foo/...`), or any combination of the three. - `-formatter [NAME]` - formatter to be used for the output. The currently available formatters are: - `default` - will output the failures the same way that `golint` does. @@ -211,13 +212,13 @@ Please notice that if no particular configuration is provided, `revive` will beh ### Sample Invocations ```shell -revive -config revive.toml -exclude file1.go -exclude file2.go -formatter friendly github.com/mgechev/revive package/... +revive -config revive.toml -exclude file1.go -exclude file2.go -formatter friendly github.com/deepsourcelabs/revive package/... ``` - The command above will use the configuration from `revive.toml` - `revive` will ignore `file1.go` and `file2.go` - The output will be formatted with the `friendly` formatter -- The linter will analyze `github.com/mgechev/revive` and the files in `package` +- The linter will analyze `github.com/deepsourcelabs/revive` and the files in `package` ### Comment Directives @@ -350,6 +351,8 @@ enableAllRules = true Arguments = [7] [rule.function-result-limit] Arguments = [3] +[rule.error-strings] + Arguments = ["mypackage.Error"] ``` ### Default Configuration @@ -357,18 +360,18 @@ enableAllRules = true The default configuration of `revive` can be found at `defaults.toml`. This will enable all rules available in `golint` and use their default configuration (i.e. the way they are hardcoded in `golint`). ```shell -revive -config defaults.toml github.com/mgechev/revive +revive -config defaults.toml github.com/deepsourcelabs/revive ``` -This will use the configuration file `defaults.toml`, the `default` formatter, and will run linting over the `github.com/mgechev/revive` package. +This will use the configuration file `defaults.toml`, the `default` formatter, and will run linting over the `github.com/deepsourcelabs/revive` package. ### Custom Configuration ```shell -revive -config config.toml -formatter friendly github.com/mgechev/revive +revive -config config.toml -formatter friendly github.com/deepsourcelabs/revive ``` -This will use `config.toml`, the `friendly` formatter, and will run linting over the `github.com/mgechev/revive` package. +This will use `config.toml`, the `friendly` formatter, and will run linting over the `github.com/deepsourcelabs/revive` package. ### Recommended Configuration @@ -410,7 +413,6 @@ warningCode = 0 ## Available Rules List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golint` column. - | Name | Config | Description | `golint` | Typed | | --------------------- | :----: | :--------------------------------------------------------------- | :------: | :---: | | [`context-keys-type`](./RULES_DESCRIPTIONS.md#context-key-types) | n/a | Disallows the usage of basic types in `context.WithValue`. | yes | yes | @@ -423,7 +425,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`context-as-argument`](./RULES_DESCRIPTIONS.md#context-as-argument) | n/a | `context.Context` should be the first argument of a function. | yes | no | | [`dot-imports`](./RULES_DESCRIPTIONS.md#dot-imports) | n/a | Forbids `.` imports. | yes | no | | [`error-return`](./RULES_DESCRIPTIONS.md#error-return) | n/a | The error return parameter should be last. | yes | no | -| [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings) | n/a | Conventions around error strings. | yes | no | +| [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings) | []string | Conventions around error strings. | yes | no | | [`error-naming`](./RULES_DESCRIPTIONS.md#error-naming) | n/a | Naming of error variables. | yes | no | | [`exported`](./RULES_DESCRIPTIONS.md#exported) | []string | Naming and commenting conventions on exported symbols. | yes | no | | [`if-return`](./RULES_DESCRIPTIONS.md#if-return) | n/a | Redundant if when returning an error. | no | no | @@ -482,6 +484,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`banned-characters`](./RULES_DESCRIPTIONS.md#banned-characters) | n/a | Checks banned characters in identifiers | no | no | | [`optimize-operands-order`](./RULES_DESCRIPTIONS.md#optimize-operands-order) | n/a | Checks inefficient conditional expressions | no | no | | [`use-any`](./RULES_DESCRIPTIONS.md#use-any) | n/a | Proposes to replace `interface{}` with its alias `any` | no | no | +| [`datarace`](./RULES_DESCRIPTIONS.md#datarace) | n/a | Spots potential dataraces | no | no | ## Configurable rules @@ -755,6 +758,18 @@ REVIVE_FORCE_COLOR=1 revive -formatter friendly ./... | tee revive.log :---: |:---: |:---: |:---: |:---: | [cce](https://github.com/cce) |[haya14busa](https://github.com/haya14busa) |[jmckenzieark](https://github.com/jmckenzieark) |[techknowlogick](https://github.com/techknowlogick) |[zeripath](https://github.com/zeripath) | +[ldez](https://github.com/ldez) |[mathieu-aubin](https://github.com/mathieu-aubin) |[michalhisim](https://github.com/michalhisim) |[pa-m](https://github.com/pa-m) |[paul-at-start](https://github.com/paul-at-start) |[weastur](https://github.com/weastur) | +:---: |:---: |:---: |:---: |:---: |:---: | +[ldez](https://github.com/ldez) |[mathieu-aubin](https://github.com/mathieu-aubin) |[michalhisim](https://github.com/michalhisim) |[pa-m](https://github.com/pa-m) |[paul-at-start](https://github.com/paul-at-start) |[weastur](https://github.com/weastur) | + +[paco0x](https://github.com/paco0x) |[rdeusser](https://github.com/rdeusser) |[ridvansumset](https://github.com/ridvansumset) |[tartale](https://github.com/tartale) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) | +:---: |:---: |:---: |:---: |:---: |:---: | +[paco0x](https://github.com/paco0x) |[rdeusser](https://github.com/rdeusser) |[ridvansumset](https://github.com/ridvansumset) |[tartale](https://github.com/tartale) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) | + +[cce](https://github.com/cce) |[haya14busa](https://github.com/haya14busa) |[jmckenzieark](https://github.com/jmckenzieark) |[techknowlogick](https://github.com/techknowlogick) |[zeripath](https://github.com/zeripath) | +:---: |:---: |:---: |:---: |:---: | +[cce](https://github.com/cce) |[haya14busa](https://github.com/haya14busa) |[jmckenzieark](https://github.com/jmckenzieark) |[techknowlogick](https://github.com/techknowlogick) |[zeripath](https://github.com/zeripath) | + ## License MIT diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index a788e4a69..e51e5f12c 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -19,6 +19,7 @@ List of all available rules. - [context-as-argument](#context-as-argument) - [context-keys-type](#context-keys-type) - [cyclomatic](#cyclomatic) + - [datarace](#datarace) - [deep-exit](#deep-exit) - [defer](#defer) - [dot-imports](#dot-imports) @@ -217,6 +218,11 @@ Example: [rule.cyclomatic] arguments =[3] ``` +## datarace + +_Description_: This rule spots potential dataraces caused by go-routines capturing (by-reference) particular identifiers of the function from which go-routines are created. The rule is able to spot two of such cases: go-routines capturing named return values, and capturing `for-range` values. + +_Configuration_: N/A ## deep-exit @@ -426,13 +432,13 @@ _Configuration_: N/A _Description_: Warns when importing black-listed packages. -_Configuration_: black-list of package names +_Configuration_: black-list of package names (or regular expression package names). Example: ```toml [imports-blacklist] - arguments =["crypto/md5", "crypto/sha1"] + arguments =["crypto/md5", "crypto/sha1", "crypto/**/pkix"] ``` ## import-shadowing diff --git a/config/config.go b/config/config.go index c6457d8c6..e9d6d9c12 100644 --- a/config/config.go +++ b/config/config.go @@ -85,6 +85,7 @@ var allRules = append([]lint.Rule{ &rule.BannedCharsRule{}, &rule.OptimizeOperandsOrderRule{}, &rule.UseAnyRule{}, + &rule.DataRaceRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ @@ -122,7 +123,7 @@ func GetLintingRules(config *lint.Config, extraRules []lint.Rule) ([]lint.Rule, var lintingRules []lint.Rule for name, ruleConfig := range config.Rules { - rule, ok := rulesMap[name] + r, ok := rulesMap[name] if !ok { return nil, fmt.Errorf("cannot find rule: %s", name) } @@ -131,7 +132,7 @@ func GetLintingRules(config *lint.Config, extraRules []lint.Rule) ([]lint.Rule, continue // skip disabled rules } - lintingRules = append(lintingRules, rule) + lintingRules = append(lintingRules, r) } return lintingRules, nil @@ -155,8 +156,8 @@ func normalizeConfig(config *lint.Config) { } if config.EnableAllRules { // Add to the configuration all rules not yet present in it - for _, rule := range allRules { - ruleName := rule.Name() + for _, r := range allRules { + ruleName := r.Name() _, alreadyInConf := config.Rules[ruleName] if alreadyInConf { continue @@ -207,15 +208,15 @@ func GetConfig(configPath string) (*lint.Config, error) { // GetFormatter yields the formatter for lint failures func GetFormatter(formatterName string) (lint.Formatter, error) { formatters := getFormatters() - formatter := formatters["default"] + fmtr := formatters["default"] if formatterName != "" { f, ok := formatters[formatterName] if !ok { return nil, fmt.Errorf("unknown formatter %v", formatterName) } - formatter = f + fmtr = f } - return formatter, nil + return fmtr, nil } func defaultConfig() *lint.Config { diff --git a/formatter/checkstyle.go b/formatter/checkstyle.go index 4f568e0f7..b7a2d6658 100644 --- a/formatter/checkstyle.go +++ b/formatter/checkstyle.go @@ -15,7 +15,7 @@ type Checkstyle struct { } // Name returns the name of the formatter -func (f *Checkstyle) Name() string { +func (*Checkstyle) Name() string { return "checkstyle" } @@ -29,7 +29,7 @@ type issue struct { } // Format formats the failures gotten from the lint. -func (f *Checkstyle) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { +func (*Checkstyle) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { issues := map[string][]issue{} for failure := range failures { buf := new(bytes.Buffer) diff --git a/formatter/default.go b/formatter/default.go index 9711d4806..8e66a8413 100644 --- a/formatter/default.go +++ b/formatter/default.go @@ -13,12 +13,12 @@ type Default struct { } // Name returns the name of the formatter -func (f *Default) Name() string { +func (*Default) Name() string { return "default" } // Format formats the failures gotten from the lint. -func (f *Default) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { +func (*Default) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { for failure := range failures { fmt.Printf("%v: %s\n", failure.Position.Start, failure.Failure) } diff --git a/formatter/friendly.go b/formatter/friendly.go index eddf18945..8ee21247f 100644 --- a/formatter/friendly.go +++ b/formatter/friendly.go @@ -10,16 +10,6 @@ import ( "github.com/olekukonko/tablewriter" ) -var newLines = map[rune]bool{ - 0x000A: true, - 0x000B: true, - 0x000C: true, - 0x000D: true, - 0x0085: true, - 0x2028: true, - 0x2029: true, -} - func getErrorEmoji() string { return color.RedString("✘") } @@ -35,7 +25,7 @@ type Friendly struct { } // Name returns the name of the formatter -func (f *Friendly) Name() string { +func (*Friendly) Name() string { return "friendly" } @@ -78,7 +68,7 @@ func (f *Friendly) printHeaderRow(failure lint.Failure, severity lint.Severity) fmt.Print(f.table([][]string{{emoji, "https://revive.run/r#" + failure.RuleName, color.GreenString(failure.Failure)}})) } -func (f *Friendly) printFilePosition(failure lint.Failure) { +func (*Friendly) printFilePosition(failure lint.Failure) { fmt.Printf(" %s:%d:%d", failure.GetFilename(), failure.Position.Start.Line, failure.Position.Start.Column) } @@ -87,7 +77,7 @@ type statEntry struct { failures int } -func (f *Friendly) printSummary(errors, warnings int) { +func (*Friendly) printSummary(errors, warnings int) { emoji := getWarningEmoji() if errors > 0 { emoji = getErrorEmoji() @@ -136,7 +126,7 @@ func (f *Friendly) printStatistics(header string, stats map[string]int) { fmt.Println(f.table(formatted)) } -func (f *Friendly) table(rows [][]string) string { +func (*Friendly) table(rows [][]string) string { buf := new(bytes.Buffer) table := tablewriter.NewWriter(buf) table.SetBorder(false) diff --git a/formatter/json.go b/formatter/json.go index 4c919886b..b6ab2cfa7 100644 --- a/formatter/json.go +++ b/formatter/json.go @@ -13,7 +13,7 @@ type JSON struct { } // Name returns the name of the formatter -func (f *JSON) Name() string { +func (*JSON) Name() string { return "json" } @@ -24,7 +24,7 @@ type jsonObject struct { } // Format formats the failures gotten from the lint. -func (f *JSON) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { +func (*JSON) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { var slice []jsonObject for failure := range failures { obj := jsonObject{} diff --git a/formatter/ndjson.go b/formatter/ndjson.go index 69cf96a0c..6e8965b38 100644 --- a/formatter/ndjson.go +++ b/formatter/ndjson.go @@ -14,12 +14,12 @@ type NDJSON struct { } // Name returns the name of the formatter -func (f *NDJSON) Name() string { +func (*NDJSON) Name() string { return "ndjson" } // Format formats the failures gotten from the lint. -func (f *NDJSON) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { +func (*NDJSON) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { enc := json.NewEncoder(os.Stdout) for failure := range failures { obj := jsonObject{} diff --git a/formatter/plain.go b/formatter/plain.go index 859547800..6abfa57b4 100644 --- a/formatter/plain.go +++ b/formatter/plain.go @@ -13,12 +13,12 @@ type Plain struct { } // Name returns the name of the formatter -func (f *Plain) Name() string { +func (*Plain) Name() string { return "plain" } // Format formats the failures gotten from the lint. -func (f *Plain) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { +func (*Plain) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { for failure := range failures { fmt.Printf("%v: %s %s\n", failure.Position.Start, failure.Failure, "https://revive.run/r#"+failure.RuleName) } diff --git a/formatter/sarif.go b/formatter/sarif.go index 9bf1c1636..f7ec9d54f 100644 --- a/formatter/sarif.go +++ b/formatter/sarif.go @@ -16,14 +16,14 @@ type Sarif struct { } // Name returns the name of the formatter -func (f *Sarif) Name() string { +func (*Sarif) Name() string { return "sarif" } const reviveSite = "https://revive.run" // Format formats the failures gotten from the lint. -func (f *Sarif) Format(failures <-chan lint.Failure, cfg lint.Config) (string, error) { +func (*Sarif) Format(failures <-chan lint.Failure, cfg lint.Config) (string, error) { sarifLog := newReviveRunLog(cfg) for failure := range failures { diff --git a/formatter/stylish.go b/formatter/stylish.go index 9a530715f..aa09e2caa 100644 --- a/formatter/stylish.go +++ b/formatter/stylish.go @@ -16,7 +16,7 @@ type Stylish struct { } // Name returns the name of the formatter -func (f *Stylish) Name() string { +func (*Stylish) Name() string { return "stylish" } @@ -32,7 +32,7 @@ func formatFailure(failure lint.Failure, severity lint.Severity) []string { } // Format formats the failures gotten from the lint. -func (f *Stylish) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { +func (*Stylish) Format(failures <-chan lint.Failure, config lint.Config) (string, error) { var result [][]string totalErrors := 0 total := 0 diff --git a/formatter/unix.go b/formatter/unix.go index b97d2e8e7..8f5a1df16 100644 --- a/formatter/unix.go +++ b/formatter/unix.go @@ -14,12 +14,12 @@ type Unix struct { } // Name returns the name of the formatter -func (f *Unix) Name() string { +func (*Unix) Name() string { return "unix" } // Format formats the failures gotten from the lint. -func (f *Unix) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { +func (*Unix) Format(failures <-chan lint.Failure, _ lint.Config) (string, error) { for failure := range failures { fmt.Printf("%v: [%s] %s\n", failure.Position.Start, failure.RuleName, failure.Failure) } diff --git a/go.mod b/go.mod index 81ae2400c..619d30b95 100644 --- a/go.mod +++ b/go.mod @@ -4,14 +4,14 @@ go 1.17 require ( github.com/BurntSushi/toml v1.1.0 - github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4 + github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 github.com/fatih/color v1.13.0 github.com/fatih/structtag v1.2.0 github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517 github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.1.10 + golang.org/x/tools v0.1.11 ) require ( diff --git a/go.sum b/go.sum index 7d3148637..b8ff66a3c 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,10 @@ github.com/BurntSushi/toml v1.1.0 h1:ksErzDEI1khOiGPgpwuI7x2ebx/uXQNw7xJpn9Eq1+I= github.com/BurntSushi/toml v1.1.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= -github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4 h1:tFXjAxje9thrTF4h57Ckik+scJjTWdwAtZqZPtOT48M= -github.com/chavacava/garif v0.0.0-20220316182200-5cad0b5181d4/go.mod h1:W8EnPSQ8Nv4fUjc/v1/8tHFqhuOJXnRub0dTfuAQktU= -github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 h1:E7LT642ysztPWE0dfz43cWOvMiF42DyTRC+eZIaO4yI= +github.com/chavacava/garif v0.0.0-20220630083739-93517212f375/go.mod h1:4m1Rv7xfuwWPNKXlThldNuJvutYM6J95wNuuVmn55To= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4= @@ -30,12 +31,14 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.1 h1:5TQK59W5E3v0r2duFAb7P95B6hEeOyEnHRa8MjYSMTY= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/yuin/goldmark v1.4.1/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY= +golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= @@ -59,11 +62,10 @@ golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= -golang.org/x/tools v0.1.10 h1:QjFRCZxdOhBJ/UNgnBZLbNV13DlbnK0quyivTnXJM20= -golang.org/x/tools v0.1.10/go.mod h1:Uh6Zz+xoGYZom868N8YTex3t7RhtHDBrE8Gzo9bV56E= +golang.org/x/tools v0.1.11 h1:loJ25fNOEhSXfHrpoGj91eCUThwdNX6u24rO1xnNteY= +golang.org/x/tools v0.1.11/go.mod h1:SgwaegtQh8clINPpECJMqnxLv9I09HLqnW3RMqW0CA4= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= 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= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/typeparams/typeparams.go b/internal/typeparams/typeparams.go new file mode 100644 index 000000000..33c4f203e --- /dev/null +++ b/internal/typeparams/typeparams.go @@ -0,0 +1,29 @@ +// Package typeparams provides utilities for working with Go ASTs with support +// for type parameters when built with Go 1.18 and higher. +package typeparams + +import ( + "go/ast" +) + +// Enabled reports whether type parameters are enabled in the current build +// environment. +func Enabled() bool { + return enabled +} + +// ReceiverType returns the named type of the method receiver, sans "*" and type +// parameters, or "invalid-type" if fn.Recv is ill formed. +func ReceiverType(fn *ast.FuncDecl) string { + e := fn.Recv.List[0].Type + if s, ok := e.(*ast.StarExpr); ok { + e = s.X + } + if enabled { + e = unpackIndexExpr(e) + } + if id, ok := e.(*ast.Ident); ok { + return id.Name + } + return "invalid-type" +} diff --git a/internal/typeparams/typeparams_go117.go b/internal/typeparams/typeparams_go117.go new file mode 100644 index 000000000..913a7316e --- /dev/null +++ b/internal/typeparams/typeparams_go117.go @@ -0,0 +1,12 @@ +//go:build !go1.18 +// +build !go1.18 + +package typeparams + +import ( + "go/ast" +) + +const enabled = false + +func unpackIndexExpr(e ast.Expr) ast.Expr { return e } diff --git a/internal/typeparams/typeparams_go118.go b/internal/typeparams/typeparams_go118.go new file mode 100644 index 000000000..0f7fd88d3 --- /dev/null +++ b/internal/typeparams/typeparams_go118.go @@ -0,0 +1,20 @@ +//go:build go1.18 +// +build go1.18 + +package typeparams + +import ( + "go/ast" +) + +const enabled = true + +func unpackIndexExpr(e ast.Expr) ast.Expr { + switch e := e.(type) { + case *ast.IndexExpr: + return e.X + case *ast.IndexListExpr: + return e.X + } + return e +} diff --git a/lint/file.go b/lint/file.go index 6505760f5..0d1ac1667 100644 --- a/lint/file.go +++ b/lint/file.go @@ -78,7 +78,7 @@ func (f *File) IsUntypedConst(expr ast.Expr) (defType string, ok bool) { // Re-evaluate expr outside its context to see if it's untyped. // (An expr evaluated within, for example, an assignment context will get the type of the LHS.) exprStr := f.Render(expr) - tv, err := types.Eval(f.Pkg.fset, f.Pkg.TypesPkg, expr.Pos(), exprStr) + tv, err := types.Eval(f.Pkg.fset, f.Pkg.TypesPkg(), expr.Pos(), exprStr) if err != nil { return "", false } @@ -259,7 +259,7 @@ func (f *File) disabledIntervals(rules []Rule, mustSpecifyDisableReason bool, fa return getEnabledDisabledIntervals() } -func (f *File) filterFailures(failures []Failure, disabledIntervals disabledIntervalsMap) []Failure { +func (File) filterFailures(failures []Failure, disabledIntervals disabledIntervalsMap) []Failure { result := []Failure{} for _, failure := range failures { fStart := failure.Position.Start.Line diff --git a/lint/linter.go b/lint/linter.go index 940c858b0..933082c26 100644 --- a/lint/linter.go +++ b/lint/linter.go @@ -85,7 +85,6 @@ func (l *Linter) lintPackage(filenames []string, ruleSet []Rule, config Config, pkg := &Package{ fset: token.NewFileSet(), files: map[string]*File{}, - mu: sync.Mutex{}, } for _, filename := range filenames { content, err := l.readFile(filename) @@ -141,7 +140,7 @@ func addInvalidFileFailure(filename, errStr string, failures chan Failure) { // errPosRegexp matches with an NewFile error message // i.e. : corrupted.go:10:4: expected '}', found 'EOF // first group matches the line and the second group, the column -var errPosRegexp = regexp.MustCompile(".*:(\\d*):(\\d*):.*$") +var errPosRegexp = regexp.MustCompile(`.*:(\d*):(\d*):.*$`) // getPositionInvalidFile gets the position of the error in an invalid file func getPositionInvalidFile(filename, s string) FailurePosition { diff --git a/lint/package.go b/lint/package.go index fd858c56a..41ab68b3e 100644 --- a/lint/package.go +++ b/lint/package.go @@ -9,6 +9,8 @@ import ( "sync" "golang.org/x/tools/go/gcexportdata" + + "github.com/deepsourcelabs/revive/internal/typeparams" ) // Package represents a package in the project. @@ -16,14 +18,14 @@ type Package struct { fset *token.FileSet files map[string]*File - TypesPkg *types.Package - TypesInfo *types.Info + typesPkg *types.Package + typesInfo *types.Info // sortable is the set of types in the package that implement sort.Interface. - Sortable map[string]bool + sortable map[string]bool // main is whether this is a "main" package. main int - mu sync.Mutex + sync.RWMutex } var newImporter = func(fset *token.FileSet) types.ImporterFrom { @@ -36,8 +38,16 @@ var ( notSet = 3 ) +// Files return package's files. +func (p *Package) Files() map[string]*File { + return p.files +} + // IsMain returns if that's the main package. func (p *Package) IsMain() bool { + p.Lock() + defer p.Unlock() + if p.main == trueValue { return true } else if p.main == falseValue { @@ -53,13 +63,35 @@ func (p *Package) IsMain() bool { return false } +// TypesPkg yields information on this package +func (p *Package) TypesPkg() *types.Package { + p.RLock() + defer p.RUnlock() + return p.typesPkg +} + +// TypesInfo yields type information of this package identifiers +func (p *Package) TypesInfo() *types.Info { + p.RLock() + defer p.RUnlock() + return p.typesInfo +} + +// Sortable yields a map of sortable types in this package +func (p *Package) Sortable() map[string]bool { + p.RLock() + defer p.RUnlock() + return p.sortable +} + // TypeCheck performs type checking for given package. func (p *Package) TypeCheck() error { - p.mu.Lock() + p.Lock() + defer p.Unlock() + // If type checking has already been performed // skip it. - if p.TypesInfo != nil || p.TypesPkg != nil { - p.mu.Unlock() + if p.typesInfo != nil || p.typesPkg != nil { return nil } config := &types.Config{ @@ -84,9 +116,9 @@ func (p *Package) TypeCheck() error { // Remember the typechecking info, even if config.Check failed, // since we will get partial information. - p.TypesPkg = typesPkg - p.TypesInfo = info - p.mu.Unlock() + p.typesPkg = typesPkg + p.typesInfo = info + return err } @@ -106,10 +138,10 @@ func check(config *types.Config, n string, fset *token.FileSet, astFiles []*ast. // TypeOf returns the type of an expression. func (p *Package) TypeOf(expr ast.Expr) types.Type { - if p.TypesInfo == nil { + if p.typesInfo == nil { return nil } - return p.TypesInfo.TypeOf(expr) + return p.typesInfo.TypeOf(expr) } type walker struct { @@ -123,7 +155,7 @@ func (w *walker) Visit(n ast.Node) ast.Visitor { return w } // TODO(dsymonds): We could check the signature to be more precise. - recv := receiverType(fn) + recv := typeparams.ReceiverType(fn) if i, ok := w.nmap[fn.Name.Name]; ok { w.has[recv] |= i } @@ -131,7 +163,7 @@ func (w *walker) Visit(n ast.Node) ast.Visitor { } func (p *Package) scanSortable() { - p.Sortable = make(map[string]bool) + p.sortable = make(map[string]bool) // bitfield for which methods exist on each type. const ( @@ -146,7 +178,7 @@ func (p *Package) scanSortable() { } for typ, ms := range has { if ms == Len|Less|Swap { - p.Sortable[typ] = true + p.sortable[typ] = true } } } diff --git a/revivelib/core.go b/revivelib/core.go index c2af9e8bd..ca83ec652 100644 --- a/revivelib/core.go +++ b/revivelib/core.go @@ -28,7 +28,7 @@ func New( maxOpenFiles int, extraRules ...ExtraRule, ) (*Revive, error) { - log, err := logging.GetLogger() + logger, err := logging.GetLogger() if err != nil { return nil, errors.Wrap(err, "initializing revive - getting logger") } @@ -55,10 +55,10 @@ func New( return nil, errors.Wrap(err, "initializing revive - gettint lint rules") } - log.Println("Config loaded") + logger.Println("Config loaded") return &Revive{ - logger: log, + logger: logger, config: conf, lintingRules: lintingRules, maxOpenFiles: maxOpenFiles, diff --git a/revivelib/core_internal_test.go b/revivelib/core_internal_test.go index f0d85b597..8f19a2201 100644 --- a/revivelib/core_internal_test.go +++ b/revivelib/core_internal_test.go @@ -39,11 +39,11 @@ func TestReviveCreateInstance(t *testing.T) { type mockRule struct { } -func (r *mockRule) Name() string { +func (*mockRule) Name() string { return "mock-rule" } -func (r *mockRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (*mockRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { return nil } diff --git a/rule/add-constant.go b/rule/add-constant.go index f3f5f5662..8f05e1fce 100644 --- a/rule/add-constant.go +++ b/rule/add-constant.go @@ -5,6 +5,7 @@ import ( "go/ast" "strconv" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -33,53 +34,12 @@ func (wl whiteList) add(kind, list string) { type AddConstantRule struct { whiteList whiteList strLitLimit int + sync.Mutex } // Apply applies the rule to given file. func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { - if r.whiteList == nil { - r.strLitLimit = defaultStrLitLimit - r.whiteList = newWhiteList() - if len(arguments) > 0 { - args, ok := arguments[0].(map[string]interface{}) - if !ok { - panic(fmt.Sprintf("Invalid argument to the add-constant rule. Expecting a k,v map, got %T", arguments[0])) - } - for k, v := range args { - kind := "" - switch k { - case "allowFloats": - kind = kindFLOAT - fallthrough - case "allowInts": - if kind == "" { - kind = kindINT - } - fallthrough - case "allowStrs": - if kind == "" { - kind = kindSTRING - } - list, ok := v.(string) - if !ok { - panic(fmt.Sprintf("Invalid argument to the add-constant rule, string expected. Got '%v' (%T)", v, v)) - } - r.whiteList.add(kind, list) - case "maxLitCount": - sl, ok := v.(string) - if !ok { - panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v' (%T)", v, v)) - } - - limit, err := strconv.Atoi(sl) - if err != nil { - panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)) - } - r.strLitLimit = limit - } - } - } - } + r.configure(arguments) var failures []lint.Failure @@ -95,7 +55,7 @@ func (r *AddConstantRule) Apply(file *lint.File, arguments lint.Arguments) []lin } // Name returns the rule name. -func (r *AddConstantRule) Name() string { +func (*AddConstantRule) Name() string { return "add-constant" } @@ -154,3 +114,52 @@ func (w lintAddConstantRule) checkNumLit(kind string, n *ast.BasicLit) { Failure: fmt.Sprintf("avoid magic numbers like '%s', create a named constant for it", n.Value), }) } + +func (r *AddConstantRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.whiteList == nil { + r.strLitLimit = defaultStrLitLimit + r.whiteList = newWhiteList() + if len(arguments) > 0 { + args, ok := arguments[0].(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule. Expecting a k,v map, got %T", arguments[0])) + } + for k, v := range args { + kind := "" + switch k { + case "allowFloats": + kind = kindFLOAT + fallthrough + case "allowInts": + if kind == "" { + kind = kindINT + } + fallthrough + case "allowStrs": + if kind == "" { + kind = kindSTRING + } + list, ok := v.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, string expected. Got '%v' (%T)", v, v)) + } + r.whiteList.add(kind, list) + case "maxLitCount": + sl, ok := v.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v' (%T)", v, v)) + } + + limit, err := strconv.Atoi(sl) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the add-constant rule, expecting string representation of an integer. Got '%v'", v)) + } + r.strLitLimit = limit + } + } + } + } +} diff --git a/rule/argument-limit.go b/rule/argument-limit.go index e3048111e..47c0f5e34 100644 --- a/rule/argument-limit.go +++ b/rule/argument-limit.go @@ -3,6 +3,7 @@ package rule import ( "fmt" "go/ast" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -10,10 +11,11 @@ import ( // ArgumentsLimitRule lints given else constructs. type ArgumentsLimitRule struct { total int + sync.Mutex } -// Apply applies the rule to given file. -func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) { + r.Lock() if r.total == 0 { checkNumberOfArguments(1, arguments, r.Name()) @@ -23,14 +25,21 @@ func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) [] } r.total = int(total) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } walker := lintArgsNum{ - total: r.total, - onFailure: func(failure lint.Failure) { - failures = append(failures, failure) - }, + total: r.total, + onFailure: onFailure, } ast.Walk(walker, file.AST) @@ -39,7 +48,7 @@ func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) [] } // Name returns the rule name. -func (r *ArgumentsLimitRule) Name() string { +func (*ArgumentsLimitRule) Name() string { return "argument-limit" } diff --git a/rule/atomic.go b/rule/atomic.go index 7c6be3a47..84ec1384b 100644 --- a/rule/atomic.go +++ b/rule/atomic.go @@ -12,10 +12,10 @@ import ( type AtomicRule struct{} // Apply applies the rule to given file. -func (r *AtomicRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*AtomicRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := atomic{ - pkgTypesInfo: file.Pkg.TypesInfo, + pkgTypesInfo: file.Pkg.TypesInfo(), onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, @@ -27,7 +27,7 @@ func (r *AtomicRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *AtomicRule) Name() string { +func (*AtomicRule) Name() string { return "atomic" } diff --git a/rule/banned-characters.go b/rule/banned-characters.go index 42238e528..3b9362544 100644 --- a/rule/banned-characters.go +++ b/rule/banned-characters.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -11,16 +12,23 @@ import ( // BannedCharsRule checks if a file contains banned characters. type BannedCharsRule struct { bannedCharList []string + sync.Mutex } const bannedCharsRuleName = "banned-characters" -// Apply applied the rule to the given file. -func (r *BannedCharsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *BannedCharsRule) configure(arguments lint.Arguments) { + r.Lock() if r.bannedCharList == nil { checkNumberOfArguments(1, arguments, bannedCharsRuleName) r.bannedCharList = r.getBannedCharsList(arguments) } + r.Unlock() +} + +// Apply applied the rule to the given file. +func (r *BannedCharsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -31,12 +39,13 @@ func (r *BannedCharsRule) Apply(file *lint.File, arguments lint.Arguments) []lin bannedChars: r.bannedCharList, onFailure: onFailure, } + ast.Walk(w, file.AST) return failures } // Name returns the rule name -func (r *BannedCharsRule) Name() string { +func (*BannedCharsRule) Name() string { return bannedCharsRuleName } diff --git a/rule/bare-return.go b/rule/bare-return.go index f628ca6cd..965cfd6d0 100644 --- a/rule/bare-return.go +++ b/rule/bare-return.go @@ -10,7 +10,7 @@ import ( type BareReturnRule struct{} // Apply applies the rule to given file. -func (r *BareReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*BareReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -23,7 +23,7 @@ func (r *BareReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *BareReturnRule) Name() string { +func (*BareReturnRule) Name() string { return "bare-return" } diff --git a/rule/blank-imports.go b/rule/blank-imports.go index 58a79933e..c861d1208 100644 --- a/rule/blank-imports.go +++ b/rule/blank-imports.go @@ -11,7 +11,7 @@ import ( type BlankImportsRule struct{} // Name returns the rule name. -func (r *BlankImportsRule) Name() string { +func (*BlankImportsRule) Name() string { return "blank-imports" } @@ -62,7 +62,7 @@ func (r *BlankImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu return failures } -func (r *BlankImportsRule) fileHasValidEmbedComment(fileAst *ast.File) bool { +func (*BlankImportsRule) fileHasValidEmbedComment(fileAst *ast.File) bool { for _, commentGroup := range fileAst.Comments { for _, comment := range commentGroup.List { if strings.HasPrefix(comment.Text, "//go:embed ") { diff --git a/rule/bool-literal-in-expr.go b/rule/bool-literal-in-expr.go index a0504a079..892ad3316 100644 --- a/rule/bool-literal-in-expr.go +++ b/rule/bool-literal-in-expr.go @@ -11,7 +11,7 @@ import ( type BoolLiteralRule struct{} // Apply applies the rule to given file. -func (r *BoolLiteralRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*BoolLiteralRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -26,7 +26,7 @@ func (r *BoolLiteralRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *BoolLiteralRule) Name() string { +func (*BoolLiteralRule) Name() string { return "bool-literal-in-expr" } diff --git a/rule/call-to-gc.go b/rule/call-to-gc.go index 0d621b48c..8c5ab3826 100644 --- a/rule/call-to-gc.go +++ b/rule/call-to-gc.go @@ -10,7 +10,7 @@ import ( type CallToGCRule struct{} // Apply applies the rule to given file. -func (r *CallToGCRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*CallToGCRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -27,7 +27,7 @@ func (r *CallToGCRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *CallToGCRule) Name() string { +func (*CallToGCRule) Name() string { return "call-to-gc" } diff --git a/rule/cognitive-complexity.go b/rule/cognitive-complexity.go index 9ea5c11ed..d19f0a2ea 100644 --- a/rule/cognitive-complexity.go +++ b/rule/cognitive-complexity.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/token" + "sync" "github.com/deepsourcelabs/revive/lint" "golang.org/x/tools/go/ast/astutil" @@ -12,10 +13,11 @@ import ( // CognitiveComplexityRule lints given else constructs. type CognitiveComplexityRule struct { maxComplexity int + sync.Mutex } -// Apply applies the rule to given file. -func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) { + r.Lock() if r.maxComplexity == 0 { checkNumberOfArguments(1, arguments, r.Name()) @@ -25,8 +27,15 @@ func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Argument } r.maxComplexity = int(complexity) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure + linter := cognitiveComplexityLinter{ file: file, maxComplexity: r.maxComplexity, @@ -35,13 +44,13 @@ func (r *CognitiveComplexityRule) Apply(file *lint.File, arguments lint.Argument }, } - linter.lint() + linter.lintCognitiveComplexity() return failures } // Name returns the rule name. -func (r *CognitiveComplexityRule) Name() string { +func (*CognitiveComplexityRule) Name() string { return "cognitive-complexity" } @@ -51,7 +60,7 @@ type cognitiveComplexityLinter struct { onFailure func(lint.Failure) } -func (w cognitiveComplexityLinter) lint() { +func (w cognitiveComplexityLinter) lintCognitiveComplexity() { f := w.file for _, decl := range f.AST.Decls { if fn, ok := decl.(*ast.FuncDecl); ok && fn.Body != nil { diff --git a/rule/confusing-naming.go b/rule/confusing-naming.go index fd4bd74d1..15344e513 100644 --- a/rule/confusing-naming.go +++ b/rule/confusing-naming.go @@ -48,7 +48,7 @@ var allPkgs = packages{pkgs: make([]pkgMethods, 1)} type ConfusingNamingRule struct{} // Apply applies the rule to given file. -func (r *ConfusingNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ConfusingNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST pkgm := allPkgs.methodNames(file.Pkg) @@ -66,7 +66,7 @@ func (r *ConfusingNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *ConfusingNamingRule) Name() string { +func (*ConfusingNamingRule) Name() string { return "confusing-naming" } diff --git a/rule/confusing-results.go b/rule/confusing-results.go index 1da413ea7..b175488c8 100644 --- a/rule/confusing-results.go +++ b/rule/confusing-results.go @@ -10,7 +10,7 @@ import ( type ConfusingResultsRule struct{} // Apply applies the rule to given file. -func (r *ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -26,7 +26,7 @@ func (r *ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.F } // Name returns the rule name. -func (r *ConfusingResultsRule) Name() string { +func (*ConfusingResultsRule) Name() string { return "confusing-results" } @@ -60,7 +60,6 @@ func (w lintConfusingResults) Visit(n ast.Node) ast.Visitor { break } lastType = t.Name - } return w diff --git a/rule/constant-logical-expr.go b/rule/constant-logical-expr.go index c398ae8b2..a29f86dfc 100644 --- a/rule/constant-logical-expr.go +++ b/rule/constant-logical-expr.go @@ -25,7 +25,7 @@ func (r *ConstantLogicalExprRule) Apply(file *lint.File, _ lint.Arguments) []lin } // Name returns the rule name. -func (r *ConstantLogicalExprRule) Name() string { +func (*ConstantLogicalExprRule) Name() string { return "constant-logical-expr" } diff --git a/rule/context-as-argument.go b/rule/context-as-argument.go index f062b095b..b8cc157ed 100644 --- a/rule/context-as-argument.go +++ b/rule/context-as-argument.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -11,21 +12,26 @@ import ( // ContextAsArgumentRule lints given else constructs. type ContextAsArgumentRule struct { allowTypesLUT map[string]struct{} + sync.Mutex } // Apply applies the rule to given file. func (r *ContextAsArgumentRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.Lock() if r.allowTypesLUT == nil { r.allowTypesLUT = getAllowTypesFromArguments(args) } + r.Unlock() var failures []lint.Failure + r.Lock() walker := lintContextArguments{ allowTypesLUT: r.allowTypesLUT, onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, } + r.Unlock() ast.Walk(walker, file.AST) @@ -33,7 +39,7 @@ func (r *ContextAsArgumentRule) Apply(file *lint.File, args lint.Arguments) []li } // Name returns the rule name. -func (r *ContextAsArgumentRule) Name() string { +func (*ContextAsArgumentRule) Name() string { return "context-as-argument" } diff --git a/rule/context-keys-type.go b/rule/context-keys-type.go index 9c933db7e..87fcc74a1 100644 --- a/rule/context-keys-type.go +++ b/rule/context-keys-type.go @@ -12,7 +12,7 @@ import ( type ContextKeysType struct{} // Apply applies the rule to given file. -func (r *ContextKeysType) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ContextKeysType) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -31,7 +31,7 @@ func (r *ContextKeysType) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *ContextKeysType) Name() string { +func (*ContextKeysType) Name() string { return "context-keys-type" } @@ -68,7 +68,7 @@ func checkContextKeyType(w lintContextKeyTypes, x *ast.CallExpr) { if len(x.Args) != 3 { return } - key := f.Pkg.TypesInfo.Types[x.Args[1]] + key := f.Pkg.TypesInfo().Types[x.Args[1]] if ktyp, ok := key.Type.(*types.Basic); ok && ktyp.Kind() != types.Invalid { w.onFailure(lint.Failure{ diff --git a/rule/cyclomatic.go b/rule/cyclomatic.go index 23928a37b..0c6741414 100644 --- a/rule/cyclomatic.go +++ b/rule/cyclomatic.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/token" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -13,10 +14,11 @@ import ( // CyclomaticRule lints given else constructs. type CyclomaticRule struct { maxComplexity int + sync.Mutex } -// Apply applies the rule to given file. -func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *CyclomaticRule) configure(arguments lint.Arguments) { + r.Lock() if r.maxComplexity == 0 { checkNumberOfArguments(1, arguments, r.Name()) @@ -26,9 +28,16 @@ func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint } r.maxComplexity = int(complexity) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure fileAst := file.AST + walker := lintCyclomatic{ file: file, complexity: r.maxComplexity, @@ -43,7 +52,7 @@ func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint } // Name returns the rule name. -func (r *CyclomaticRule) Name() string { +func (*CyclomaticRule) Name() string { return "cyclomatic" } diff --git a/rule/datarace.go b/rule/datarace.go new file mode 100644 index 000000000..d1822e930 --- /dev/null +++ b/rule/datarace.go @@ -0,0 +1,142 @@ +package rule + +import ( + "fmt" + "go/ast" + + "github.com/deepsourcelabs/revive/lint" +) + +// DataRaceRule lints assignments to value method-receivers. +type DataRaceRule struct{} + +// Apply applies the rule to given file. +func (*DataRaceRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + w := lintDataRaces{onFailure: onFailure} + + ast.Walk(w, file.AST) + + return failures +} + +// Name returns the rule name. +func (*DataRaceRule) Name() string { + return "datarace" +} + +type lintDataRaces struct { + onFailure func(failure lint.Failure) +} + +func (w lintDataRaces) Visit(n ast.Node) ast.Visitor { + node, ok := n.(*ast.FuncDecl) + if !ok { + return w // not function declaration + } + if node.Body == nil { + return nil // empty body + } + + results := node.Type.Results + + returnIDs := map[*ast.Object]struct{}{} + if results != nil { + returnIDs = w.ExtractReturnIDs(results.List) + } + fl := &lintFunctionForDataRaces{onFailure: w.onFailure, returnIDs: returnIDs, rangeIDs: map[*ast.Object]struct{}{}} + ast.Walk(fl, node.Body) + + return nil +} + +func (w lintDataRaces) ExtractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} { + r := map[*ast.Object]struct{}{} + for _, f := range fields { + for _, id := range f.Names { + r[id.Obj] = struct{}{} + } + } + + return r +} + +type lintFunctionForDataRaces struct { + _ struct{} + onFailure func(failure lint.Failure) + returnIDs map[*ast.Object]struct{} + rangeIDs map[*ast.Object]struct{} +} + +func (w lintFunctionForDataRaces) Visit(node ast.Node) ast.Visitor { + switch n := node.(type) { + case *ast.RangeStmt: + if n.Body == nil { + return nil + } + + getIds := func(exprs ...ast.Expr) []*ast.Ident { + r := []*ast.Ident{} + for _, expr := range exprs { + if id, ok := expr.(*ast.Ident); ok { + r = append(r, id) + } + } + return r + } + + ids := getIds(n.Key, n.Value) + for _, id := range ids { + w.rangeIDs[id.Obj] = struct{}{} + } + + ast.Walk(w, n.Body) + + for _, id := range ids { + delete(w.rangeIDs, id.Obj) + } + + return nil // do not visit the body of the range, it has been already visited + case *ast.GoStmt: + f := n.Call.Fun + funcLit, ok := f.(*ast.FuncLit) + if !ok { + return nil + } + selectIDs := func(n ast.Node) bool { + _, ok := n.(*ast.Ident) + return ok + } + + ids := pick(funcLit.Body, selectIDs, nil) + for _, id := range ids { + id := id.(*ast.Ident) + _, isRangeID := w.rangeIDs[id.Obj] + _, isReturnID := w.returnIDs[id.Obj] + + switch { + case isRangeID: + w.onFailure(lint.Failure{ + Confidence: 1, + Node: id, + Category: "logic", + Failure: fmt.Sprintf("datarace: range value %s is captured (by-reference) in goroutine", id.Name), + }) + case isReturnID: + w.onFailure(lint.Failure{ + Confidence: 0.8, + Node: id, + Category: "logic", + Failure: fmt.Sprintf("potential datarace: return value %s is captured (by-reference) in goroutine", id.Name), + }) + } + } + + return nil + } + + return w +} diff --git a/rule/deep-exit.go b/rule/deep-exit.go index 68d246502..5ea7fbd73 100644 --- a/rule/deep-exit.go +++ b/rule/deep-exit.go @@ -11,7 +11,7 @@ import ( type DeepExitRule struct{} // Apply applies the rule to given file. -func (r *DeepExitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*DeepExitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -36,7 +36,7 @@ func (r *DeepExitRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *DeepExitRule) Name() string { +func (*DeepExitRule) Name() string { return "deep-exit" } diff --git a/rule/defer.go b/rule/defer.go index f6d47f5f7..432b12bcf 100644 --- a/rule/defer.go +++ b/rule/defer.go @@ -3,6 +3,7 @@ package rule import ( "fmt" "go/ast" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -10,18 +11,25 @@ import ( // DeferRule lints unused params in functions. type DeferRule struct { allow map[string]bool + sync.Mutex } -// Apply applies the rule to given file. -func (r *DeferRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *DeferRule) configure(arguments lint.Arguments) { + r.Lock() if r.allow == nil { r.allow = r.allowFromArgs(arguments) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *DeferRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) } - w := lintDeferRule{onFailure: onFailure, allow: r.allow} ast.Walk(w, file.AST) @@ -30,11 +38,11 @@ func (r *DeferRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Fail } // Name returns the rule name. -func (r *DeferRule) Name() string { +func (*DeferRule) Name() string { return "defer" } -func (r *DeferRule) allowFromArgs(args lint.Arguments) map[string]bool { +func (*DeferRule) allowFromArgs(args lint.Arguments) map[string]bool { if len(args) < 1 { allow := map[string]bool{ "loop": true, diff --git a/rule/dot-imports.go b/rule/dot-imports.go index 12cf25ce0..dfe2ebc62 100644 --- a/rule/dot-imports.go +++ b/rule/dot-imports.go @@ -10,7 +10,7 @@ import ( type DotImportsRule struct{} // Apply applies the rule to given file. -func (r *DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -28,7 +28,7 @@ func (r *DotImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *DotImportsRule) Name() string { +func (*DotImportsRule) Name() string { return "dot-imports" } diff --git a/rule/duplicated-imports.go b/rule/duplicated-imports.go index a2f5249e7..6767f829a 100644 --- a/rule/duplicated-imports.go +++ b/rule/duplicated-imports.go @@ -10,7 +10,7 @@ import ( type DuplicatedImportsRule struct{} // Apply applies the rule to given file. -func (r *DuplicatedImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*DuplicatedImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure impPaths := map[string]struct{}{} @@ -34,6 +34,6 @@ func (r *DuplicatedImportsRule) Apply(file *lint.File, _ lint.Arguments) []lint. } // Name returns the rule name. -func (r *DuplicatedImportsRule) Name() string { +func (*DuplicatedImportsRule) Name() string { return "duplicated-imports" } diff --git a/rule/early-return.go b/rule/early-return.go index eef99f97f..c28d4a362 100644 --- a/rule/early-return.go +++ b/rule/early-return.go @@ -10,7 +10,7 @@ import ( type EarlyReturnRule struct{} // Apply applies the rule to given file. -func (r *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -23,7 +23,7 @@ func (r *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *EarlyReturnRule) Name() string { +func (*EarlyReturnRule) Name() string { return "early-return" } diff --git a/rule/empty-block.go b/rule/empty-block.go index 6767afa2b..42572e298 100644 --- a/rule/empty-block.go +++ b/rule/empty-block.go @@ -10,7 +10,7 @@ import ( type EmptyBlockRule struct{} // Apply applies the rule to given file. -func (r *EmptyBlockRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*EmptyBlockRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -23,7 +23,7 @@ func (r *EmptyBlockRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *EmptyBlockRule) Name() string { +func (*EmptyBlockRule) Name() string { return "empty-block" } diff --git a/rule/empty-lines.go b/rule/empty-lines.go index 9c0f84b4a..7b7ef749c 100644 --- a/rule/empty-lines.go +++ b/rule/empty-lines.go @@ -18,25 +18,25 @@ func (r *EmptyLinesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure failures = append(failures, failure) } - w := lintEmptyLines{file, file.CommentMap(), onFailure} + w := lintEmptyLines{file, r.commentLines(file.CommentMap(), file), onFailure} ast.Walk(w, file.AST) return failures } // Name returns the rule name. -func (r *EmptyLinesRule) Name() string { +func (*EmptyLinesRule) Name() string { return "empty-lines" } type lintEmptyLines struct { file *lint.File - cmap ast.CommentMap + cmap map[int]struct{} onFailure func(lint.Failure) } func (w lintEmptyLines) Visit(node ast.Node) ast.Visitor { block, ok := node.(*ast.BlockStmt) - if !ok { + if !ok || len(block.List) == 0 { return w } @@ -47,67 +47,59 @@ func (w lintEmptyLines) Visit(node ast.Node) ast.Visitor { } func (w lintEmptyLines) checkStart(block *ast.BlockStmt) { - if len(block.List) == 0 { - return - } - - start := w.position(block.Lbrace) + blockStart := w.position(block.Lbrace) firstNode := block.List[0] + firstStmt := w.position(firstNode.Pos()) - if w.commentBetween(start, firstNode) { + firstBlockLineIsStmt := firstStmt.Line-(blockStart.Line+1) == 0 + _, firstBlockLineIsComment := w.cmap[blockStart.Line+1] + if firstBlockLineIsStmt || firstBlockLineIsComment { return } - first := w.position(firstNode.Pos()) - if first.Line-start.Line > 1 { - w.onFailure(lint.Failure{ - Confidence: 1, - Node: block, - Category: "style", - Failure: "extra empty line at the start of a block", - }) - } + w.onFailure(lint.Failure{ + Confidence: 1, + Node: block, + Category: "style", + Failure: "extra empty line at the start of a block", + }) } func (w lintEmptyLines) checkEnd(block *ast.BlockStmt) { - if len(block.List) < 1 { - return - } - - end := w.position(block.Rbrace) + blockEnd := w.position(block.Rbrace) lastNode := block.List[len(block.List)-1] + lastStmt := w.position(lastNode.End()) - if w.commentBetween(end, lastNode) { + lastBlockLineIsStmt := (blockEnd.Line-1)-lastStmt.Line == 0 + _, lastBlockLineIsComment := w.cmap[blockEnd.Line-1] + if lastBlockLineIsStmt || lastBlockLineIsComment { return } - last := w.position(lastNode.End()) - if end.Line-last.Line > 1 { - w.onFailure(lint.Failure{ - Confidence: 1, - Node: lastNode, - Category: "style", - Failure: "extra empty line at the end of a block", - }) - } + w.onFailure(lint.Failure{ + Confidence: 1, + Node: block, + Category: "style", + Failure: "extra empty line at the end of a block", + }) } -func (w lintEmptyLines) commentBetween(position token.Position, node ast.Node) bool { - comments := w.cmap.Filter(node).Comments() - if len(comments) == 0 { - return false - } +func (w lintEmptyLines) position(pos token.Pos) token.Position { + return w.file.ToPosition(pos) +} + +func (*EmptyLinesRule) commentLines(cmap ast.CommentMap, file *lint.File) map[int]struct{} { + result := map[int]struct{}{} - for _, comment := range comments { - start, end := w.position(comment.Pos()), w.position(comment.End()) - if start.Line-position.Line == 1 || position.Line-end.Line == 1 { - return true + for _, comments := range cmap { + for _, comment := range comments { + start := file.ToPosition(comment.Pos()) + end := file.ToPosition(comment.End()) + for i := start.Line; i <= end.Line; i++ { + result[i] = struct{}{} + } } } - return false -} - -func (w lintEmptyLines) position(pos token.Pos) token.Position { - return w.file.ToPosition(pos) + return result } diff --git a/rule/error-naming.go b/rule/error-naming.go index 0d92ffe08..4496b637e 100644 --- a/rule/error-naming.go +++ b/rule/error-naming.go @@ -13,7 +13,7 @@ import ( type ErrorNamingRule struct{} // Apply applies the rule to given file. -func (r *ErrorNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ErrorNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -31,7 +31,7 @@ func (r *ErrorNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *ErrorNamingRule) Name() string { +func (*ErrorNamingRule) Name() string { return "error-naming" } diff --git a/rule/error-return.go b/rule/error-return.go index a0c724d92..9de592a55 100644 --- a/rule/error-return.go +++ b/rule/error-return.go @@ -10,7 +10,7 @@ import ( type ErrorReturnRule struct{} // Apply applies the rule to given file. -func (r *ErrorReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ErrorReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -28,7 +28,7 @@ func (r *ErrorReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *ErrorReturnRule) Name() string { +func (*ErrorReturnRule) Name() string { return "error-return" } diff --git a/rule/error-strings.go b/rule/error-strings.go index 746c617c8..3e0e9f4d4 100644 --- a/rule/error-strings.go +++ b/rule/error-strings.go @@ -4,6 +4,8 @@ import ( "go/ast" "go/token" "strconv" + "strings" + "sync" "unicode" "unicode/utf8" @@ -11,13 +13,20 @@ import ( ) // ErrorStringsRule lints given else constructs. -type ErrorStringsRule struct{} +type ErrorStringsRule struct { + errorFunctions map[string]map[string]struct{} + sync.Mutex +} -// Apply applies the rule to given file. -func (r *ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - var failures []lint.Failure +func (r *ErrorStringsRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() - errorFunctions := map[string]map[string]struct{}{ + if r.errorFunctions != nil { + return + } + + r.errorFunctions = map[string]map[string]struct{}{ "fmt": { "Errorf": {}, }, @@ -31,11 +40,33 @@ func (r *ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu }, } + var invalidCustomFunctions []string + for _, argument := range arguments { + if functionName, ok := argument.(string); ok { + fields := strings.Split(strings.TrimSpace(functionName), ".") + if len(fields) != 2 || len(fields[0]) == 0 || len(fields[1]) == 0 { + invalidCustomFunctions = append(invalidCustomFunctions, functionName) + continue + } + r.errorFunctions[fields[0]] = map[string]struct{}{fields[1]: {}} + } + } + if len(invalidCustomFunctions) != 0 { + panic("found invalid custom function: " + strings.Join(invalidCustomFunctions, ",")) + } +} + +// Apply applies the rule to given file. +func (r *ErrorStringsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + + r.configure(arguments) + fileAst := file.AST walker := lintErrorStrings{ file: file, fileAst: fileAst, - errorFunctions: errorFunctions, + errorFunctions: r.errorFunctions, onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, @@ -47,7 +78,7 @@ func (r *ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu } // Name returns the rule name. -func (r *ErrorStringsRule) Name() string { +func (*ErrorStringsRule) Name() string { return "error-strings" } diff --git a/rule/errorf.go b/rule/errorf.go index 41231ffc1..08d4be3e1 100644 --- a/rule/errorf.go +++ b/rule/errorf.go @@ -13,7 +13,7 @@ import ( type ErrorfRule struct{} // Apply applies the rule to given file. -func (r *ErrorfRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ErrorfRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -32,7 +32,7 @@ func (r *ErrorfRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *ErrorfRule) Name() string { +func (*ErrorfRule) Name() string { return "errorf" } diff --git a/rule/exported.go b/rule/exported.go index 11c4b0316..6c2b1e878 100644 --- a/rule/exported.go +++ b/rule/exported.go @@ -5,9 +5,11 @@ import ( "go/ast" "go/token" "strings" + "sync" "unicode" "unicode/utf8" + "github.com/deepsourcelabs/revive/internal/typeparams" "github.com/deepsourcelabs/revive/lint" ) @@ -17,19 +19,14 @@ type ExportedRule struct { checkPrivateReceivers bool disableStutteringCheck bool stuttersMsg string + sync.Mutex } -// Apply applies the rule to given file. -func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { - var failures []lint.Failure - - if file.IsTest() { - return failures - } - +func (r *ExportedRule) configure(arguments lint.Arguments) { + r.Lock() if !r.configured { var sayRepetitiveInsteadOfStutters bool - r.checkPrivateReceivers, r.disableStutteringCheck, sayRepetitiveInsteadOfStutters = r.getConf(args) + r.checkPrivateReceivers, r.disableStutteringCheck, sayRepetitiveInsteadOfStutters = r.getConf(arguments) r.stuttersMsg = "stutters" if sayRepetitiveInsteadOfStutters { r.stuttersMsg = "is repetitive" @@ -37,8 +34,20 @@ func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failur r.configured = true } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) + + var failures []lint.Failure + if file.IsTest() { + return failures + } fileAst := file.AST + walker := lintExported{ file: file, fileAst: fileAst, @@ -57,7 +66,7 @@ func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *ExportedRule) Name() string { +func (*ExportedRule) Name() string { return "exported" } @@ -108,7 +117,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { if fn.Recv != nil && len(fn.Recv.List) > 0 { // method kind = "method" - recv := receiverType(fn) + recv := typeparams.ReceiverType(fn) if !w.checkPrivateReceivers && !ast.IsExported(recv) { // receiver is unexported return @@ -118,7 +127,8 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { } switch name { case "Len", "Less", "Swap": - if w.file.Pkg.Sortable[recv] { + sortables := w.file.Pkg.Sortable() + if sortables[recv] { return } } @@ -241,7 +251,7 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD return } - if vs.Doc == nil && gd.Doc == nil { + if vs.Doc == nil && vs.Comment == nil && gd.Doc == nil { if genDeclMissingComments[gd] { return } @@ -264,10 +274,16 @@ func (w *lintExported) lintValueSpecDoc(vs *ast.ValueSpec, gd *ast.GenDecl, genD } // The relevant text to check will be on either vs.Doc or gd.Doc. // Use vs.Doc preferentially. - doc := vs.Doc - if doc == nil { + var doc *ast.CommentGroup + switch { + case vs.Doc != nil: + doc = vs.Doc + case vs.Comment != nil && gd.Doc == nil: + doc = vs.Comment + default: doc = gd.Doc } + prefix := name + " " s := normalizeText(doc.Text()) if !strings.HasPrefix(s, prefix) { diff --git a/rule/file-header.go b/rule/file-header.go index cc21794fc..2fe40fb9b 100644 --- a/rule/file-header.go +++ b/rule/file-header.go @@ -3,6 +3,7 @@ package rule import ( "fmt" "regexp" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -10,15 +11,16 @@ import ( // FileHeaderRule lints given else constructs. type FileHeaderRule struct { header string + sync.Mutex } var ( - multiRegexp = regexp.MustCompile("^/\\*") + multiRegexp = regexp.MustCompile(`^/\*`) singleRegexp = regexp.MustCompile("^//") ) -// Apply applies the rule to given file. -func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *FileHeaderRule) configure(arguments lint.Arguments) { + r.Lock() if r.header == "" { checkNumberOfArguments(1, arguments, r.Name()) var ok bool @@ -27,6 +29,12 @@ func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint panic(fmt.Sprintf("invalid argument for \"file-header\" rule: first argument should be a string, got %T", arguments[0])) } } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) failure := []lint.Failure{ { @@ -67,6 +75,6 @@ func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint } // Name returns the rule name. -func (r *FileHeaderRule) Name() string { +func (*FileHeaderRule) Name() string { return "file-header" } diff --git a/rule/flag-param.go b/rule/flag-param.go index 579d3243d..f2b022510 100644 --- a/rule/flag-param.go +++ b/rule/flag-param.go @@ -11,7 +11,7 @@ import ( type FlagParamRule struct{} // Apply applies the rule to given file. -func (r *FlagParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*FlagParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -24,7 +24,7 @@ func (r *FlagParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *FlagParamRule) Name() string { +func (*FlagParamRule) Name() string { return "flag-parameter" } diff --git a/rule/function-length.go b/rule/function-length.go index c8b0e1747..0b7a9dc89 100644 --- a/rule/function-length.go +++ b/rule/function-length.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "reflect" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -12,15 +13,22 @@ import ( type FunctionLength struct { maxStmt int maxLines int + sync.Mutex } -// Apply applies the rule to given file. -func (r *FunctionLength) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *FunctionLength) configure(arguments lint.Arguments) { + r.Lock() if r.maxLines == 0 { maxStmt, maxLines := r.parseArguments(arguments) r.maxStmt = int(maxStmt) r.maxLines = int(maxLines) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *FunctionLength) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure @@ -39,11 +47,11 @@ func (r *FunctionLength) Apply(file *lint.File, arguments lint.Arguments) []lint } // Name returns the rule name. -func (r *FunctionLength) Name() string { +func (*FunctionLength) Name() string { return "function-length" } -func (r *FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxLines int64) { +func (*FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxLines int64) { if len(arguments) != 2 { panic(fmt.Sprintf(`invalid configuration for "function-length" rule, expected 2 arguments but got %d`, len(arguments))) } @@ -64,7 +72,7 @@ func (r *FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxL panic(fmt.Sprintf(`the configuration value for max statements in "function-length" rule cannot be negative, got %d`, maxLines)) } - return + return maxStmt, maxLines } type lintFuncLength struct { diff --git a/rule/function-result-limit.go b/rule/function-result-limit.go index 4aeb5c0f4..e6fef3e00 100644 --- a/rule/function-result-limit.go +++ b/rule/function-result-limit.go @@ -3,6 +3,7 @@ package rule import ( "fmt" "go/ast" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -10,10 +11,11 @@ import ( // FunctionResultsLimitRule lints given else constructs. type FunctionResultsLimitRule struct { max int + sync.Mutex } -// Apply applies the rule to given file. -func (r *FunctionResultsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *FunctionResultsLimitRule) configure(arguments lint.Arguments) { + r.Lock() if r.max == 0 { checkNumberOfArguments(1, arguments, r.Name()) @@ -26,6 +28,12 @@ func (r *FunctionResultsLimitRule) Apply(file *lint.File, arguments lint.Argumen } r.max = int(max) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *FunctionResultsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure @@ -42,7 +50,7 @@ func (r *FunctionResultsLimitRule) Apply(file *lint.File, arguments lint.Argumen } // Name returns the rule name. -func (r *FunctionResultsLimitRule) Name() string { +func (*FunctionResultsLimitRule) Name() string { return "function-result-limit" } diff --git a/rule/get-return.go b/rule/get-return.go index 845c3b13c..a1ac1a424 100644 --- a/rule/get-return.go +++ b/rule/get-return.go @@ -12,7 +12,7 @@ import ( type GetReturnRule struct{} // Apply applies the rule to given file. -func (r *GetReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*GetReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -25,7 +25,7 @@ func (r *GetReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *GetReturnRule) Name() string { +func (*GetReturnRule) Name() string { return "get-return" } diff --git a/rule/identical-branches.go b/rule/identical-branches.go index 7752a5186..30d98d0a1 100644 --- a/rule/identical-branches.go +++ b/rule/identical-branches.go @@ -10,7 +10,7 @@ import ( type IdenticalBranchesRule struct{} // Apply applies the rule to given file. -func (r *IdenticalBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*IdenticalBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -24,7 +24,7 @@ func (r *IdenticalBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint. } // Name returns the rule name. -func (r *IdenticalBranchesRule) Name() string { +func (*IdenticalBranchesRule) Name() string { return "identical-branches" } @@ -57,7 +57,7 @@ func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor { return w } -func (w *lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool { +func (lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool { if len(branches) < 2 { return false } diff --git a/rule/if-return.go b/rule/if-return.go index 76f9382c9..0eccf23e4 100644 --- a/rule/if-return.go +++ b/rule/if-return.go @@ -12,7 +12,7 @@ import ( type IfReturnRule struct{} // Apply applies the rule to given file. -func (r *IfReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*IfReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -26,7 +26,7 @@ func (r *IfReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *IfReturnRule) Name() string { +func (*IfReturnRule) Name() string { return "if-return" } diff --git a/rule/import-shadowing.go b/rule/import-shadowing.go index 6eb103aa6..cc8829c43 100644 --- a/rule/import-shadowing.go +++ b/rule/import-shadowing.go @@ -13,7 +13,7 @@ import ( type ImportShadowingRule struct{} // Apply applies the rule to given file. -func (r *ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure importNames := map[string]struct{}{} @@ -37,7 +37,7 @@ func (r *ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *ImportShadowingRule) Name() string { +func (*ImportShadowingRule) Name() string { return "import-shadowing" } diff --git a/rule/imports-blacklist.go b/rule/imports-blacklist.go index 888df899d..ecf855c40 100644 --- a/rule/imports-blacklist.go +++ b/rule/imports-blacklist.go @@ -2,42 +2,63 @@ package rule import ( "fmt" + "regexp" + "sync" "github.com/deepsourcelabs/revive/lint" ) // ImportsBlacklistRule lints given else constructs. type ImportsBlacklistRule struct { - blacklist map[string]bool + blacklist []*regexp.Regexp + sync.Mutex } -// Apply applies the rule to given file. -func (r *ImportsBlacklistRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { - var failures []lint.Failure +var replaceRegexp = regexp.MustCompile(`/?\*\*/?`) - if file.IsTest() { - return failures // skip, test file - } +func (r *ImportsBlacklistRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() if r.blacklist == nil { - r.blacklist = make(map[string]bool, len(arguments)) + r.blacklist = make([]*regexp.Regexp, 0) for _, arg := range arguments { argStr, ok := arg.(string) if !ok { panic(fmt.Sprintf("Invalid argument to the imports-blacklist rule. Expecting a string, got %T", arg)) } - // we add quotes if not present, because when parsed, the value of the AST node, will be quoted - if len(argStr) > 2 && argStr[0] != '"' && argStr[len(argStr)-1] != '"' { - argStr = fmt.Sprintf(`%q`, argStr) + regStr, err := regexp.Compile(fmt.Sprintf(`(?m)"%s"$`, replaceRegexp.ReplaceAllString(argStr, `(\W|\w)*`))) + if err != nil { + panic(fmt.Sprintf("Invalid argument to the imports-blacklist rule. Expecting %q to be a valid regular expression, got: %v", argStr, err)) } - r.blacklist[argStr] = true + r.blacklist = append(r.blacklist, regStr) + } + } +} + +func (r *ImportsBlacklistRule) isBlacklisted(path string) bool { + for _, regex := range r.blacklist { + if regex.MatchString(path) { + return true } } + return false +} + +// Apply applies the rule to given file. +func (r *ImportsBlacklistRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + + var failures []lint.Failure + + if file.IsTest() { + return failures // skip, test file + } for _, is := range file.AST.Imports { path := is.Path - if path != nil && r.blacklist[path.Value] { + if path != nil && r.isBlacklisted(path.Value) { failures = append(failures, lint.Failure{ Confidence: 1, Failure: "should not use the following blacklisted import: " + path.Value, @@ -51,6 +72,6 @@ func (r *ImportsBlacklistRule) Apply(file *lint.File, arguments lint.Arguments) } // Name returns the rule name. -func (r *ImportsBlacklistRule) Name() string { +func (*ImportsBlacklistRule) Name() string { return "imports-blacklist" } diff --git a/rule/increment-decrement.go b/rule/increment-decrement.go index 2ee04176d..ca21c126b 100644 --- a/rule/increment-decrement.go +++ b/rule/increment-decrement.go @@ -12,7 +12,7 @@ import ( type IncrementDecrementRule struct{} // Apply applies the rule to given file. -func (r *IncrementDecrementRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*IncrementDecrementRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -29,13 +29,12 @@ func (r *IncrementDecrementRule) Apply(file *lint.File, _ lint.Arguments) []lint } // Name returns the rule name. -func (r *IncrementDecrementRule) Name() string { +func (*IncrementDecrementRule) Name() string { return "increment-decrement" } type lintIncrementDecrement struct { file *lint.File - fileAst *ast.File onFailure func(lint.Failure) } diff --git a/rule/indent-error-flow.go b/rule/indent-error-flow.go index 985e9c3a5..d4ae8af8a 100644 --- a/rule/indent-error-flow.go +++ b/rule/indent-error-flow.go @@ -11,7 +11,7 @@ import ( type IndentErrorFlowRule struct{} // Apply applies the rule to given file. -func (r *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -24,7 +24,7 @@ func (r *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *IndentErrorFlowRule) Name() string { +func (*IndentErrorFlowRule) Name() string { return "indent-error-flow" } diff --git a/rule/line-length-limit.go b/rule/line-length-limit.go index 0cab76daa..eccf66009 100644 --- a/rule/line-length-limit.go +++ b/rule/line-length-limit.go @@ -6,6 +6,7 @@ import ( "fmt" "go/token" "strings" + "sync" "unicode/utf8" "github.com/deepsourcelabs/revive/lint" @@ -14,10 +15,11 @@ import ( // LineLengthLimitRule lints given else constructs. type LineLengthLimitRule struct { max int + sync.Mutex } -// Apply applies the rule to given file. -func (r *LineLengthLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *LineLengthLimitRule) configure(arguments lint.Arguments) { + r.Lock() if r.max == 0 { checkNumberOfArguments(1, arguments, r.Name()) @@ -28,8 +30,15 @@ func (r *LineLengthLimitRule) Apply(file *lint.File, arguments lint.Arguments) [ r.max = int(max) } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *LineLengthLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure + checker := lintLineLengthNum{ max: r.max, file: file, @@ -44,7 +53,7 @@ func (r *LineLengthLimitRule) Apply(file *lint.File, arguments lint.Arguments) [ } // Name returns the rule name. -func (r *LineLengthLimitRule) Name() string { +func (*LineLengthLimitRule) Name() string { return "line-length-limit" } diff --git a/rule/max-public-structs.go b/rule/max-public-structs.go index 4db32ab07..86bc65c99 100644 --- a/rule/max-public-structs.go +++ b/rule/max-public-structs.go @@ -3,6 +3,7 @@ package rule import ( "go/ast" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -10,10 +11,11 @@ import ( // MaxPublicStructsRule lints given else constructs. type MaxPublicStructsRule struct { max int64 + sync.Mutex } -// Apply applies the rule to given file. -func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) { + r.Lock() if r.max < 1 { checkNumberOfArguments(1, arguments, r.Name()) @@ -23,10 +25,17 @@ func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments) } r.max = max } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) var failures []lint.Failure fileAst := file.AST + walker := &lintMaxPublicStructs{ fileAst: fileAst, onFailure: func(failure lint.Failure) { @@ -49,7 +58,7 @@ func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments) } // Name returns the rule name. -func (r *MaxPublicStructsRule) Name() string { +func (*MaxPublicStructsRule) Name() string { return "max-public-structs" } diff --git a/rule/modifies-param.go b/rule/modifies-param.go index 54cb8975b..b133a6f13 100644 --- a/rule/modifies-param.go +++ b/rule/modifies-param.go @@ -11,7 +11,7 @@ import ( type ModifiesParamRule struct{} // Apply applies the rule to given file. -func (r *ModifiesParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ModifiesParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -24,7 +24,7 @@ func (r *ModifiesParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail } // Name returns the rule name. -func (r *ModifiesParamRule) Name() string { +func (*ModifiesParamRule) Name() string { return "modifies-parameter" } diff --git a/rule/modifies-value-receiver.go b/rule/modifies-value-receiver.go index ff9e4ddd3..242826756 100644 --- a/rule/modifies-value-receiver.go +++ b/rule/modifies-value-receiver.go @@ -11,7 +11,7 @@ import ( type ModifiesValRecRule struct{} // Apply applies the rule to given file. -func (r *ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -26,7 +26,7 @@ func (r *ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fai } // Name returns the rule name. -func (r *ModifiesValRecRule) Name() string { +func (*ModifiesValRecRule) Name() string { return "modifies-value-receiver" } diff --git a/rule/nested-structs.go b/rule/nested-structs.go index a9403bacc..61ddea03b 100644 --- a/rule/nested-structs.go +++ b/rule/nested-structs.go @@ -10,13 +10,9 @@ import ( type NestedStructs struct{} // Apply applies the rule to given file. -func (r *NestedStructs) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (*NestedStructs) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure - if len(arguments) > 0 { - panic(r.Name() + " doesn't take any arguments") - } - walker := &lintNestedStructs{ fileAST: file.AST, onFailure: func(failure lint.Failure) { @@ -30,7 +26,7 @@ func (r *NestedStructs) Apply(file *lint.File, arguments lint.Arguments) []lint. } // Name returns the rule name. -func (r *NestedStructs) Name() string { +func (*NestedStructs) Name() string { return "nested-structs" } diff --git a/rule/optimize-operands-order.go b/rule/optimize-operands-order.go index 840e280de..dcd75c7eb 100644 --- a/rule/optimize-operands-order.go +++ b/rule/optimize-operands-order.go @@ -12,7 +12,7 @@ import ( type OptimizeOperandsOrderRule struct{} // Apply applies the rule to given file. -func (r *OptimizeOperandsOrderRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*OptimizeOperandsOrderRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -26,7 +26,7 @@ func (r *OptimizeOperandsOrderRule) Apply(file *lint.File, _ lint.Arguments) []l } // Name returns the rule name. -func (r *OptimizeOperandsOrderRule) Name() string { +func (*OptimizeOperandsOrderRule) Name() string { return "optimize-operands-order" } diff --git a/rule/package-comments.go b/rule/package-comments.go index 1da633399..3dd9e9875 100644 --- a/rule/package-comments.go +++ b/rule/package-comments.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/token" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -14,7 +15,9 @@ import ( // This has a notable false positive in that a package comment // could rightfully appear in a different file of the same package, // but that's not easy to fix since this linter is file-oriented. -type PackageCommentsRule struct{} +type PackageCommentsRule struct { + checkPackageCommentCache sync.Map +} // Apply applies the rule to given file. func (r *PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { @@ -29,13 +32,13 @@ func (r *PackageCommentsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } fileAst := file.AST - w := &lintPackageComments{fileAst, file, onFailure} + w := &lintPackageComments{fileAst, file, onFailure, r} ast.Walk(w, fileAst) return failures } // Name returns the rule name. -func (r *PackageCommentsRule) Name() string { +func (*PackageCommentsRule) Name() string { return "package-comments" } @@ -43,6 +46,49 @@ type lintPackageComments struct { fileAst *ast.File file *lint.File onFailure func(lint.Failure) + rule *PackageCommentsRule +} + +func (l *lintPackageComments) checkPackageComment() []lint.Failure { + // deduplicate warnings in package + if _, exists := l.rule.checkPackageCommentCache.LoadOrStore(l.file.Pkg, struct{}{}); exists { + return nil + } + var docFile *ast.File // which name is doc.go + var packageFile *ast.File // which name is $package.go + var firstFile *ast.File + var firstFileName string + for name, file := range l.file.Pkg.Files() { + if file.AST.Doc != nil { + return nil + } + if name == "doc.go" { + docFile = file.AST + } + if name == file.AST.Name.String()+".go" { + packageFile = file.AST + } + if firstFileName == "" || firstFileName > name { + firstFile = file.AST + firstFileName = name + } + } + // prefer warning on doc.go, $package.go over first file + if docFile == nil { + docFile = packageFile + } + if docFile == nil { + docFile = firstFile + } + if docFile != nil { + return []lint.Failure{{ + Category: "comments", + Node: docFile, + Confidence: 1, + Failure: "should have a package comment", + }} + } + return nil } func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor { @@ -50,7 +96,6 @@ func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor { return nil } - const ref = styleGuideBase + "#package-comments" prefix := "Package " + l.fileAst.Name.Name + " " // Look for a detached package comment. @@ -90,12 +135,9 @@ func (l *lintPackageComments) Visit(_ ast.Node) ast.Visitor { } if l.fileAst.Doc == nil { - l.onFailure(lint.Failure{ - Category: "comments", - Node: l.fileAst, - Confidence: 0.2, - Failure: "should have a package comment, unless it's in another file for this package", - }) + for _, failure := range l.checkPackageComment() { + l.onFailure(failure) + } return nil } s := l.fileAst.Doc.Text() diff --git a/rule/range-val-address.go b/rule/range-val-address.go index b5c710324..6ba47f653 100644 --- a/rule/range-val-address.go +++ b/rule/range-val-address.go @@ -13,7 +13,7 @@ import ( type RangeValAddress struct{} // Apply applies the rule to given file. -func (r *RangeValAddress) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*RangeValAddress) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := rangeValAddress{ @@ -30,7 +30,7 @@ func (r *RangeValAddress) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *RangeValAddress) Name() string { +func (*RangeValAddress) Name() string { return "range-val-address" } diff --git a/rule/range-val-in-closure.go b/rule/range-val-in-closure.go index 119c218f8..b40644128 100644 --- a/rule/range-val-in-closure.go +++ b/rule/range-val-in-closure.go @@ -11,7 +11,7 @@ import ( type RangeValInClosureRule struct{} // Apply applies the rule to given file. -func (r *RangeValInClosureRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*RangeValInClosureRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := rangeValInClosure{ @@ -26,7 +26,7 @@ func (r *RangeValInClosureRule) Apply(file *lint.File, _ lint.Arguments) []lint. } // Name returns the rule name. -func (r *RangeValInClosureRule) Name() string { +func (*RangeValInClosureRule) Name() string { return "range-val-in-closure" } diff --git a/rule/range.go b/rule/range.go index 4209d3542..157071767 100644 --- a/rule/range.go +++ b/rule/range.go @@ -12,7 +12,7 @@ import ( type RangeRule struct{} // Apply applies the rule to given file. -func (r *RangeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*RangeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -25,7 +25,7 @@ func (r *RangeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *RangeRule) Name() string { +func (*RangeRule) Name() string { return "range" } diff --git a/rule/receiver-naming.go b/rule/receiver-naming.go index 5e4423abc..04200efe6 100644 --- a/rule/receiver-naming.go +++ b/rule/receiver-naming.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" + "github.com/deepsourcelabs/revive/internal/typeparams" "github.com/deepsourcelabs/revive/lint" ) @@ -11,7 +12,7 @@ import ( type ReceiverNamingRule struct{} // Apply applies the rule to given file. -func (r *ReceiverNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*ReceiverNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -28,7 +29,7 @@ func (r *ReceiverNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fai } // Name returns the rule name. -func (r *ReceiverNamingRule) Name() string { +func (*ReceiverNamingRule) Name() string { return "receiver-naming" } @@ -47,7 +48,6 @@ func (w lintReceiverName) Visit(n ast.Node) ast.Visitor { return w } name := names[0].Name - const ref = styleGuideBase + "#receiver-names" if name == "_" { w.onFailure(lint.Failure{ Node: n, @@ -66,7 +66,7 @@ func (w lintReceiverName) Visit(n ast.Node) ast.Visitor { }) return w } - recv := receiverType(fn) + recv := typeparams.ReceiverType(fn) if prev, ok := w.typeReceiver[recv]; ok && prev != name { w.onFailure(lint.Failure{ Node: n, diff --git a/rule/redefines-builtin-id.go b/rule/redefines-builtin-id.go index 89d497572..b8ed973e0 100644 --- a/rule/redefines-builtin-id.go +++ b/rule/redefines-builtin-id.go @@ -34,39 +34,34 @@ var builtFunctions = map[string]bool{ } var builtInTypes = map[string]bool{ - "ComplexType": true, - "FloatType": true, - "IntegerType": true, - "Type": true, - "Type1": true, - "bool": true, - "byte": true, - "complex128": true, - "complex64": true, - "error": true, - "float32": true, - "float64": true, - "int": true, - "int16": true, - "int32": true, - "int64": true, - "int8": true, - "rune": true, - "string": true, - "uint": true, - "uint16": true, - "uint32": true, - "uint64": true, - "uint8": true, - "uintptr": true, - "any": true, + "bool": true, + "byte": true, + "complex128": true, + "complex64": true, + "error": true, + "float32": true, + "float64": true, + "int": true, + "int16": true, + "int32": true, + "int64": true, + "int8": true, + "rune": true, + "string": true, + "uint": true, + "uint16": true, + "uint32": true, + "uint64": true, + "uint8": true, + "uintptr": true, + "any": true, } // RedefinesBuiltinIDRule warns when a builtin identifier is shadowed. type RedefinesBuiltinIDRule struct{} // Apply applies the rule to given file. -func (r *RedefinesBuiltinIDRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*RedefinesBuiltinIDRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -81,7 +76,7 @@ func (r *RedefinesBuiltinIDRule) Apply(file *lint.File, _ lint.Arguments) []lint } // Name returns the rule name. -func (r *RedefinesBuiltinIDRule) Name() string { +func (*RedefinesBuiltinIDRule) Name() string { return "redefines-builtin-id" } @@ -136,7 +131,6 @@ func (w *lintRedefinesBuiltinID) Visit(node ast.Node) ast.Visitor { if ok, bt := w.isBuiltIn(id.Name); ok { var msg string - println(bt, id.Name) switch bt { case "constant or variable": if n.Tok == token.DEFINE { @@ -165,7 +159,7 @@ func (w lintRedefinesBuiltinID) addFailure(node ast.Node, msg string) { }) } -func (w lintRedefinesBuiltinID) isBuiltIn(id string) (r bool, builtInKind string) { +func (lintRedefinesBuiltinID) isBuiltIn(id string) (r bool, builtInKind string) { if builtFunctions[id] { return true, "function" } diff --git a/rule/string-format.go b/rule/string-format.go index ed92cbf81..21e4374f1 100644 --- a/rule/string-format.go +++ b/rule/string-format.go @@ -16,7 +16,7 @@ import ( type StringFormatRule struct{} // Apply applies the rule to the given file. -func (r *StringFormatRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { +func (*StringFormatRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -31,12 +31,12 @@ func (r *StringFormatRule) Apply(file *lint.File, arguments lint.Arguments) []li } // Name returns the rule name. -func (r *StringFormatRule) Name() string { +func (*StringFormatRule) Name() string { return "string-format" } // ParseArgumentsTest is a public wrapper around w.parseArguments used for testing. Returns the error message provided to panic, or nil if no error was encountered -func (r *StringFormatRule) ParseArgumentsTest(arguments lint.Arguments) *string { +func (StringFormatRule) ParseArgumentsTest(arguments lint.Arguments) *string { w := lintStringFormatRule{} c := make(chan interface{}) // Parse the arguments in a goroutine, defer a recover() call, return the error encountered (or nil if there was no error) @@ -61,9 +61,7 @@ func (r *StringFormatRule) ParseArgumentsTest(arguments lint.Arguments) *string type lintStringFormatRule struct { onFailure func(lint.Failure) - - rules []stringFormatSubrule - stringDeclarations map[string]string + rules []stringFormatSubrule } type stringFormatSubrule struct { @@ -161,12 +159,12 @@ func (w lintStringFormatRule) parseArgument(argument interface{}, ruleNum int) ( } // Report an invalid config, this is specifically the user's fault -func (w lintStringFormatRule) configError(msg string, ruleNum, option int) { +func (lintStringFormatRule) configError(msg string, ruleNum, option int) { panic(fmt.Sprintf("invalid configuration for string-format: %s [argument %d, option %d]", msg, ruleNum, option)) } // Report a general config parsing failure, this may be the user's fault, but it isn't known for certain -func (w lintStringFormatRule) parseError(msg string, ruleNum, option int) { +func (lintStringFormatRule) parseError(msg string, ruleNum, option int) { panic(fmt.Sprintf("failed to parse configuration for string-format: %s [argument %d, option %d]", msg, ruleNum, option)) } @@ -197,7 +195,7 @@ func (w lintStringFormatRule) Visit(node ast.Node) ast.Visitor { } // Return the name of a call expression in the form of package.Func or Func -func (w lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok bool) { +func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok bool) { if ident, ok := call.Fun.(*ast.Ident); ok { // Local function call return ident.Name, true @@ -220,14 +218,14 @@ func (w lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, // #region Linting logic // Apply a single format rule to a call expression (should be done after verifying the that the call expression matches the rule's scope) -func (rule stringFormatSubrule) Apply(call *ast.CallExpr) { - if len(call.Args) <= rule.scope.argument { +func (r *stringFormatSubrule) Apply(call *ast.CallExpr) { + if len(call.Args) <= r.scope.argument { return } - arg := call.Args[rule.scope.argument] + arg := call.Args[r.scope.argument] var lit *ast.BasicLit - if len(rule.scope.field) > 0 { + if len(r.scope.field) > 0 { // Try finding the scope's Field, treating arg as a composite literal composite, ok := arg.(*ast.CompositeLit) if !ok { @@ -239,7 +237,7 @@ func (rule stringFormatSubrule) Apply(call *ast.CallExpr) { continue } key, ok := kv.Key.(*ast.Ident) - if !ok || key.Name != rule.scope.field { + if !ok || key.Name != r.scope.field { continue } @@ -259,21 +257,21 @@ func (rule stringFormatSubrule) Apply(call *ast.CallExpr) { } // Unquote the string literal before linting unquoted := lit.Value[1 : len(lit.Value)-1] - rule.lintMessage(unquoted, lit) + r.lintMessage(unquoted, lit) } -func (rule stringFormatSubrule) lintMessage(s string, node ast.Node) { +func (r *stringFormatSubrule) lintMessage(s string, node ast.Node) { // Fail if the string doesn't match the user's regex - if rule.regexp.MatchString(s) { + if r.regexp.MatchString(s) { return } var failure string - if len(rule.errorMessage) > 0 { - failure = rule.errorMessage + if len(r.errorMessage) > 0 { + failure = r.errorMessage } else { - failure = fmt.Sprintf("string literal doesn't match user defined regex /%s/", rule.regexp.String()) + failure = fmt.Sprintf("string literal doesn't match user defined regex /%s/", r.regexp.String()) } - rule.parent.onFailure(lint.Failure{ + r.parent.onFailure(lint.Failure{ Confidence: 1, Failure: failure, Node: node, diff --git a/rule/string-of-int.go b/rule/string-of-int.go index 9c9bba2db..877f64f4f 100644 --- a/rule/string-of-int.go +++ b/rule/string-of-int.go @@ -11,7 +11,7 @@ import ( type StringOfIntRule struct{} // Apply applies the rule to given file. -func (r *StringOfIntRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*StringOfIntRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -28,7 +28,7 @@ func (r *StringOfIntRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *StringOfIntRule) Name() string { +func (*StringOfIntRule) Name() string { return "string-of-int" } diff --git a/rule/struct-tag.go b/rule/struct-tag.go index dae6743f9..80106cf4f 100644 --- a/rule/struct-tag.go +++ b/rule/struct-tag.go @@ -14,7 +14,7 @@ import ( type StructTagRule struct{} // Apply applies the rule to given file. -func (r *StructTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*StructTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -29,13 +29,14 @@ func (r *StructTagRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *StructTagRule) Name() string { +func (*StructTagRule) Name() string { return "struct-tag" } type lintStructTagRule struct { - onFailure func(lint.Failure) - usedTagNbr map[string]bool // list of used tag numbers + onFailure func(lint.Failure) + usedTagNbr map[string]bool // list of used tag numbers + usedTagName map[string]bool // list of used tag keys } func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { @@ -44,7 +45,8 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { if n.Fields == nil || n.Fields.NumFields() < 1 { return nil // skip empty structs } - w.usedTagNbr = map[string]bool{} // init + w.usedTagNbr = map[string]bool{} // init + w.usedTagName = map[string]bool{} // init for _, f := range n.Fields.List { if f.Tag != nil { w.checkTaggedField(f) @@ -55,6 +57,49 @@ func (w lintStructTagRule) Visit(node ast.Node) ast.Visitor { return w } +func (w lintStructTagRule) checkTagNameIfNeed(tag *structtag.Tag) (string, bool) { + isUnnamedTag := tag.Name == "" || tag.Name == "-" + if isUnnamedTag { + return "", true + } + + needsToCheckTagName := tag.Key == "bson" || + tag.Key == "json" || + tag.Key == "xml" || + tag.Key == "yaml" || + tag.Key == "protobuf" + + if !needsToCheckTagName { + return "", true + } + + tagName := w.getTagName(tag) + // We concat the key and name as the mapping key here + // to allow the same tag name in different tag type. + key := tag.Key + ":" + tagName + if _, ok := w.usedTagName[key]; ok { + return fmt.Sprintf("duplicate tag name: '%s'", tagName), false + } + + w.usedTagName[key] = true + + return "", true +} + +func (lintStructTagRule) getTagName(tag *structtag.Tag) string { + switch tag.Key { + case "protobuf": + for _, option := range tag.Options { + if strings.HasPrefix(option, "name=") { + return strings.TrimLeft(option, "name=") + } + } + return "protobuf tag lacks name" + default: + return tag.Name + } +} + // checkTaggedField checks the tag of the given field. // precondition: the field has a tag func (w lintStructTagRule) checkTaggedField(f *ast.Field) { @@ -69,6 +114,10 @@ func (w lintStructTagRule) checkTaggedField(f *ast.Field) { } for _, tag := range tags.Tags() { + if msg, ok := w.checkTagNameIfNeed(tag); !ok { + w.addFailure(f.Tag, msg) + } + switch key := tag.Key; key { case "asn1": msg, ok := w.checkASN1Tag(f.Type, tag) @@ -148,7 +197,7 @@ func (w lintStructTagRule) checkASN1Tag(t ast.Expr, tag *structtag.Tag) (string, return "", true } -func (w lintStructTagRule) checkBSONTag(options []string) (string, bool) { +func (lintStructTagRule) checkBSONTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "inline", "minsize", "omitempty": @@ -160,7 +209,7 @@ func (w lintStructTagRule) checkBSONTag(options []string) (string, bool) { return "", true } -func (w lintStructTagRule) checkJSONTag(name string, options []string) (string, bool) { +func (lintStructTagRule) checkJSONTag(name string, options []string) (string, bool) { for _, opt := range options { switch opt { case "omitempty", "string": @@ -177,7 +226,7 @@ func (w lintStructTagRule) checkJSONTag(name string, options []string) (string, return "", true } -func (w lintStructTagRule) checkXMLTag(options []string) (string, bool) { +func (lintStructTagRule) checkXMLTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "any", "attr", "cdata", "chardata", "comment", "innerxml", "omitempty", "typeattr": @@ -189,7 +238,7 @@ func (w lintStructTagRule) checkXMLTag(options []string) (string, bool) { return "", true } -func (w lintStructTagRule) checkYAMLTag(options []string) (string, bool) { +func (lintStructTagRule) checkYAMLTag(options []string) (string, bool) { for _, opt := range options { switch opt { case "flow", "inline", "omitempty": @@ -201,7 +250,7 @@ func (w lintStructTagRule) checkYAMLTag(options []string) (string, bool) { return "", true } -func (w lintStructTagRule) typeValueMatch(t ast.Expr, val string) bool { +func (lintStructTagRule) typeValueMatch(t ast.Expr, val string) bool { tID, ok := t.(*ast.Ident) if !ok { return true diff --git a/rule/superfluous-else.go b/rule/superfluous-else.go index 302c6586d..a1deb3241 100644 --- a/rule/superfluous-else.go +++ b/rule/superfluous-else.go @@ -12,7 +12,7 @@ import ( type SuperfluousElseRule struct{} // Apply applies the rule to given file. -func (r *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -36,7 +36,7 @@ func (r *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *SuperfluousElseRule) Name() string { +func (*SuperfluousElseRule) Name() string { return "superfluous-else" } @@ -82,9 +82,9 @@ func (w lintSuperfluousElse) Visit(node ast.Node) ast.Visitor { lastStmt := ifStmt.Body.List[len(ifStmt.Body.List)-1] switch stmt := lastStmt.(type) { case *ast.BranchStmt: - token := stmt.Tok.String() - if token != "fallthrough" { - w.onFailure(newFailure(ifStmt.Else, "if block ends with a "+token+" statement, so drop this else and outdent its block"+extra)) + tok := stmt.Tok.String() + if tok != "fallthrough" { + w.onFailure(newFailure(ifStmt.Else, "if block ends with a "+tok+" statement, so drop this else and outdent its block"+extra)) } case *ast.ExprStmt: if ce, ok := stmt.X.(*ast.CallExpr); ok { // it's a function call diff --git a/rule/time-naming.go b/rule/time-naming.go index b3d866080..624afcc6b 100644 --- a/rule/time-naming.go +++ b/rule/time-naming.go @@ -13,7 +13,7 @@ import ( type TimeNamingRule struct{} // Apply applies the rule to given file. -func (r *TimeNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*TimeNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -28,7 +28,7 @@ func (r *TimeNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure } // Name returns the rule name. -func (r *TimeNamingRule) Name() string { +func (*TimeNamingRule) Name() string { return "time-naming" } diff --git a/rule/unconditional-recursion.go b/rule/unconditional-recursion.go index bcc4bbdda..bb1b23e98 100644 --- a/rule/unconditional-recursion.go +++ b/rule/unconditional-recursion.go @@ -10,7 +10,7 @@ import ( type UnconditionalRecursionRule struct{} // Apply applies the rule to given file. -func (r *UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -23,7 +23,7 @@ func (r *UnconditionalRecursionRule) Apply(file *lint.File, _ lint.Arguments) [] } // Name returns the rule name. -func (r *UnconditionalRecursionRule) Name() string { +func (*UnconditionalRecursionRule) Name() string { return "unconditional-recursion" } @@ -151,7 +151,7 @@ var exitFunctions = map[string]map[string]bool{ }, } -func (w *lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool { +func (lintUnconditionalRecursionRule) hasControlExit(node ast.Node) bool { // isExit returns true if the given node makes control exit the function isExit := func(node ast.Node) bool { switch n := node.(type) { diff --git a/rule/unexported-naming.go b/rule/unexported-naming.go index 58ddc8005..a0aeaba85 100644 --- a/rule/unexported-naming.go +++ b/rule/unexported-naming.go @@ -12,7 +12,7 @@ import ( type UnexportedNamingRule struct{} // Apply applies the rule to given file. -func (r *UnexportedNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnexportedNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -25,7 +25,7 @@ func (r *UnexportedNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.F } // Name returns the rule name. -func (r *UnexportedNamingRule) Name() string { +func (*UnexportedNamingRule) Name() string { return "unexported-naming" } diff --git a/rule/unexported-return.go b/rule/unexported-return.go index 69fe32c33..65318da5a 100644 --- a/rule/unexported-return.go +++ b/rule/unexported-return.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/types" + "github.com/deepsourcelabs/revive/internal/typeparams" "github.com/deepsourcelabs/revive/lint" ) @@ -12,7 +13,7 @@ import ( type UnexportedReturnRule struct{} // Apply applies the rule to given file. -func (r *UnexportedReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnexportedReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -31,7 +32,7 @@ func (r *UnexportedReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.F } // Name returns the rule name. -func (r *UnexportedReturnRule) Name() string { +func (*UnexportedReturnRule) Name() string { return "unexported-return" } @@ -55,7 +56,7 @@ func (w lintUnexportedReturn) Visit(n ast.Node) ast.Visitor { thing := "func" if fn.Recv != nil && len(fn.Recv.List) > 0 { thing = "method" - if !ast.IsExported(receiverType(fn)) { + if !ast.IsExported(typeparams.ReceiverType(fn)) { // Don't report exported methods of unexported types, // such as private implementations of sort.Interface. return nil @@ -82,24 +83,24 @@ func (w lintUnexportedReturn) Visit(n ast.Node) ast.Visitor { // It is imprecise, and will err on the side of returning true, // such as for composite types. func exportedType(typ types.Type) bool { - switch T := typ.(type) { + switch t := typ.(type) { case *types.Named: - obj := T.Obj() + obj := t.Obj() switch { // Builtin types have no package. case obj.Pkg() == nil: case obj.Exported(): default: - _, ok := T.Underlying().(*types.Interface) + _, ok := t.Underlying().(*types.Interface) return ok } return true case *types.Map: - return exportedType(T.Key()) && exportedType(T.Elem()) + return exportedType(t.Key()) && exportedType(t.Elem()) case interface { Elem() types.Type }: // array, slice, pointer, chan - return exportedType(T.Elem()) + return exportedType(t.Elem()) } // Be conservative about other types, such as struct, interface, etc. return true diff --git a/rule/unhandled-error.go b/rule/unhandled-error.go index 62eb9e9d1..011178a90 100644 --- a/rule/unhandled-error.go +++ b/rule/unhandled-error.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/types" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -11,16 +12,17 @@ import ( // UnhandledErrorRule lints given else constructs. type UnhandledErrorRule struct { ignoreList ignoreListType + sync.Mutex } type ignoreListType map[string]struct{} -// Apply applies the rule to given file. -func (r *UnhandledErrorRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { +func (r *UnhandledErrorRule) configure(arguments lint.Arguments) { + r.Lock() if r.ignoreList == nil { - r.ignoreList = make(ignoreListType, len(args)) + r.ignoreList = make(ignoreListType, len(arguments)) - for _, arg := range args { + for _, arg := range arguments { argStr, ok := arg.(string) if !ok { panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg)) @@ -29,6 +31,12 @@ func (r *UnhandledErrorRule) Apply(file *lint.File, args lint.Arguments) []lint. r.ignoreList[argStr] = struct{}{} } } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *UnhandledErrorRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) var failures []lint.Failure @@ -47,7 +55,7 @@ func (r *UnhandledErrorRule) Apply(file *lint.File, args lint.Arguments) []lint. } // Name returns the rule name. -func (r *UnhandledErrorRule) Name() string { +func (*UnhandledErrorRule) Name() string { return "unhandled-error" } diff --git a/rule/unnecessary-stmt.go b/rule/unnecessary-stmt.go index 56f4c3dae..07bf0b4c3 100644 --- a/rule/unnecessary-stmt.go +++ b/rule/unnecessary-stmt.go @@ -11,7 +11,7 @@ import ( type UnnecessaryStmtRule struct{} // Apply applies the rule to given file. -func (r *UnnecessaryStmtRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnnecessaryStmtRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -23,7 +23,7 @@ func (r *UnnecessaryStmtRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *UnnecessaryStmtRule) Name() string { +func (*UnnecessaryStmtRule) Name() string { return "unnecessary-stmt" } diff --git a/rule/unreachable-code.go b/rule/unreachable-code.go index 14c83440d..bec87e980 100644 --- a/rule/unreachable-code.go +++ b/rule/unreachable-code.go @@ -10,7 +10,7 @@ import ( type UnreachableCodeRule struct{} // Apply applies the rule to given file. -func (r *UnreachableCodeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnreachableCodeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) @@ -34,7 +34,7 @@ func (r *UnreachableCodeRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *UnreachableCodeRule) Name() string { +func (*UnreachableCodeRule) Name() string { return "unreachable-code" } diff --git a/rule/unused-param.go b/rule/unused-param.go index 5c76b9c62..d66797cb0 100644 --- a/rule/unused-param.go +++ b/rule/unused-param.go @@ -11,7 +11,7 @@ import ( type UnusedParamRule struct{} // Apply applies the rule to given file. -func (r *UnusedParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UnusedParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -26,7 +26,7 @@ func (r *UnusedParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failur } // Name returns the rule name. -func (r *UnusedParamRule) Name() string { +func (*UnusedParamRule) Name() string { return "unused-parameter" } diff --git a/rule/use-any.go b/rule/use-any.go index 467fa1f54..df51897fd 100644 --- a/rule/use-any.go +++ b/rule/use-any.go @@ -10,7 +10,7 @@ import ( type UseAnyRule struct{} // Apply applies the rule to given file. -func (r *UseAnyRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UseAnyRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := lintUseAny{ @@ -25,7 +25,7 @@ func (r *UseAnyRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *UseAnyRule) Name() string { +func (*UseAnyRule) Name() string { return "use-any" } diff --git a/rule/useless-break.go b/rule/useless-break.go index ce0233f83..83c739c21 100644 --- a/rule/useless-break.go +++ b/rule/useless-break.go @@ -11,7 +11,7 @@ import ( type UselessBreak struct{} // Apply applies the rule to given file. -func (r *UselessBreak) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*UselessBreak) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -25,7 +25,7 @@ func (r *UselessBreak) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { } // Name returns the rule name. -func (r *UselessBreak) Name() string { +func (*UselessBreak) Name() string { return "useless-break" } diff --git a/rule/utils.go b/rule/utils.go index 9f2962237..844fcadaf 100644 --- a/rule/utils.go +++ b/rule/utils.go @@ -13,8 +13,6 @@ import ( "github.com/deepsourcelabs/revive/lint" ) -const styleGuideBase = "https://golang.org/wiki/CodeReviewComments" - // isBlank returns whether id is the blank identifier "_". // If id == nil, the answer is false. func isBlank(id *ast.Ident) bool { return id != nil && id.Name == "_" } @@ -28,19 +26,6 @@ var commonMethods = map[string]bool{ "Unwrap": true, } -func receiverType(fn *ast.FuncDecl) string { - switch e := fn.Recv.List[0].Type.(type) { - case *ast.Ident: - return e.Name - case *ast.StarExpr: - if id, ok := e.X.(*ast.Ident); ok { - return id.Name - } - } - // The parser accepts much more than just the legal forms. - return "invalid-type" -} - var knownNameExceptions = map[string]bool{ "LastInsertId": true, // must match database/sql "kWh": true, @@ -82,10 +67,10 @@ var zeroLiteral = map[string]bool{ "0i": true, } -func validType(T types.Type) bool { - return T != nil && - T != types.Typ[types.Invalid] && - !strings.Contains(T.String(), "invalid type") // good but not foolproof +func validType(t types.Type) bool { + return t != nil && + t != types.Typ[types.Invalid] && + !strings.Contains(t.String(), "invalid type") // good but not foolproof } // isPkgDot checks if the expression is . diff --git a/rule/var-declarations.go b/rule/var-declarations.go index d664e0a5d..e449754f0 100644 --- a/rule/var-declarations.go +++ b/rule/var-declarations.go @@ -13,7 +13,7 @@ import ( type VarDeclarationsRule struct{} // Apply applies the rule to given file. -func (r *VarDeclarationsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*VarDeclarationsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure fileAst := file.AST @@ -32,7 +32,7 @@ func (r *VarDeclarationsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa } // Name returns the rule name. -func (r *VarDeclarationsRule) Name() string { +func (*VarDeclarationsRule) Name() string { return "var-declaration" } diff --git a/rule/var-naming.go b/rule/var-naming.go index 5ecab37c2..d0166a71d 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -5,6 +5,7 @@ import ( "go/ast" "go/token" "strings" + "sync" "github.com/deepsourcelabs/revive/lint" ) @@ -14,12 +15,11 @@ type VarNamingRule struct { configured bool whitelist []string blacklist []string + sync.Mutex } -// Apply applies the rule to given file. -func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { - var failures []lint.Failure - +func (r *VarNamingRule) configure(arguments lint.Arguments) { + r.Lock() if !r.configured { if len(arguments) >= 1 { r.whitelist = getList(arguments[0], "whitelist") @@ -30,8 +30,17 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. } r.configured = true } + r.Unlock() +} + +// Apply applies the rule to given file. +func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + r.configure(arguments) + + var failures []lint.Failure fileAst := file.AST + walker := lintNames{ file: file, fileAst: fileAst, @@ -47,7 +56,7 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. walker.onFailure(lint.Failure{ Failure: "don't use an underscore in package name", Confidence: 1, - Node: walker.fileAst, + Node: walker.fileAst.Name, Category: "naming", }) } @@ -58,7 +67,7 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. } // Name returns the rule name. -func (r *VarNamingRule) Name() string { +func (*VarNamingRule) Name() string { return "var-naming" } @@ -124,13 +133,11 @@ func check(id *ast.Ident, thing string, w *lintNames) { } type lintNames struct { - file *lint.File - fileAst *ast.File - lastGen *ast.GenDecl - genDeclMissingComments map[*ast.GenDecl]bool - onFailure func(lint.Failure) - whitelist []string - blacklist []string + file *lint.File + fileAst *ast.File + onFailure func(lint.Failure) + whitelist []string + blacklist []string } func (w *lintNames) Visit(n ast.Node) ast.Visitor { diff --git a/rule/waitgroup-by-value.go b/rule/waitgroup-by-value.go index fb8ecdb7f..c1d1ae203 100644 --- a/rule/waitgroup-by-value.go +++ b/rule/waitgroup-by-value.go @@ -10,7 +10,7 @@ import ( type WaitGroupByValueRule struct{} // Apply applies the rule to given file. -func (r *WaitGroupByValueRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (*WaitGroupByValueRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure onFailure := func(failure lint.Failure) { @@ -23,7 +23,7 @@ func (r *WaitGroupByValueRule) Apply(file *lint.File, _ lint.Arguments) []lint.F } // Name returns the rule name. -func (r *WaitGroupByValueRule) Name() string { +func (*WaitGroupByValueRule) Name() string { return "waitgroup-by-value" } diff --git a/test/datarace_test.go b/test/datarace_test.go new file mode 100644 index 000000000..ecd1b0894 --- /dev/null +++ b/test/datarace_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/deepsourcelabs/revive/rule" +) + +func TestDatarace(t *testing.T) { + testRule(t, "datarace", &rule.DataRaceRule{}) +} diff --git a/test/error-strings-custom-functions_test.go b/test/error-strings-custom-functions_test.go new file mode 100644 index 000000000..30d19af5a --- /dev/null +++ b/test/error-strings-custom-functions_test.go @@ -0,0 +1,15 @@ +package test + +import ( + "testing" + + "github.com/deepsourcelabs/revive/lint" + "github.com/deepsourcelabs/revive/rule" +) + +func TestErrorStringsWithCustomFunctions(t *testing.T) { + args := []interface{}{"pkgErrors.Wrap"} + testRule(t, "error-strings-with-custom-functions", &rule.ErrorStringsRule{}, &lint.RuleConfig{ + Arguments: args, + }) +} diff --git a/test/import-blacklist_test.go b/test/import-blacklist_test.go index 78ba5aa13..cdbdb5bde 100644 --- a/test/import-blacklist_test.go +++ b/test/import-blacklist_test.go @@ -7,16 +7,24 @@ import ( "github.com/deepsourcelabs/revive/rule" ) -func TestImportsBlacklist(t *testing.T) { +func TestImportsBlacklistOriginal(t *testing.T) { args := []interface{}{"crypto/md5", "crypto/sha1"} + testRule(t, "imports-blacklist-original", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{ + Arguments: args, + }) +} + +func TestImportsBlacklist(t *testing.T) { + args := []interface{}{"github.com/full/match", "wildcard/**/between", "wildcard/backward/**", "**/wildcard/forward", "full"} + testRule(t, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{ Arguments: args, }) } func BenchmarkImportsBlacklist(b *testing.B) { - args := []interface{}{"crypto/md5", "crypto/sha1"} + args := []interface{}{"github.com/full/match", "wildcard/**/between", "wildcard/backward/**", "**/wildcard/forward", "full"} var t *testing.T for i := 0; i <= b.N; i++ { testRule(t, "imports-blacklist", &rule.ImportsBlacklistRule{}, &lint.RuleConfig{ diff --git a/test/receiver-naming_test.go b/test/receiver-naming_test.go new file mode 100644 index 000000000..f49e607e0 --- /dev/null +++ b/test/receiver-naming_test.go @@ -0,0 +1,15 @@ +package test + +import ( + "testing" + + "github.com/deepsourcelabs/revive/internal/typeparams" + "github.com/deepsourcelabs/revive/rule" +) + +func TestReceiverNamingTypeParams(t *testing.T) { + if !typeparams.Enabled() { + t.Skip("type parameters are not enabled in the current build environment") + } + testRule(t, "receiver-naming-issue-669", &rule.ReceiverNamingRule{}) +} diff --git a/test/utils.go b/test/utils.go index 290d8a03b..20e274df6 100644 --- a/test/utils.go +++ b/test/utils.go @@ -6,7 +6,6 @@ import ( "fmt" "go/ast" "go/parser" - "go/printer" "go/token" "go/types" "io/ioutil" @@ -200,14 +199,6 @@ func extractReplacement(line string) (string, bool) { return line[a+len(start) : b], true } -func render(fset *token.FileSet, x interface{}) string { - var buf bytes.Buffer - if err := printer.Fprint(&buf, fset, x); err != nil { - panic(err) - } - return buf.String() -} - func srcLine(src []byte, p token.Position) string { // Run to end of line in both directions if not at line start/end. lo, hi := p.Offset, p.Offset+1 @@ -288,16 +279,16 @@ func TestLintName(t *testing.T) { //revive:disable-line:exported // It is imprecise, and will err on the side of returning true, // such as for composite types. func exportedType(typ types.Type) bool { - switch T := typ.(type) { + switch t := typ.(type) { case *types.Named: // Builtin types have no package. - return T.Obj().Pkg() == nil || T.Obj().Exported() + return t.Obj().Pkg() == nil || t.Obj().Exported() case *types.Map: - return exportedType(T.Key()) && exportedType(T.Elem()) + return exportedType(t.Key()) && exportedType(t.Elem()) case interface { Elem() types.Type }: // array, slice, pointer, chan - return exportedType(T.Elem()) + return exportedType(t.Elem()) } // Be conservative about other types, such as struct, interface, etc. return true diff --git a/testdata/datarace.go b/testdata/datarace.go new file mode 100644 index 000000000..37e94a3f6 --- /dev/null +++ b/testdata/datarace.go @@ -0,0 +1,30 @@ +package fixtures + +func datarace() (r int, c char) { + for _, p := range []int{1, 2} { + go func() { + print(r) // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/ + print(p) // MATCH /datarace: range value p is captured (by-reference) in goroutine/ + }() + for i, p1 := range []int{1, 2} { + a := p1 + go func() { + print(r) // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/ + print(p) // MATCH /datarace: range value p is captured (by-reference) in goroutine/ + print(p1) // MATCH /datarace: range value p1 is captured (by-reference) in goroutine/ + print(a) + print(i) // MATCH /datarace: range value i is captured (by-reference) in goroutine/ + }() + print(i) + print(p) + go func() { + _ = c // MATCH /potential datarace: return value c is captured (by-reference) in goroutine/ + }() + } + print(p1) + } + go func() { + print(r) // MATCH /potential datarace: return value r is captured (by-reference) in goroutine/ + }() + print(r) +} diff --git a/testdata/empty-lines.go b/testdata/empty-lines.go index 0e9d57cc0..a37a778db 100644 --- a/testdata/empty-lines.go +++ b/testdata/empty-lines.go @@ -9,14 +9,14 @@ func f1(x *int) bool { // MATCH /extra empty line at the start of a block/ return x > 2 } -func f2(x *int) bool { - return x > 2 // MATCH /extra empty line at the end of a block/ +func f2(x *int) bool { // MATCH /extra empty line at the end of a block/ + return x > 2 } func f3(x *int) bool { // MATCH /extra empty line at the start of a block/ - return x > 2 // MATCH /extra empty line at the end of a block/ + return x > 2 // MATCH:17 /extra empty line at the end of a block/ } @@ -36,8 +36,8 @@ func f6(x *int) bool { // This is fine. } -func f7(x *int) bool { - return x > 2 // MATCH /extra empty line at the end of a block/ +func f7(x *int) bool { // MATCH /extra empty line at the end of a block/ + return x > 2 // This is _not_ fine. } @@ -52,8 +52,8 @@ func f8(*int) bool { } func f9(*int) bool { - if x > 2 { - return true // MATCH /extra empty line at the end of a block/ + if x > 2 { // MATCH /extra empty line at the end of a block/ + return true } @@ -75,8 +75,8 @@ func f11(x *int) bool { } } -func f12(x *int) bool { - if x > 2 { // MATCH /extra empty line at the end of a block/ +func f12(x *int) bool { // MATCH /extra empty line at the end of a block/ + if x > 2 { return true } @@ -89,8 +89,8 @@ func f13(x *int) bool { } } -func f14(x *int) bool { - switch { // MATCH /extra empty line at the end of a block/ +func f14(x *int) bool { // MATCH /extra empty line at the end of a block/ + switch { case x == 2: return false } @@ -98,8 +98,8 @@ func f14(x *int) bool { } func f15(x *int) bool { - switch { - case x == 2: // MATCH /extra empty line at the end of a block/ + switch { // MATCH /extra empty line at the end of a block/ + case x == 2: return false } @@ -111,8 +111,8 @@ func f16(x *int) bool { ).Execute() } -func f17(x *int) bool { - return Query( // MATCH /extra empty line at the end of a block/ +func f17(x *int) bool { // MATCH /extra empty line at the end of a block/ + return Query( qm("x = ?", x), ).Execute() @@ -120,7 +120,7 @@ func f17(x *int) bool { func f18(x *int) bool { if true { - if true { // MATCH /extra empty line at the end of a block/ + if true { return true } @@ -160,3 +160,11 @@ func x() { } } } + +func ShouldNotWarn() { + // comment + + println() + + // comment +} diff --git a/testdata/error-strings-with-custom-functions.go b/testdata/error-strings-with-custom-functions.go new file mode 100644 index 000000000..ec8a4055e --- /dev/null +++ b/testdata/error-strings-with-custom-functions.go @@ -0,0 +1,12 @@ +package fixtures + +import ( + pkgErrors "github.com/pkg/errors" +) + +// Check for the error strings themselves. + +func errorsStrings(x int) error { + var err error + return pkgErrors.Wrap(err, "This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/ +} diff --git a/testdata/golint/const-block.go b/testdata/golint/const-block.go index d4d6ec16c..fcaf86f0c 100644 --- a/testdata/golint/const-block.go +++ b/testdata/golint/const-block.go @@ -4,11 +4,14 @@ package foo const ( + InlineComment = "ShouldBeOK" // InlineComment is a valid comment + // Prefix for something. // MATCH /comment on exported const InlineWhatever should be of the form "InlineWhatever ..."/ InlineWhatever = "blah" - Whatsit = "missing_comment" // MATCH /exported const Whatsit should have comment (or a comment on this block) or be unexported/ + Whatsit = "missing_comment" + // MATCH:13 /exported const Whatsit should have comment (or a comment on this block) or be unexported/ // We should only warn once per block for missing comments, // but always complain about malformed comments. @@ -29,8 +32,11 @@ const ( // The comment on the previous const block shouldn't flow through to here. -const UndocAgain = 6 // MATCH /exported const UndocAgain should have comment or be unexported/ +const UndocAgain = 6 + +// MATCH:35 /exported const UndocAgain should have comment or be unexported/ const ( - SomeUndocumented = 7 // MATCH /exported const SomeUndocumented should have comment (or a comment on this block) or be unexported/ + SomeUndocumented = 7 + // MATCH:40 /exported const SomeUndocumented should have comment (or a comment on this block) or be unexported/ ) diff --git a/testdata/golint/error-naming.go b/testdata/golint/error-naming.go index 11df3522d..e8c7bbf15 100644 --- a/testdata/golint/error-naming.go +++ b/testdata/golint/error-naming.go @@ -11,7 +11,9 @@ import ( var unexp = errors.New("some unexported error") // MATCH /error var unexp should have name of the form errFoo/ // Exp ... -var Exp = errors.New("some exported error") // MATCH /error var Exp should have name of the form ErrFoo/ +var Exp = errors.New("some exported error") + +// MATCH:14 /error var Exp should have name of the form ErrFoo/ var ( e1 = fmt.Errorf("blah %d", 4) // MATCH /error var e1 should have name of the form errFoo/ diff --git a/testdata/golint/package-doc1.go b/testdata/golint/package-doc1.go index 1ab0a7ce5..4c649c2f7 100644 --- a/testdata/golint/package-doc1.go +++ b/testdata/golint/package-doc1.go @@ -1,3 +1,3 @@ // Test of missing package comment. -package foo // MATCH /should have a package comment, unless it's in another file for this package/ +package foo // MATCH /should have a package comment/ diff --git a/testdata/golint/value-spec.go b/testdata/golint/value-spec.go index 4471539b2..ccf827447 100644 --- a/testdata/golint/value-spec.go +++ b/testdata/golint/value-spec.go @@ -24,15 +24,22 @@ type V string func (*V) H() {} // MATCH /exported method V.H should have comment or be unexported/ -var W = "foo" // MATCH /exported var W should have comment or be unexported/ +var W = "foo" -const X = "bar" // MATCH /exported const X should have comment or be unexported/ +// MATCH:27 /exported var W should have comment or be unexported/ -var Y, Z int // MATCH /exported var Z should have its own declaration/ +const X = "bar" + +// MATCH:31 /exported const X should have comment or be unexported/ + +var Y, Z int + +// MATCH:35 /exported var Z should have its own declaration/ // Location should be okay, since the other var name is an underscore. var Location, _ = time.LoadLocation("Europe/Istanbul") // not Constantinople // this is improperly documented -// MATCH /comment on exported const Thing should be of the form "Thing ..."/ const Thing = "wonderful" + +// MATCH:42 /comment on exported const Thing should be of the form "Thing ..."/ diff --git a/testdata/imports-blacklist-original.go b/testdata/imports-blacklist-original.go new file mode 100644 index 000000000..e8d6f7d9d --- /dev/null +++ b/testdata/imports-blacklist-original.go @@ -0,0 +1,8 @@ +package fixtures + +import ( + "crypto/md5" // MATCH /should not use the following blacklisted import: "crypto/md5"/ + "crypto/sha1" // MATCH /should not use the following blacklisted import: "crypto/sha1"/ + "strings" +) + diff --git a/testdata/imports-blacklist.go b/testdata/imports-blacklist.go index cdd5ef4f5..aca95fc71 100644 --- a/testdata/imports-blacklist.go +++ b/testdata/imports-blacklist.go @@ -1,7 +1,19 @@ package fixtures import ( - "crypto/md5" // MATCH /should not use the following blacklisted import: "crypto/md5"/ - "crypto/sha1" // MATCH /should not use the following blacklisted import: "crypto/sha1"/ + "github.com/full/match" // MATCH /should not use the following blacklisted import: "github.com/full/match"/ + "bithub.com/full/match" + "github.com/full/matche" + "wildcard/between" // MATCH /should not use the following blacklisted import: "wildcard/between"/ + "wildcard/pkg1/between" // MATCH /should not use the following blacklisted import: "wildcard/pkg1/between"/ + "wildcard/pkg1/pkg2/between" // MATCH /should not use the following blacklisted import: "wildcard/pkg1/pkg2/between"/ + "wildcard/backward" // MATCH /should not use the following blacklisted import: "wildcard/backward"/ + "wildcard/backward/pkg" // MATCH /should not use the following blacklisted import: "wildcard/backward/pkg"/ + "wildcard/backward/pkg/pkg1" // MATCH /should not use the following blacklisted import: "wildcard/backward/pkg/pkg1"/ + "wildcard/forward" // MATCH /should not use the following blacklisted import: "wildcard/forward"/ + "pkg/wildcard/forward" // MATCH /should not use the following blacklisted import: "pkg/wildcard/forward"/ + "pkg/pkg1/wildcard/forward" // MATCH /should not use the following blacklisted import: "pkg/pkg1/wildcard/forward"/ + "full" // MATCH /should not use the following blacklisted import: "full"/ + "github.com/partical/match/fully" "strings" ) diff --git a/testdata/receiver-naming-issue-669.go b/testdata/receiver-naming-issue-669.go new file mode 100644 index 000000000..58c530174 --- /dev/null +++ b/testdata/receiver-naming-issue-669.go @@ -0,0 +1,37 @@ +package fixtures + +type gen1[T any] struct{} + +func (g gen1[T]) f1() {} + +func (g gen1[U]) f2() {} + +func (n gen1[T]) f3() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +func (n gen1[U]) f4() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +func (n gen1[V]) f5() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +func (n *gen1[T]) f6() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +func (n *gen1[U]) f7() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +func (n *gen1[V]) f8() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen1/ + +type gen2[T1, T2 any] struct{} + +func (g gen2[T1, T2]) f1() {} + +func (g gen2[U1, U2]) f2() {} + +func (n gen2[T1, T2]) f3() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ + +func (n gen2[U1, U2]) f4() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ + +func (n gen2[V1, V2]) f5() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ + +func (n *gen2[T1, T2]) f6() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ + +func (n *gen2[U1, U2]) f7() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ + +func (n *gen2[V1, V2]) f8() {} // MATCH /receiver name n should be consistent with previous receiver name g for gen2/ diff --git a/testdata/redefines-builtin-id.go b/testdata/redefines-builtin-id.go index 86545235b..059379903 100644 --- a/testdata/redefines-builtin-id.go +++ b/testdata/redefines-builtin-id.go @@ -9,7 +9,7 @@ func append(i, j int) { // MATCH /redefinition of the built-in function append/ } -type Type int16 // MATCH /redefinition of the built-in type Type/ +type string int16 // MATCH /redefinition of the built-in type string/ func delete(set []int64, i int) (y []int64) { // MATCH /redefinition of the built-in function delete/ for j, v := range set { diff --git a/testdata/struct-tag.go b/testdata/struct-tag.go index e87f4a68f..702c81a53 100644 --- a/testdata/struct-tag.go +++ b/testdata/struct-tag.go @@ -4,19 +4,21 @@ import "time" type decodeAndValidateRequest struct { // BEAWRE : the flag of URLParam should match the const string URLParam - URLParam string `json:"-" path:"url_param" validate:"numeric"` - Text string `json:"text" validate:"max=10"` - DefaultInt int `json:"defaultInt" default:"10.0"` // MATCH /field's type and default value's type mismatch/ - DefaultInt2 int `json:"defaultInt" default:"10"` + URLParam string `json:"-" path:"url_param" validate:"numeric"` + Text string `json:"text" validate:"max=10"` + DefaultInt int `json:"defaultInt" default:"10.0"` // MATCH /field's type and default value's type mismatch/ + DefaultInt2 int `json:"defaultInt2" default:"10"` + // MATCH:12 /unknown option 'inline' in JSON tag/ + DefaultInt3 int `json:"defaultInt2,inline" default:"11"` // MATCH /duplicate tag name: 'defaultInt2'/ DefaultString string `json:"defaultString" default:"foo"` DefaultBool bool `json:"defaultBool" default:"trues"` // MATCH /field's type and default value's type mismatch/ - DefaultBool2 bool `json:"defaultBool" default:"true"` - DefaultBool3 bool `json:"defaultBool" default:"false"` + DefaultBool2 bool `json:"defaultBool2" default:"true"` + DefaultBool3 bool `json:"defaultBool3" default:"false"` DefaultFloat float64 `json:"defaultFloat" default:"f10.0"` // MATCH /field's type and default value's type mismatch/ - DefaultFloat2 float64 `json:"defaultFloat" default:"10.0"` + DefaultFloat2 float64 `json:"defaultFloat2" default:"10.0"` MandatoryStruct mandatoryStruct `json:"mandatoryStruct" required:"trues"` // MATCH /required should be 'true' or 'false'/ - MandatoryStruct2 mandatoryStruct `json:"mandatoryStruct" required:"true"` - MandatoryStruct4 mandatoryStruct `json:"mandatoryStruct" required:"false"` + MandatoryStruct2 mandatoryStruct `json:"mandatoryStruct2" required:"true"` + MandatoryStruct4 mandatoryStruct `json:"mandatoryStruct4" required:"false"` OptionalStruct *optionalStruct `json:"optionalStruct,omitempty"` OptionalQuery string `json:"-" querystring:"queryfoo"` optionalQuery string `json:"-" querystring:"queryfoo"` // MATCH /tag on not-exported field optionalQuery/ @@ -57,3 +59,46 @@ type VirtualMachineRelocateSpecDiskLocator struct { DiskBackingInfo BaseVirtualDeviceBackingInfo `xml:"diskBackingInfo,omitempty,any"` Profile []BaseVirtualMachineProfileSpec `xml:"profile,omitempty,other"` // MATCH /unknown option 'other' in XML tag/ } + +type TestDuplicatedXMLTags struct { + A int `xml:"a"` + B int `xml:"a"` // MATCH /duplicate tag name: 'a'/ + C int `xml:"c"` +} + +type TestDuplicatedBSONTags struct { + A int `bson:"b"` + B int `bson:"b"` // MATCH /duplicate tag name: 'b'/ + C int `bson:"c"` +} + +type TestDuplicatedYAMLTags struct { + A int `yaml:"b"` + B int `yaml:"c"` + C int `yaml:"c"` // MATCH /duplicate tag name: 'c'/ +} + +type TestDuplicatedProtobufTags struct { + A int `protobuf:"varint,name=b"` + B int `protobuf:"varint,name=c"` + C int `protobuf:"varint,name=c"` // MATCH /duplicate tag name: 'c'/ +} + +// test case from +// sigs.k8s.io/kustomize/api/types/helmchartargs.go + +type HelmChartArgs struct { + ChartName string `json:"chartName,omitempty" yaml:"chartName,omitempty"` + ChartVersion string `json:"chartVersion,omitempty" yaml:"chartVersion,omitempty"` + ChartRepoURL string `json:"chartRepoUrl,omitempty" yaml:"chartRepoUrl,omitempty"` + ChartHome string `json:"chartHome,omitempty" yaml:"chartHome,omitempty"` + ChartRepoName string `json:"chartRepoName,omitempty" yaml:"chartRepoName,omitempty"` + HelmBin string `json:"helmBin,omitempty" yaml:"helmBin,omitempty"` + HelmHome string `json:"helmHome,omitempty" yaml:"helmHome,omitempty"` + Values string `json:"values,omitempty" yaml:"values,omitempty"` + ValuesLocal map[string]interface{} `json:"valuesLocal,omitempty" yaml:"valuesLocal,omitempty"` + ValuesMerge string `json:"valuesMerge,omitempty" yaml:"valuesMerge,omitempty"` + ReleaseName string `json:"releaseName,omitempty" yaml:"releaseName,omitempty"` + ReleaseNamespace string `json:"releaseNamespace,omitempty" yaml:"releaseNamespace,omitempty"` + ExtraArgs []string `json:"extraArgs,omitempty" yaml:"extraArgs,omitempty"` +}