Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

whitespace: update after moving to the analysis package #4003

Merged
merged 9 commits into from
Nov 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ require (
github.com/tomarrell/wrapcheck/v2 v2.8.1
github.com/tommy-muehle/go-mnd/v2 v2.5.1
github.com/ultraware/funlen v0.1.0
github.com/ultraware/whitespace v0.0.5
github.com/ultraware/whitespace v0.1.0
github.com/uudashr/gocognit v1.1.2
github.com/valyala/quicktemplate v1.7.0
github.com/xen0n/gosmopolitan v1.2.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/golinters/revive.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type jsonObject struct {

// NewRevive returns a new Revive linter.
//
//nolint:dupl

func NewRevive(settings *config.ReviveSettings) *goanalysis.Linter {
var mu sync.Mutex
var resIssues []goanalysis.Issue
Expand Down
86 changes: 40 additions & 46 deletions pkg/golinters/whitespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package golinters

import (
"fmt"
"go/token"
"sync"

"github.com/ultraware/whitespace"
Expand All @@ -16,33 +15,29 @@ import (

const whitespaceName = "whitespace"

//nolint:dupl
func NewWhitespace(settings *config.WhitespaceSettings) *goanalysis.Linter {
var mu sync.Mutex
var resIssues []goanalysis.Issue

var wsSettings whitespace.Settings
if settings != nil {
wsSettings = whitespace.Settings{
Mode: whitespace.RunningModeGolangCI,
MultiIf: settings.MultiIf,
MultiFunc: settings.MultiFunc,
}
}

analyzer := &analysis.Analyzer{
Name: whitespaceName,
Doc: goanalysis.TheOnlyanalyzerDoc,
Run: goanalysis.DummyRun,
}
a := whitespace.NewAnalyzer(&wsSettings)

return goanalysis.NewLinter(
whitespaceName,
"Tool for detection of leading and trailing whitespace",
[]*analysis.Analyzer{analyzer},
a.Name,
a.Doc,
[]*analysis.Analyzer{a},
nil,
).WithContextSetter(func(lintCtx *linter.Context) {
analyzer.Run = func(pass *analysis.Pass) (any, error) {
issues, err := runWhitespace(lintCtx, pass, wsSettings)
a.Run = func(pass *analysis.Pass) (any, error) {
issues, err := runWhitespace(pass, wsSettings)
if err != nil {
return nil, err
}
Expand All @@ -62,46 +57,45 @@ func NewWhitespace(settings *config.WhitespaceSettings) *goanalysis.Linter {
}).WithLoadMode(goanalysis.LoadModeSyntax)
}

func runWhitespace(lintCtx *linter.Context, pass *analysis.Pass, wsSettings whitespace.Settings) ([]goanalysis.Issue, error) {
var messages []whitespace.Message
for _, file := range pass.Files {
messages = append(messages, whitespace.Run(file, pass.Fset, wsSettings)...)
}

if len(messages) == 0 {
return nil, nil
}
func runWhitespace(pass *analysis.Pass, wsSettings whitespace.Settings) ([]goanalysis.Issue, error) {
lintIssues := whitespace.Run(pass, &wsSettings)

issues := make([]goanalysis.Issue, len(messages))
for k, i := range messages {
issue := result.Issue{
Pos: token.Position{
Filename: i.Pos.Filename,
Line: i.Pos.Line,
},
LineRange: &result.Range{From: i.Pos.Line, To: i.Pos.Line},
Text: i.Message,
FromLinter: whitespaceName,
Replacement: &result.Replacement{},
issues := make([]goanalysis.Issue, len(lintIssues))
for i, issue := range lintIssues {
report := &result.Issue{
FromLinter: whitespaceName,
Pos: pass.Fset.PositionFor(issue.Diagnostic, false),
Text: issue.Message,
}

bracketLine, err := lintCtx.LineCache.GetLine(issue.Pos.Filename, issue.Pos.Line)
if err != nil {
return nil, fmt.Errorf("failed to get line %s:%d: %w", issue.Pos.Filename, issue.Pos.Line, err)
}
switch issue.MessageType {
case whitespace.MessageTypeRemove:
if len(issue.LineNumbers) == 0 {
continue
}

report.LineRange = &result.Range{
From: issue.LineNumbers[0],
To: issue.LineNumbers[len(issue.LineNumbers)-1],
}

report.Replacement = &result.Replacement{NeedOnlyDelete: true}

case whitespace.MessageTypeAdd:
report.Pos = pass.Fset.PositionFor(issue.FixStart, false)
report.Replacement = &result.Replacement{
Inline: &result.InlineFix{
StartCol: 0,
Length: 1,
NewString: "\n\t",
},
}

switch i.Type {
case whitespace.MessageTypeLeading:
issue.LineRange.To++ // cover two lines by the issue: opening bracket "{" (issue.Pos.Line) and following empty line
case whitespace.MessageTypeTrailing:
issue.LineRange.From-- // cover two lines by the issue: closing bracket "}" (issue.Pos.Line) and preceding empty line
issue.Pos.Line-- // set in sync with LineRange.From to not break fixer and other code features
case whitespace.MessageTypeAddAfter:
bracketLine += "\n"
default:
return nil, fmt.Errorf("unknown message type: %v", issue.MessageType)
}
issue.Replacement.NewLines = []string{bracketLine}

issues[k] = goanalysis.NewIssue(&issue, pass)
issues[i] = goanalysis.NewIssue(report, pass)
}

return issues, nil
Expand Down
23 changes: 23 additions & 0 deletions test/testdata/fix/in/whitespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package p

import "fmt"

//line yaccpar:1
func oneLeadingNewline() {

fmt.Println("Hello world")
Expand Down Expand Up @@ -58,4 +59,26 @@ func multiIfFunc() {
2 == 2 {
fmt.Println("Hello multi-line world")
}

if true {
if true {
if true {
if 1 == 1 &&
2 == 2 {
fmt.Println("Hello nested multi-line world")
}
}
}
}
}

func notGoFmted() {




fmt.Println("Hello world")



}
18 changes: 17 additions & 1 deletion test/testdata/fix/out/whitespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package p

import "fmt"

//line yaccpar:1
func oneLeadingNewline() {
fmt.Println("Hello world")
}
Expand Down Expand Up @@ -40,7 +41,6 @@ func oneLeadingNewlineWithCommentFunc() {
}

func twoLeadingNewlines() {

fmt.Println("Hello world")
}

Expand All @@ -56,4 +56,20 @@ func multiIfFunc() {

fmt.Println("Hello multi-line world")
}

if true {
if true {
if true {
if 1 == 1 &&
2 == 2 {

fmt.Println("Hello nested multi-line world")
}
}
}
}
}

func notGoFmted() {
fmt.Println("Hello world")
Comment on lines +73 to +74
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed here that we have the same situation as in #4003 (comment). Basically this results in the standalone linter and golangci-lint not fixing the same way, golangci-lint will leave the sapces before fmt whereas the linter itself will replace it with a tab.

I don't think there's anything we should do extra in this specific linter for this scenario (and also that's the same behaviour as before, we just didn't have a standalone linter) but yet another reason to try to integrate the suggested fixes with the golangci-lint fixer.

}
Loading