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

Jump to bottom

cmd/vet: check files not in current build context #29901

Closed
nhooyr opened this issue Jan 23, 2019 · 4 comments
Closed

cmd/vet: check files not in current build context #29901

nhooyr opened this issue Jan 23, 2019 · 4 comments
Labels
FrozenDueToAge

Comments

@nhooyr
Copy link
Contributor

nhooyr commented Jan 23, 2019

Lets say this file was in a package (it's used to go generate a revision file):

// +build ignore

package main

import (
	"context"
	"fmt"
	"go/format"
	"io/ioutil"
	"log"
	"os/exec"
	"strings"
	"time"
)

func main() {
	log.SetFlags(0)

	log.Println("%v", 3)
	revision := revision()
	writeRevision(revision)
}

func revision() string {
	ctx := context.Background()
	ctx, cancel := context.WithTimeout(ctx, time.Second*30)
	defer cancel()

	git := exec.CommandContext(ctx, "git", "describe", "--dirty", "--always")
	revisionBytes, err := git.Output()
	if err != nil {
		log.Fatalf("failed to run `git describe`: %v", err)
	}

	revision := string(revisionBytes)
	return strings.TrimSpace(revision)
}

func writeRevision(revision string) {
	file := revisionFile(revision)

	formattedFile, err := format.Source(file)
	if err != nil {
		log.Fatalf("failed to format revision file %q: %v", string(file), err)
	}

	err = ioutil.WriteFile("revision.go", formattedFile, 0644)
	if err != nil {
		log.Fatalf("failed to write revision.go: %v", err)
	}
}

func revisionFile(revision string) []byte {
	tmpl := `package main

//go:generate go run revision_gen.go
var revision = %q
`

	return []byte(fmt.Sprintf(tmpl, revision))
}

go veting the package would not also vet this file. You can confirm by passing the file in explicitly afterwards and seeing vet complain about line 19.
@ianlancetaylor ianlancetaylor changed the title vet: check files not in current build context cmd/vet: check files not in current build context Jan 24, 2019
@ianlancetaylor
Copy link
Contributor

It's generally correct for cmd/vet to ignore files with a +build ignore build constraint. vet respects build tags.

You should be able to run vet on that file via go vet ignoredfile.go.

@nhooyr
Copy link
Contributor Author

nhooyr commented Jan 24, 2019

Why is that considered correct? vet isn't building those files, its vetting them and that should happen to all files regardless of whether or not they are in the current build. Vet reports the package's correctness, which should include reporting on files that will not be built on the current platform.

@ianlancetaylor
Copy link
Contributor

vet type checks the package, which can't be done if it reads files with ignored build tags. The type checking enables much more powerful checks.

I'm not saying that it's not useful to look at all files, just that vet is not the tool for that.

@FiloSottile
Copy link
Contributor

Agreed, when vet is asked to vet a package, it should look only at the files that make up that package. This is even more necessary now that vet gets complete type information.

If you want vet to look at a different set of files, you can pass the list to it just like you can do to go build. Even if we wanted to check all files, we can't know which combination of them would compile. In your example that file should be compiled on its own, while in other cases the extra files should be considered as part of the package they are in.

@golang golang locked and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@FiloSottile @ianlancetaylor @gopherbot @nhooyr