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.

Sign up for GitHub

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

Update WSL to v1.2.1 #794

Merged
merged 2 commits into from
Oct 8, 2019
Merged

Update WSL to v1.2.1 #794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a96b501
Update WSL to v1.2.1
bombsimon Oct 7, 2019
b005b2d
Add new tests for fixed false positives, don't derive defaults from WSL
bombsimon Oct 7, 2019
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
9 changes: 9 additions & 0 deletions .golangci.example.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,15 @@ linters-settings:
whitespace:
multi-if: false # Enforces newlines (or comments) after every multi-line if statement
multi-func: false # Enforces newlines (or comments) after every multi-line function signature
wsl:
# If true append is only allowed to be cuddled if appending value is
# matching variables, fields or types on line above. Default is true.
strict-append: true
# Allow calls and assignments to be cuddled as long as the lines have any
# matching variables, fields or types. Default is true.
allow-assign-and-call: true
# Allow multiline assignments to be cuddled. Default is true.
allow-multiline-assign: true

linters:
enable:
Expand Down
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,15 @@ linters-settings:
whitespace:
multi-if: false # Enforces newlines (or comments) after every multi-line if statement
multi-func: false # Enforces newlines (or comments) after every multi-line function signature
wsl:
# If true append is only allowed to be cuddled if appending value is
# matching variables, fields or types on line above. Default is true.
strict-append: true
# Allow calls and assignments to be cuddled as long as the lines have any
# matching variables, fields or types. Default is true.
allow-assign-and-call: true
# Allow multiline assignments to be cuddled. Default is true.
allow-multiline-assign: true

linters:
enable:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.12

require (
github.com/OpenPeeDeeP/depguard v1.0.1
github.com/bombsimon/wsl v1.0.1
github.com/bombsimon/wsl v1.2.1
github.com/fatih/color v1.7.0
github.com/go-critic/go-critic v0.3.5-0.20190904082202-d79a9f0c64db
github.com/go-lintpack/lintpack v0.5.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/bombsimon/wsl v1.0.1 h1:GkrpOj7ajds8re6EJXN+k6UtatYSktupigQ2ZfOUIOU=
github.com/bombsimon/wsl v1.0.1/go.mod h1:DSRwCD8c7NecM11/LnZcTS8nS8OND5ybj04DWM4l8mc=
github.com/bombsimon/wsl v1.2.1 h1:DcLf3V66dJi4a+KHt+F1FdOeBZ05adHqTMYFvjgv06k=
github.com/bombsimon/wsl v1.2.1/go.mod h1:43lEF/i0kpXbLCeDXL9LMT8c92HyBywXb0AsgMHYngM=
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
Expand Down
12 changes: 12 additions & 0 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ type LintersSettings struct {
MultiFunc bool `mapstructure:"multi-func"`
}

WSL WSLSettings
Lll LllSettings
Unparam UnparamSettings
Nakedret NakedretSettings
Expand Down Expand Up @@ -249,6 +250,12 @@ type GocognitSettings struct {
MinComplexity int `mapstructure:"min-complexity"`
}

type WSLSettings struct {
StrictAppend bool `mapstructure:"strict-append"`
AllowAssignAndCallCuddle bool `mapstructure:"allow-assign-and-call"`
AllowMultiLineAssignCuddle bool `mapstructure:"allow-multiline-assign"`
}

var defaultLintersSettings = LintersSettings{
Lll: LllSettings{
LineLength: 120,
Expand Down Expand Up @@ -277,6 +284,11 @@ var defaultLintersSettings = LintersSettings{
Gocognit: GocognitSettings{
MinComplexity: 30,
},
WSL: WSLSettings{
StrictAppend: true,
AllowAssignAndCallCuddle: true,
AllowMultiLineAssignCuddle: true,
},
}

type Linters struct {
Expand Down
14 changes: 12 additions & 2 deletions pkg/golinters/wsl.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,23 @@ func NewWSL() *goanalysis.Linter {
nil,
).WithContextSetter(func(lintCtx *linter.Context) {
analyzer.Run = func(pass *analysis.Pass) (interface{}, error) {
files := []string{}
var (
files = []string{}
linterCfg = lintCtx.Cfg.LintersSettings.WSL
processorCfg = wsl.DefaultConfig()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary given the defaults added to config.go. We probably only want one place the default values can be set and sourced from.

Copy link
Member Author

@bombsimon bombsimon Oct 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that there should be one place for the defaults and right now they're all true which is kind of bad if you want to marshal the configuration from .golangci.yml but good if you don't want negative names. :)

How do you propose I should attack this issue? I want the values to be true by default and therefore I don't want them in the .golangci.yaml. However if they're not present and the the configuration is being unmarshalled their default values will be false (unless I use pointers). Because of this I added the WSLSettings which is set in config.go to default them to true if the config is empty. So the only way to set them to false right now is to explicitly specify that in the configuration, otherwise the defaults will be made.

Do you think I should re-write how this is handled in the linter itself (e.g. don't expose DefaultConfig()) or am I missing something obvious about how the defaults are set in golangci-lint? I still think there might be use-cases for WSL outside golangci-lint. The only other idea I have right now is to change the config to be negative such as NoStrictAppend and NoMultiLineAssign but then we're at negative names again.

Copy link
Contributor

@tpounds tpounds Oct 7, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't have a strong recommendation but if you want golangci-lint's setting to derive from the linter directly I would set that within the config.go file. However, my general feeling is it might be better to just have golangci-lint have a copy of its own defaults that way the defaults can't change when upgrading the linter in the future (potentially a breaking change to users). This implies removing the wsl.DefaultConfig() assignment here and mapping the struct fields 1:1.

ctxCfg := lintCtx.Cfg.LintersSettings.WSL
wslCfg := wsl.Configuration{
    StrictAppend:               ctxCfg.StrictAppend,
    AllowAssignAndCallCuddle:   ctxCfg.AllowAssignAndCallCuddle,
    AllowMultiLineAssignCuddle: ctxCfg.AllowMultiLineAssignCuddle,
}

)

processorCfg.StrictAppend = linterCfg.StrictAppend
processorCfg.AllowAssignAndCallCuddle = linterCfg.AllowAssignAndCallCuddle
processorCfg.AllowMultiLineAssignCuddle = linterCfg.AllowMultiLineAssignCuddle

for _, file := range pass.Files {
files = append(files, pass.Fset.Position(file.Pos()).Filename)
}

wslErrors, _ := wsl.ProcessFiles(files)
wslErrors, _ := wsl.NewProcessorWithConfig(processorCfg).
ProcessFiles(files)

if len(wslErrors) == 0 {
return nil, nil
}
Expand Down
21 changes: 21 additions & 0 deletions vendor/github.com/bombsimon/wsl/README.md
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion vendor/github.com/bombsimon/wsl/go.mod
View file
Open in desktop

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

2 changes: 0 additions & 2 deletions vendor/github.com/bombsimon/wsl/go.sum
View file
Open in desktop

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

Loading