Update WSL to v1.2.1 #794
Update WSL to v1.2.1 #794
Conversation
tpounds
added
dependencies
pkg/golinters/wsl.go
Outdated
| var ( | ||
| files = []string{} | ||
| linterCfg = lintCtx.Cfg.LintersSettings.WSL | ||
| processorCfg = wsl.DefaultConfig() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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,
}
|
@bombsimon Thanks for the updated version! You mentioned this fixes some false positives. Would it be possible to add those as additional test cases? This will help ensure |
|
@tpounds You are absolutely right! I'll add a test case which is what I resolved. |
$ git cherry --abbrev -v 0af0999fabfb ee9bf5809ead + abd8436 all: enable Go modules on CI (golangci#753) + 3c9d0fb checkers: recognize //line and //nolint in commentFormatting (golangci#756) + 0b517d7 checkers: extend deprecatedComment patterns (golangci#757) + 09100f6 checkers: use astcast package instead of coerce.go (golangci#758) + 2e9e97f checker: simplify boolExprSimplify (golangci#759) + 575701e make: add go-consistent to CI checks list (golangci#761) + b55f431 checkers: fix unlambda handling of builtins (golangci#763) + 5a7dee3 checker: handle lambdas properly in boolExprSimplify (golangci#765) + 5ce3939 checkers: teach boolExprSimplify a few new tricks (golangci#766) + 04d160f checkers: add new patterns to boolExprSimplify (golangci#768) + 09582e2 make: collect coverprofile separately from goveralls (golangci#769) + d8d0ee4 checkers: recognize NOTE pattern in deprecatedComment (golangci#770) + 12f0f85 Update copyright notice to 2019 (golangci#771) + f54bdb6 checkers: add stringXbytes checker + 170d65c checkers: followup for golangci#773 (golangci#774) + 84e9e83 checkers: make stringXbytes more linear (golangci#775) + a800815 checkers: add Depreacted typo pattern (golangci#776) + 6751be9 checkers: add hexLiterals (golangci#772) + ac61906 checkers: add typeAssertChain checker (golangci#782) + d19dbf1 checkers: add codegenComment checker (golangci#783) + d82b576 checkers: proper pkg/obj check for flagName (golangci#786) + dfcf754 ci: enable integration tests (golangci#787) + 5dafc45 checkers: fix equalFold false positive (golangci#788) + ed5e8e7 checkers: refactor and fix hexLiteral checker (golangci#789) + e704e07 checkers: add argOrder checker (golangci#790) + 34c1dc8 checkers: add Split handling to argOrder checker (golangci#791) + cbe095d checkers: add math.Max and math.Min to dupArg (golangci#792) + c986ee5 checkers: add checkers info fields test (golangci#794) + 66e5832 cmd/makedocs: use lintpack, fix build (golangci#793) + 6bce9d0 cmd/makedocs: add enabled/disabled by default info (golangci#795) + 4adbf9a checkers: simplify flagName (golangci#799) + 07de34a checkers: add octalLiteral checker (golangci#798) + 765907a cmd/makedocs: add checker param docs (golangci#796) + ee9bf58 cmd/makedocs: fix headers formatting (golangci#803)
ldez
added
linter: update version
After WSL v1.0.1 got merged last week I found a few false positives which is resolved in this version. I've also added support for some configuration which is bundled in this commit.
I aimed to use non-negative configuration names so defaulting them to
truerequired me to create a config struct, hope this is OK.I sent an email to Denis yesterday with a few questions regarding this linter but decided to create this PR even though I haven't gotten any response yet since I don't want to risk miss this update if a new release of
golangci-lintis being created.