diff options
Diffstat (limited to 'vendor/github.com/bombsimon')
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/.gitignore (renamed from vendor/github.com/bombsimon/wsl/v3/.gitignore) | 0 | ||||
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/.golangci.yml (renamed from vendor/github.com/bombsimon/wsl/v3/.golangci.yml) | 3 | ||||
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/LICENSE (renamed from vendor/github.com/bombsimon/wsl/v3/LICENSE) | 0 | ||||
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/README.md (renamed from vendor/github.com/bombsimon/wsl/v3/README.md) | 84 | ||||
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/analyzer.go | 141 | ||||
| -rw-r--r-- | vendor/github.com/bombsimon/wsl/v4/wsl.go (renamed from vendor/github.com/bombsimon/wsl/v3/wsl.go) | 749 |
6 files changed, 620 insertions, 357 deletions
diff --git a/vendor/github.com/bombsimon/wsl/v3/.gitignore b/vendor/github.com/bombsimon/wsl/v4/.gitignore index 1c8eba613..1c8eba613 100644 --- a/vendor/github.com/bombsimon/wsl/v3/.gitignore +++ b/vendor/github.com/bombsimon/wsl/v4/.gitignore diff --git a/vendor/github.com/bombsimon/wsl/v3/.golangci.yml b/vendor/github.com/bombsimon/wsl/v4/.golangci.yml index 336ad4bc8..543012008 100644 --- a/vendor/github.com/bombsimon/wsl/v3/.golangci.yml +++ b/vendor/github.com/bombsimon/wsl/v4/.golangci.yml @@ -39,6 +39,7 @@ linters: disable: - cyclop - deadcode + - depguard - dupl - dupword - exhaustivestruct @@ -62,11 +63,13 @@ linters: - nosnakecase - paralleltest - prealloc + - rowserrcheck - scopelint - structcheck - testpackage - varcheck - varnamelen + - wastedassign fast: false diff --git a/vendor/github.com/bombsimon/wsl/v3/LICENSE b/vendor/github.com/bombsimon/wsl/v4/LICENSE index 4dade6d1c..4dade6d1c 100644 --- a/vendor/github.com/bombsimon/wsl/v3/LICENSE +++ b/vendor/github.com/bombsimon/wsl/v4/LICENSE diff --git a/vendor/github.com/bombsimon/wsl/v3/README.md b/vendor/github.com/bombsimon/wsl/v4/README.md index 8ff74392b..0bcf01d96 100644 --- a/vendor/github.com/bombsimon/wsl/v3/README.md +++ b/vendor/github.com/bombsimon/wsl/v4/README.md @@ -1,4 +1,4 @@ -# WSL - Whitespace Linter +# wsl - Whitespace Linter [](https://forthebadge.com) [](https://forthebadge.com) @@ -6,83 +6,58 @@ [](https://github.com/bombsimon/wsl/actions/workflows/go.yml) [](https://coveralls.io/github/bombsimon/wsl?branch=master) -WSL is a linter that enforces a very **non scientific** vision of how to make +`wsl` is a linter that enforces a very **non scientific** vision of how to make code more readable by enforcing empty lines at the right places. -I think too much code out there is to cuddly and a bit too warm for it's own -good, making it harder for other people to read and understand. The linter will -warn about newlines in and around blocks, in the beginning of files and other -places in the code. - -**I know this linter is aggressive** and a lot of projects I've tested it on -have failed miserably. For this linter to be useful at all I want to be open to -new ideas, configurations and discussions! Also note that some of the warnings -might be bugs or unintentional false positives so I would love an +**This linter is aggressive** and a lot of projects I've tested it on have +failed miserably. For this linter to be useful at all I want to be open to new +ideas, configurations and discussions! Also note that some of the warnings might +be bugs or unintentional false positives so I would love an [issue](https://github.com/bombsimon/wsl/issues/new) to fix, discuss, change or make something configurable! ## Installation -### By `go get` (local installation) - -You can do that by using: - ```sh -go get -u github.com/bombsimon/wsl/v3/cmd/... -``` - -### By golangci-lint (CI automation) +# Latest release +go install github.com/bombsimon/wsl/v4/cmd/wsl -`wsl` is already integrated with -[golangci-lint](https://github.com/golangci/golangci-lint). Please refer to the -instructions there. +# Main branch +go install github.com/bombsimon/wsl/v4/cmd/wsl@master +``` ## Usage -How to use depends on how you install `wsl`. +> **Note**: This linter provides a fixer that can fix most issues with the +> `--fix` flag. However, currently `golangci-lint` [does not support suggested +> fixes](https://github.com/golangci/golangci-lint/issues/1779) so the `--fix` +> flag in `golangci-lint` will **not** work. -### With local binary - -The general command format for `wsl` is: +`wsl` uses the [analysis](https://pkg.go.dev/golang.org/x/tools/go/analysis) +package meaning it will operate on package level with the default analysis flags +and way of working. ```sh -$ wsl [flags] <file1> [files...] -$ wsl [flags] </path/to/package/...> - -# Examples +wsl --help +wsl [flags] </path/to/package/...> -$ wsl ./main.go -$ wsl --no-test ./main.go -$ wsl --allow-cuddle-declarations ./main.go -$ wsl --no-test --allow-cuddle-declaration ./main.go -$ wsl --no-test --allow-trailing-comment ./myProject/... +wsl --allow-cuddle-declarations --fix ./... ``` -The "..." wildcard is not used like other `go` commands but instead can only -be to a relative or absolute path. - -By default, the linter will run on `./...` which means all go files in the -current path and all subsequent paths, including test files. To disable linting -test files, use `-n` or `--no-test`. - -### By `golangci-lint` (CI automation) - -The recommended command is: +`wsl` is also integrated in [`golangci-lint`](https://golangci-lint.run) ```sh -golangci-lint run --disable-all --enable wsl +golangci-lint run --no-config --disable-all --enable wsl ``` -For more information, please refer to -[golangci-lint](https://github.com/golangci/golangci-lint)'s documentation. - ## Issues and configuration The linter suppers a few ways to configure it to satisfy more than one kind of code style. These settings could be set either with flags or with YAML configuration if used via `golangci-lint`. -The supported configuration can be found [in the documentation](doc/configuration.md). +The supported configuration can be found [in the +documentation](doc/configuration.md). Below are the available checklist for any hit from `wsl`. If you do not see any, feel free to raise an [issue](https://github.com/bombsimon/wsl/issues/new). @@ -106,14 +81,11 @@ feel free to raise an [issue](https://github.com/bombsimon/wsl/issues/new). * [For statements should only be cuddled with assignments used in the iteration](doc/rules.md#for-statements-should-only-be-cuddled-with-assignments-used-in-the-iteration) * [Go statements can only invoke functions assigned on line above](doc/rules.md#go-statements-can-only-invoke-functions-assigned-on-line-above) * [If statements should only be cuddled with assignments](doc/rules.md#if-statements-should-only-be-cuddled-with-assignments) -* [If statements should only be cuddled with assignments used in the if - statement - itself](doc/rules.md#if-statements-should-only-be-cuddled-with-assignments-used-in-the-if-statement-itself) +* [If statements should only be cuddled with assignments used in the if statement itself](doc/rules.md#if-statements-should-only-be-cuddled-with-assignments-used-in-the-if-statement-itself) * [If statements that check an error must be cuddled with the statement that assigned the error](doc/rules.md#if-statements-that-check-an-error-must-be-cuddled-with-the-statement-that-assigned-the-error) -* [Only cuddled expressions if assigning variable or using from line - above](doc/rules.md#only-cuddled-expressions-if-assigning-variable-or-using-from-line-above) +* [Only cuddled expressions if assigning variable or using from line above](doc/rules.md#only-cuddled-expressions-if-assigning-variable-or-using-from-line-above) * [Only one cuddle assignment allowed before defer statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-defer-statement) -* [Only one cuddle assginment allowed before for statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-for-statement) +* [Only one cuddle assignment allowed before for statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-for-statement) * [Only one cuddle assignment allowed before go statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-go-statement) * [Only one cuddle assignment allowed before if statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-if-statement) * [Only one cuddle assignment allowed before range statement](doc/rules.md#only-one-cuddle-assignment-allowed-before-range-statement) diff --git a/vendor/github.com/bombsimon/wsl/v4/analyzer.go b/vendor/github.com/bombsimon/wsl/v4/analyzer.go new file mode 100644 index 000000000..b8eac1587 --- /dev/null +++ b/vendor/github.com/bombsimon/wsl/v4/analyzer.go @@ -0,0 +1,141 @@ +package wsl + +import ( + "flag" + "strings" + + "golang.org/x/tools/go/analysis" +) + +func NewAnalyzer(config *Configuration) *analysis.Analyzer { + wa := &wslAnalyzer{config: config} + + return &analysis.Analyzer{ + Name: "wsl", + Doc: "add or remove empty lines", + Flags: wa.flags(), + Run: wa.run, + RunDespiteErrors: true, + } +} + +func defaultConfig() *Configuration { + return &Configuration{ + AllowAssignAndAnythingCuddle: false, + AllowAssignAndCallCuddle: true, + AllowCuddleDeclaration: false, + AllowMultiLineAssignCuddle: true, + AllowSeparatedLeadingComment: false, + AllowTrailingComment: false, + ForceCuddleErrCheckAndAssign: false, + ForceExclusiveShortDeclarations: false, + StrictAppend: true, + AllowCuddleWithCalls: []string{"Lock", "RLock"}, + AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, + ErrorVariableNames: []string{"err"}, + ForceCaseTrailingWhitespaceLimit: 0, + } +} + +// wslAnalyzer is a wrapper around the configuration which is used to be able to +// set the configuration when creating the analyzer and later be able to update +// flags and running method. +type wslAnalyzer struct { + config *Configuration +} + +func (wa *wslAnalyzer) flags() flag.FlagSet { + flags := flag.NewFlagSet("", flag.ExitOnError) + + // If we have a configuration set we're not running from the command line so + // we don't use any flags. + if wa.config != nil { + return *flags + } + + wa.config = defaultConfig() + + flags.BoolVar(&wa.config.AllowAssignAndAnythingCuddle, "allow-assign-and-anything", false, "Allow assignments and anything to be cuddled") + flags.BoolVar(&wa.config.AllowAssignAndCallCuddle, "allow-assign-and-call", true, "Allow assignments and calls to be cuddled (if using same variable/type)") + flags.BoolVar(&wa.config.AllowCuddleDeclaration, "allow-cuddle-declarations", false, "Allow declarations to be cuddled") + flags.BoolVar(&wa.config.AllowMultiLineAssignCuddle, "allow-multi-line-assign", true, "Allow cuddling with multi line assignments") + flags.BoolVar(&wa.config.AllowSeparatedLeadingComment, "allow-separated-leading-comment", false, "Allow empty newlines in leading comments") + flags.BoolVar(&wa.config.AllowTrailingComment, "allow-trailing-comment", false, "Allow blocks to end with a comment") + flags.BoolVar(&wa.config.ForceCuddleErrCheckAndAssign, "force-err-cuddling", false, "Force cuddling of error checks with error var assignment") + flags.BoolVar(&wa.config.ForceExclusiveShortDeclarations, "force-short-decl-cuddling", false, "Force short declarations to cuddle by themselves") + flags.BoolVar(&wa.config.StrictAppend, "strict-append", true, "Strict rules for append") + flags.IntVar(&wa.config.ForceCaseTrailingWhitespaceLimit, "force-case-trailing-whitespace", 0, "Force newlines for case blocks > this number.") + + flags.Var(&multiStringValue{slicePtr: &wa.config.AllowCuddleWithCalls}, "allow-cuddle-with-calls", "Comma separated list of idents that can have cuddles after") + flags.Var(&multiStringValue{slicePtr: &wa.config.AllowCuddleWithRHS}, "allow-cuddle-with-rhs", "Comma separated list of idents that can have cuddles before") + flags.Var(&multiStringValue{slicePtr: &wa.config.ErrorVariableNames}, "error-variable-names", "Comma separated list of error variable names") + + return *flags +} + +func (wa *wslAnalyzer) run(pass *analysis.Pass) (interface{}, error) { + for _, file := range pass.Files { + filename := pass.Fset.PositionFor(file.Pos(), false).Filename + if !strings.HasSuffix(filename, ".go") { + continue + } + + processor := newProcessorWithConfig(file, pass.Fset, wa.config) + processor.parseAST() + + for pos, fix := range processor.result { + textEdits := []analysis.TextEdit{} + for _, f := range fix.fixRanges { + textEdits = append(textEdits, analysis.TextEdit{ + Pos: f.fixRangeStart, + End: f.fixRangeEnd, + NewText: []byte("\n"), + }) + } + + pass.Report(analysis.Diagnostic{ + Pos: pos, + Category: "whitespace", + Message: fix.reason, + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: textEdits, + }, + }, + }) + } + } + + //nolint:nilnil // A pass don't need to return anything. + return nil, nil +} + +// multiStringValue is a flag that supports multiple values. It's implemented to +// contain a pointer to a string slice that will be overwritten when the flag's +// `Set` method is called. +type multiStringValue struct { + slicePtr *[]string +} + +// Set implements the flag.Value interface and will overwrite the pointer to the +// slice with a new pointer after splitting the flag by comma. +func (m *multiStringValue) Set(value string) error { + s := []string{} + + for _, v := range strings.Split(value, ",") { + s = append(s, strings.TrimSpace(v)) + } + + *m.slicePtr = s + + return nil +} + +// Set implements the flag.Value interface. +func (m *multiStringValue) String() string { + if m.slicePtr == nil { + return "" + } + + return strings.Join(*m.slicePtr, ", ") +} diff --git a/vendor/github.com/bombsimon/wsl/v3/wsl.go b/vendor/github.com/bombsimon/wsl/v4/wsl.go index 1b139c047..6fd33335a 100644 --- a/vendor/github.com/bombsimon/wsl/v3/wsl.go +++ b/vendor/github.com/bombsimon/wsl/v4/wsl.go @@ -3,45 +3,44 @@ package wsl import ( "fmt" "go/ast" - "go/parser" "go/token" - "os" "reflect" + "sort" "strings" ) // Error reason strings. const ( - reasonMustCuddleErrCheck = "if statements that check an error must be cuddled with the statement that assigned the error" - reasonOnlyCuddleIfWithAssign = "if statements should only be cuddled with assignments" - reasonOnlyOneCuddle = "only one cuddle assignment allowed before if statement" - reasonOnlyCuddleWithUsedAssign = "if statements should only be cuddled with assignments used in the if statement itself" - reasonOnlyCuddle2LineReturn = "return statements should not be cuddled if block has more than two lines" - reasonMultiLineBranchCuddle = "branch statements should not be cuddled if block has more than two lines" + reasonAnonSwitchCuddled = "anonymous switch statements should never be cuddled" reasonAppendCuddledWithoutUse = "append only allowed to cuddle with appended value" reasonAssignsCuddleAssign = "assignments should only be cuddled with other assignments" - reasonNeverCuddleDeclare = "declarations should never be cuddled" - reasonExpressionCuddledWithDeclOrRet = "expressions should not be cuddled with declarations or returns" - reasonExpressionCuddledWithBlock = "expressions should not be cuddled with blocks" - reasonExprCuddlingNonAssignedVar = "only cuddled expressions if assigning variable or using from line above" - reasonOneCuddleBeforeRange = "only one cuddle assignment allowed before range statement" - reasonRangeCuddledWithoutUse = "ranges should only be cuddled with assignments used in the iteration" - reasonOneCuddleBeforeDefer = "only one cuddle assignment allowed before defer statement" + reasonBlockEndsWithWS = "block should not end with a whitespace (or comment)" + reasonBlockStartsWithWS = "block should not start with a whitespace" + reasonCaseBlockTooCuddly = "case block should end with newline at this size" reasonDeferCuddledWithOtherVar = "defer statements should only be cuddled with expressions on same variable" - reasonForWithoutCondition = "for statement without condition should never be cuddled" - reasonForWithMoreThanOneCuddle = "only one cuddle assignment allowed before for statement" + reasonExprCuddlingNonAssignedVar = "only cuddled expressions if assigning variable or using from line above" + reasonExpressionCuddledWithBlock = "expressions should not be cuddled with blocks" + reasonExpressionCuddledWithDeclOrRet = "expressions should not be cuddled with declarations or returns" reasonForCuddledAssignWithoutUse = "for statements should only be cuddled with assignments used in the iteration" - reasonOneCuddleBeforeGo = "only one cuddle assignment allowed before go statement" + reasonForWithoutCondition = "for statement without condition should never be cuddled" reasonGoFuncWithoutAssign = "go statements can only invoke functions assigned on line above" - reasonSwitchManyCuddles = "only one cuddle assignment allowed before switch statement" - reasonAnonSwitchCuddled = "anonymous switch statements should never be cuddled" + reasonMultiLineBranchCuddle = "branch statements should not be cuddled if block has more than two lines" + reasonMustCuddleErrCheck = "if statements that check an error must be cuddled with the statement that assigned the error" + reasonNeverCuddleDeclare = "declarations should never be cuddled" + reasonOnlyCuddle2LineReturn = "return statements should not be cuddled if block has more than two lines" + reasonOnlyCuddleIfWithAssign = "if statements should only be cuddled with assignments" + reasonOnlyCuddleWithUsedAssign = "if statements should only be cuddled with assignments used in the if statement itself" + reasonOnlyOneCuddleBeforeDefer = "only one cuddle assignment allowed before defer statement" + reasonOnlyOneCuddleBeforeFor = "only one cuddle assignment allowed before for statement" + reasonOnlyOneCuddleBeforeGo = "only one cuddle assignment allowed before go statement" + reasonOnlyOneCuddleBeforeIf = "only one cuddle assignment allowed before if statement" + reasonOnlyOneCuddleBeforeRange = "only one cuddle assignment allowed before range statement" + reasonOnlyOneCuddleBeforeSwitch = "only one cuddle assignment allowed before switch statement" + reasonOnlyOneCuddleBeforeTypeSwitch = "only one cuddle assignment allowed before type switch statement" + reasonRangeCuddledWithoutUse = "ranges should only be cuddled with assignments used in the iteration" + reasonShortDeclNotExclusive = "short declaration should cuddle only with other short declarations" reasonSwitchCuddledWithoutUse = "switch statements should only be cuddled with variables switched" - reasonTypeSwitchTooCuddled = "only one cuddle assignment allowed before type switch statement" reasonTypeSwitchCuddledWithoutUse = "type switch statements should only be cuddled with variables switched" - reasonBlockStartsWithWS = "block should not start with a whitespace" - reasonBlockEndsWithWS = "block should not end with a whitespace (or comment)" - reasonCaseBlockTooCuddly = "case block should end with newline at this size" - reasonShortDeclNotExclusive = "short declaration should cuddle only with other short declarations" ) // Warning strings. @@ -54,6 +53,7 @@ const ( warnUnknownRHS = "UNKNOWN RHS" ) +// Configuration represents configurable settings for the linter. type Configuration struct { // 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 @@ -82,7 +82,7 @@ type Configuration struct { // x = AnotherAssign() AllowAssignAndCallCuddle bool - // AllowAssignAndCallCuddle allows assignments to be cuddled with anything. + // AllowAssignAndAnythingCuddle allows assignments to be cuddled with anything. // Example supported with this set to true: // if x == 1 { // x = 0 @@ -176,94 +176,40 @@ type Configuration struct { ForceExclusiveShortDeclarations bool } -// DefaultConfig returns default configuration. -func DefaultConfig() Configuration { - return Configuration{ - StrictAppend: true, - AllowAssignAndCallCuddle: true, - AllowAssignAndAnythingCuddle: false, - AllowMultiLineAssignCuddle: true, - AllowTrailingComment: false, - AllowSeparatedLeadingComment: false, - ForceCuddleErrCheckAndAssign: false, - ForceExclusiveShortDeclarations: false, - ForceCaseTrailingWhitespaceLimit: 0, - AllowCuddleWithCalls: []string{"Lock", "RLock"}, - AllowCuddleWithRHS: []string{"Unlock", "RUnlock"}, - ErrorVariableNames: []string{"err"}, - } +// fix is a range to fixup. +type fix struct { + fixRangeStart token.Pos + fixRangeEnd token.Pos } -// Result represents the result of one error. -type Result struct { - FileName string - LineNumber int - Position token.Position - Reason string +// result represents the result of one error. +type result struct { + fixRanges []fix + 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) -} - -type Processor struct { - config Configuration - result []Result - warnings []string - fileSet *token.FileSet +// processor is the type that keeps track of the file and fileset and holds the +// results from parsing the AST. +type processor struct { + config *Configuration file *ast.File + fileSet *token.FileSet + result map[token.Pos]result + warnings []string } -// NewProcessor will create a Processor. -// -//nolint:gocritic // It's fine to copy config struct -func NewProcessorWithConfig(cfg Configuration) *Processor { - return &Processor{ - result: []Result{}, - config: cfg, - } -} - -// NewProcessor will create a Processor. -func NewProcessor() *Processor { - return NewProcessorWithConfig(DefaultConfig()) -} - -// ProcessFiles takes a string slice with file names (full paths) and lints -// them. -// -//nolint:gocritic // Don't want named returns -func (p *Processor) ProcessFiles(filenames []string) ([]Result, []string) { - for _, filename := range filenames { - data, err := os.ReadFile(filename) - if err != nil { - panic(err) - } - - p.process(filename, data) +// newProcessorWithConfig will create a Processor with the passed configuration. +func newProcessorWithConfig(file *ast.File, fileSet *token.FileSet, cfg *Configuration) *processor { + return &processor{ + config: cfg, + file: file, + fileSet: fileSet, + result: make(map[token.Pos]result), } - - return p.result, p.warnings } -func (p *Processor) process(filename string, data []byte) { - fileSet := token.NewFileSet() - file, err := parser.ParseFile(fileSet, filename, data, parser.ParseComments) - // If the file is not parsable let's add a syntax error and move on. - if err != nil { - p.result = append(p.result, Result{ - FileName: filename, - LineNumber: 0, - Reason: fmt.Sprintf("invalid syntax, file cannot be linted (%s)", err.Error()), - }) - - return - } - - p.fileSet = fileSet - p.file = file - +// parseAST will parse the AST attached to the Processor instance. +func (p *processor) parseAST() { for _, d := range p.file.Decls { switch v := d.(type) { case *ast.FuncDecl: @@ -279,7 +225,7 @@ func (p *Processor) process(filename string, data []byte) { // parseBlockBody will parse any kind of block statements such as switch cases // and if statements. A list of Result is returned. -func (p *Processor) parseBlockBody(ident *ast.Ident, block *ast.BlockStmt) { +func (p *processor) parseBlockBody(ident *ast.Ident, block *ast.BlockStmt) { // Nothing to do if there's no value. if reflect.ValueOf(block).IsNil() { return @@ -294,7 +240,7 @@ func (p *Processor) parseBlockBody(ident *ast.Ident, block *ast.BlockStmt) { // parseBlockStatements will parse all the statements found in the body of a // node. A list of Result is returned. -func (p *Processor) parseBlockStatements(statements []ast.Stmt) { +func (p *processor) parseBlockStatements(statements []ast.Stmt) { for i, stmt := range statements { // Start by checking if this statement is another block (other than if, // for and range). This could be assignment to a function, defer or go @@ -380,14 +326,38 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - moreThanOneStatementAbove := func() bool { - if i < 2 { + nStatementsBefore := func(n int) bool { + if i < n { return false } - statementBeforePreviousStatement := statements[i-2] + for j := 1; j < n; j++ { + s1 := statements[i-j] + s2 := statements[i-(j+1)] + + if p.nodeStart(s1)-1 != p.nodeEnd(s2) { + return false + } + } - return p.nodeStart(previousStatement)-1 == p.nodeEnd(statementBeforePreviousStatement) + return true + } + + nStatementsAfter := func(n int) bool { + if len(statements)-1 < i+n { + return false + } + + for j := 0; j < n; j++ { + s1 := statements[i+j] + s2 := statements[i+j+1] + + if p.nodeEnd(s1)+1 != p.nodeStart(s2) { + return false + } + } + + return true } isLastStatementInBlockOfOnlyTwoLines := func() bool { @@ -413,9 +383,30 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // it was and use *that* statement's position if p.config.ForceExclusiveShortDeclarations && cuddledWithLastStmt { if p.isShortDecl(stmt) && !p.isShortDecl(previousStatement) { - p.addError(stmt.Pos(), reasonShortDeclNotExclusive) + var reportNode ast.Node = previousStatement + + cm := ast.NewCommentMap(p.fileSet, stmt, p.file.Comments) + if cg, ok := cm[stmt]; ok && len(cg) > 0 { + for _, c := range cg { + if c.Pos() > previousStatement.End() && c.End() < stmt.Pos() { + reportNode = c + } + } + } + + p.addErrorRange( + stmt.Pos(), + reportNode.End(), + reportNode.End(), + reasonShortDeclNotExclusive, + ) } else if p.isShortDecl(previousStatement) && !p.isShortDecl(stmt) { - p.addError(previousStatement.Pos(), reasonShortDeclNotExclusive) + p.addErrorRange( + previousStatement.Pos(), + stmt.Pos(), + stmt.Pos(), + reasonShortDeclNotExclusive, + ) } } @@ -428,6 +419,31 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } } + reportNewlineTwoLinesAbove := func(n1, n2 ast.Node, reason string) { + if atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) || + atLeastOneInListsMatch(assignedOnLineAbove, calledOrAssignedFirstInBlock) { + // If both the assignment on the line above _and_ the assignment + // two lines above is part of line or first in block, add the + // newline as if non were. + _, isAssignmentTwoLinesAbove := statements[i-2].(*ast.AssignStmt) + assignedTwoLinesAbove := p.findLHS(statements[i-2]) + + if isAssignmentTwoLinesAbove && + (atLeastOneInListsMatch(rightAndLeftHandSide, assignedTwoLinesAbove) || + atLeastOneInListsMatch(assignedTwoLinesAbove, calledOrAssignedFirstInBlock)) { + p.addWhitespaceBeforeError(n1, reason) + } else { + // If the variable on the line above is allowed to be + // cuddled, break two lines above so we keep the proper + // cuddling. + p.addErrorRange(n1.Pos(), n2.Pos(), n2.Pos(), reason) + } + } else { + // If not, break here so we separate the cuddled variable. + p.addWhitespaceBeforeError(n1, reason) + } + } + switch t := stmt.(type) { case *ast.IfStmt: checkingErrInitializedInline := func() bool { @@ -460,7 +476,12 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } if atLeastOneInListsMatch(assignedOnLineAbove, p.config.ErrorVariableNames) { - p.addError(t.Pos(), reasonMustCuddleErrCheck) + p.addErrorRange( + stmt.Pos(), + previousStatement.End(), + stmt.Pos(), + reasonMustCuddleErrCheck, + ) } } @@ -468,12 +489,12 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } if len(assignedOnLineAbove) == 0 { - p.addError(t.Pos(), reasonOnlyCuddleIfWithAssign) + p.addWhitespaceBeforeError(t, reasonOnlyCuddleIfWithAssign) continue } - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonOnlyOneCuddle) + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeIf) continue } @@ -485,33 +506,34 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - p.addError(t.Pos(), reasonOnlyCuddleWithUsedAssign) + p.addWhitespaceBeforeError(t, reasonOnlyCuddleWithUsedAssign) case *ast.ReturnStmt: if isLastStatementInBlockOfOnlyTwoLines() { continue } - p.addError(t.Pos(), reasonOnlyCuddle2LineReturn) + p.addWhitespaceBeforeError(t, reasonOnlyCuddle2LineReturn) case *ast.BranchStmt: if isLastStatementInBlockOfOnlyTwoLines() { continue } - p.addError(t.Pos(), reasonMultiLineBranchCuddle) + p.addWhitespaceBeforeError(t, reasonMultiLineBranchCuddle) case *ast.AssignStmt: // 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.StrictAppend { if !atLeastOneInListsMatch(calledOrAssignedOnLineAbove, rightHandSide) { - p.addError(t.Pos(), reasonAppendCuddledWithoutUse) + p.addWhitespaceBeforeError(t, reasonAppendCuddledWithoutUse) } } continue } - if _, ok := previousStatement.(*ast.AssignStmt); ok { + switch previousStatement.(type) { + case *ast.AssignStmt, *ast.IncDecStmt: continue } @@ -535,10 +557,18 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } } - p.addError(t.Pos(), reasonAssignsCuddleAssign) + p.addWhitespaceBeforeError(t, reasonAssignsCuddleAssign) + case *ast.IncDecStmt: + switch previousStatement.(type) { + case *ast.AssignStmt, *ast.IncDecStmt: + continue + } + + p.addWhitespaceBeforeError(t, reasonAssignsCuddleAssign) + case *ast.DeclStmt: if !p.config.AllowCuddleDeclaration { - p.addError(t.Pos(), reasonNeverCuddleDeclare) + p.addWhitespaceBeforeError(t, reasonNeverCuddleDeclare) } case *ast.ExprStmt: switch previousStatement.(type) { @@ -547,9 +577,9 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - p.addError(t.Pos(), reasonExpressionCuddledWithDeclOrRet) + p.addWhitespaceBeforeError(t, reasonExpressionCuddledWithDeclOrRet) case *ast.IfStmt, *ast.RangeStmt, *ast.SwitchStmt: - p.addError(t.Pos(), reasonExpressionCuddledWithBlock) + p.addWhitespaceBeforeError(t, reasonExpressionCuddledWithBlock) } // If the expression is called on a type or variable used or @@ -567,17 +597,17 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // If we assigned variables on the line above but didn't use them in // this expression there should probably be a newline between them. if len(assignedOnLineAbove) > 0 && !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { - p.addError(t.Pos(), reasonExprCuddlingNonAssignedVar) + p.addWhitespaceBeforeError(t, reasonExprCuddlingNonAssignedVar) } case *ast.RangeStmt: - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonOneCuddleBeforeRange) + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeRange) continue } if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { if !atLeastOneInListsMatch(assignedOnLineAbove, calledOrAssignedFirstInBlock) { - p.addError(t.Pos(), reasonRangeCuddledWithoutUse) + p.addWhitespaceBeforeError(t, reasonRangeCuddledWithoutUse) } } case *ast.DeferStmt: @@ -586,27 +616,38 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - // Special treatment of deferring body closes after error checking - // according to best practices. See - // https://github.com/bombsimon/wsl/issues/31 which links to - // discussion about error handling after HTTP requests. This is hard - // coded and very specific but for now this is to be seen as a - // special case. What this does is that it *only* allows a defer - // statement with `Close` on the right hand side to be cuddled with - // an if-statement to support this: - // resp, err := client.Do(req) - // if err != nil { - // return err - // } - // defer resp.Body.Close() - if _, ok := previousStatement.(*ast.IfStmt); ok { - if atLeastOneInListsMatch(rightHandSide, []string{"Close"}) { - continue + if nStatementsBefore(2) { + // We allow cuddling defer if the defer references something + // used two lines above. + // There are several reasons to why we do this. + // Originally there was a special use case only for "Close" + // + // https://github.com/bombsimon/wsl/issues/31 which links to + // resp, err := client.Do(req) + // if err != nil { + // return err + // } + // defer resp.Body.Close() + // + // After a discussion in a followup issue it makes sense to not + // only hard code `Close` but for anything that's referenced two + // statements above. + // + // https://github.com/bombsimon/wsl/issues/85 + // db, err := OpenDB() + // require.NoError(t, err) + // defer db.Close() + // + // All of this is only allowed if there's exactly three cuddled + // statements, otherwise the regular rules apply. + if !nStatementsBefore(3) && !nStatementsAfter(1) { + variablesTwoLinesAbove := append(p.findLHS(statements[i-2]), p.findRHS(statements[i-2])...) + if atLeastOneInListsMatch(rightHandSide, variablesTwoLinesAbove) { + continue + } } - } - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonOneCuddleBeforeDefer) + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeDefer) continue } @@ -620,7 +661,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } // Allow use to cuddled defer func literals with usages on line - // abouve. Example: + // above. Example: // b := getB() // defer func() { // makesSenseToUse(b) @@ -638,18 +679,16 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { - p.addError(t.Pos(), reasonDeferCuddledWithOtherVar) + p.addWhitespaceBeforeError(t, reasonDeferCuddledWithOtherVar) } case *ast.ForStmt: if len(rightAndLeftHandSide) == 0 { - p.addError(t.Pos(), reasonForWithoutCondition) - + p.addWhitespaceBeforeError(t, reasonForWithoutCondition) continue } - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonForWithMoreThanOneCuddle) - + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeFor) continue } @@ -658,7 +697,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // first line in the block for details. if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { if !atLeastOneInListsMatch(assignedOnLineAbove, calledOrAssignedFirstInBlock) { - p.addError(t.Pos(), reasonForCuddledAssignWithoutUse) + p.addWhitespaceBeforeError(t, reasonForCuddledAssignWithoutUse) } } case *ast.GoStmt: @@ -666,9 +705,8 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { continue } - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonOneCuddleBeforeGo) - + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeGo) continue } @@ -689,26 +727,24 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { } if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { - p.addError(t.Pos(), reasonGoFuncWithoutAssign) + p.addWhitespaceBeforeError(t, reasonGoFuncWithoutAssign) } case *ast.SwitchStmt: - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonSwitchManyCuddles) - + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeSwitch) continue } if !atLeastOneInListsMatch(rightAndLeftHandSide, assignedOnLineAbove) { if len(rightAndLeftHandSide) == 0 { - p.addError(t.Pos(), reasonAnonSwitchCuddled) + p.addWhitespaceBeforeError(t, reasonAnonSwitchCuddled) } else { - p.addError(t.Pos(), reasonSwitchCuddledWithoutUse) + p.addWhitespaceBeforeError(t, reasonSwitchCuddledWithoutUse) } } case *ast.TypeSwitchStmt: - if moreThanOneStatementAbove() { - p.addError(t.Pos(), reasonTypeSwitchTooCuddled) - + if nStatementsBefore(2) { + reportNewlineTwoLinesAbove(t, statements[i-1], reasonOnlyOneCuddleBeforeTypeSwitch) continue } @@ -717,11 +753,11 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // Allow type assertion on variables used in the first case // immediately. if !atLeastOneInListsMatch(assignedOnLineAbove, calledOrAssignedFirstInBlock) { - p.addError(t.Pos(), reasonTypeSwitchCuddledWithoutUse) + p.addWhitespaceBeforeError(t, reasonTypeSwitchCuddledWithoutUse) } } case *ast.CaseClause, *ast.CommClause: - // Case clauses will be checked by not allowing leading ot trailing + // Case clauses will be checked by not allowing leading of trailing // whitespaces within the block. There's nothing in the case itself // that may be cuddled. default: @@ -735,7 +771,7 @@ func (p *Processor) parseBlockStatements(statements []ast.Stmt) { // directly as the first argument inside a body. // The body will then be parsed as a *ast.BlockStmt (regular block) or as a list // of []ast.Stmt (case block). -func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node { +func (p *processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node { stmt := allStmt[i] // Start by checking if the statement has a body (probably if-statement, @@ -766,7 +802,14 @@ func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node { } } - p.parseBlockBody(nil, statementBodyContent) + // If statement bodies will be parsed already when finding block bodies. + // The reason is because if/else-if/else chains is nested in the AST + // where the else bit is a part of the if statement. Since if statements + // is the only statement that can be chained like this we exclude it + // from parsing it again here. + if _, ok := stmt.(*ast.IfStmt); !ok { + p.parseBlockBody(nil, statementBodyContent) + } case []ast.Stmt: // The Body field for an *ast.CaseClause or *ast.CommClause is of type // []ast.Stmt. We must check leading and trailing whitespaces and then @@ -791,7 +834,7 @@ func (p *Processor) firstBodyStatement(i int, allStmt []ast.Stmt) ast.Node { return firstBodyStatement } -func (p *Processor) findLHS(node ast.Node) []string { +func (p *processor) findLHS(node ast.Node) []string { var lhs []string if node == nil { @@ -850,7 +893,7 @@ func (p *Processor) findLHS(node ast.Node) []string { return lhs } -func (p *Processor) findRHS(node ast.Node) []string { +func (p *processor) findRHS(node ast.Node) []string { var rhs []string if node == nil { @@ -934,7 +977,7 @@ func (p *Processor) findRHS(node ast.Node) []string { return rhs } -func (p *Processor) isShortDecl(node ast.Node) bool { +func (p *processor) isShortDecl(node ast.Node) bool { if t, ok := node.(*ast.AssignStmt); ok { return t.Tok == token.DEFINE } @@ -942,16 +985,22 @@ func (p *Processor) isShortDecl(node ast.Node) bool { return false } -func (p *Processor) findBlockStmt(node ast.Node) []*ast.BlockStmt { +func (p *processor) findBlockStmt(node ast.Node) []*ast.BlockStmt { var blocks []*ast.BlockStmt switch t := node.(type) { + case *ast.BlockStmt: + return []*ast.BlockStmt{t} case *ast.AssignStmt: for _, x := range t.Rhs { blocks = append(blocks, p.findBlockStmt(x)...) } case *ast.CallExpr: blocks = append(blocks, p.findBlockStmt(t.Fun)...) + + for _, x := range t.Args { + blocks = append(blocks, p.findBlockStmt(x)...) + } case *ast.FuncLit: blocks = append(blocks, t.Body) case *ast.ExprStmt: @@ -964,6 +1013,8 @@ func (p *Processor) findBlockStmt(node ast.Node) []*ast.BlockStmt { blocks = append(blocks, p.findBlockStmt(t.Call)...) case *ast.GoStmt: blocks = append(blocks, p.findBlockStmt(t.Call)...) + case *ast.IfStmt: + blocks = append([]*ast.BlockStmt{t.Body}, p.findBlockStmt(t.Else)...) } return blocks @@ -1020,15 +1071,15 @@ func atLeastOneInListsMatch(listOne, listTwo []string) bool { // findLeadingAndTrailingWhitespaces will find leading and trailing whitespaces // in a node. The method takes comments in consideration which will make the // parser more gentle. -func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, nextStatement ast.Node) { +func (p *processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, nextStatement ast.Node) { var ( - allowedLinesBeforeFirstStatement = 1 - commentMap = ast.NewCommentMap(p.fileSet, stmt, p.file.Comments) - blockStatements []ast.Stmt - blockStartLine int - blockEndLine int - blockStartPos token.Pos - blockEndPos token.Pos + commentMap = ast.NewCommentMap(p.fileSet, stmt, p.file.Comments) + blockStatements []ast.Stmt + blockStartLine int + blockEndLine int + blockStartPos token.Pos + blockEndPos token.Pos + isCase bool ) // Depending on the block type, get the statements in the block and where @@ -1041,9 +1092,11 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne case *ast.CaseClause: blockStatements = t.Body blockStartPos = t.Colon + isCase = true case *ast.CommClause: blockStatements = t.Body blockStartPos = t.Colon + isCase = true default: p.addWarning(warnWSNodeTypeNotImplemented, stmt.Pos(), stmt) @@ -1055,8 +1108,8 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne return } - blockStartLine = p.fileSet.Position(blockStartPos).Line - blockEndLine = p.fileSet.Position(blockEndPos).Line + blockStartLine = p.fileSet.PositionFor(blockStartPos, false).Line + blockEndLine = p.fileSet.PositionFor(blockEndPos, false).Line // No whitespace possible if LBrace and RBrace is on the same line. if blockStartLine == blockEndLine { @@ -1064,152 +1117,238 @@ func (p *Processor) findLeadingAndTrailingWhitespaces(ident *ast.Ident, stmt, ne } var ( - firstStatement = blockStatements[0] - lastStatement = blockStatements[len(blockStatements)-1] - seenCommentGroups = 0 + firstStatement = blockStatements[0] + lastStatement = blockStatements[len(blockStatements)-1] ) - // Get the comment related to the first statement, we do allow commends in + // Get the comment related to the first statement, we do allow comments in // the beginning of a block before the first statement. - if c, ok := commentMap[firstStatement]; ok { - for _, commentGroup := range c { - // If the comment group is on the same line as the block start - // (LBrace) we should not consider it. - if p.nodeStart(commentGroup) == blockStartLine { + var ( + openingNodePos = blockStartPos + 1 + lastLeadingComment ast.Node + ) + + var ( + firstStatementCommentGroups []*ast.CommentGroup + lastStatementCommentGroups []*ast.CommentGroup + ) + + if cg, ok := commentMap[firstStatement]; ok && !isCase { + firstStatementCommentGroups = cg + } else { + // TODO: Just like with trailing whitespaces comments in a case block is + // tied to the last token of the first statement. For now we iterate over + // all comments in the stmt and grab those that's after colon and before + // first statement. + for _, cg := range commentMap { + if len(cg) < 1 { continue } - // We only care about comments before our statement from the comment - // map. As soon as we hit comments after our statement let's break - // out! - if commentGroup.Pos() > firstStatement.Pos() { - break + // If we have comments and the last comment ends before the first + // statement and the node is after the colon, this must be the node + // mapped to comments. + for _, c := range cg { + if c.End() < firstStatement.Pos() && c.Pos() > blockStartPos { + firstStatementCommentGroups = append(firstStatementCommentGroups, c) + } } - // We store number of seen comment groups because we allow multiple - // groups with a newline between them; but if the first one has WS - // before it, we're not going to count it to force an error. - if p.config.AllowSeparatedLeadingComment { - cg := p.fileSet.Position(commentGroup.Pos()).Line + // And same if we have comments where the first comment is after the + // last statement but before the next statement (next case). As with + // the other things, if there is not next statement it's no next + // case and the logic will be handled when parsing the block. + if nextStatement == nil { + continue + } - if seenCommentGroups > 0 || cg == blockStartLine+1 { - seenCommentGroups++ + for _, c := range cg { + if c.Pos() > lastStatement.End() && c.End() < nextStatement.Pos() { + lastStatementCommentGroups = append(lastStatementCommentGroups, c) } - } else { - seenCommentGroups++ } + } + + // Since the comments come from a map they might not be ordered meaning + // that the last and first comment groups can be in the wrong order. We + // fix this by sorting all comments by pos after adding them all to the + // slice. + sort.Slice(firstStatementCommentGroups, func(i, j int) bool { + return firstStatementCommentGroups[i].Pos() < firstStatementCommentGroups[j].Pos() + }) + + sort.Slice(lastStatementCommentGroups, func(i, j int) bool { + return lastStatementCommentGroups[i].Pos() < lastStatementCommentGroups[j].Pos() + }) + } - // Support both /* multiline */ and //single line comments - for _, c := range commentGroup.List { - allowedLinesBeforeFirstStatement += len(strings.Split(c.Text, "\n")) + for _, commentGroup := range firstStatementCommentGroups { + // If the comment group is on the same line as the block start + // (LBrace) we should not consider it. + if p.nodeEnd(commentGroup) == blockStartLine { + openingNodePos = commentGroup.End() + continue + } + + // We only care about comments before our statement from the comment + // map. As soon as we hit comments after our statement let's break + // out! + if commentGroup.Pos() > firstStatement.Pos() { + break + } + + // We never allow leading whitespace for the first comment. + if lastLeadingComment == nil && p.nodeStart(commentGroup)-1 != blockStartLine { + p.addErrorRange( + openingNodePos, + openingNodePos, + commentGroup.Pos(), + reasonBlockStartsWithWS, + ) + } + + // If lastLeadingComment is set this is not the first comment so we + // should remove whitespace between them if we don't explicitly + // allow it. + if lastLeadingComment != nil && !p.config.AllowSeparatedLeadingComment { + if p.nodeStart(commentGroup)+1 != p.nodeEnd(lastLeadingComment) { + p.addErrorRange( + openingNodePos, + lastLeadingComment.End(), + commentGroup.Pos(), + reasonBlockStartsWithWS, + ) } } + + lastLeadingComment = commentGroup } - // If we allow separated comments, allow for a space after each group - if p.config.AllowSeparatedLeadingComment { - if seenCommentGroups > 1 { - allowedLinesBeforeFirstStatement += seenCommentGroups - 1 - } else if seenCommentGroups == 1 { - allowedLinesBeforeFirstStatement++ - } + lastNodePos := openingNodePos + if lastLeadingComment != nil { + lastNodePos = lastLeadingComment.End() + blockStartLine = p.nodeEnd(lastLeadingComment) } - // And now if the first statement is passed the number of allowed lines, - // then we had extra WS, possibly before the first comment group. - if p.nodeStart(firstStatement) > blockStartLine+allowedLinesBeforeFirstStatement { - p.addError( - blockStartPos, + // Check if we have a whitespace between the last node which can be the + // Lbrace, a comment on the same line or the last comment if we have + // comments inside the actual block and the first statement. This is never + // allowed. + if p.nodeStart(firstStatement)-1 != blockStartLine { + p.addErrorRange( + openingNodePos, + lastNodePos, + firstStatement.Pos(), reasonBlockStartsWithWS, ) } // If the blockEndLine is not 0 we're a regular block (not case). if blockEndLine != 0 { - if p.config.AllowTrailingComment { - if lastComment, ok := commentMap[lastStatement]; ok { - var ( - lastCommentGroup = lastComment[len(lastComment)-1] - lastCommentLine = lastCommentGroup.List[len(lastCommentGroup.List)-1] - countNewlines = 0 - ) + // We don't want to reject example functions since they have to end with + // a comment. + if isExampleFunc(ident) { + return + } - countNewlines += len(strings.Split(lastCommentLine.Text, "\n")) + var ( + lastNode ast.Node = lastStatement + trailingComments []ast.Node + ) - // No newlines between trailing comments and end of block. - if p.nodeStart(lastCommentLine)+countNewlines != blockEndLine-1 { - return + // Check if we have an comments _after_ the last statement and update + // the last node if so. + if c, ok := commentMap[lastStatement]; ok { + lastComment := c[len(c)-1] + if lastComment.Pos() > lastStatement.End() && lastComment.Pos() < stmt.End() { + lastNode = lastComment + } + } + + // TODO: This should be improved. + // The trailing comments are mapped to the last statement item which can + // be anything depending on what the last statement is. + // In `fmt.Println("hello")`, trailing comments will be mapped to + // `*ast.BasicLit` for the "hello" string. + // A short term improvement can be to cache this but for now we naively + // iterate over all items when we check a block. + for _, commentGroups := range commentMap { + for _, commentGroup := range commentGroups { + if commentGroup.Pos() < lastNode.End() || commentGroup.End() > stmt.End() { + continue } + + trailingComments = append(trailingComments, commentGroup) } } - if p.nodeEnd(lastStatement) != blockEndLine-1 && !isExampleFunc(ident) { - p.addError(blockEndPos, reasonBlockEndsWithWS) + // TODO: Should this be relaxed? + // Given the old code we only allowed trailing newline if it was + // directly tied to the last statement so for backwards compatibility + // we'll do the same. This means we fail all but the last whitespace + // even when allowing trailing comments. + for _, comment := range trailingComments { + if p.nodeStart(comment)-p.nodeEnd(lastNode) > 1 { + p.addErrorRange( + blockEndPos, + lastNode.End(), + comment.Pos(), + reasonBlockEndsWithWS, + ) + } + + lastNode = comment + } + + if !p.config.AllowTrailingComment && p.nodeEnd(stmt)-1 != p.nodeEnd(lastStatement) { + p.addErrorRange( + blockEndPos, + lastNode.End(), + stmt.End()-1, + reasonBlockEndsWithWS, + ) } return } + // Nothing to do if we're not looking for enforced newline. + if p.config.ForceCaseTrailingWhitespaceLimit == 0 { + return + } + // If we don't have any nextStatement the trailing whitespace will be // handled when parsing the switch. If we do have a next statement we can // see where it starts by getting it's colon position. We set the end of the // current case to the position of the next case. - switch n := nextStatement.(type) { - case *ast.CaseClause: - blockEndPos = n.Case - case *ast.CommClause: - blockEndPos = n.Case + switch nextStatement.(type) { + case *ast.CaseClause, *ast.CommClause: default: // No more cases return } - blockEndLine = p.fileSet.Position(blockEndPos).Line - 1 - - var ( - blockSize = blockEndLine - blockStartLine - caseTrailingCommentLines int - ) - - // TODO: I don't know what comments are bound to in cases. For regular - // blocks the last comment is bound to the last statement but for cases - // they are bound to the case clause expression. This will however get us all - // comments and depending on the case expression this gets tricky. - // - // To handle this I get the comment map from the current statement (the case - // itself) and iterate through all groups and all comment within all groups. - // I then get the comments after the last statement but before the next case - // clause and just map each line of comment that way. - for _, commentGroups := range commentMap { - for _, commentGroup := range commentGroups { - for _, comment := range commentGroup.List { - commentLine := p.fileSet.Position(comment.Pos()).Line - - // Ignore comments before the last statement. - if commentLine <= p.nodeStart(lastStatement) { - continue - } - - // Ignore comments after the end of this case. - if commentLine > blockEndLine { - continue - } - - // This allows /* multiline */ comments with newlines as well - // as regular (//) ones - caseTrailingCommentLines += len(strings.Split(comment.Text, "\n")) - } - } + var closingNode ast.Node = lastStatement + for _, commentGroup := range lastStatementCommentGroups { + // TODO: In future versions we might want to close the gaps between + // comments. However this is not currently reported in v3 so we + // won't add this for now. + // if p.nodeStart(commentGroup)-1 != p.nodeEnd(closingNode) {} + closingNode = commentGroup } - hasTrailingWhitespace := p.nodeEnd(lastStatement)+caseTrailingCommentLines != blockEndLine + totalRowsInCase := p.nodeEnd(closingNode) - blockStartLine + if totalRowsInCase < p.config.ForceCaseTrailingWhitespaceLimit { + return + } - // If the force trailing limit is configured and we don't end with a newline. - if p.config.ForceCaseTrailingWhitespaceLimit > 0 && !hasTrailingWhitespace { - // Check if the block size is too big to miss the newline. - if blockSize >= p.config.ForceCaseTrailingWhitespaceLimit { - p.addError(lastStatement.Pos(), reasonCaseBlockTooCuddly) - } + if p.nodeEnd(closingNode)+1 == p.nodeStart(nextStatement) { + p.addErrorRange( + closingNode.Pos(), + closingNode.End(), + closingNode.End(), + reasonCaseBlockTooCuddly, + ) } } @@ -1217,15 +1356,15 @@ func isExampleFunc(ident *ast.Ident) bool { return ident != nil && strings.HasPrefix(ident.Name, "Example") } -func (p *Processor) nodeStart(node ast.Node) int { - return p.fileSet.Position(node.Pos()).Line +func (p *processor) nodeStart(node ast.Node) int { + return p.fileSet.PositionFor(node.Pos(), false).Line } -func (p *Processor) nodeEnd(node ast.Node) int { - line := p.fileSet.Position(node.End()).Line +func (p *processor) nodeEnd(node ast.Node) int { + line := p.fileSet.PositionFor(node.End(), false).Line if isEmptyLabeledStmt(node) { - return p.fileSet.Position(node.Pos()).Line + return p.fileSet.PositionFor(node.Pos(), false).Line } return line @@ -1242,21 +1381,29 @@ func isEmptyLabeledStmt(node ast.Node) bool { return empty } -// Add an error for the file and line number for the current token.Pos with the -// given reason. -func (p *Processor) addError(pos token.Pos, reason string) { - position := p.fileSet.Position(pos) +func (p *processor) addWhitespaceBeforeError(node ast.Node, reason string) { + p.addErrorRange(node.Pos(), node.Pos(), node.Pos(), reason) +} + +func (p *processor) addErrorRange(reportAt, start, end token.Pos, reason string) { + report, ok := p.result[reportAt] + if !ok { + report = result{ + reason: reason, + fixRanges: []fix{}, + } + } - p.result = append(p.result, Result{ - FileName: position.Filename, - LineNumber: position.Line, - Position: position, - Reason: reason, + report.fixRanges = append(report.fixRanges, fix{ + fixRangeStart: start, + fixRangeEnd: end, }) + + p.result[reportAt] = report } -func (p *Processor) addWarning(w string, pos token.Pos, t interface{}) { - position := p.fileSet.Position(pos) +func (p *processor) addWarning(w string, pos token.Pos, t interface{}) { + position := p.fileSet.PositionFor(pos, false) p.warnings = append(p.warnings, fmt.Sprintf("%s:%d: %s (%T)", position.Filename, position.Line, w, t), |