Merge remote-tracking branch 'upstream/master'

Conflicts: config.go
2025-01-24 03:47:45 +02:00 · 2019-03-18 14:03:06 +01:00 · 2019-03-18 14:03:06 +01:00 · c14c00c892
commit c14c00c892
parent 4bb1b4d361 6a62ee9f02
11 changed files with 210 additions and 46 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -5,13 +5,15 @@ script:
 - make build
 - make test
 - git clone https://github.com/mgechev/revive.run revive-docs && cd revive-docs
- npm i && npm run build || true
+- npm i || true
+- npm run build || true
 - git config --global user.email "mgechev@gmail.com"
 - git config --global user.name "mgechev"
 - git add .
 - git commit -am 'Travis CI deployment'
- git push --force --quiet "https://${GH_TOKEN}@${GH_REF}"
+- git push --force --quiet "https://${GH_TOKEN}@${GH_REF}" || true
 env:
  global:
  - GH_REF: github.com/mgechev/revive.run.git
  - secure: Bp2/fJOVEor5aj3rNwA3+4/wecCmX2mVQ2pQt1AJ1n+pT6MjKYywswTDT6kzK/Cu1bPcfEGh3a7XKieAhIWVKvchoyYVV8J80F76HGyqgSBuzFXvV0c32zFn2LtxQxvmtCNynjmGAV57dHtsxGmHxkX9u8JIJ4J06E2Eq9nuuflTCf5o5gHtaE7P7hQT2WL/JRJVABHUMa0XzsMuUdRNO0OBXGMm+SqiWEcZetf2Vq3tfo2LL4ula99oTKKspg0iRKiauCZZaRxyZG/V3QiR0Rl9nhTVnb6hx6/RFrxru63Pm1FUaK1gIqEq9EUMpZRTddGW77UPp9GSB81/GaUm+e0GNFjUAL2e59t72wMxCQEOT+835hVbeCjgdksg0IDbn7sR/S+rYbiCyxTuCA/4YNlDoEajl9RMxK4culsq35LnibE1x7L4Q/5blD7HwVSMhA33HSDCC5MINwTdWwsdHYiAvFo0RCi5B0GngMzE6/pJxvYhWV3YhKWrgSmhafV1QO3Qu9dCn6P+7KsEVDbUeA8Yxnugd60kQNh2vG7bdTYKaZ6GhfU12zAM15xd2SSrKl6szSAo64CYOTznNFMBMskpm05SubTW1w+xDQy8vGjIHqb6zntiqUhFhTDd326iRYfrQuhAK53XbU1NUFFOyZa8kCTlSsPWDoSOX68oH9Q=
+
--- a/README.md
+++ b/README.md
@ -12,17 +12,28 @@ Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in r

 Here's how `revive` is different from `golint`:

- Allows us to enable or disable rules using a configuration file.
- Allows us to configure the linting rules with a TOML file.
+- Allows to enable or disable rules using a configuration file.
+- Allows to configure the linting rules with a TOML file.
 - 2x faster running the same rules as golint.
 - Provides functionality for disabling a specific rule or the entire linter for a file or a range of lines.
  - `golint` allows this only for generated files.
 - Optional type checking. Most rules in golint do not require type checking. If you disable them in the config file, revive will run over 6x faster than golint.
 - Provides multiple formatters which let us customize the output.
- Allows us to customize the return code for the entire linter or based on the failure of only some rules.
+- Allows to customize the return code for the entire linter or based on the failure of only some rules.
 - _Everyone can extend it easily with custom rules or formatters._
 - `Revive` provides more rules compared to `golint`.

