gojp / goreportcard

A report card for your Go application
https://goreportcard.com
Apache License 2.0
1.99k stars 252 forks source link

gofmt warns on Go 1.17+ `//go:build` lines #363

Open kolyshkin opened 2 years ago

kolyshkin commented 2 years ago

We have recently added Go 1.17+ go:build tags to runc (see https://github.com/opencontainers/runc/pull/3185/commits/d8da00355e305f0564a9de6854bf5df1aa4b73d9) and that resulted in goreportcard giving warnings on gofmt -s, like this (from https://goreportcard.com/report/github.com/opencontainers/runc):

gofmt 40%
Gofmt formats Go programs. We run gofmt -s on your code, where -s is for the "simplify" command

runc/libcontainer/configs/namespaces_unsupported.go
Line 1: warning: file is not gofmted with -s (gofmt)
runc/libcontainer/network_linux.go
Line 1: warning: file is not gofmted with -s (gofmt)

Apparently, some older version of gofmt is used, which does not understand go:build tags.

kolyshkin commented 2 years ago

Same issue with another repo: https://goreportcard.com/report/github.com/moby/moby

shawnps commented 2 years ago

@kolyshkin I think this may be the same issue as https://github.com/gojp/goreportcard/issues/364, meaning these errors wouldn't show up if a new version were released (not totally sure though).

kolyshkin commented 2 years ago

Hmm. Now the links in warnings point out to v1.0.2 release, which does not have //go:build tags, so I'm totally unsure what it complains about.

shawnps commented 2 years ago

@kolyshkin I took one file and ran gofmt -s -w on it as a test:

libcontainer/apparmor/apparmor_unsupported.go

Before gofmt -s:

// +build !linux

package apparmor

func isEnabled() bool {
    return false
}

func applyProfile(name string) error {
    if name != "" {
        return ErrApparmorNotEnabled
    }
    return nil
}

After:

//go:build !linux
// +build !linux

package apparmor

func isEnabled() bool {
    return false
}

func applyProfile(name string) error {
    if name != "" {
        return ErrApparmorNotEnabled
    }
    return nil
}
kolyshkin commented 2 years ago

Ah, so it's vice versa -- it doesn't like the absense of go:build tags.

This is going to be a problem with lots of packages I guess, since those tags are not really required until at least Go 1.19.

OTOH this points out to a potential future problem, so maybe it's not a bug, it's a feature!

gen2brain commented 2 years ago

I am using just //go:build tags, have go 1.17 requirement in go.mod and still get this warning. I have tags in 250+ files so I get a bad report.

I cannot even add old style with gofmt (i.e. add just blank // +build and gofmt should do the job) because it is "too complex":

//go:build ((aix || dragonfly || freebsd || linux || netbsd || openbsd || solaris || illumos) && !motif) || gtk
// +build error: expression too complex for // +build lines

I would not want to maintain this manually in two different styles where the old one is not readable at all, and I already need some other features from Go 1.17.