Skip to content

Commit

Permalink
Code Quality Improvements (bazel-contrib#1197)
Browse files Browse the repository at this point in the history
  • Loading branch information
sluongng authored Apr 11, 2022
1 parent e0dc92a commit ec1c643
Show file tree
Hide file tree
Showing 78 changed files with 400 additions and 357 deletions.
1 change: 1 addition & 0 deletions cmd/autogazelle/client_unix.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build darwin || dragonfly || freebsd || linux || nacl || netbsd || openbsd || solaris || windows
// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris windows

/* Copyright 2018 The Bazel Authors. All rights reserved.
Expand Down
3 changes: 2 additions & 1 deletion cmd/autogazelle/server_unix.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build darwin || dragonfly || freebsd || linux || nacl || netbsd || openbsd || solaris || windows
// +build darwin dragonfly freebsd linux nacl netbsd openbsd solaris windows

/* Copyright 2018 The Bazel Authors. All rights reserved.
Expand Down Expand Up @@ -66,7 +67,7 @@ func startServer() error {
// with SIGINT or SIGTERM.
func runServer() error {
// Begin logging to the log file.
logFile, err := os.OpenFile(*logPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666)
logFile, err := os.OpenFile(*logPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o666)
if err != nil {
return err
}
Expand Down
12 changes: 5 additions & 7 deletions cmd/fetch_repo/fetch_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@ import (
"golang.org/x/tools/go/vcs"
)

var (
root = &vcs.RepoRoot{
VCS: vcs.ByCmd("git"),
Repo: "https://github.com/bazeltest/rules_go",
Root: "github.com/bazeltest/rules_go",
}
)
var root = &vcs.RepoRoot{
VCS: vcs.ByCmd("git"),
Repo: "https://github.com/bazeltest/rules_go",
Root: "github.com/bazeltest/rules_go",
}

func TestMain(m *testing.M) {
// Replace vcs.RepoRootForImportPath to disable any network calls.
Expand Down
4 changes: 2 additions & 2 deletions cmd/fetch_repo/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func fetchModule(dest, importpath, version, sum string) error {
// Download the module. In Go 1.11, this command must be run in a module,
// so we create a dummy module in the current directory (which should be
// empty).
w, err := os.OpenFile("go.mod", os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0666)
w, err := os.OpenFile("go.mod", os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o666)
if err != nil {
return fmt.Errorf("error creating temporary go.mod: %v", err)
}
Expand Down Expand Up @@ -132,7 +132,7 @@ func copyTree(destRoot, srcRoot string) error {
dest := filepath.Join(destRoot, rel)

if info.IsDir() {
return os.Mkdir(dest, 0777)
return os.Mkdir(dest, 0o777)
} else {
r, err := os.Open(src)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions cmd/gazelle/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/pmezard/go-difflib/difflib"
)

var exitError = fmt.Errorf("encountered changes while running diff")
var errExit = fmt.Errorf("encountered changes while running diff")

func diffFile(c *config.Config, f *rule.File) error {
rel, err := filepath.Rel(c.RepoRoot, f.Path)
Expand Down Expand Up @@ -60,7 +60,7 @@ func diffFile(c *config.Config, f *rule.File) error {
}

if len(f.Content) != 0 {
diff.A = difflib.SplitLines(string(f.Content))
diff.A = difflib.SplitLines(string(f.Content))
}

diff.B = difflib.SplitLines(string(newContent))
Expand All @@ -80,7 +80,7 @@ func diffFile(c *config.Config, f *rule.File) error {
return fmt.Errorf("error diffing %s: %v", f.Path, err)
}
if ds, _ := difflib.GetUnifiedDiffString(diff); ds != "" {
return exitError
return errExit
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion cmd/gazelle/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ func TestDiffExisting(t *testing.T) {
Content: `
# gazelle:prefix example.com/hello
`,
}, {
},
{
Path: "hello.go",
Content: `package hello`,
},
Expand Down
13 changes: 4 additions & 9 deletions cmd/gazelle/fix-update.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,6 @@ type visitRecord struct {
mappedKindInfo map[string]rule.KindInfo
}

type byPkgRel []visitRecord

func (vs byPkgRel) Len() int { return len(vs) }
func (vs byPkgRel) Less(i, j int) bool { return vs[i].pkgRel < vs[j].pkgRel }
func (vs byPkgRel) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] }

var genericLoads = []rule.LoadInfo{
{
Name: "@bazel_gazelle//:def.bzl",
Expand Down Expand Up @@ -306,7 +300,8 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
RegularFiles: regularFiles,
GenFiles: genFiles,
OtherEmpty: empty,
OtherGen: gen})
OtherGen: gen,
})
if len(res.Gen) != len(res.Imports) {
log.Panicf("%s: language %s generated %d rules but returned %d imports", rel, l.Name(), len(res.Gen), len(res.Imports))
}
Expand Down Expand Up @@ -387,15 +382,15 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
for _, v := range visits {
merger.FixLoads(v.file, applyKindMappings(v.mappedKinds, loads))
if err := uc.emit(v.c, v.file); err != nil {
if err == exitError {
if err == errExit {
exit = err
} else {
log.Print(err)
}
}
}
if uc.patchPath != "" {
if err := ioutil.WriteFile(uc.patchPath, uc.patchBuffer.Bytes(), 0666); err != nil {
if err := ioutil.WriteFile(uc.patchPath, uc.patchBuffer.Bytes(), 0o666); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/gazelle/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ func fixFile(c *config.Config, f *rule.File) error {
return nil
}
outPath := findOutputPath(c, f)
if err := os.MkdirAll(filepath.Dir(outPath), 0777); err != nil {
if err := os.MkdirAll(filepath.Dir(outPath), 0o777); err != nil {
return err
}
if err := ioutil.WriteFile(outPath, newContent, 0666); err != nil {
if err := ioutil.WriteFile(outPath, newContent, 0o666); err != nil {
return err
}
f.Content = newContent
Expand Down
39 changes: 25 additions & 14 deletions cmd/gazelle/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@ func TestMain(m *testing.M) {
// to any files that don't have it. Files and directories in the module cache
// are read-only, and on Windows, the read-only bit prevents deletion and
// prevents Bazel from cleaning up the source tree.
filepath.Walk(tmpDir, func(path string, info os.FileInfo, err error) error {
_ = filepath.Walk(tmpDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if mode := info.Mode(); mode&0200 == 0 {
err = os.Chmod(path, mode|0200)
if mode := info.Mode(); mode&0o200 == 0 {
err = os.Chmod(path, mode|0o200)
}
return err
})
Expand Down Expand Up @@ -112,12 +112,14 @@ func TestCreateFile(t *testing.T) {
defer os.RemoveAll(dir)

goFile := filepath.Join(dir, "main.go")
if err = ioutil.WriteFile(goFile, []byte("package main"), 0600); err != nil {
if err = ioutil.WriteFile(goFile, []byte("package main"), 0o600); err != nil {
t.Fatalf("error writing file %q: %v", goFile, err)
}

// Check that Gazelle creates a new file named "BUILD.bazel".
run(dir, defaultArgs(dir))
if err = run(dir, defaultArgs(dir)); err != nil {
t.Fatalf("run failed: %v", err)
}

buildFile := filepath.Join(dir, "BUILD.bazel")
if _, err = os.Stat(buildFile); err != nil {
Expand All @@ -135,17 +137,20 @@ func TestUpdateFile(t *testing.T) {
defer os.RemoveAll(dir)

goFile := filepath.Join(dir, "main.go")
if err = ioutil.WriteFile(goFile, []byte("package main"), 0600); err != nil {
if err = ioutil.WriteFile(goFile, []byte("package main"), 0o600); err != nil {
t.Fatalf("error writing file %q: %v", goFile, err)
}

buildFile := filepath.Join(dir, "BUILD")
if err = ioutil.WriteFile(buildFile, nil, 0600); err != nil {
if err = ioutil.WriteFile(buildFile, nil, 0o600); err != nil {
t.Fatalf("error writing file %q: %v", buildFile, err)
}

// Check that Gazelle updates the BUILD file in place.
run(dir, defaultArgs(dir))
if err = run(dir, defaultArgs(dir)); err != nil {
t.Fatalf("run failed: %v", err)
}

if st, err := os.Stat(buildFile); err != nil {
t.Errorf("could not stat BUILD: %v", err)
} else if st.Size() == 0 {
Expand All @@ -167,7 +172,7 @@ func TestNoChanges(t *testing.T) {
defer os.RemoveAll(dir)

goFile := filepath.Join(dir, "main.go")
if err = ioutil.WriteFile(goFile, []byte("package main"), 0600); err != nil {
if err = ioutil.WriteFile(goFile, []byte("package main"), 0o600); err != nil {
t.Fatalf("error writing file %q: %v", goFile, err)
}

Expand All @@ -186,7 +191,7 @@ go_binary(
embed = [":go_default_library"],
visibility = ["//visibility:public"],
)
`), 0600); err != nil {
`), 0o600); err != nil {
t.Fatalf("error writing file %q: %v", buildFile, err)
}
st, err := os.Stat(buildFile)
Expand All @@ -196,7 +201,10 @@ go_binary(
modTime := st.ModTime()

// Ensure that Gazelle does not write to the BUILD file.
run(dir, defaultArgs(dir))
if err = run(dir, defaultArgs(dir)); err != nil {
t.Fatalf("run failed: %v", err)
}

if st, err := os.Stat(buildFile); err != nil {
t.Errorf("could not stat BUILD: %v", err)
} else if !modTime.Equal(st.ModTime()) {
Expand Down Expand Up @@ -228,7 +236,8 @@ package main
func main() {}
`,
}, {
},
{
Path: "out/BUILD",
Content: `this should get replaced`,
},
Expand Down Expand Up @@ -388,12 +397,14 @@ go_library(
defer cleanup()

// Check that Gazelle does not update the BUILD file, due to lang filter.
run(dir, []string{
if err := run(dir, []string{
"-repo_root", dir,
"-go_prefix", "example.com/repo",
"-lang=proto",
dir,
})
}); err != nil {
t.Fatalf("run failed: %v", err)
}

testtools.CheckFiles(t, dir, fixture)
}
2 changes: 1 addition & 1 deletion cmd/gazelle/gazelle.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func main() {
}

if err := run(wd, os.Args[1:]); err != nil && err != flag.ErrHelp {
if err == exitError {
if err == errExit {
os.Exit(1)
} else {
log.Fatal(err)
Expand Down
Loading

0 comments on commit ec1c643

Please sign in to comment.