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

Comma in exclude pattern leads to unexpected results #665

Closed
xrstf opened this issue Sep 6, 2019 · 3 comments · Fixed by #1917
Closed

Comma in exclude pattern leads to unexpected results #665

xrstf opened this issue Sep 6, 2019 · 3 comments · Fixed by #1917
Labels
bug Something isn't working
Projects

Comments

@xrstf
Copy link

xrstf commented Sep 6, 2019

Bug Report

I tried to configure an exclude pattern to get rid of some goconst errors. These errors take the form of

string `get` has 12 occurrences, make it a constant

So I made an exclude filter like this:

issues:
  exclude:
  - "string `get` has [0-9]+ occurences, make it a constant"

and then noticed that all goconst errors disappeared. After adding some debugging statements to the code, I could see that the final assembled regex is

(?i)(string `get` has [0-9]+ occurences| make it a constant|Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked|(comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)|func name will be used as test\.Test.* by other packages, and that stutters; consider calling this|(possible misuse of unsafe.Pointer|should have signature)|ineffective break statement. Did you mean to break out of the outer loop|Use of unsafe calls should be audited|Subprocess launch(ed with variable|ing should be audited)|G104|(Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less)|Potential file inclusion via variable)

Notice how the comma turned the exclusion pattern into two distinct elements. The documentation in the readme just says

List of regexps of issue texts to exclude, empty list by default.

I don't really know why this is tagged as a mapstructure in the code, but right now the behaviour or the documentation is misleading.

Version of golangci-lint:

1.17.1 or master branch

Config file: cat .golangci.yml

linters:
  enable:
   - goconst
  disable-all: true

issues:
  exclude:
  - iamnotarealstring, make it a constant

Go environment: go version && go env

go version go1.13 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/xrstf/.cache/go-build"
GOENV="/home/xrstf/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/xrstf/gospace"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build282403496=/tmp/go-build -gno-record-gcc-switches"

cc @alvaroaleman

@tpounds tpounds added the bug Something isn't working label Sep 25, 2019
@jirfag jirfag added this to Low priority in Zero Bug Oct 15, 2019
@stale
Copy link

stale bot commented Mar 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Mar 23, 2020
@ernado ernado removed the stale No recent correspondence or work activity label Mar 23, 2020
@stale
Copy link

stale bot commented Apr 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Apr 18, 2021
@ldez ldez removed the stale No recent correspondence or work activity label Apr 19, 2021
@ldez
Copy link
Member

ldez commented Apr 19, 2021

The problem is related to the custom parsing of slice flags:

fixSlicesFlags(e.runCmd.Flags())

func fixSlicesFlags(fs *pflag.FlagSet) {
// It's a dirty hack to set flag.Changed to true for every string slice flag.
// It's necessary to merge config and command-line slices: otherwise command-line
// flags will always overwrite ones from the config.
fs.VisitAll(func(f *pflag.Flag) {
if f.Value.Type() != "stringSlice" {
return
}
s, err := fs.GetStringSlice(f.Name)
if err != nil {
return
}
if s == nil { // assume that every string slice flag has nil as the default
return
}
// calling Set sets Changed to true: next Set calls will append, not overwrite
_ = f.Value.Set(strings.Join(s, ","))
})
}

Zero Bug automation moved this from Low priority to Closed Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Zero Bug
  
Closed
Development

Successfully merging a pull request may close this issue.

4 participants