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

golangci-lint reports issues #281

Closed
mem opened this issue Mar 8, 2021 · 2 comments · Fixed by #316
Closed

golangci-lint reports issues #281

mem opened this issue Mar 8, 2021 · 2 comments · Fixed by #316

Comments

@mem
Copy link
Contributor

mem commented Mar 8, 2021

$ git --no-pager branch --contains 47ee79a16821e03c31ca741659d4f7919aed91c1
* main
* 
$ git --no-pager branch --remotes --contains 47ee79a16821e03c31ca741659d4f7919aed91c1
  origin/main

$ make
>> checking code style
>> checking license header
>> running golangci-lint
GO111MODULE=on go list -e -compiled -test=true -export=false -deps=true -find=false -tags= -- ./... > /dev/null
GO111MODULE=on /home/marcelo/devel/grafana/.direnv/go/bin/golangci-lint run  ./...
config/http_config_test.go:312:6: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                                        fmt.Fprintf(w, ExpectedMessage)
                                        ^
config/http_config_test.go:336:6: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
                                        fmt.Fprintf(w, ExpectedMessage)
                                        ^
expfmt/encode.go:21:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead. (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
expfmt/text_parse.go:27:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead. (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
expfmt/decode_test.go:24:2: SA1019: package github.com/golang/protobuf/proto is deprecated: Use the "google.golang.org/protobuf/proto" package instead. (staticcheck)
        "github.com/golang/protobuf/proto"
        ^
make: *** [Makefile.common:192: common-lint] Error 1

the first two are simple, the others not so much.

The reason why these packages import github.com/golang/protobuf/proto is to have access to proto.MarshalTextString, which has been removed. That's replaced by prototext.Marshal, but that in turn requires that the Go code be generated with a newer protoc-gen-go, so this affects prometheus/client_model.

@SuperQ
Copy link
Member

SuperQ commented Jun 3, 2021

Yes, there's a bunch of stuff to untangle here.

CC @kakkoyun

@mem
Copy link
Contributor Author

mem commented Jul 15, 2021

This has become a nuisance. For now, I'm going to add comments to turn off the errors.

mem added a commit that referenced this issue Jul 15, 2021
The linter is correctly flagging this bit:

	fmt.Fprintf(w, ExpectedMessage)

because it's a formatting function that doesn't appear to be formatting
anything (since there are not additional arguments after the formatting
string).

staticcheck has started flagging this import:

	"github.com/golang/protobuf/proto"

because that package has been rewritten and published as
google.golang.org/protobuf. The new package is not a drop-in replacement
for the old one, even if the staticcheck diagnostic suggests otherwise.
Newer versions of the old package are actually implemented in terms of
the new one. For this code base in particular, some of the text
marshalling functions have disappeared in the new package and they need
to be implemented in terms of the new reflection package. Until an
adequate solution is found, simply ignore the error.

Closes: #281

Signed-off-by: Marcelo E. Magallon <marcelo.magallon@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants