mirror of
				https://github.com/mgechev/revive.git
				synced 2025-10-30 23:37:49 +02:00 
			
		
		
		
	Exported config (#565)
* working version * adds flag for replacing "stutters"
This commit is contained in:
		| @@ -408,7 +408,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | ||||
| | [`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-naming`](./RULES_DESCRIPTIONS.md#error-naming)        |  n/a   | Naming of error variables.                                       |   yes    |  no   | | ||||
| | [`exported`](./RULES_DESCRIPTIONS.md#exported)            |  n/a   | Naming and commenting conventions on exported symbols.           |   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   | | ||||
| | [`increment-decrement`](./RULES_DESCRIPTIONS.md#increment-decrement) |  n/a   | Use `i++` and `i--` instead of `i += 1` and `i -= 1`.            |   yes    |  no   | | ||||
| | [`var-naming`](./RULES_DESCRIPTIONS.md#var-naming)          |  whitelist & blacklist of initialisms   | Naming rules.                                                    |   yes    |  no   | | ||||
|   | ||||
| @@ -299,7 +299,20 @@ _Description_: Exported function and methods should have comments. This warns on | ||||
|  | ||||
| More information [here](https://github.com/golang/go/wiki/CodeReviewComments#doc-comments) | ||||
|  | ||||
| _Configuration_: N/A | ||||
| _Configuration_: ([]string) rule flags.  | ||||
| Please notice that without configuration, the default behavior of the rule is that of its `golint` counterpart.  | ||||
| Available flags are: | ||||
|  | ||||
| * _checkPrivateReceivers_ enables checking public methods of private types | ||||
| * _disableStutteringCheck_ disables checking for method names that stutter with the package name (i.e. avoid failure messages of the form _type name will be used as x.XY by other packages, and that stutters; consider calling this Y_) | ||||
| * _sayRepetitiveInsteadOfStutters_ replaces the use of the term _stutters_ by _repetitive_ in failure messages | ||||
|  | ||||
| Example: | ||||
|  | ||||
| ```toml | ||||
| [rule.exported] | ||||
|   arguments =["checkPrivateReceivers","disableStutteringCheck"] | ||||
| ``` | ||||
|  | ||||
| ## file-header | ||||
|  | ||||
|   | ||||
| @@ -15,13 +15,20 @@ import ( | ||||
| type ExportedRule struct{} | ||||
|  | ||||
| // Apply applies the rule to given file. | ||||
| func (r *ExportedRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { | ||||
| func (r *ExportedRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { | ||||
| 	var failures []lint.Failure | ||||
|  | ||||
| 	if isTest(file) { | ||||
| 		return failures | ||||
| 	} | ||||
|  | ||||
| 	checkPrivateReceivers, disableStutteringCheck, sayRepetitiveInsteadOfStutters := r.getConf(args) | ||||
|  | ||||
| 	stuttersMsg := "stutters" | ||||
| 	if sayRepetitiveInsteadOfStutters { | ||||
| 		stuttersMsg = "is repetitive" | ||||
| 	} | ||||
|  | ||||
| 	fileAst := file.AST | ||||
| 	walker := lintExported{ | ||||
| 		file:    file, | ||||
| @@ -30,6 +37,9 @@ func (r *ExportedRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { | ||||
| 			failures = append(failures, failure) | ||||
| 		}, | ||||
| 		genDeclMissingComments: make(map[*ast.GenDecl]bool), | ||||
| 		checkPrivateReceivers:  checkPrivateReceivers, | ||||
| 		disableStutteringCheck: disableStutteringCheck, | ||||
| 		stuttersMsg:            stuttersMsg, | ||||
| 	} | ||||
|  | ||||
| 	ast.Walk(&walker, fileAst) | ||||
| @@ -42,12 +52,41 @@ func (r *ExportedRule) Name() string { | ||||
| 	return "exported" | ||||
| } | ||||
|  | ||||
| func (r *ExportedRule) getConf(args lint.Arguments) (checkPrivateReceivers bool, disableStutteringCheck bool, sayRepetitiveInsteadOfStutters bool) { | ||||
| 	// if any, we expect a slice of strings as configuration | ||||
| 	if len(args) < 1 { | ||||
| 		return | ||||
| 	} | ||||
| 	for _, flag := range args { | ||||
| 		flagStr, ok := flag.(string) | ||||
| 		if !ok { | ||||
| 			panic(fmt.Sprintf("Invalid argument for the %s rule: expecting a string, got %T", r.Name(), flag)) | ||||
| 		} | ||||
|  | ||||
| 		switch flagStr { | ||||
| 		case "checkPrivateReceivers": | ||||
| 			checkPrivateReceivers = true | ||||
| 		case "disableStutteringCheck": | ||||
| 			disableStutteringCheck = true | ||||
| 		case "sayRepetitiveInsteadOfStutters": | ||||
| 			sayRepetitiveInsteadOfStutters = true | ||||
| 		default: | ||||
| 			panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name())) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	return | ||||
| } | ||||
|  | ||||
| type lintExported struct { | ||||
| 	file                   *lint.File | ||||
| 	fileAst                *ast.File | ||||
| 	lastGen                *ast.GenDecl | ||||
| 	genDeclMissingComments map[*ast.GenDecl]bool | ||||
| 	onFailure              func(lint.Failure) | ||||
| 	checkPrivateReceivers  bool | ||||
| 	disableStutteringCheck bool | ||||
| 	stuttersMsg            string | ||||
| } | ||||
|  | ||||
| func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { | ||||
| @@ -61,7 +100,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { | ||||
| 		// method | ||||
| 		kind = "method" | ||||
| 		recv := receiverType(fn) | ||||
| 		if !ast.IsExported(recv) { | ||||
| 		if !ast.IsExported(recv) && !w.checkPrivateReceivers { | ||||
| 			// receiver is unexported | ||||
| 			return | ||||
| 		} | ||||
| @@ -98,6 +137,10 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) { | ||||
| } | ||||
|  | ||||
| func (w *lintExported) checkStutter(id *ast.Ident, thing string) { | ||||
| 	if w.disableStutteringCheck { | ||||
| 		return | ||||
| 	} | ||||
|  | ||||
| 	pkg, name := w.fileAst.Name.Name, id.Name | ||||
| 	if !ast.IsExported(name) { | ||||
| 		// unexported name | ||||
| @@ -122,7 +165,7 @@ func (w *lintExported) checkStutter(id *ast.Ident, thing string) { | ||||
| 			Node:       id, | ||||
| 			Confidence: 0.8, | ||||
| 			Category:   "naming", | ||||
| 			Failure:    fmt.Sprintf("%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem), | ||||
| 			Failure:    fmt.Sprintf("%s name will be used as %s.%s by other packages, and that %s; consider calling this %s", thing, pkg, name, w.stuttersMsg, rem), | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|   | ||||
							
								
								
									
										26
									
								
								test/exported_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										26
									
								
								test/exported_test.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,26 @@ | ||||
| package test | ||||
|  | ||||
| import ( | ||||
| 	"testing" | ||||
|  | ||||
| 	"github.com/mgechev/revive/lint" | ||||
| 	"github.com/mgechev/revive/rule" | ||||
| ) | ||||
|  | ||||
| func TestExportedWithDisableStutteringCheck(t *testing.T) { | ||||
| 	args := []interface{}{"disableStutteringCheck"} | ||||
|  | ||||
| 	testRule(t, "exported-issue-555", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) | ||||
| } | ||||
|  | ||||
| func TestExportedWithChecksOnMethodsOfPrivateTypes(t *testing.T) { | ||||
| 	args := []interface{}{"checkPrivateReceivers"} | ||||
|  | ||||
| 	testRule(t, "exported-issue-552", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) | ||||
| } | ||||
|  | ||||
| func TestExportedReplacingStuttersByRepetitive(t *testing.T) { | ||||
| 	args := []interface{}{"sayRepetitiveInsteadOfStutters"} | ||||
|  | ||||
| 	testRule(t, "exported-issue-519", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args}) | ||||
| } | ||||
							
								
								
									
										7
									
								
								testdata/exported-issue-519.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										7
									
								
								testdata/exported-issue-519.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,7 @@ | ||||
| // Package golint comment | ||||
| package golint | ||||
|  | ||||
| // Test case for the configuration option tp replace the word "stutters" by "repetitive" failure messages | ||||
|  | ||||
| //  GolintRepetitive is a dummy function | ||||
| func GolintRepetitive() {} // MATCH /func name will be used as golint.GolintRepetitive by other packages, and that is repetitive; consider calling this Repetitive/ | ||||
							
								
								
									
										10
									
								
								testdata/exported-issue-552.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										10
									
								
								testdata/exported-issue-552.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,10 @@ | ||||
| // Package golint comment | ||||
| package golint | ||||
|  | ||||
| // Test cases for enabling checks of exported methods of private types in exported rule | ||||
| type private struct { | ||||
| } | ||||
|  | ||||
| // MATCH /comment on exported method private.Method should be of the form "Method ..."/ | ||||
| func (p *private) Method() { | ||||
| } | ||||
							
								
								
									
										7
									
								
								testdata/exported-issue-555.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										7
									
								
								testdata/exported-issue-555.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,7 @@ | ||||
| // Package golint comment | ||||
| package golint | ||||
|  | ||||
| // Test case for disabling stuttering check in exported rule | ||||
|  | ||||
| //  GolintSomething is a dummy function | ||||
| func GolintSomething() {} | ||||
							
								
								
									
										5
									
								
								testdata/golint/exported.go
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										5
									
								
								testdata/golint/exported.go
									
									
									
									
										vendored
									
									
								
							| @@ -3,6 +3,9 @@ package golint | ||||
|  | ||||
| import "net/http" | ||||
|  | ||||
| //  GolintFoo is a dummy function | ||||
| func GolintFoo() {} // MATCH /func name will be used as golint.GolintFoo by other packages, and that stutters; consider calling this Foo/ | ||||
|  | ||||
| type ( | ||||
| 	// O is a shortcut (alias) for map[string]interface{}, e.g. a JSON object. | ||||
| 	O = map[string]interface{} | ||||
| @@ -53,3 +56,5 @@ func ServeHTTP(w http.ResponseWriter, r *http.Request) {}                // MATC | ||||
| func Read(p []byte) (n int, err error)                 { return 0, nil } // MATCH /exported function Read should have comment or be unexported/ | ||||
| func Write(p []byte) (n int, err error)                { return 0, nil } // MATCH /exported function Write should have comment or be unexported/ | ||||
| func Unwrap(err error) error                           { return nil }    // MATCH /exported function Unwrap should have comment or be unexported/ | ||||
|  | ||||
| // The following cases are tests for issue 555 | ||||
|   | ||||
		Reference in New Issue
	
	Block a user