From e183f831246f1e05031123b53dba2b631442bd5c Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Mon, 7 Oct 2019 18:44:41 +0200 Subject: [PATCH] Better field names for Configuration --- cmd/wsl/main.go | 6 ++--- wsl.go | 62 +++++++++++++++++++++++++++++++------------------ wsl_test.go | 30 ++++++++++++++++++++---- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/cmd/wsl/main.go b/cmd/wsl/main.go index 8399388..e699f41 100644 --- a/cmd/wsl/main.go +++ b/cmd/wsl/main.go @@ -29,9 +29,9 @@ func main() { flag.BoolVar(&showWarnings, "w", false, "Show warnings (ignored rules)") flag.BoolVar(&showWarnings, "warnings", false, "") - flag.BoolVar(&config.CheckAppend, "check-append", true, "Strict rules for append") - flag.BoolVar(&config.AllowAssignAndCallsCuddle, "allow-assign-and-call", true, "Allow assignments and calls to be cuddled (if using same variable/type)") - flag.BoolVar(&config.AllowMultiLineAssignmentCuddled, "allow-multi-line-assignment", true, "Allow cuddling with multi line assignments") + flag.BoolVar(&config.StrictAppend, "strict-append", true, "Strict rules for append") + flag.BoolVar(&config.AllowAssignAndCallCuddle, "allow-assign-and-call", true, "Allow assignments and calls to be cuddled (if using same variable/type)") + flag.BoolVar(&config.AllowMultiLineAssignCuddle, "allow-multi-line-assign", true, "Allow cuddling with multi line assignments") flag.Parse() diff --git a/wsl.go b/wsl.go index 6c9a11c..3dcc32b 100644 --- a/wsl.go +++ b/wsl.go @@ -10,36 +10,49 @@ import ( ) type Configuration struct { - // CheckAppend will do special handling when assigning from append (x = + // StrictAppend will do strict checking when assigning from append (x = // append(x, y)). If this is set to true the append call must append either - // a variable assigned on the line above or a variable called on the line - // above. - CheckAppend bool - - // AllowAssignAndCallsCuddle allows assignments to be cuddled with variables + // a variable assigned, called or used on the line above. Example on not + // allowed when this is true: + // + // x := []string{} + // y := "not going in X" + // x = append(x, "not y") // This is not allowed with StrictAppend + // z := "going in X" + // + // x = append(x, z) // This is allowed with StrictAppend + // + // m := transform(z) + // x = append(x, z) // So is this because Z is used above. + StrictAppend bool + + // AllowAssignAndCallCuddle allows assignments to be cuddled with variables // used in calls on line above and calls to be cuddled with assignments of // variables used in call on line above. // Example supported with this set to true: + // // x.Call() - // y = Assign() + // x = Assign() // x.AnotherCall() - // y = AnotherAssign() - AllowAssignAndCallsCuddle bool + // x = AnotherAssign() + AllowAssignAndCallCuddle bool - // AllowMultiLineAssignmentCuddled allows cuddling to assignments even if - // they span over multiple lines. This defaults to true which allows the + // AllowMultiLineAssignCuddle allows cuddling to assignments even if they + // span over multiple lines. This defaults to true which allows the // following example: + // // err := function( // "multiple", "lines", // ) // if err != nil { // // ... // } - AllowMultiLineAssignmentCuddled bool + AllowMultiLineAssignCuddle bool // AllowCuddleWithCalls is a list of call idents that everything can be // cuddled with. Defaults to calls looking like locks to support a flow like // this: + // // mu.Lock() // allow := thisAssignment AllowCuddleWithCalls []string @@ -47,6 +60,7 @@ type Configuration struct { // AllowCuddleWithRHS is a list of right hand side variables that is allowed // to be cuddled with anything. Defaults to assignments or calls looking // like unlocks to support a flow like this: + // // allow := thisAssignment() // mu.Unlock() AllowCuddleWithRHS []string @@ -55,14 +69,15 @@ type Configuration struct { // DefaultConfig returns default configuration func DefaultConfig() Configuration { return Configuration{ - CheckAppend: true, - AllowAssignAndCallsCuddle: true, - AllowMultiLineAssignmentCuddled: true, - AllowCuddleWithCalls: []string{"Lock", "RLock"}, - AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, + StrictAppend: true, + AllowAssignAndCallCuddle: true, + AllowMultiLineAssignCuddle: true, + AllowCuddleWithCalls: []string{"Lock", "RLock"}, + AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, } } +// Result represents the result of one error. type Result struct { FileName string LineNumber int @@ -70,6 +85,7 @@ type Result struct { Reason string } +// String returns the filename, line number and reason of a Result. func (r *Result) String() string { return fmt.Sprintf("%s:%d: %s", r.FileName, r.LineNumber, r.Reason) } @@ -207,7 +223,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // If previous assignment is multi line and we allow it, fetch // assignments (but only assignments). - if isMultiLineAssignment && p.config.AllowMultiLineAssignmentCuddled { + if isMultiLineAssignment && p.config.AllowMultiLineAssignCuddle { if _, ok := previousStatement.(*ast.AssignStmt); ok { assignedOnLineAbove = p.findLHS(previousStatement) } @@ -308,7 +324,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // append is usually an assignment but should not be allowed to be // cuddled with anything not appended. if len(rightHandSide) > 0 && rightHandSide[len(rightHandSide)-1] == "append" { - if p.config.CheckAppend { + if p.config.StrictAppend { if !atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightHandSide) { p.addError(t.Pos(), "append only allowed to cuddle with appended value") } @@ -322,12 +338,12 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } // If the assignment is from a type or variable called on the line - // above we can allow it by setting AllowAssignAndCallsCuddle to + // above we can allow it by setting AllowAssignAndCallCuddle to // true. // Example (x is used): // x.function() // a.Field = x.anotherFunction() - if p.config.AllowAssignAndCallsCuddle { + if p.config.AllowAssignAndCallCuddle { if atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightAndLeftHandSide) { continue } @@ -346,11 +362,11 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // If the expression is called on a type or variable used or // assigned on the line we can allow it by setting - // AllowAssignAndCallsCuddle to true. + // AllowAssignAndCallCuddle to true. // Example of allowed cuddled (x is used): // a.Field = x.func() // x.function() - if p.config.AllowAssignAndCallsCuddle { + if p.config.AllowAssignAndCallCuddle { if atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightAndLeftHandSide) { continue } diff --git a/wsl_test.go b/wsl_test.go index 7d1e42c..c37b903 100644 --- a/wsl_test.go +++ b/wsl_test.go @@ -948,7 +948,7 @@ func TestShouldAddEmptyLines(t *testing.T) { }`), }, { - description: "AllowAssignAndCallsCuddle", + description: "AllowAssignAndCallCuddle", code: []byte(`package main func main() { @@ -991,7 +991,7 @@ func TestWithConfig(t *testing.T) { customConfig *Configuration }{ { - description: "AllowAssignAndCallsCuddle", + description: "AllowAssignAndCallCuddle", code: []byte(`package main func main() { @@ -1001,7 +1001,7 @@ func TestWithConfig(t *testing.T) { d.Size = p.uint() }`), customConfig: &Configuration{ - AllowAssignAndCallsCuddle: true, + AllowAssignAndCallCuddle: true, }, }, { @@ -1020,7 +1020,7 @@ func TestWithConfig(t *testing.T) { } }`), customConfig: &Configuration{ - AllowMultiLineAssignmentCuddled: false, + AllowMultiLineAssignCuddle: false, }, expectedErrorStrings: []string{"if statements should only be cuddled with assignments"}, }, @@ -1049,10 +1049,30 @@ func TestWithConfig(t *testing.T) { } }`), customConfig: &Configuration{ - AllowMultiLineAssignmentCuddled: true, + AllowMultiLineAssignCuddle: true, }, expectedErrorStrings: []string{"if statements should only be cuddled with assignments used in the if statement itself"}, }, + { + description: "strict append", + code: []byte(`package main + + func main() { + x := []string{} + y := "not in x" + x = append(x, "not y") // Should not be allowed + + z := "in x" + x = append(x, z) // Should be allowed + + m := transform(z) + x := append(x, z) // Should be allowed + }`), + customConfig: &Configuration{ + StrictAppend: true, + }, + expectedErrorStrings: []string{"append only allowed to cuddle with appended value"}, + }, } for _, tc := range cases {