From e527b2789bcd90b7637741197bd0aede06e83f0f Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Thu, 2 Nov 2023 12:10:04 +0100 Subject: [PATCH] Use go modules, analysis package and add test (#8) * Use go modules, analysis package and add test * Use go modules * Use `analysis.Analyzer` for the linter * Introduce running mode to use from `golangci-lint` * Add tests using the testing package and assert fixer * Add standalone linter (`cmd/whitespace`) * Fix comments and typos, add another test * Add GitHub actions * Make linter work on non formatted files Instead of relying on the file being `gofmt`:ed and only containing one empty line at most this commit will keep track of comments after the opening bracket of block statements and set the fix start to where this comment ends. This means that we will fix all the way from the left bracket or comment til the first statement no matter how many empty lines this is. This will also keep a list of lines affected by the linter to support fixing multiple empty lines in `golangci-lint`. * Fix typos, add tests from PR * Multiline comments ending after opening pos is ok * Fix typo --- .github/workflows/go.yaml | 22 ++ README.md | 4 +- cmd/whitespace/main.go | 10 + go.mod | 10 + go.sum | 8 + main.go | 162 --------- .../src/whitespace_multiline/multiline.go | 85 +++++ .../whitespace_multiline/multiline.go.golden | 85 +++++ .../whitespace_no_multiline/no_multiline.go | 171 ++++++++++ .../no_multiline.go.golden | 122 +++++++ whitespace.go | 307 ++++++++++++++++++ whitespace_test.go | 29 ++ 12 files changed, 852 insertions(+), 163 deletions(-) create mode 100644 .github/workflows/go.yaml create mode 100644 cmd/whitespace/main.go create mode 100644 go.mod create mode 100644 go.sum delete mode 100644 main.go create mode 100644 testdata/src/whitespace_multiline/multiline.go create mode 100644 testdata/src/whitespace_multiline/multiline.go.golden create mode 100644 testdata/src/whitespace_no_multiline/no_multiline.go create mode 100644 testdata/src/whitespace_no_multiline/no_multiline.go.golden create mode 100644 whitespace.go create mode 100644 whitespace_test.go diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml new file mode 100644 index 0000000..0142102 --- /dev/null +++ b/.github/workflows/go.yaml @@ -0,0 +1,22 @@ +--- +name: Build and test +on: + push: + pull_request: + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Set up Go + uses: actions/setup-go@v4 + with: + go-version: '1.21' + + - name: Build + run: go build -v ./... + + - name: Test + run: go test -v ./... diff --git a/README.md b/README.md index 2a88f13..f99ecce 100644 --- a/README.md +++ b/README.md @@ -4,4 +4,6 @@ Whitespace is a linter that checks for unnecessary newlines at the start and end ## Installation guide -Whitespace is included in [golangci-lint](https://github.com/golangci/golangci-lint/). Install it and enable whitespace. +To install as a standalone linter, run `go install github.com/ultraware/whitespace`. + +Whitespace is also included in [golangci-lint](https://github.com/golangci/golangci-lint/). Install it and enable whitespace. diff --git a/cmd/whitespace/main.go b/cmd/whitespace/main.go new file mode 100644 index 0000000..73a78ed --- /dev/null +++ b/cmd/whitespace/main.go @@ -0,0 +1,10 @@ +package main + +import ( + "github.com/ultraware/whitespace" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(whitespace.NewAnalyzer(nil)) +} diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..306a134 --- /dev/null +++ b/go.mod @@ -0,0 +1,10 @@ +module github.com/ultraware/whitespace + +go 1.20 + +require golang.org/x/tools v0.12.0 + +require ( + golang.org/x/mod v0.12.0 // indirect + golang.org/x/sys v0.11.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..06d171f --- /dev/null +++ b/go.sum @@ -0,0 +1,8 @@ +golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= +golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= +golang.org/x/tools v0.12.0 h1:YW6HUoUmYBpwSgyaGaZq1fHjrBjX1rlpZ54T6mu2kss= +golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= diff --git a/main.go b/main.go deleted file mode 100644 index a5ff70a..0000000 --- a/main.go +++ /dev/null @@ -1,162 +0,0 @@ -package whitespace - -import ( - "go/ast" - "go/token" -) - -// Message contains a message -type Message struct { - Pos token.Position - Type MessageType - Message string -} - -// MessageType describes what should happen to fix the warning -type MessageType uint8 - -// List of MessageTypes -const ( - MessageTypeLeading MessageType = iota + 1 - MessageTypeTrailing - MessageTypeAddAfter -) - -// Settings contains settings for edge-cases -type Settings struct { - MultiIf bool - MultiFunc bool -} - -// Run runs this linter on the provided code -func Run(file *ast.File, fset *token.FileSet, settings Settings) []Message { - var messages []Message - - for _, f := range file.Decls { - decl, ok := f.(*ast.FuncDecl) - if !ok || decl.Body == nil { // decl.Body can be nil for e.g. cgo - continue - } - - vis := visitor{file.Comments, fset, nil, make(map[*ast.BlockStmt]bool), settings} - ast.Walk(&vis, decl) - - messages = append(messages, vis.messages...) - } - - return messages -} - -type visitor struct { - comments []*ast.CommentGroup - fset *token.FileSet - messages []Message - wantNewline map[*ast.BlockStmt]bool - settings Settings -} - -func (v *visitor) Visit(node ast.Node) ast.Visitor { - if node == nil { - return v - } - - if stmt, ok := node.(*ast.IfStmt); ok && v.settings.MultiIf { - checkMultiLine(v, stmt.Body, stmt.Cond) - } - - if stmt, ok := node.(*ast.FuncLit); ok && v.settings.MultiFunc { - checkMultiLine(v, stmt.Body, stmt.Type) - } - - if stmt, ok := node.(*ast.FuncDecl); ok && v.settings.MultiFunc { - checkMultiLine(v, stmt.Body, stmt.Type) - } - - if stmt, ok := node.(*ast.BlockStmt); ok { - wantNewline := v.wantNewline[stmt] - - comments := v.comments - if wantNewline { - comments = nil // Comments also count as a newline if we want a newline - } - first, last := firstAndLast(comments, v.fset, stmt.Pos(), stmt.End(), stmt.List) - - startMsg := checkStart(v.fset, stmt.Lbrace, first) - - if wantNewline && startMsg == nil { - v.messages = append(v.messages, Message{v.fset.PositionFor(stmt.Pos(), false), MessageTypeAddAfter, `multi-line statement should be followed by a newline`}) - } else if !wantNewline && startMsg != nil { - v.messages = append(v.messages, *startMsg) - } - - if msg := checkEnd(v.fset, stmt.Rbrace, last); msg != nil { - v.messages = append(v.messages, *msg) - } - } - - return v -} - -func checkMultiLine(v *visitor, body *ast.BlockStmt, stmtStart ast.Node) { - start, end := posLine(v.fset, stmtStart.Pos()), posLine(v.fset, stmtStart.End()) - - if end > start { // Check only multi line conditions - v.wantNewline[body] = true - } -} - -func posLine(fset *token.FileSet, pos token.Pos) int { - return fset.PositionFor(pos, false).Line -} - -func firstAndLast(comments []*ast.CommentGroup, fset *token.FileSet, start, end token.Pos, stmts []ast.Stmt) (ast.Node, ast.Node) { - if len(stmts) == 0 { - return nil, nil - } - - first, last := ast.Node(stmts[0]), ast.Node(stmts[len(stmts)-1]) - - for _, c := range comments { - if posLine(fset, c.Pos()) == posLine(fset, start) || posLine(fset, c.End()) == posLine(fset, end) { - continue - } - - if c.Pos() < start || c.End() > end { - continue - } - if c.Pos() < first.Pos() { - first = c - } - if c.End() > last.End() { - last = c - } - } - - return first, last -} - -func checkStart(fset *token.FileSet, start token.Pos, first ast.Node) *Message { - if first == nil { - return nil - } - - if posLine(fset, start)+1 < posLine(fset, first.Pos()) { - pos := fset.PositionFor(start, false) - return &Message{pos, MessageTypeLeading, `unnecessary leading newline`} - } - - return nil -} - -func checkEnd(fset *token.FileSet, end token.Pos, last ast.Node) *Message { - if last == nil { - return nil - } - - if posLine(fset, end)-1 > posLine(fset, last.End()) { - pos := fset.PositionFor(end, false) - return &Message{pos, MessageTypeTrailing, `unnecessary trailing newline`} - } - - return nil -} diff --git a/testdata/src/whitespace_multiline/multiline.go b/testdata/src/whitespace_multiline/multiline.go new file mode 100644 index 0000000..cdf2b9c --- /dev/null +++ b/testdata/src/whitespace_multiline/multiline.go @@ -0,0 +1,85 @@ +package whitespace + +import "fmt" + +// No comments, only whitespace. +func fn1() { // want "unnecessary leading newline" + + fmt.Println("Hello, World") + +} // want "unnecessary trailing newline" + +// Whitespace before leading and after trailing comment. +func fn2() { // want "unnecessary leading newline" + + // Some leading comments - this should yield error. + fmt.Println("Hello, World") + // And some trailing comments as well! + +} // want "unnecessary trailing newline" + +// Whitespace after leading and before trailing comment - no diagnostic. +func fn3() { + // This is fine, newline is after comment + + fmt.Println("Hello, World") + + // This is fine, newline before comment +} + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) without newlinew. +func fn4( + arg1 int, + arg2 int, +) { // want "multi-line statement should be followed by a newline" + fmt.Println("Hello, World") + + if true && + false { // want "multi-line statement should be followed by a newline" + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + fmt.Println("Hello, World") + } + } +} + + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) with comments counting +// as newlines. +func fn5( + arg1 int, + arg2 int, +) { + // Comment count as newline. + fmt.Println("Hello, World") + + if true && + false { + // Comment count as newline. + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { + // Comment count as newline. + fmt.Println("Hello, World") + } +} diff --git a/testdata/src/whitespace_multiline/multiline.go.golden b/testdata/src/whitespace_multiline/multiline.go.golden new file mode 100644 index 0000000..d0ca256 --- /dev/null +++ b/testdata/src/whitespace_multiline/multiline.go.golden @@ -0,0 +1,85 @@ +package whitespace + +import "fmt" + +// No comments, only whitespace. +func fn1() { // want "unnecessary leading newline" + fmt.Println("Hello, World") +} // want "unnecessary trailing newline" + +// Whitespace before leading and after trailing comment. +func fn2() { // want "unnecessary leading newline" + // Some leading comments - this should yield error. + fmt.Println("Hello, World") + // And some trailing comments as well! +} // want "unnecessary trailing newline" + +// Whitespace after leading and before trailing comment - no diagnostic. +func fn3() { + // This is fine, newline is after comment + + fmt.Println("Hello, World") + + // This is fine, newline before comment +} + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) without newlinew. +func fn4( + arg1 int, + arg2 int, +) { // want "multi-line statement should be followed by a newline" + + fmt.Println("Hello, World") + + if true && + false { // want "multi-line statement should be followed by a newline" + + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + + _ = func( + arg1 int, + arg2 int, + ) { // want "multi-line statement should be followed by a newline" + + fmt.Println("Hello, World") + } + } +} + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) with comments counting +// as newlines. +func fn5( + arg1 int, + arg2 int, +) { + // Comment count as newline. + fmt.Println("Hello, World") + + if true && + false { + // Comment count as newline. + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { + // Comment count as newline. + fmt.Println("Hello, World") + } +} diff --git a/testdata/src/whitespace_no_multiline/no_multiline.go b/testdata/src/whitespace_no_multiline/no_multiline.go new file mode 100644 index 0000000..4038fb3 --- /dev/null +++ b/testdata/src/whitespace_no_multiline/no_multiline.go @@ -0,0 +1,171 @@ +package whitespace + +import "fmt" + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) want to remove newlinne. +func fn1( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + + fmt.Println("Hello, World") + + if true && + false { // want "unnecessary leading newline" + + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "unnecessary leading newline" + + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { + _ = func( + arg1 int, + arg2 int, + ) { // want "unnecessary leading newline" + + fmt.Println("Hello, World") + } + } +} + +// MultiFunc (FuncDecl) with comment. +func fn2( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + + // A comment. + fmt.Println("Hello, World") +} + +// MultiFunc (FuncDecl) that's not `gofmt`:ed. +func fn3( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + + + + // A comment. + fmt.Println("Hello, World") + + if true { // want "unnecessary leading newline" + + + + fmt.Println("No comments") + + + } // want "unnecessary trailing newline" + // Also at end + + + +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. +func fn4() { // want "unnecessary leading newline" + + + + + fmt.Println("Hello, World") + + if true { // want "unnecessary leading newline" + + + + fmt.Println("No comments") + + + } // want "unnecessary trailing newline" + + + + +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. with comments +func fn5() { + // A comment that should still exist after this + // This one should also still exist + + + + fmt.Println("Hello, World") + + if true { + // A comment that should still exist after this + // This one should also still exist + + + + fmt.Println("No comments") + + + } // want "unnecessary trailing newline" + + + + +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. with comment blocks +func fn6() { + /* + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + Curabitur ornare dolor at nulla ultrices cursus. + Mauris pharetra metus ac condimentum sodales. + Fusce viverra libero vitae tellus dictum, sed congue risus sodales. + */ + + + + fmt.Println("Hello, World") + + if true { + /* + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + Curabitur ornare dolor at nulla ultrices cursus. + Mauris pharetra metus ac condimentum sodales. + Fusce viverra libero vitae tellus dictum, sed congue risus sodales. + */ + + + fmt.Println("No comments") + + + } // want "unnecessary trailing newline" + + + + +} // want "unnecessary trailing newline" + +func fn7() { /* Multiline spaning 1 line */ // want "unnecessary leading newline" + + fmt.Println("No comments") +} + +func fn8() { /* Multiline spaning + over several lines */ + + fmt.Println("No comments") +} + +func fn9() { /* Multiline spaning + + over several lines */ + + fmt.Println("No comments") +} diff --git a/testdata/src/whitespace_no_multiline/no_multiline.go.golden b/testdata/src/whitespace_no_multiline/no_multiline.go.golden new file mode 100644 index 0000000..6cbf7de --- /dev/null +++ b/testdata/src/whitespace_no_multiline/no_multiline.go.golden @@ -0,0 +1,122 @@ +package whitespace + +import "fmt" + +// MultiFunc (FuncDecl), MultiIf and MultiFunc(FuncLit) want to remove newlinne. +func fn1( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + fmt.Println("Hello, World") + + if true && + false { // want "unnecessary leading newline" + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { // want "unnecessary leading newline" + fmt.Println("Hello, World") + } + + _ = func( + arg1 int, + arg2 int, + ) { + _ = func( + arg1 int, + arg2 int, + ) { // want "unnecessary leading newline" + fmt.Println("Hello, World") + } + } +} + +// MultiFunc (FuncDecl) with comment. +func fn2( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + // A comment. + fmt.Println("Hello, World") +} + +// MultiFunc (FuncDecl) that's not `gofmt`:ed. +func fn3( + arg1 int, + arg2 int, +) { // want "unnecessary leading newline" + // A comment. + fmt.Println("Hello, World") + + if true { // want "unnecessary leading newline" + fmt.Println("No comments") + } // want "unnecessary trailing newline" + // Also at end +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. +func fn4() { // want "unnecessary leading newline" + fmt.Println("Hello, World") + + if true { // want "unnecessary leading newline" + fmt.Println("No comments") + } // want "unnecessary trailing newline" +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. with comments +func fn5() { + // A comment that should still exist after this + // This one should also still exist + + fmt.Println("Hello, World") + + if true { + // A comment that should still exist after this + // This one should also still exist + + fmt.Println("No comments") + } // want "unnecessary trailing newline" +} // want "unnecessary trailing newline" + +// Regular func (FuncDecl) that's not `gofmt`:ed. with comment blocks +func fn6() { + /* + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + Curabitur ornare dolor at nulla ultrices cursus. + Mauris pharetra metus ac condimentum sodales. + Fusce viverra libero vitae tellus dictum, sed congue risus sodales. + */ + + fmt.Println("Hello, World") + + if true { + /* + Lorem ipsum dolor sit amet, consectetur adipiscing elit. + Curabitur ornare dolor at nulla ultrices cursus. + Mauris pharetra metus ac condimentum sodales. + Fusce viverra libero vitae tellus dictum, sed congue risus sodales. + */ + + fmt.Println("No comments") + } // want "unnecessary trailing newline" +} // want "unnecessary trailing newline" + +func fn7() { /* Multiline spaning 1 line */ // want "unnecessary leading newline" + fmt.Println("No comments") +} + +func fn8() { /* Multiline spaning + over several lines */ + + fmt.Println("No comments") +} + +func fn9() { /* Multiline spaning + + over several lines */ + + fmt.Println("No comments") +} diff --git a/whitespace.go b/whitespace.go new file mode 100644 index 0000000..350e9b7 --- /dev/null +++ b/whitespace.go @@ -0,0 +1,307 @@ +package whitespace + +import ( + "flag" + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" +) + +// MessageType describes what should happen to fix the warning. +type MessageType uint8 + +// List of MessageTypes. +const ( + MessageTypeRemove MessageType = iota + 1 + MessageTypeAdd +) + +// RunningMode describes the mode the linter is run in. This can be either +// native or golangci-lint. +type RunningMode uint8 + +const ( + RunningModeNative RunningMode = iota + RunningModeGolangCI +) + +// Message contains a message and diagnostic information. +type Message struct { + // Diagnostic is what position the diagnostic should be put at. This isn't + // always the same as the fix start, f.ex. when we fix trailing newlines we + // put the diagnostic at the right bracket but we fix between the end of the + // last statement and the bracket. + Diagnostic token.Pos + + // FixStart is the span start of the fix. + FixStart token.Pos + + // FixEnd is the span end of the fix. + FixEnd token.Pos + + // LineNumbers represent the actual line numbers in the file. This is set + // when finding the diagnostic to make it easier to suggest fixes in + // golangci-lint. + LineNumbers []int + + // MessageType represents the type of message it is. + MessageType MessageType + + // Message is the diagnostic to show. + Message string +} + +// Settings contains settings for edge-cases. +type Settings struct { + Mode RunningMode + MultiIf bool + MultiFunc bool +} + +// NewAnalyzer creates a new whitespace analyzer. +func NewAnalyzer(settings *Settings) *analysis.Analyzer { + if settings == nil { + settings = &Settings{} + } + + return &analysis.Analyzer{ + Name: "whitespace", + Doc: "Whitespace is a linter that checks for unnecessary newlines at the start and end of functions, if, for, etc.", + Flags: flags(settings), + Run: func(p *analysis.Pass) (any, error) { + Run(p, settings) + return nil, nil + }, + RunDespiteErrors: true, + } +} + +func flags(settings *Settings) flag.FlagSet { + flags := flag.NewFlagSet("", flag.ExitOnError) + flags.BoolVar(&settings.MultiIf, "multi-if", settings.MultiIf, "Check that multi line if-statements have a leading newline") + flags.BoolVar(&settings.MultiFunc, "multi-func", settings.MultiFunc, "Check that multi line functions have a leading newline") + + return *flags +} + +func Run(pass *analysis.Pass, settings *Settings) []Message { + messages := []Message{} + + for _, file := range pass.Files { + filename := pass.Fset.Position(file.Pos()).Filename + if !strings.HasSuffix(filename, ".go") { + continue + } + + fileMessages := runFile(file, pass.Fset, *settings) + + if settings.Mode == RunningModeGolangCI { + messages = append(messages, fileMessages...) + continue + } + + for _, message := range fileMessages { + pass.Report(analysis.Diagnostic{ + Pos: message.Diagnostic, + Category: "whitespace", + Message: message.Message, + SuggestedFixes: []analysis.SuggestedFix{ + { + TextEdits: []analysis.TextEdit{ + { + Pos: message.FixStart, + End: message.FixEnd, + NewText: []byte("\n"), + }, + }, + }, + }, + }) + } + } + + return messages +} + +func runFile(file *ast.File, fset *token.FileSet, settings Settings) []Message { + var messages []Message + + for _, f := range file.Decls { + decl, ok := f.(*ast.FuncDecl) + if !ok || decl.Body == nil { // decl.Body can be nil for e.g. cgo + continue + } + + vis := visitor{file.Comments, fset, nil, make(map[*ast.BlockStmt]bool), settings} + ast.Walk(&vis, decl) + + messages = append(messages, vis.messages...) + } + + return messages +} + +type visitor struct { + comments []*ast.CommentGroup + fset *token.FileSet + messages []Message + wantNewline map[*ast.BlockStmt]bool + settings Settings +} + +func (v *visitor) Visit(node ast.Node) ast.Visitor { + if node == nil { + return v + } + + if stmt, ok := node.(*ast.IfStmt); ok && v.settings.MultiIf { + checkMultiLine(v, stmt.Body, stmt.Cond) + } + + if stmt, ok := node.(*ast.FuncLit); ok && v.settings.MultiFunc { + checkMultiLine(v, stmt.Body, stmt.Type) + } + + if stmt, ok := node.(*ast.FuncDecl); ok && v.settings.MultiFunc { + checkMultiLine(v, stmt.Body, stmt.Type) + } + + if stmt, ok := node.(*ast.BlockStmt); ok { + wantNewline := v.wantNewline[stmt] + + comments := v.comments + if wantNewline { + comments = nil // Comments also count as a newline if we want a newline + } + + opening, first, last := firstAndLast(comments, v.fset, stmt) + startMsg := checkStart(v.fset, opening, first) + + if wantNewline && startMsg == nil && len(stmt.List) >= 1 { + v.messages = append(v.messages, Message{ + Diagnostic: opening, + FixStart: stmt.List[0].Pos(), + FixEnd: stmt.List[0].Pos(), + LineNumbers: []int{v.fset.PositionFor(stmt.List[0].Pos(), false).Line}, + MessageType: MessageTypeAdd, + Message: "multi-line statement should be followed by a newline", + }) + } else if !wantNewline && startMsg != nil { + v.messages = append(v.messages, *startMsg) + } + + if msg := checkEnd(v.fset, stmt.Rbrace, last); msg != nil { + v.messages = append(v.messages, *msg) + } + } + + return v +} + +func checkMultiLine(v *visitor, body *ast.BlockStmt, stmtStart ast.Node) { + start, end := posLine(v.fset, stmtStart.Pos()), posLine(v.fset, stmtStart.End()) + + if end > start { // Check only multi line conditions + v.wantNewline[body] = true + } +} + +func posLine(fset *token.FileSet, pos token.Pos) int { + return fset.PositionFor(pos, false).Line +} + +func firstAndLast(comments []*ast.CommentGroup, fset *token.FileSet, stmt *ast.BlockStmt) (token.Pos, ast.Node, ast.Node) { + openingPos := stmt.Lbrace + 1 + + if len(stmt.List) == 0 { + return openingPos, nil, nil + } + + first, last := ast.Node(stmt.List[0]), ast.Node(stmt.List[len(stmt.List)-1]) + + for _, c := range comments { + // If the comment is on the same line as the opening pos (initially the + // left bracket) but it starts after the pos the comment must be after + // the bracket and where that comment ends should be considered where + // the fix should start. + if posLine(fset, c.Pos()) == posLine(fset, openingPos) && c.Pos() > openingPos { + if posLine(fset, c.End()) != posLine(fset, openingPos) { + // This is a multiline comment that spans from the `LBrace` line + // to a line further down. This should always be seen as ok! + first = c + } else { + openingPos = c.End() + } + } + + if posLine(fset, c.Pos()) == posLine(fset, stmt.Pos()) || posLine(fset, c.End()) == posLine(fset, stmt.End()) { + continue + } + + if c.Pos() < stmt.Pos() || c.End() > stmt.End() { + continue + } + + if c.Pos() < first.Pos() { + first = c + } + + if c.End() > last.End() { + last = c + } + } + + return openingPos, first, last +} + +func checkStart(fset *token.FileSet, start token.Pos, first ast.Node) *Message { + if first == nil { + return nil + } + + if posLine(fset, start)+1 < posLine(fset, first.Pos()) { + return &Message{ + Diagnostic: start, + FixStart: start, + FixEnd: first.Pos(), + LineNumbers: linesBetween(fset, start, first.Pos()), + MessageType: MessageTypeRemove, + Message: "unnecessary leading newline", + } + } + + return nil +} + +func checkEnd(fset *token.FileSet, end token.Pos, last ast.Node) *Message { + if last == nil { + return nil + } + + if posLine(fset, end)-1 > posLine(fset, last.End()) { + return &Message{ + Diagnostic: end, + FixStart: last.End(), + FixEnd: end, + LineNumbers: linesBetween(fset, last.End(), end), + MessageType: MessageTypeRemove, + Message: "unnecessary trailing newline", + } + } + + return nil +} + +func linesBetween(fset *token.FileSet, a, b token.Pos) []int { + lines := []int{} + aPosition := fset.PositionFor(a, false) + bPosition := fset.PositionFor(b, false) + + for i := aPosition.Line + 1; i < bPosition.Line; i++ { + lines = append(lines, i) + } + + return lines +} diff --git a/whitespace_test.go b/whitespace_test.go new file mode 100644 index 0000000..bbcc4d6 --- /dev/null +++ b/whitespace_test.go @@ -0,0 +1,29 @@ +package whitespace + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestWantMultiline(t *testing.T) { + testdata := analysistest.TestData() + analyzer := NewAnalyzer(&Settings{ + Mode: RunningModeNative, + MultiIf: true, + MultiFunc: true, + }) + + analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "whitespace_multiline") +} + +func TestNoMultiline(t *testing.T) { + testdata := analysistest.TestData() + analyzer := NewAnalyzer(&Settings{ + Mode: RunningModeNative, + MultiIf: false, + MultiFunc: false, + }) + + analysistest.RunWithSuggestedFixes(t, testdata, analyzer, "whitespace_no_multiline") +}