+## Who uses Revive
+
+- [`tidb`](https://github.com/pingcap/tidb) - TiDB is a distributed HTAP database compatible with the MySQL protocol
+- [`ferret`](https://github.com/MontFerret/ferret) - Declarative web scraping
+- [`gopass`](https://github.com/gopasspw/gopass) - The slightly more awesome standard unix password manager for teams
+- [`gitea`](https://github.com/go-gitea/gitea) - Git with a cup of tea, painless self-hosted git service
+- [`excelize`](https://github.com/360EntSecGroup-Skylar/excelize) - Go library for reading and writing Microsoft Excel™ (XLSX) files
+- [`aurora`](https://github.com/xuri/aurora) - aurora is a web-based Beanstalk queue server console written in Go
+
+*Open a PR to add your project*.
+
 <p align="center">
  <img src="./assets/demo.svg" alt="" width="700">
 </p>
@ -47,6 +58,8 @@ Here's how `revive` is different from `golint`:
    - [Friendly](#friendly)
    - [Stylish](#stylish)
    - [Default](#default)
+    - [Plain](#plain)
+    - [Unix](#unix)
  - [Extensibility](#extensibility)
    - [Custom Rule](#custom-rule)
      - [Example](#example)
@ -277,7 +290,8 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`waitgroup-by-value`](./RULES_DESCRIPTIONS.md#waitgroup-by-value)  |  n/a   | Warns on functions taking sync.WaitGroup as a by-value parameter |    no    |  no   |
 | [`atomic`](./RULES_DESCRIPTIONS.md#atomic)              |  n/a   | Check for common mistaken usages of the `sync/atomic` package    |    no    |  no   |
 | [`empty-lines`](./RULES_DESCRIPTIONS.md#empty-lines)   | n/a | Warns when there are heading or trailing newlines in a block              |    no    |  no   |
-| [`line-lenght-limit`](./RULES_DESCRIPTIONS.md#line-lenght-limit)   | int    | Specifies the maximum number of characters in a line             |    no    |  no   |
+| [`line-length-limit`](./RULES_DESCRIPTIONS.md#line-length-limit)   | int    | Specifies the maximum number of characters in a line             |    no    |  no   |
+| [`call-to-gc`](./RULES_DESCRIPTIONS.md#call-to-gc)   | n/a    | Warns on explicit call to the garbage collector    |    no    |  no   |

 ## Configurable rules

@ -414,14 +428,19 @@ Currently, type checking is enabled by default. If you want to run the linter wi

 ## Contributors

-[<img alt="mgechev" src="https://avatars1.githubusercontent.com/u/455023?v=4&s=117" width="117">](https://github.com/mgechev) |[<img alt="chavacava" src="https://avatars2.githubusercontent.com/u/25788468?v=4&s=117" width="117">](https://github.com/chavacava) |[<img alt="xuri" src="https://avatars2.githubusercontent.com/u/2809468?v=4&s=117" width="117">](https://github.com/xuri) |[<img alt="gsamokovarov" src="https://avatars0.githubusercontent.com/u/604618?v=4&s=117" width="117">](https://github.com/gsamokovarov) |[<img alt="morphy2k" src="https://avatars2.githubusercontent.com/u/4280578?v=4&s=117" width="117">](https://github.com/morphy2k) |[<img alt="z0mbie42" src="https://avatars0.githubusercontent.com/u/6172808?v=4&s=117" width="117">](https://github.com/z0mbie42) |
+[<img alt="mgechev" src="https://avatars1.githubusercontent.com/u/455023?v=4&s=117" width="117">](https://github.com/mgechev) |[<img alt="chavacava" src="https://avatars2.githubusercontent.com/u/25788468?v=4&s=117" width="117">](https://github.com/chavacava) |[<img alt="xuri" src="https://avatars2.githubusercontent.com/u/2809468?v=4&s=117" width="117">](https://github.com/xuri) |[<img alt="gsamokovarov" src="https://avatars0.githubusercontent.com/u/604618?v=4&s=117" width="117">](https://github.com/gsamokovarov) |[<img alt="morphy2k" src="https://avatars2.githubusercontent.com/u/4280578?v=4&s=117" width="117">](https://github.com/morphy2k) |[<img alt="tamird" src="https://avatars0.githubusercontent.com/u/1535036?v=4&s=117" width="117">](https://github.com/tamird) |
 :---: |:---: |:---: |:---: |:---: |:---: |
-[mgechev](https://github.com/mgechev) |[chavacava](https://github.com/chavacava) |[xuri](https://github.com/xuri) |[gsamokovarov](https://github.com/gsamokovarov) |[morphy2k](https://github.com/morphy2k) |[z0mbie42](https://github.com/z0mbie42) |
+[mgechev](https://github.com/mgechev) |[chavacava](https://github.com/chavacava) |[xuri](https://github.com/xuri) |[gsamokovarov](https://github.com/gsamokovarov) |[morphy2k](https://github.com/morphy2k) |[tamird](https://github.com/tamird) |

-[<img alt="tamird" src="https://avatars0.githubusercontent.com/u/1535036?v=4&s=117" width="117">](https://github.com/tamird) |[<img alt="paul-at-start" src="https://avatars2.githubusercontent.com/u/5486775?v=4&s=117" width="117">](https://github.com/paul-at-start) |[<img alt="psapezhko" src="https://avatars3.githubusercontent.com/u/10865586?v=4&s=117" width="117">](https://github.com/psapezhko) |[<img alt="Jarema" src="https://avatars0.githubusercontent.com/u/7369771?v=4&s=117" width="117">](https://github.com/Jarema) |[<img alt="vkrol" src="https://avatars3.githubusercontent.com/u/153412?v=4&s=117" width="117">](https://github.com/vkrol) |[<img alt="haya14busa" src="https://avatars0.githubusercontent.com/u/3797062?v=4&s=117" width="117">](https://github.com/haya14busa) |
+[<img alt="AragurDEV" src="https://avatars0.githubusercontent.com/u/11004008?v=4&s=117" width="117">](https://github.com/AragurDEV) |[<img alt="jamesmaidment" src="https://avatars3.githubusercontent.com/u/2050324?v=4&s=117" width="117">](https://github.com/jamesmaidment) |[<img alt="mapreal19" src="https://avatars2.githubusercontent.com/u/3055997?v=4&s=117" width="117">](https://github.com/mapreal19) |[<img alt="paul-at-start" src="https://avatars2.githubusercontent.com/u/5486775?v=4&s=117" width="117">](https://github.com/paul-at-start) |[<img alt="psapezhko" src="https://avatars3.githubusercontent.com/u/10865586?v=4&s=117" width="117">](https://github.com/psapezhko) |[<img alt="ridvansumset" src="https://avatars2.githubusercontent.com/u/26631560?v=4&s=117" width="117">](https://github.com/ridvansumset) |
 :---: |:---: |:---: |:---: |:---: |:---: |
-[tamird](https://github.com/tamird) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) |[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) |
+[AragurDEV](https://github.com/AragurDEV) |[jamesmaidment](https://github.com/jamesmaidment) |[mapreal19](https://github.com/mapreal19) |[paul-at-start](https://github.com/paul-at-start) |[psapezhko](https://github.com/psapezhko) |[ridvansumset](https://github.com/ridvansumset) |
+
+[<img alt="Jarema" src="https://avatars0.githubusercontent.com/u/7369771?v=4&s=117" width="117">](https://github.com/Jarema) |[<img alt="vkrol" src="https://avatars3.githubusercontent.com/u/153412?v=4&s=117" width="117">](https://github.com/vkrol) |[<img alt="haya14busa" src="https://avatars0.githubusercontent.com/u/3797062?v=4&s=117" width="117">](https://github.com/haya14busa) |
+:---: |:---: |:---: |
+[Jarema](https://github.com/Jarema) |[vkrol](https://github.com/vkrol) |[haya14busa](https://github.com/haya14busa) |

 ## License

 MIT
+
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@ -9,6 +9,7 @@ List of all available rules.
  - [atomic](#atomic)
  - [blank-imports](#blank-imports)
  - [bool-literal-in-expr](#bool-literal-in-expr)
+  - [call-to-gc](#call-to-gc)
  - [confusing-naming](#confusing-naming)
  - [confusing-results](#confusing-results)
  - [constant-logical-expr](#constant-logical-expr)
@ -32,7 +33,7 @@ List of all available rules.
  - [increment-decrement](#increment-decrement)
  - [indent-error-flow](#indent-error-flow)
  - [imports-blacklist](#imports-blacklist)
-  - [line-lenght-limit](#line-lenght-limit)
+  - [line-length-limit](#line-length-limit)
  - [max-public-structs](#max-public-structs)
  - [modifies-parameter](#modifies-parameter)
  - [modifies-value-receiver](#modifies-value-receiver)
@ -80,7 +81,7 @@ _Configuration_: (int) the maximum number of parameters allowed per function.
 Example:

 ```toml
-[argument-limit]
+[rule.argument-limit]
  arguments =[4]
 ```

@ -102,6 +103,14 @@ _Description_: Using Boolean literals (`true`, `false`) in logic expressions may

 _Configuration_: N/A

+## call-to-gc
+
+_Description_:  Explicitly invoking the garbage collector is, except for specific uses in benchmarking, very dubious.
+
+The garbage collector can be configured through environment variables as described [here](https://golang.org/pkg/runtime/).
+
+_Configuration_: N/A
+
 ## confusing-naming

 _Description_: Methods or fields of `struct` that have names different only by capitalization could be confusing.
@ -141,7 +150,7 @@ _Configuration_: (int) the maximum function complexity
 Example:

 ```toml
-[cyclomatic]
+[rule.cyclomatic]
  arguments =[3]
 ```

@ -214,7 +223,7 @@ _Configuration_: (string) the header to look for in source files.
 Example:

 ```toml
-[file-header]
+[rule.file-header]
  arguments =["This is the text that must appear at the top of source files."]
 ```

@ -235,7 +244,7 @@ _Configuration_: (int) the maximum allowed return values
 Example:

 ```toml
-[function-result-limit]
+[rule.function-result-limit]
  arguments =[3]
 ```

@ -280,7 +289,7 @@ Example:
  arguments =["crypto/md5", "crypto/sha1"]
 ```

-## line-lenght-limit
+## line-length-limit

 _Description_: Warns in the presence of code lines longer than a configured maximum.

@ -289,7 +298,7 @@ _Configuration_: (int) maximum line length in characters.
 Example:

 ```toml
-[line-lenght-limit]
+[rule.line-length-limit]
  arguments =[80]
 ```

@ -305,7 +314,7 @@ _Configuration_: (int) the maximum allowed public structs
 Example:

 ```toml
-[max-public-structs]
+[rule.max-public-structs]
  arguments =[3]
 ```

--- a/config.go
+++ b/config.go
@ -54,10 +54,6 @@ var allRules = append([]lint.Rule{
 	&rule.ModifiesParamRule{},
 	&rule.ConfusingResultsRule{},
 	&rule.DeepExitRule{},
-<<<<<<< HEAD
-	&rule.ADSPrintRule{},
-	&rule.ADSLostErrRule{},
-=======
 	&rule.UnusedParamRule{},
 	&rule.UnreachableCodeRule{},
 	&rule.AddConstantRule{},
@ -76,7 +72,7 @@ var allRules = append([]lint.Rule{
 	&rule.AtomicRule{},
 	&rule.EmptyLinesRule{},
 	&rule.LineLengthLimitRule{},
->>>>>>> ad7df7dc375879b9c25628acf9d643378cfaf102
+	&rule.CallToGCRule{},
 }, defaultRules...)

 var allFormatters = []lint.Formatter{
--- a/fixtures/call-to-gc.go
+++ b/fixtures/call-to-gc.go
@ -0,0 +1,17 @@
+package fixtures
+
+import (
+	"fmt"
+	"runtime"
+)
+
+func GC() {
+}
+
+func foo() {
+	fmt.Println("just testing")
+	GC()
+	fixtures.GC()
+	runtime.Goexit()
+	runtime.GC() // MATCH /explicit call to the garbage collector/
+}
--- a/fixtures/disable-annotations2.go
+++ b/fixtures/disable-annotations2.go
@ -0,0 +1,24 @@
+package fixtures
+
+func foo1() {
+	//revive:disable-next-line:var-naming
+	var invalid_name = 0
+	var invalid_name2 = 1 //revive:disable-line:var-naming
+}
+
+func foo2() {
+	// 		revive:disable-next-line:var-naming
+	//revive:disable
+	var invalid_name = 0
+	var invalid_name2 = 1
+}
+
+func foo3() {
+	//revive:enable
+	// revive:disable-next-line:var-naming
+	var invalid_name = 0
+	// not a valid annotation revive:disable-next-line:var-naming
+	var invalid_name2 = 1 // MATCH /don't use underscores in Go names; var invalid_name2 should be invalidName2/
+	/* revive:disable-next-line:var-naming */
+	var invalid_name3 = 0 // MATCH /don't use underscores in Go names; var invalid_name3 should be invalidName3/
+}
--- a/fixtures/line-length-limit.go
+++ b/fixtures/line-length-limit.go
@ -5,3 +5,13 @@ import "fmt"
 func foo(a, b int) {
 	fmt.Printf("single line characters out of limit") // MATCH /line is 105 characters, out of limit 100/
 }
+
+// revive:disable-next-line:line-length-limit
+// The length of this comment line is over 80 characters, this is bad for readability.
+
+// Warn: the testing framework does not allow to check for failures in comments
+
+func toto() {
+	// revive:disable-next-line:line-length-limit
+	fmt.Println("This line is way too long. In my opinion, it should be shortened.")
+}
--- a/lint/file.go
+++ b/lint/file.go
@ -127,7 +127,7 @@ type enableDisableConfig struct {
 }

 func (f *File) disabledIntervals(rules []Rule) disabledIntervalsMap {
-	re := regexp.MustCompile(`^\s*revive:(enable|disable)(?:-(line|next-line))?(:|\s|$)`)
+	re := regexp.MustCompile(`^//[\s]*revive:(enable|disable)(?:-(line|next-line))?(?::([^\s]+))?[\s]*$`)

 	enabledDisabledRulesMap := make(map[string][]enableDisableConfig)

@ -194,37 +194,38 @@ func (f *File) disabledIntervals(rules []Rule) disabledIntervalsMap {
 	}

 	handleComment := func(filename string, c *ast.CommentGroup, line int) {
-		text := c.Text()
-		parts := re.FindStringSubmatch(text)
-		if len(parts) == 0 {
-			return
-		}
-		str := re.FindString(text)
-		ruleNamesString := strings.Split(text, str)
-		ruleNames := []string{}
-		if len(ruleNamesString) == 2 {
-			tempNames := strings.Split(ruleNamesString[1], ",")
-			for _, name := range tempNames {
-				name = strings.Trim(name, "\n")
-				if len(name) > 0 {
-					ruleNames = append(ruleNames, name)
+		comments := c.List
+		for _, c := range comments {
+			match := re.FindStringSubmatch(c.Text)
+			if len(match) == 0 {
+				return
+			}
+
+			ruleNames := []string{}
+			if len(match) > 2 {
+				tempNames := strings.Split(match[3], ",")
+				for _, name := range tempNames {
+					name = strings.Trim(name, "\n")
+					if len(name) > 0 {
+						ruleNames = append(ruleNames, name)
+					}
 				}
 			}
-		}

-		// TODO: optimize
-		if len(ruleNames) == 0 {
-			for _, rule := range rules {
-				ruleNames = append(ruleNames, rule.Name())
+			// TODO: optimize
+			if len(ruleNames) == 0 {
+				for _, rule := range rules {
+					ruleNames = append(ruleNames, rule.Name())
+				}
 			}
-		}

-		handleRules(filename, parts[2], parts[1] == "enable", line, ruleNames)
+			handleRules(filename, match[2], match[1] == "enable", line, ruleNames)
+		}
 	}

 	comments := f.AST.Comments
 	for _, c := range comments {
-		handleComment(f.Name, c, f.ToPosition(c.Pos()).Line)
+		handleComment(f.Name, c, f.ToPosition(c.End()).Line)
 	}

 	return getEnabledDisabledIntervals()
--- a/rule/call-to-gc.go
+++ b/rule/call-to-gc.go
@ -0,0 +1,70 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// CallToGCRule lints calls to the garbage collector.
+type CallToGCRule struct{}
+
+// Apply applies the rule to given file.
+func (r *CallToGCRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	var gcTriggeringFunctions = map[string]map[string]bool{
+		"runtime": map[string]bool{"GC": true},
+	}
+
+	w := lintCallToGC{onFailure, gcTriggeringFunctions}
+	ast.Walk(w, file.AST)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *CallToGCRule) Name() string {
+	return "call-to-gc"
+}
+
+type lintCallToGC struct {
+	onFailure             func(lint.Failure)
+	gcTriggeringFunctions map[string]map[string]bool
+}
+
+func (w lintCallToGC) Visit(node ast.Node) ast.Visitor {
+	ce, ok := node.(*ast.CallExpr)
+	if !ok {
+		return w // nothing to do, the node is not a call
+	}
+
+	fc, ok := ce.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return nil // nothing to do, the call is not of the form pkg.func(...)
+	}
+
+	id, ok := fc.X.(*ast.Ident)
+
+	if !ok {
+		return nil // in case X is not an id (it should be!)
+	}
+
+	fn := fc.Sel.Name
+	pkg := id.Name
+	if !w.gcTriggeringFunctions[pkg][fn] {
+		return nil // it isn't a call to a GC triggering function
+	}
+
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       node,
+		Category:   "bad practice",
+		Failure:    "explicit call to the garbage collector",
+	})
+
+	return w
+}
--- a/test/call-to-gc_test.go
+++ b/test/call-to-gc_test.go
@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// TestCallToGC test call-to-gc rule
+func TestCallToGC(t *testing.T) {
+	testRule(t, "call-to-gc", &rule.CallToGCRule{})
+}
--- a/test/disable-annotations_test.go
+++ b/test/disable-annotations_test.go
@ -10,3 +10,7 @@ import (
 func TestDisabledAnnotations(t *testing.T) {
 	testRule(t, "disable-annotations", &rule.ExportedRule{}, &lint.RuleConfig{})
 }
+
+func TestModifiedAnnotations(t *testing.T) {
+	testRule(t, "disable-annotations2", &rule.VarNamingRule{}, &lint.RuleConfig{})
+}