Cleanup of various lint issues across the code base
Fixing a variety of issues discovered by running staticcheck locally,
along with other linting issues found via golangci-lint in my IDE.
This does NOT eliminate all of the findings that staticcheck has in
its default configuration, just significantly reduces them. Some
issues require more extensive changes/investigation than I was willing
to put into this CL, or probably should just be suppressed since it is
unlikely we will fix them.
Types of issues identified by staticcheck and fixed:
- should use fmt.Errorf(...) instead of errors.New(fmt.Sprintf(...)) (S1028)
- var <name> is unused (U1000)
- func <name> is unused (U1000)
- type <name> is unused (U1000)
- this value of err is never used (SA4006)
- package <name> is being imported more than once (ST1019)
- unnecessary assignment to the blank identifier (S1005)
- Various "io/ioutil" has been deprecated since Go 1.19
- reflect.PtrTo has been deprecated since Go 1.22
- should use time.Since instead of time.Now().Sub (S1012)
- should use copy(to, from) instead of a loop (S1001)
Other types of issues fixed:
- Various pkg/var name collisions ('auth', 'git', 'gerrit', etc)
- Various direct comparison against errors instead of using errors.Is()
- Redundant conversions, e.g. string("literal")
- Struct <name> has methods on both value and pointer receivers,
i.e. mixing (c Cmd) and (c *Cmd), there seemed to be lots of false
positives for this, not clear why
- Ineffectual assignments, e.g. val := "", then immediately assigning
to val, sometimes these assignments hid unused variables
- Instances where types could be omitted
- Snake case naming
I intentionally did not fix the following findings:
- error strings should not be capitalized (ST1005), since a lot of the
existing tests are doing string comparison for error checking, so
would need to be updated also
- strings.Title being deprecated, since it requires a new library call
to fix, which has different semantics
- Fixing slices being initialized with empty literal instead of nil,
e.g. var := []int{}. Technically nil for a slice can be appended to
like an empty slice and some other usages and potentially save an
allocation if not used, but in other cases, like working with JSON
marshaling/unmarshaling nil vs an empty slice have semantic
differences. I think the nil-behaviour of slices was a mistake,
since it is a bit of a gotcha and really only exists for a niche
optimization.
- Unused params, which could be fixed via '_'. There are hundreds of
these in the code base and it is debatable if they should be fixed.
- Warnings about documentation, broadly ignored, since often they were
related to having a '// TODO...' with no other documentation. Fixed
examples that just needed moving text around instead of adding
documentation
- Unhand-led errors, since these are often just being used in defer or
from a printing statement, and wrapping them to be explicitly
ignored is verbose
- Some examples of unused vars in tests, because they smelt of
incomplete tests, i.e. there should have cases that used
them.
- Some name collisions, especially with 'len' and 'delete', because
they smelt of overly smart meta-programming going on, and I didn't
feel like sussing out what was going on
Bug: 439814281
Change-Id: Ib828330bfcffb1290fc15909218fb1eb432c45b5
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/258134
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@google.com>
Commit-Queue: Brian Sheedy <bsheedy@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
diff --git a/tools/src/bench/bench.go b/tools/src/bench/bench.go
index 4f442a4..f0273f3 100644
--- a/tools/src/bench/bench.go
+++ b/tools/src/bench/bench.go
@@ -93,13 +93,13 @@
type Parser = func(s string) (Run, error)
for _, parser := range []Parser{parseConsole, parseJSON} {
r, err := parser(s)
- switch err {
- case nil:
- return r, nil
- case errWrongFormat:
- default:
+ if err != nil {
+ if errors.Is(err, errWrongFormat) {
+ continue
+ }
return Run{}, err
}
+ return r, nil
}
return Run{}, errors.New("Unrecognised file format")
@@ -318,21 +318,21 @@
// Draw table
b := &strings.Builder{}
- horizontal_bar := func() {
+ horizontalBar := func() {
for i := range header {
fmt.Fprintf(b, "+%v", strings.Repeat("-", 2+widths[i]))
}
fmt.Fprintln(b, "+")
}
- horizontal_bar()
+ horizontalBar()
for i, h := range header {
fmt.Fprintf(b, "| %v ", pad(h, widths[i]))
}
fmt.Fprintln(b, "|")
- horizontal_bar()
+ horizontalBar()
for _, row := range columns {
for i, cell := range row {
@@ -341,7 +341,7 @@
fmt.Fprintln(b, "|")
}
- horizontal_bar()
+ horizontalBar()
return b.String()
}
diff --git a/tools/src/cmd/add-gerrit-hashtags/main.go b/tools/src/cmd/add-gerrit-hashtags/main.go
index d65e2da..76bb746 100644
--- a/tools/src/cmd/add-gerrit-hashtags/main.go
+++ b/tools/src/cmd/add-gerrit-hashtags/main.go
@@ -53,14 +53,13 @@
)
var (
- repoFlag = flag.String("repo", "dawn", "the project (tint or dawn)")
- userFlag = flag.String("user", defaultUser(), "user name / email")
- afterFlag = flag.String("after", "", "start date")
- beforeFlag = flag.String("before", "", "end date")
- daysFlag = flag.Int("days", 30, "interval in days (used if --after is not specified)")
- verboseFlag = flag.Bool("v", false, "verbose mode - lists all the changes")
- dryrunFlag = flag.Bool("dry", false, "dry mode. Don't apply any changes")
- authFlags = authcli.Flags{}
+ repoFlag = flag.String("repo", "dawn", "the project (tint or dawn)")
+ userFlag = flag.String("user", defaultUser(), "user name / email")
+ afterFlag = flag.String("after", "", "start date")
+ beforeFlag = flag.String("before", "", "end date")
+ daysFlag = flag.Int("days", 30, "interval in days (used if --after is not specified)")
+ dryrunFlag = flag.Bool("dry", false, "dry mode. Don't apply any changes")
+ authFlags = authcli.Flags{}
)
func defaultUser() string {
@@ -119,12 +118,12 @@
}
ctx := context.Background()
- auth, err := authFlags.Options()
+ options, err := authFlags.Options()
if err != nil {
return err
}
- g, err := gerrit.New(ctx, auth, dawn.GerritURL)
+ g, err := gerrit.New(ctx, options, dawn.GerritURL)
if err != nil {
return err
}
@@ -179,10 +178,6 @@
return out
}
-func today() time.Time {
- return time.Now()
-}
-
func date(t time.Time) string {
return t.Format(yyyymmdd)
}
diff --git a/tools/src/cmd/auto-submit/main.go b/tools/src/cmd/auto-submit/main.go
index 56681ae..e81eab8 100644
--- a/tools/src/cmd/auto-submit/main.go
+++ b/tools/src/cmd/auto-submit/main.go
@@ -103,12 +103,12 @@
}
ctx := context.Background()
- auth, err := authFlags.Options()
+ options, err := authFlags.Options()
if err != nil {
return err
}
- g, err := gerrit.New(ctx, auth, dawn.GerritURL)
+ g, err := gerrit.New(ctx, options, dawn.GerritURL)
if err != nil {
return err
}
diff --git a/tools/src/cmd/benchdiff/main.go b/tools/src/cmd/benchdiff/main.go
index 87645c4..a8362fb 100644
--- a/tools/src/cmd/benchdiff/main.go
+++ b/tools/src/cmd/benchdiff/main.go
@@ -33,9 +33,7 @@
"errors"
"flag"
"fmt"
- "io/ioutil"
"os"
- "path/filepath"
"time"
"dawn.googlesource.com/dawn/tools/src/bench"
@@ -69,7 +67,7 @@
}
func run(pathA, pathB string) error {
- fileA, err := ioutil.ReadFile(pathA)
+ fileA, err := os.ReadFile(pathA)
if err != nil {
return err
}
@@ -78,7 +76,7 @@
return err
}
- fileB, err := ioutil.ReadFile(pathB)
+ fileB, err := os.ReadFile(pathB)
if err != nil {
return err
}
@@ -102,8 +100,3 @@
return nil
}
-
-func fileName(path string) string {
- _, name := filepath.Split(path)
- return name
-}
diff --git a/tools/src/cmd/check-spec-examples/main.go b/tools/src/cmd/check-spec-examples/main.go
index 7350ea2..031bd85 100644
--- a/tools/src/cmd/check-spec-examples/main.go
+++ b/tools/src/cmd/check-spec-examples/main.go
@@ -43,7 +43,6 @@
"flag"
"fmt"
"io"
- "io/ioutil"
"net/http"
"net/url"
"os"
@@ -79,16 +78,15 @@
}
err := run()
- switch err {
- case nil:
- return
- case errInvalidArg:
- fmt.Fprintf(os.Stderr, "Error: %v\n\n", err)
- flag.Usage()
- default:
- fmt.Fprintf(os.Stderr, "%v\n", err)
+ if err != nil {
+ if errors.Is(err, errInvalidArg) {
+ fmt.Fprintf(os.Stderr, "Error: %v\n\n", err)
+ flag.Usage()
+ } else {
+ fmt.Fprintf(os.Stderr, "%v\n", err)
+ }
+ os.Exit(1)
}
- os.Exit(1)
}
func run() error {
@@ -171,7 +169,7 @@
}
// Create a temporary directory to hold the examples as separate files
- tmpDir, err := ioutil.TempDir("", "wgsl-spec-examples")
+ tmpDir, err := os.MkdirTemp("", "wgsl-spec-examples")
if err != nil {
return err
}
@@ -241,7 +239,7 @@
func compile(compiler, wd, name, code string) error {
filename := name + ".wgsl"
path := filepath.Join(wd, filename)
- if err := ioutil.WriteFile(path, []byte(code), 0666); err != nil {
+ if err := os.WriteFile(path, []byte(code), 0666); err != nil {
return fmt.Errorf("Failed to write example file '%v'", path)
}
cmd := exec.Command(compiler, filename)
@@ -301,7 +299,7 @@
}
}
-// hasClass returns true iff node is has the given "class" attribute.
+// hasClass returns true iff node has the given "class" attribute.
func hasClass(node *html.Node, class string) bool {
for _, attr := range node.Attr {
if attr.Key == "class" {
diff --git a/tools/src/cmd/cts/common/config.go b/tools/src/cmd/cts/common/config.go
index 4cf4ee4..776f327 100644
--- a/tools/src/cmd/cts/common/config.go
+++ b/tools/src/cmd/cts/common/config.go
@@ -31,7 +31,7 @@
"bytes"
"encoding/json"
"fmt"
- "io/ioutil"
+ "os"
"dawn.googlesource.com/dawn/tools/src/buildbucket"
"dawn.googlesource.com/dawn/tools/src/cts/result"
@@ -99,7 +99,7 @@
// LoadConfig loads the JSON config file at the given path
func LoadConfig(path string) (*Config, error) {
- data, err := ioutil.ReadFile(path)
+ data, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to open '%v': %w", path, err)
}
diff --git a/tools/src/cmd/cts/common/export.go b/tools/src/cmd/cts/common/export.go
index 9294225..f1c2dcf 100644
--- a/tools/src/cmd/cts/common/export.go
+++ b/tools/src/cmd/cts/common/export.go
@@ -88,6 +88,9 @@
// Fetch the table column names
columns, err := fetchRow[string](s, spreadsheet, dataSheet, 0)
+ if err != nil {
+ return err
+ }
counts := container.NewMap[string, int]()
for _, r := range results {
@@ -96,7 +99,7 @@
// Check the unimplemented query tree to see if this should be classed as
// 'Unimplemented' instead.
if node := unimplemented.Get(r.Query); node != nil && node.Data != nil {
- counts[string("Unimplemented")]++
+ counts[("Unimplemented")]++
continue
}
}
@@ -154,16 +157,6 @@
return fmt.Sprintf("%v!A%v:%v", sheet.Properties.Title, index+1, index+1)
}
-// columnRange returns a sheets range ("name!i1:i") for the entire column with
-// the given index.
-func columnRange(index int, sheet *sheets.Sheet) string {
- col := 'A' + index
- if index > 25 {
- panic("UNIMPLEMENTED")
- }
- return fmt.Sprintf("%v!%c1:%c", sheet.Properties.Title, col, col)
-}
-
// fetchRow returns all the values in the given sheet's row.
func fetchRow[T any](srv *sheets.Service, spreadsheet *sheets.Spreadsheet, sheet *sheets.Sheet, row int) ([]T, error) {
rng := rowRange(row, sheet)
diff --git a/tools/src/cmd/cts/common/results.go b/tools/src/cmd/cts/common/results.go
index d2d840c..571246d 100644
--- a/tools/src/cmd/cts/common/results.go
+++ b/tools/src/cmd/cts/common/results.go
@@ -29,7 +29,6 @@
import (
"context"
- "errors"
"flag"
"fmt"
"log"
@@ -130,17 +129,17 @@
// CTS roll.
if ps.Change == 0 {
fmt.Println("no change specified, scanning gerrit for last CTS roll...")
- gerrit, err := gerrit.New(ctx, auth, cfg.Gerrit.Host)
+ instance, err := gerrit.New(ctx, auth, cfg.Gerrit.Host)
if err != nil {
return nil, err
}
- latest, err := LatestCTSRoll(gerrit)
+ latest, err := LatestCTSRoll(instance)
if err != nil {
return nil, err
}
fmt.Printf("scanning for latest patchset of %v...\n", latest.Number)
var resultsByExecutionMode result.ResultsByExecutionMode
- resultsByExecutionMode, *ps, err = getRecentResults(ctx, cfg, r.CacheDir, gerrit, bb, client, latest.Number)
+ resultsByExecutionMode, *ps, err = getRecentResults(ctx, cfg, r.CacheDir, instance, bb, client, latest.Number)
if err != nil {
return nil, err
}
@@ -151,11 +150,11 @@
// If a change, but no patchset was specified, then query the most recent
// patchset.
if ps.Patchset == 0 {
- gerrit, err := gerrit.New(ctx, auth, cfg.Gerrit.Host)
+ instance, err := gerrit.New(ctx, auth, cfg.Gerrit.Host)
if err != nil {
return nil, err
}
- *ps, err = gerrit.LatestPatchset(strconv.Itoa(ps.Change))
+ *ps, err = instance.LatestPatchset(strconv.Itoa(ps.Change))
if err != nil {
err := fmt.Errorf("failed to find latest patchset of change %v: %w",
ps.Change, err)
@@ -351,7 +350,7 @@
}
if !strings.HasPrefix(r.TestId, prefix) {
- return errors.New(fmt.Sprintf("Test ID %s did not start with %s even though query should have filtered.", r.TestId, prefix))
+ return fmt.Errorf("Test ID %s did not start with %s even though query should have filtered.", r.TestId, prefix)
}
testName := r.TestId[len(prefix):]
@@ -588,7 +587,7 @@
return resultsByExecutionMode["compat"], nil
}
-// CacheRecentRexpectationAffectedCiResults looks in the cache at 'cacheDir' for
+// CacheRecentUniqueSuppressedResults looks in the cache at 'cacheDir' for
// CI results from the recent history. If the cache contains the results, they
// are loaded and returned. If the cache does not contain the results, they are
// fetched, cleaned with CleanResults(), saved to the cache directory, and
diff --git a/tools/src/cmd/cts/expectationcoverage/expectationcoverage.go b/tools/src/cmd/cts/expectationcoverage/expectationcoverage.go
index fee6657..eac6877 100644
--- a/tools/src/cmd/cts/expectationcoverage/expectationcoverage.go
+++ b/tools/src/cmd/cts/expectationcoverage/expectationcoverage.go
@@ -68,11 +68,11 @@
Count int
}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "expectation-coverage"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return "checks how much test coverage is lost due to expectations"
}
@@ -131,7 +131,7 @@
return err
}
if c.flags.verbose {
- fmt.Printf("Took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Took %s\n", time.Since(startTime).String())
fmt.Printf("Got %d chunks/individual expectations\n", len(content.Chunks))
}
@@ -150,7 +150,7 @@
return err
}
if c.flags.verbose {
- fmt.Printf("Took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Took %s\n", time.Since(startTime).String())
fmt.Printf("Got %d unique results\n", len(uniqueResults))
}
@@ -159,7 +159,7 @@
startTime = time.Now()
orderedChunks := getChunksOrderedByCoverageLoss(&content, &uniqueResults)
if c.flags.verbose {
- fmt.Printf("Took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Took %s\n", time.Since(startTime).String())
}
// Output results.
@@ -312,7 +312,7 @@
uniqueResults *result.List) []ChunkWithCounter {
affectedChunks := make([]ChunkWithCounter, len(content.Chunks))
- for i, _ := range content.Chunks {
+ for i := range content.Chunks {
affectedChunks[i].Chunk = &(content.Chunks[i])
}
@@ -328,7 +328,7 @@
// Each of the ChunkWithCounter will have its Count filled in place by a
// worker when picked up.
- for i, _ := range affectedChunks {
+ for i := range affectedChunks {
workQueue <- &(affectedChunks[i])
}
close(workQueue)
@@ -415,9 +415,9 @@
numApplicableResults := 0
for i := workerNumber; i < len(*uniqueResults); i += numWorkers {
- result := (*uniqueResults)[i]
+ r := (*uniqueResults)[i]
for _, expectation := range chunkWithCounter.Chunk.Expectations {
- if expectation.AppliesToResult(result) {
+ if expectation.AppliesToResult(r) {
numApplicableResults += 1
break
}
diff --git a/tools/src/cmd/cts/roll/roll.go b/tools/src/cmd/cts/roll/roll.go
index b5d54d9..2c9dfe3 100644
--- a/tools/src/cmd/cts/roll/roll.go
+++ b/tools/src/cmd/cts/roll/roll.go
@@ -32,7 +32,7 @@
"encoding/json"
"flag"
"fmt"
- "io/ioutil"
+ "io"
"log"
"net/http"
"os"
@@ -104,11 +104,11 @@
flags rollerFlags
}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "roll"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return "roll CTS and re-generate expectations"
}
@@ -140,7 +140,7 @@
func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
// Validate command line arguments
- auth, err := c.flags.auth.Options()
+ options, err := c.flags.auth.Options()
if err != nil {
return fmt.Errorf("failed to obtain authentication options: %w", err)
}
@@ -160,11 +160,11 @@
// Create the various service clients and ensure required permissions are
// available.
- git, err := git.New(c.flags.gitPath)
+ gitInstance, err := git.New(c.flags.gitPath)
if err != nil {
return fmt.Errorf("failed to obtain authentication options: %w", err)
}
- gerrit, err := gerrit.New(ctx, auth, cfg.Gerrit.Host)
+ gerritInstance, err := gerrit.New(ctx, options, cfg.Gerrit.Host)
if err != nil {
return err
}
@@ -172,7 +172,7 @@
if err != nil {
return err
}
- bb, err := buildbucket.New(ctx, auth)
+ bb, err := buildbucket.New(ctx, options)
if err != nil {
return err
}
@@ -205,12 +205,12 @@
r := roller{
cfg: cfg,
flags: c.flags,
- auth: auth,
+ auth: options,
bb: bb,
parentSwarmingRunID: c.flags.parentSwarmingRunID,
client: client,
- git: git,
- gerrit: gerrit,
+ git: gitInstance,
+ gerrit: gerritInstance,
gitiles: gitilesRepos{dawn: dawn},
ctsDir: ctsDir,
}
@@ -542,7 +542,7 @@
}
defer resp.Body.Close()
- jsonResponse, err := ioutil.ReadAll(resp.Body)
+ jsonResponse, err := io.ReadAll(resp.Body)
if err != nil {
return err
}
diff --git a/tools/src/cmd/cts/treemap/treemap.go b/tools/src/cmd/cts/treemap/treemap.go
index bcf2d67..9f8366f 100644
--- a/tools/src/cmd/cts/treemap/treemap.go
+++ b/tools/src/cmd/cts/treemap/treemap.go
@@ -31,6 +31,7 @@
"bufio"
"context"
"encoding/json"
+ "errors"
"flag"
"fmt"
"io"
@@ -65,11 +66,11 @@
}
}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "treemap"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return `displays a treemap visualization of the CTS tests cases
mode:
@@ -93,30 +94,34 @@
// TODO(crbug.com/344014313): Split this up into testable helper functions and
// add coverage (browser.Open() likely prevents testing of Run() itself).
func (c *cmd) Run(ctx context.Context, cfg common.Config) error {
- ctx, stop := context.WithCancel(context.Background())
+ ctx, stop := context.WithCancel(ctx)
defer stop()
// Are we visualizing cases, or timings?
var data string
- var err error
switch flag.Arg(0) {
case "case", "cases":
- data, err = loadCasesData(c.flags.testQuery, cfg.OsWrapper)
+ d, err := loadCasesData(c.flags.testQuery, cfg.OsWrapper)
+ if err != nil {
+ return err
+ }
+ data = d
case "time", "times", "timing":
// Validate command line arguments
- auth, err := c.flags.auth.Options()
+ options, err := c.flags.auth.Options()
if err != nil {
return fmt.Errorf("failed to obtain authentication options: %w", err)
}
- data, err = loadTimingData(ctx, c.flags.source, cfg, auth)
+ d, err := loadTimingData(ctx, c.flags.source, cfg, options)
+ if err != nil {
+ return err
+ }
+ data = d
default:
- err = subcmd.ErrInvalidCLA
- }
- if err != nil {
- return err
+ return subcmd.ErrInvalidCLA
}
// Kick the server
@@ -149,13 +154,11 @@
browser.Open(url)
<-ctx.Done()
- err = server.Shutdown(ctx)
- switch err {
- case nil, context.Canceled:
- return nil
- default:
+ err := server.Shutdown(ctx)
+ if err != nil && !errors.Is(err, context.Canceled) {
return err
}
+ return nil
}
// TODO(crbug.com/344014313): Add unittests for this function.
@@ -190,10 +193,10 @@
fmt.Fprint(data, `"data":[`)
fmt.Fprint(data, `["Query", "Parent", "Number of tests", "Color"],`)
fmt.Fprint(data, `["root", null, 0, 0]`)
- for _, query := range queryCounts.Keys() {
+ for _, key := range queryCounts.Keys() {
fmt.Fprint(data, ",")
- count := queryCounts[query]
- if err := json.NewEncoder(data).Encode([]any{query, parentOfOrRoot(query), count, count}); err != nil {
+ count := queryCounts[key]
+ if err := json.NewEncoder(data).Encode([]any{key, parentOfOrRoot(key), count, count}); err != nil {
return "", err
}
}
@@ -246,10 +249,10 @@
fmt.Fprint(data, `"data":[`)
fmt.Fprint(data, `["Query", "Parent", "Time (ms)", "Color"],`)
fmt.Fprint(data, `["root", null, 0, 0]`)
- for _, query := range queryTimes.Keys() {
+ for _, key := range queryTimes.Keys() {
fmt.Fprint(data, ",")
- d := queryTimes[query].average()
- if err := json.NewEncoder(data).Encode([]any{query, parentOfOrRoot(query), d.Milliseconds(), d.Milliseconds()}); err != nil {
+ d := queryTimes[key].average()
+ if err := json.NewEncoder(data).Encode([]any{key, parentOfOrRoot(key), d.Milliseconds(), d.Milliseconds()}); err != nil {
return "", err
}
}
diff --git a/tools/src/cmd/cts/update/testlist/testlist.go b/tools/src/cmd/cts/update/testlist/testlist.go
index 19d4e7d..4d82f83 100644
--- a/tools/src/cmd/cts/update/testlist/testlist.go
+++ b/tools/src/cmd/cts/update/testlist/testlist.go
@@ -29,29 +29,16 @@
import (
"context"
- "flag"
- "os"
- "strings"
-
"dawn.googlesource.com/dawn/tools/src/cmd/cts/common"
"dawn.googlesource.com/dawn/tools/src/fileutils"
+ "flag"
+ "os"
)
func init() {
common.Register(&cmd{})
}
-type arrayFlags []string
-
-func (i *arrayFlags) String() string {
- return strings.Join((*i), " ")
-}
-
-func (i *arrayFlags) Set(value string) error {
- *i = append(*i, value)
- return nil
-}
-
type cmd struct {
flags struct {
ctsDir string
@@ -61,11 +48,11 @@
}
}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "update-testlist"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return "updates a CTS test list file"
}
diff --git a/tools/src/cmd/fix-tests/main.go b/tools/src/cmd/fix-tests/main.go
index f5ea3f2..2680f2c 100644
--- a/tools/src/cmd/fix-tests/main.go
+++ b/tools/src/cmd/fix-tests/main.go
@@ -32,7 +32,6 @@
"encoding/json"
"flag"
"fmt"
- "io/ioutil"
"os"
"os/exec"
"path/filepath"
@@ -78,7 +77,7 @@
wd := filepath.Dir(exe) // The directory holding the test exe
// Create a temporary directory to hold the 'test-results.json' file
- tmpDir, err := ioutil.TempDir("", "fix-tests")
+ tmpDir, err := os.MkdirTemp("", "fix-tests")
if err != nil {
return err
}
@@ -174,55 +173,56 @@
return mr.end - mr.start
}
-// Look for 'search_str' in the 'doc_string' and return the longest matching substring of 'search_str' as start/end indices
-func longestSubstringMatch(search_str string, doc_string string) MatchRange {
+// longestSubstringMatch finds the longest matching substring of searchStr in docString and
+// returns the start/end indices
+func longestSubstringMatch(searchStr string, docString string) MatchRange {
// Brute force algorithm is n*m for string sizes n and m
- best_idx_start := 0
- best_idx_end := 0 // exclusive
+ bestIdxStart := 0
+ bestIdxEnd := 0 // exclusive
// Scan the document with the search string from the highest offset to the lowest. This will be out of bound for search string but that is because we are looking of a substring.
- for doc_offset := -len(search_str); doc_offset < len(doc_string)+len(search_str); doc_offset++ {
- curr_idx_start := 0
- curr_idx_end := 0 // exclusive
- for search_idx := 0; search_idx < len(search_str); search_idx++ {
- doc_offset := search_idx + doc_offset
- is_match := false
+ for docOffset := -len(searchStr); docOffset < len(docString)+len(searchStr); docOffset++ {
+ currIdxStart := 0
+ currIdxEnd := 0 // exclusive
+ for searchIdx := 0; searchIdx < len(searchStr); searchIdx++ {
+ docOffset := searchIdx + docOffset
+ isMatch := false
// basic range checking for the doc
- if doc_offset >= 0 && doc_offset < len(doc_string) {
- if search_str[search_idx] == doc_string[doc_offset] {
- is_match = true
+ if docOffset >= 0 && docOffset < len(docString) {
+ if searchStr[searchIdx] == docString[docOffset] {
+ isMatch = true
}
}
- if is_match {
- if curr_idx_end == 0 {
+ if isMatch {
+ if currIdxEnd == 0 {
// first time matching
- curr_idx_start = search_idx
- curr_idx_end = curr_idx_start + 1 // exclusive
+ currIdxStart = searchIdx
+ currIdxEnd = currIdxStart + 1 // exclusive
} else {
// continue current matching.
- curr_idx_end++
+ currIdxEnd++
}
}
// check if our match is the best
- best_size := best_idx_end - best_idx_start
- curr_size := curr_idx_end - curr_idx_start
+ bestSize := bestIdxEnd - bestIdxStart
+ currSize := currIdxEnd - currIdxStart
- if best_size < curr_size {
- best_idx_start = curr_idx_start
- best_idx_end = curr_idx_end
+ if bestSize < currSize {
+ bestIdxStart = currIdxStart
+ bestIdxEnd = currIdxEnd
}
- if !is_match {
+ if !isMatch {
// reset
- curr_idx_start = 0
- curr_idx_end = 0
+ currIdxStart = 0
+ currIdxEnd = 0
}
}
}
- return MatchRange{best_idx_start, best_idx_end}
+ return MatchRange{bestIdxStart, bestIdxEnd}
}
func processFailure(test, wd, failure string) error {
@@ -257,63 +257,63 @@
fix = func(testSource string) (string, error) {
// This code is written so it can insert changes as fixes for expects that only match a substring.
// An example of where this is required is the "glsl/writer/builtin_test.cc" due to the programmatic header (GlslHeader())
- // We don't know if a or b is the expected. We also dont know if it should be escaped for R"(...)" strings
- a_esc, b_esc := escape(a), escape(b)
+ // We don't know if a or b is the expected. We also don't know if it should be escaped for R"(...)" strings
+ aEsc, bEsc := escape(a), escape(b)
// Find the longest match. We have (unfortunately) 4 options.
- mr_a := longestSubstringMatch(a, testSource)
- mr_b := longestSubstringMatch(b, testSource)
- mr_a_esc := longestSubstringMatch(a_esc, testSource)
- mr_b_esc := longestSubstringMatch(b_esc, testSource)
+ mrA := longestSubstringMatch(a, testSource)
+ mrB := longestSubstringMatch(b, testSource)
+ mrAEsc := longestSubstringMatch(aEsc, testSource)
+ mrBEsc := longestSubstringMatch(bEsc, testSource)
// Perfect matches are prioritized for the cases where there may be more than matchable string in test function.
// This is common in tint where there is both the 'src' and 'expect' strings of non-failing transform test.
- var a_perfect = len(a) == Size(mr_a)
- var b_perfect = len(b) == Size(mr_b)
- var a_esc_perfect = len(a_esc) == Size(mr_a_esc)
- var b_esc_perfect = len(b_esc) == Size(mr_b_esc)
- var has_perfect = a_perfect || b_perfect || a_esc_perfect || b_esc_perfect
+ var aPerfect = len(a) == Size(mrA)
+ var bPerfect = len(b) == Size(mrB)
+ var aEscPerfect = len(aEsc) == Size(mrAEsc)
+ var bEscPerfect = len(bEsc) == Size(mrBEsc)
+ var hasPerfect = aPerfect || bPerfect || aEscPerfect || bEscPerfect
- use_largest := func(mr MatchRange) bool {
- return Size(mr) >= Size(mr_a) && Size(mr) >= Size(mr_b) &&
- Size(mr) >= Size(mr_a_esc) && Size(mr) >= Size(mr_b_esc) && !has_perfect
+ useLargest := func(mr MatchRange) bool {
+ return Size(mr) >= Size(mrA) && Size(mr) >= Size(mrB) &&
+ Size(mr) >= Size(mrAEsc) && Size(mr) >= Size(mrBEsc) && !hasPerfect
}
// assumed mr_b_esc is best match
- expected_str := b_esc
- replace_str := a_esc
- mr_largest := mr_b_esc
+ expectedStr := bEsc
+ replaceStr := aEsc
+ mrLargest := mrBEsc
- if use_largest(mr_a) || a_perfect {
- expected_str = a
- replace_str = b
- mr_largest = mr_a
- } else if use_largest(mr_b) || b_perfect {
- expected_str = b
- replace_str = a
- mr_largest = mr_b
- } else if use_largest(mr_a_esc) || a_esc_perfect {
- expected_str = a_esc
- replace_str = b_esc
- mr_largest = mr_a_esc
+ if useLargest(mrA) || aPerfect {
+ expectedStr = a
+ replaceStr = b
+ mrLargest = mrA
+ } else if useLargest(mrB) || bPerfect {
+ expectedStr = b
+ replaceStr = a
+ mrLargest = mrB
+ } else if useLargest(mrAEsc) || aEscPerfect {
+ expectedStr = aEsc
+ replaceStr = bEsc
+ mrLargest = mrAEsc
}
// trim away the number of unmatched characters from the end of expected to the end of the replacement.
- replace_str_end := len(replace_str) - (len(expected_str) - mr_largest.end)
- if replace_str_end >= mr_largest.start && replace_str_end <= len(replace_str) {
- replace_str = replace_str[mr_largest.start:replace_str_end]
- expected_str = expected_str[mr_largest.start:mr_largest.end]
+ replaceStrEnd := len(replaceStr) - (len(expectedStr) - mrLargest.end)
+ if replaceStrEnd >= mrLargest.start && replaceStrEnd <= len(replaceStr) {
+ replaceStr = replaceStr[mrLargest.start:replaceStrEnd]
+ expectedStr = expectedStr[mrLargest.start:mrLargest.end]
} else {
- // It is not safe to attempt a replace if the replacement string would have negative (nonsense) size.
- expected_str = ""
+ // It is not safe to attempt a replacement if the replacement string would have negative (nonsense) size.
+ expectedStr = ""
}
// Do not try to replace on empty strings.
- if len(expected_str) <= 0 {
+ if len(expectedStr) <= 0 {
return "", fmt.Errorf("could not fix 'EXPECT_EQ' pattern in '%v'\n\nA: '%v'\n\nB: '%v'", file, a, b)
}
- testSource = strings.ReplaceAll(testSource, expected_str, replace_str)
+ testSource = strings.ReplaceAll(testSource, expectedStr, replaceStr)
return testSource, nil
}
} else if parts := reExpectHasSubstr.FindStringSubmatch(failure); len(parts) == 5 {
@@ -374,7 +374,7 @@
// parseSourceFile() reads the file at path, splitting the content into chunks
// for each TEST.
func parseSourceFile(path string) (sourceFile, error) {
- fileBytes, err := ioutil.ReadFile(path)
+ fileBytes, err := os.ReadFile(path)
if err != nil {
return sourceFile{}, err
}
@@ -403,7 +403,7 @@
// path.
func writeSourceFile(path string, file sourceFile) error {
body := strings.Join(file.parts, "")
- return ioutil.WriteFile(path, []byte(body), 0666)
+ return os.WriteFile(path, []byte(body), 0666)
}
type sourceFile struct {
diff --git a/tools/src/cmd/gen/main.go b/tools/src/cmd/gen/main.go
index 3f60c42..c17c58c 100644
--- a/tools/src/cmd/gen/main.go
+++ b/tools/src/cmd/gen/main.go
@@ -30,6 +30,7 @@
import (
"context"
+ "errors"
"fmt"
"os"
@@ -38,9 +39,6 @@
"dawn.googlesource.com/dawn/tools/src/cmd/gen/templates"
"dawn.googlesource.com/dawn/tools/src/oswrapper"
"dawn.googlesource.com/dawn/tools/src/subcmd"
-
- // Register sub-commands
- _ "dawn.googlesource.com/dawn/tools/src/cmd/gen/templates"
)
func main() {
@@ -50,7 +48,7 @@
cfg.RegisterFlags()
if err := subcmd.Run(ctx, cfg, common.Commands()...); err != nil {
- if err != subcmd.ErrInvalidCLA {
+ if !errors.Is(err, subcmd.ErrInvalidCLA) {
fmt.Fprintln(os.Stderr, err)
}
os.Exit(1)
@@ -64,13 +62,13 @@
type cmdAll struct {
}
-func (cmdAll) IsDefaultCommand() {}
+func (c *cmdAll) IsDefaultCommand() {}
-func (cmdAll) Name() string {
+func (c *cmdAll) Name() string {
return "all"
}
-func (cmdAll) Desc() string {
+func (c *cmdAll) Desc() string {
return `all runs all the generators`
}
@@ -78,7 +76,7 @@
return nil, nil
}
-func (c cmdAll) Run(ctx context.Context, cfg *common.Config) error {
+func (c *cmdAll) Run(ctx context.Context, cfg *common.Config) error {
templatesCmd := templates.Cmd{}
if err := templatesCmd.Run(ctx, cfg); err != nil {
return err
diff --git a/tools/src/cmd/gen/templates/templates.go b/tools/src/cmd/gen/templates/templates.go
index 2108a2b..712d867 100644
--- a/tools/src/cmd/gen/templates/templates.go
+++ b/tools/src/cmd/gen/templates/templates.go
@@ -56,11 +56,11 @@
type Cmd struct {
}
-func (Cmd) Name() string {
+func (c *Cmd) Name() string {
return "templates"
}
-func (Cmd) Desc() string {
+func (c *Cmd) Desc() string {
return `templates generates files from <file>.tmpl files found in the Tint source and test directories`
}
@@ -69,7 +69,7 @@
}
// TODO(crbug.com/344014313): Add unittest coverage.
-func (c Cmd) Run(ctx context.Context, cfg *common.Config) error {
+func (c *Cmd) Run(ctx context.Context, cfg *common.Config) error {
staleFiles := common.StaleFiles{}
projectRoot := fileutils.DawnRoot(cfg.OsWrapper)
@@ -372,7 +372,7 @@
rty := reflect.TypeOf(ty)
return func(v any) bool {
ty := reflect.TypeOf(v)
- return ty == rty || ty == reflect.PtrTo(rty)
+ return ty == rty || ty == reflect.PointerTo(rty)
}
}
diff --git a/tools/src/cmd/gerrit-stats/main.go b/tools/src/cmd/gerrit-stats/main.go
index fce2868..fd4bb5d 100644
--- a/tools/src/cmd/gerrit-stats/main.go
+++ b/tools/src/cmd/gerrit-stats/main.go
@@ -108,12 +108,12 @@
}
ctx := context.Background()
- auth, err := authFlags.Options()
+ options, err := authFlags.Options()
if err != nil {
return err
}
- g, err := gerrit.New(ctx, auth, dawn.GerritURL)
+ g, err := gerrit.New(ctx, options, dawn.GerritURL)
if err != nil {
return err
}
@@ -125,7 +125,7 @@
"before:"+date(before),
"repo:"+*repoFlag)
if err != nil {
- return fmt.Errorf("Query failed: %w", err)
+ return fmt.Errorf("query failed: %w", err)
}
reviewed, reviewQuery, err := g.QueryChanges(
@@ -135,7 +135,7 @@
"before:"+date(before),
"repo:"+*repoFlag)
if err != nil {
- return fmt.Errorf("Query failed: %w", err)
+ return fmt.Errorf("query failed: %w", err)
}
ignorelist := []*regexp.Regexp{
@@ -183,10 +183,6 @@
return nil
}
-func today() time.Time {
- return time.Now()
-}
-
func date(t time.Time) string {
return t.Format(yyyymmdd)
}
diff --git a/tools/src/cmd/get-test-plan/main.go b/tools/src/cmd/get-test-plan/main.go
index 98f1deb..864277d 100644
--- a/tools/src/cmd/get-test-plan/main.go
+++ b/tools/src/cmd/get-test-plan/main.go
@@ -46,7 +46,6 @@
"flag"
"fmt"
"io"
- "io/ioutil"
"net/http"
"net/url"
"os"
@@ -110,16 +109,15 @@
}
err := run()
- switch err {
- case nil:
- return
- case errInvalidArg:
- fmt.Fprintf(os.Stderr, "Error: %v\n\n", err)
- flag.Usage()
- default:
- fmt.Fprintf(os.Stderr, "%v\n", err)
+ if err != nil {
+ if errors.Is(err, errInvalidArg) {
+ fmt.Fprintf(os.Stderr, "Error: %v\n\n", err)
+ flag.Usage()
+ } else {
+ fmt.Fprintf(os.Stderr, "%v\n", err)
+ }
+ os.Exit(1)
}
- os.Exit(1)
}
func run() error {
@@ -217,7 +215,7 @@
continue
}
- dim := -1
+ var dim int
if len(sectionDims) == len(s) {
//x.y is the same as x.y.0
dim = 0
@@ -304,7 +302,7 @@
if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil {
return fmt.Errorf("failed to create directory for '%v': %w", path, err)
}
- if err := ioutil.WriteFile(path, []byte(content), 0666); err != nil {
+ if err := os.WriteFile(path, []byte(content), 0666); err != nil {
return fmt.Errorf("failed to write file '%v': %w", path, err)
}
return nil
@@ -416,7 +414,7 @@
}
func Parse(node *html.Node) (*Parser, error) {
- var p *Parser = new(Parser)
+ p := new(Parser)
p.firstSectionContainingRule = -1
p.lastSectionContainingRule = -1
return p, p.getRules(node)
@@ -768,7 +766,6 @@
return "", "", err
}
- builtinName = ""
index := strings.Index(desc, ":")
if strings.Contains(id, "builtin_functions") && index > -1 {
builtinName = reName.ReplaceAllString(desc[:index], "_")
@@ -952,7 +949,7 @@
continue
}
- index := -1
+ var index int
sectionDims, err := parseSection(r.SubSection)
if err != nil || len(sectionDims) == 0 {
return err
diff --git a/tools/src/cmd/git-stats/main.go b/tools/src/cmd/git-stats/main.go
index 2d72b1d..c537841 100644
--- a/tools/src/cmd/git-stats/main.go
+++ b/tools/src/cmd/git-stats/main.go
@@ -400,11 +400,3 @@
}
return out
}
-
-func today() time.Time {
- return time.Now()
-}
-
-func date(t time.Time) string {
- return t.Format(yyyymmdd)
-}
diff --git a/tools/src/cmd/idlgen/main.go b/tools/src/cmd/idlgen/main.go
index 4b5b102..9c23c2c 100644
--- a/tools/src/cmd/idlgen/main.go
+++ b/tools/src/cmd/idlgen/main.go
@@ -32,7 +32,6 @@
import (
"flag"
"fmt"
- "io/ioutil"
"os"
"path/filepath"
"reflect"
@@ -91,7 +90,7 @@
}
// Read the template file
- tmpl, err := ioutil.ReadFile(templatePath)
+ tmpl, err := os.ReadFile(templatePath)
if err != nil {
return fmt.Errorf("failed to open template file '%v'", templatePath)
}
@@ -101,7 +100,7 @@
// Parse each of the WebIDL files and add the declarations to idl
for _, path := range idlFiles {
- content, err := ioutil.ReadFile(path)
+ content, err := os.ReadFile(path)
if err != nil {
return fmt.Errorf("failed to open file '%v'", path)
}
@@ -532,7 +531,7 @@
return func(v interface{}) bool {
ty := reflect.TypeOf(v)
for _, rty := range types {
- if ty == rty || ty == reflect.PtrTo(rty) {
+ if ty == rty || ty == reflect.PointerTo(rty) {
return true
}
}
diff --git a/tools/src/cmd/run-cts/chrome/cmd.go b/tools/src/cmd/run-cts/chrome/cmd.go
index ee34531..5c0fa1b 100644
--- a/tools/src/cmd/run-cts/chrome/cmd.go
+++ b/tools/src/cmd/run-cts/chrome/cmd.go
@@ -63,11 +63,11 @@
query string
}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "chrome"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return "runs the CTS with chrome"
}
diff --git a/tools/src/cmd/run-cts/node/cmd.go b/tools/src/cmd/run-cts/node/cmd.go
index 234b0dd..8854038 100644
--- a/tools/src/cmd/run-cts/node/cmd.go
+++ b/tools/src/cmd/run-cts/node/cmd.go
@@ -75,13 +75,13 @@
query string
}
-func (cmd) IsDefaultCommand() {}
+func (c *cmd) IsDefaultCommand() {}
-func (cmd) Name() string {
+func (c *cmd) Name() string {
return "node"
}
-func (cmd) Desc() string {
+func (c *cmd) Desc() string {
return "runs the CTS with dawn.node"
}
diff --git a/tools/src/cmd/run-cts/node/server.go b/tools/src/cmd/run-cts/node/server.go
index fc76412..2ee9853 100644
--- a/tools/src/cmd/run-cts/node/server.go
+++ b/tools/src/cmd/run-cts/node/server.go
@@ -101,7 +101,6 @@
var port int
testCaseLog := &bytes.Buffer{}
- stopServer := func() {}
startServer := func() error {
args := []string{
"-e", "require('./out-node/common/runtime/server.js');",
@@ -192,7 +191,7 @@
return nil
}
- stopServer = func() {
+ stopServer := func() {
if port > 0 {
go http.Post(fmt.Sprintf("http://localhost:%v/terminate", port), "", &bytes.Buffer{})
time.Sleep(time.Millisecond * 100)
diff --git a/tools/src/cmd/snippets/main.go b/tools/src/cmd/snippets/main.go
index 075a9bd..5097b36 100644
--- a/tools/src/cmd/snippets/main.go
+++ b/tools/src/cmd/snippets/main.go
@@ -98,19 +98,19 @@
if *afterFlag != "" {
after, err = time.Parse(yyyymmdd, *afterFlag)
if err != nil {
- return fmt.Errorf("Couldn't parse after date: %w", err)
+ return fmt.Errorf("couldn't parse after date: %w", err)
}
} else {
after = before.Add(-time.Hour * time.Duration(24**daysFlag))
}
ctx := context.Background()
- auth, err := authFlags.Options()
+ options, err := authFlags.Options()
if err != nil {
return err
}
- g, err := gerrit.New(ctx, auth, dawn.GerritURL)
+ g, err := gerrit.New(ctx, options, dawn.GerritURL)
if err != nil {
return err
}
@@ -143,10 +143,6 @@
return nil
}
-func today() time.Time {
- return time.Now()
-}
-
func date(t time.Time) string {
return t.Format(yyyymmdd)
}
diff --git a/tools/src/cmd/tests/main.go b/tools/src/cmd/tests/main.go
index 7f6ccfc..82e6d75 100644
--- a/tools/src/cmd/tests/main.go
+++ b/tools/src/cmd/tests/main.go
@@ -33,6 +33,7 @@
"context"
"crypto/sha256"
"encoding/json"
+ "errors"
"flag"
"fmt"
"io"
@@ -169,7 +170,7 @@
args = defaultArgs
}
- filePredicate := func(s string) bool { return true }
+ var filePredicate func(string) bool
if m, err := match.New(ignore); err == nil {
filePredicate = func(s string) bool { return !m(s) }
} else {
@@ -193,8 +194,8 @@
switch {
case fileutils.IsDir(arg, fsReaderWriter):
// Argument is to a directory, expand out to N globs
- for _, glob := range directoryGlobs {
- globs = append(globs, path.Join(arg, glob))
+ for _, g := range directoryGlobs {
+ globs = append(globs, path.Join(arg, g))
}
case fileutils.IsFile(arg, fsReaderWriter):
// Argument is a file, append to absFiles
@@ -827,13 +828,13 @@
matched = true // test passed and matched expectations
}
- var skip_str string = "FAILED"
+ skipStr := "FAILED"
if isSkipInvalidTest {
- skip_str = "INVALID"
+ skipStr = "INVALID"
}
if timedOut {
- skip_str = "TIMEOUT"
+ skipStr = "TIMEOUT"
}
passed := ok && (matched || isSkipTimeoutTest)
@@ -852,7 +853,7 @@
case isSkipTest:
// Do not update expected if timeout test actually timed out.
if cfg.generateSkip && !(isSkipTimeoutTest && timedOut) {
- saveExpectedFile(expectedFilePath, "SKIP: "+skip_str+"\n\n"+out)
+ saveExpectedFile(expectedFilePath, "SKIP: "+skipStr+"\n\n"+out)
}
if isSkipInvalidTest {
return status{code: invalid, timeTaken: timeTaken}
@@ -863,7 +864,7 @@
case !ok:
// Compiler returned non-zero exit code
if cfg.generateSkip {
- saveExpectedFile(expectedFilePath, "SKIP: "+skip_str+"\n\n"+out)
+ saveExpectedFile(expectedFilePath, "SKIP: "+skipStr+"\n\n"+out)
}
err := fmt.Errorf("%s", out)
return status{code: fail, err: err, timeTaken: timeTaken}
@@ -871,7 +872,7 @@
default:
// Compiler returned zero exit code, or output was not as expected
if cfg.generateSkip {
- saveExpectedFile(expectedFilePath, "SKIP: "+skip_str+"\n\n"+out)
+ saveExpectedFile(expectedFilePath, "SKIP: "+skipStr+"\n\n"+out)
}
// Expected output did not match
@@ -910,9 +911,9 @@
return in, nil
}
out = in
- for _, match := range matches {
- out = strings.ReplaceAll(out, match[0], "")
- hashes = append(hashes, match[1])
+ for _, m := range matches {
+ out = strings.ReplaceAll(out, m[0], "")
+ hashes = append(hashes, m[1])
}
return out, hashes
}
@@ -959,21 +960,21 @@
// maxStringLen returns the maximum number of runes found in all the strings in
// 'l'
func maxStringLen(l []string) int {
- max := 0
+ maxLen := 0
for _, s := range l {
- if c := utf8.RuneCountInString(s); c > max {
- max = c
+ if c := utf8.RuneCountInString(s); c > maxLen {
+ maxLen = c
}
}
- return max
+ return maxLen
}
// formatWidth returns the width in runes for the outputFormat column 'b'
func formatWidth(b outputFormat) int {
- const min = 6
+ const minWidth = 6
c := utf8.RuneCountInString(string(b))
- if c < min {
- return min
+ if c < minWidth {
+ return minWidth
}
return c
}
@@ -1095,7 +1096,7 @@
out, err := cmd.CombinedOutput()
str := string(out)
if err != nil {
- if ctx.Err() == context.DeadlineExceeded {
+ if errors.Is(ctx.Err(), context.DeadlineExceeded) {
return false, fmt.Sprintf("test timed out after %v", testTimeout), true
}
if str != "" {
@@ -1178,16 +1179,16 @@
func printDuration(d time.Duration) string {
sec := int(d.Seconds())
- min := int(sec) / 60
- hour := min / 60
- sec -= min * 60
- min -= hour * 60
+ minute := int(sec) / 60
+ hour := minute / 60
+ sec -= minute * 60
+ minute -= hour * 60
sb := &strings.Builder{}
if hour > 0 {
fmt.Fprintf(sb, "%dh", hour)
}
- if min > 0 {
- fmt.Fprintf(sb, "%dm", min)
+ if minute > 0 {
+ fmt.Fprintf(sb, "%dm", minute)
}
if sec > 0 {
fmt.Fprintf(sb, "%ds", sec)
diff --git a/tools/src/cmd/time-cmd/main.go b/tools/src/cmd/time-cmd/main.go
index cd89a1e..b48ecaa 100644
--- a/tools/src/cmd/time-cmd/main.go
+++ b/tools/src/cmd/time-cmd/main.go
@@ -137,9 +137,9 @@
start := time.Now()
out, err := cmd.CombinedOutput()
- time := time.Since(start)
+ since := time.Since(start)
- result.time += time
+ result.time += since
if err != nil {
result.errs = append(result.errs, fmt.Errorf("%v", string(out)))
segError.Count++
diff --git a/tools/src/cov/import.go b/tools/src/cov/import.go
index fe3ebcf..dbfe3a6 100644
--- a/tools/src/cov/import.go
+++ b/tools/src/cov/import.go
@@ -62,10 +62,10 @@
TurboCov string // path to the turbo-cov tool (one of Cov or TurboCov must be supplied)
}
-// TODO(crbug.com/344014313): Add unittest coverage.
// AllSourceFiles returns a *Coverage containing all the source files without
// coverage data. This populates the coverage view with files even if they
// didn't get compiled.
+// TODO(crbug.com/344014313): Add unittest coverage.
func (e Env) AllSourceFiles(fsReader oswrapper.FilesystemReader) *Coverage {
var ignorePaths = map[string]bool{
//
@@ -99,10 +99,10 @@
return &cov
}
-// TODO(crbug.com/416755658): Add unittest coverage once exec is handled via
-// dependency injection.
// Import uses the llvm-profdata and llvm-cov tools to import the coverage
// information from a .profraw file.
+// TODO(crbug.com/416755658): Add unittest coverage once exec is handled via
+// dependency injection.
func (e Env) Import(profrawPath string, fsReader oswrapper.FilesystemReader) (*Coverage, error) {
profdata := profrawPath + ".profdata"
defer os.Remove(profdata)
diff --git a/tools/src/cts/expectations/expectations.go b/tools/src/cts/expectations/expectations.go
index 1545688..6a8ac60 100644
--- a/tools/src/cts/expectations/expectations.go
+++ b/tools/src/cts/expectations/expectations.go
@@ -59,7 +59,7 @@
Expectations Expectations // Expectations for the chunk
}
-// Type + enum for whether an Expectation's Query contains globs or not.
+// ExpectationType is a Type + enum for whether an Expectation's Query contains globs or not.
type ExpectationType int
const (
@@ -97,7 +97,7 @@
}
// Save saves the Content file to 'path'.
-func (c Content) Save(path string) error {
+func (c *Content) Save(path string) error {
f, err := os.Create(path)
if err != nil {
return err
@@ -108,7 +108,7 @@
}
// Clone makes a deep-copy of the Content.
-func (c Content) Clone() Content {
+func (c *Content) Clone() Content {
chunks := make([]Chunk, len(c.Chunks))
for i, c := range c.Chunks {
chunks[i] = c.Clone()
@@ -117,12 +117,12 @@
}
// Empty returns true if the Content has no chunks.
-func (c Content) Empty() bool {
+func (c *Content) Empty() bool {
return len(c.Chunks) == 0
}
// Write writes the Content, in textual form, to the writer w.
-func (c Content) Write(w io.Writer) error {
+func (c *Content) Write(w io.Writer) error {
for i, chunk := range c.Chunks {
if i > 0 {
if _, err := fmt.Fprintln(w); err != nil {
@@ -144,7 +144,7 @@
}
// String returns the Content as a string.
-func (c Content) String() string {
+func (c *Content) String() string {
sb := strings.Builder{}
c.Write(&sb)
return sb.String()
@@ -214,9 +214,7 @@
// Clone returns a deep-copy of the Chunk
func (c Chunk) Clone() Chunk {
comments := make([]string, len(c.Comments))
- for i, c := range c.Comments {
- comments[i] = c
- }
+ copy(comments, c.Comments)
expectations := make([]Expectation, len(c.Expectations))
for i, e := range c.Expectations {
expectations[i] = e.Clone()
@@ -268,7 +266,7 @@
// AppliesToResult returns whether the Expectation applies to the test + config
// represented by the Result.
-func (e Expectation) AppliesToResult(r result.Result) bool {
+func (e *Expectation) AppliesToResult(r result.Result) bool {
// Tags apply as long as the Expectation's tags are a subset of the Result's
// tags.
tagsApply := r.Tags.ContainsAll(e.Tags)
@@ -280,7 +278,7 @@
// AppliesToTest returns whether the Expectation applies to the test |name|.
// This does NOT take into account the tags contained within the Expectation,
// only whether the name matches.
-func (e Expectation) AppliesToTest(name string) bool {
+func (e *Expectation) AppliesToTest(name string) bool {
// The query is a glob expectation, we need to perform a more complex
// comparison. Otherwise, we can just check for an exact match.
if e.IsGlobExpectation() {
@@ -293,7 +291,7 @@
// AsExpectationFileString returns the human-readable form of the expectation
// that matches the syntax of the expectation files.
-func (e Expectation) AsExpectationFileString() string {
+func (e *Expectation) AsExpectationFileString() string {
parts := []string{}
if e.Bug != "" {
parts = append(parts, e.Bug)
@@ -310,7 +308,7 @@
}
// Clone makes a deep-copy of the Expectation.
-func (e Expectation) Clone() Expectation {
+func (e *Expectation) Clone() Expectation {
out := Expectation{
Line: e.Line,
Bug: e.Bug,
@@ -333,7 +331,7 @@
// 0 if a and b are identical
//
// Note: Only comparing bug, tags, and query (in that order).
-func (e Expectation) Compare(b Expectation) int {
+func (e *Expectation) Compare(b Expectation) int {
switch strings.Compare(e.Bug, b.Bug) {
case -1:
return -1
@@ -357,7 +355,7 @@
// ComparePrioritizeQuery is the same as Compare, but compares in the following
// order: query, tags, bug.
-func (e Expectation) ComparePrioritizeQuery(other Expectation) int {
+func (e *Expectation) ComparePrioritizeQuery(other Expectation) int {
switch strings.Compare(e.Query, other.Query) {
case -1:
return -1
diff --git a/tools/src/cts/expectations/update.go b/tools/src/cts/expectations/update.go
index 178d6d2..dfa3e64 100644
--- a/tools/src/cts/expectations/update.go
+++ b/tools/src/cts/expectations/update.go
@@ -57,7 +57,7 @@
//
// TODO(crbug.com/372730248): Return diagnostics.
func (c *Content) AddExpectationsForFailingResults(results result.List,
- use_explicit_tags bool, verbose bool) error {
+ useExplicitTags bool, verbose bool) error {
// Make a copy of the results. This code mutates the list.
results = append(result.List{}, results...)
@@ -66,11 +66,11 @@
return err
}
if verbose {
- fmt.Printf("Removing unknown tags took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Removing unknown tags took %s\n", time.Since(startTime).String())
}
startTime = time.Now()
- if use_explicit_tags {
+ if useExplicitTags {
if err := c.reduceTagsToMostExplicitOnly(&results); err != nil {
return err
}
@@ -80,14 +80,14 @@
}
}
if verbose {
- fmt.Printf("Reducing tags took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Reducing tags took %s\n", time.Since(startTime).String())
}
// Merge identical results.
startTime = time.Now()
results = result.Merge(results)
if verbose {
- fmt.Printf("Merging identical results took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Merging identical results took %s\n", time.Since(startTime).String())
}
startTime = time.Now()
@@ -95,7 +95,7 @@
return err
}
if verbose {
- fmt.Printf("Adding expectations took %s\n", time.Now().Sub(startTime).String())
+ fmt.Printf("Adding expectations took %s\n", time.Since(startTime).String())
}
return nil
}
@@ -221,9 +221,8 @@
ROLLER_AUTOGENERATED_FAILURES,
}
- // Bin the chunks into those that contain any of the strings in
- // mutableTokens in the comments and those that do not have these strings.
- immutableChunkIndicies, mutableChunkIndices := []int{}, []int{}
+ // Filter out chunks that do not contain any of the strings in mutableTokens in the comments
+ mutableChunkIndices := []int{}
for i, chunk := range c.Chunks {
immutable := true
@@ -237,9 +236,7 @@
}
}
- if immutable {
- immutableChunkIndicies = append(immutableChunkIndicies, i)
- } else {
+ if !immutable {
mutableChunkIndices = append(mutableChunkIndices, i)
}
}
diff --git a/tools/src/cts/expectations/validate.go b/tools/src/cts/expectations/validate.go
index 904789c..68904bd 100644
--- a/tools/src/cts/expectations/validate.go
+++ b/tools/src/cts/expectations/validate.go
@@ -57,7 +57,7 @@
out = append(out, Diagnostic{
Severity: Error,
Line: ex.Line,
- Message: fmt.Sprintf("\"Slow\" expectation is not valid here. Use slow_tests.txt instead."),
+ Message: "\"Slow\" expectation is not valid here. Use slow_tests.txt instead.",
})
}
}
diff --git a/tools/src/git/git.go b/tools/src/git/git.go
index ab30476..b984a33 100644
--- a/tools/src/git/git.go
+++ b/tools/src/git/git.go
@@ -97,8 +97,8 @@
}
// Empty return true if there's no username or password for authentication
-func (a Credentials) Empty() bool {
- return a.Username == "" && a.Password == ""
+func (c Credentials) Empty() bool {
+ return c.Username == "" && c.Password == ""
}
// addToURL returns the url with the credentials appended
@@ -138,19 +138,19 @@
}
// Clone performs a clone of the repository at url to path.
-func (g Git) Clone(path, url string, opt *CloneOptions) (*Repository, error) {
+func (g Git) Clone(path, urlStr string, opt *CloneOptions) (*Repository, error) {
if err := os.MkdirAll(path, 0777); err != nil {
return nil, err
}
if opt == nil {
opt = &CloneOptions{}
}
- url, err := opt.Credentials.addToURL(url)
+ urlStr, err := opt.Credentials.addToURL(urlStr)
if err != nil {
return nil, err
}
r := &Repository{g, path}
- args := []string{"clone", url, "."}
+ args := []string{"clone", urlStr, "."}
if opt.Branch != "" {
args = append(args, "--branch", opt.Branch)
}
@@ -215,15 +215,15 @@
if opt.Remote == "" {
opt.Remote = "origin"
}
- url, err := r.run(nil, opt.Timeout, "remote", "get-url", opt.Remote)
+ urlStr, err := r.run(nil, opt.Timeout, "remote", "get-url", opt.Remote)
if err != nil {
return err
}
- url, err = opt.Credentials.addToURL(url)
+ urlStr, err = opt.Credentials.addToURL(urlStr)
if err != nil {
return err
}
- if _, err := r.run(nil, opt.Timeout, "push", url, localRef+":"+remoteRef); err != nil {
+ if _, err := r.run(nil, opt.Timeout, "push", urlStr, localRef+":"+remoteRef); err != nil {
return err
}
return nil
@@ -267,13 +267,6 @@
opt = &CommitOptions{}
}
- args := []string{"commit"}
- if opt.Amend {
- args = append(args, "--amend")
- } else {
- args = append(args, "-m", msg)
- }
-
var env []string
if opt.AuthorName != "" || opt.AuthorEmail != "" {
env = []string{
@@ -283,7 +276,14 @@
fmt.Sprintf("GIT_COMMITTER_EMAIL=%v", opt.AuthorEmail),
}
}
- if _, err := r.run(env, opt.Timeout, "commit", "-m", msg); err != nil {
+
+ args := []string{"commit"}
+ if opt.Amend {
+ args = append(args, "--amend")
+ } else {
+ args = append(args, "-m", msg)
+ }
+ if _, err := r.run(env, opt.Timeout, args...); err != nil {
return Hash{}, err
}
out, err := r.run(nil, 0, "rev-parse", "HEAD")
diff --git a/tools/src/oswrapper/memmaposwrapper_test.go b/tools/src/oswrapper/memmaposwrapper_test.go
index 8b93d1d..286cecf 100644
--- a/tools/src/oswrapper/memmaposwrapper_test.go
+++ b/tools/src/oswrapper/memmaposwrapper_test.go
@@ -180,7 +180,7 @@
require.Equal(t, []byte("content"), buffer)
require.Equal(t, 7, bytesRead)
bytesRead, err = file.Read(buffer)
- require.Error(t, io.EOF, "Did not get EOF")
+ require.ErrorIs(t, err, io.EOF, "Did not get EOF")
require.Equal(t, 0, bytesRead)
}
@@ -408,7 +408,7 @@
if testCase.wantErr {
require.ErrorContains(t, err, testCase.wantErrMsg)
} else {
- require.NoErrorf(t, err, "Got unexpected error: %v")
+ require.NoErrorf(t, err, "Got unexpected error: %v", err)
}
// Make sure things were actually deleted.
diff --git a/tools/src/template/template.go b/tools/src/template/template.go
index 5413a74..739a370 100644
--- a/tools/src/template/template.go
+++ b/tools/src/template/template.go
@@ -32,7 +32,6 @@
import (
"fmt"
"io"
- "io/ioutil"
"os"
"path/filepath"
"reflect"
@@ -131,7 +130,7 @@
func (g *generator) bindAndParse(t *template.Template, tmpl string) error {
_, err := t.
- Funcs(map[string]any(g.funcs)).
+ Funcs(g.funcs).
Option("missingkey=error").
Parse(tmpl)
return err
@@ -192,7 +191,7 @@
if err := g.bindAndParse(t, string(data)); err != nil {
return "", fmt.Errorf("failed to parse '%v': %w", path, err)
}
- if err := t.Execute(ioutil.Discard, nil); err != nil {
+ if err := t.Execute(io.Discard, nil); err != nil {
return "", fmt.Errorf("failed to execute '%v': %w", path, err)
}
return "", nil
diff --git a/tools/src/utils/context.go b/tools/src/utils/context.go
index 41b02c0..6966925 100644
--- a/tools/src/utils/context.go
+++ b/tools/src/utils/context.go
@@ -38,7 +38,7 @@
// CancelOnInterruptContext returns a context that's cancelled if the process receives a SIGINT or
// SIGTERM interrupt.
func CancelOnInterruptContext(ctx context.Context) context.Context {
- ctx, cancel := context.WithCancel(context.Background())
+ ctx, cancel := context.WithCancel(ctx)
sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
go func() {
