Skip to content

Commit

Permalink
cmd/cue: fix the only SA4004 error
Browse files Browse the repository at this point in the history
This was introduced by https://cuelang.org/cl/1191581,
where the previous range-based logic using `break outer` was replaced
by a callback-based logic which could no longer work the same way.

The new logic always looped just once, meaning we only cared about
the top-level "commands" field existing in at least one _tool.cue file.
We don't require the command being run itself to reside in a
_tool.cue file as well; that breaks https://cuelang.org/issue/281
for which we have a regression test.

Fix the staticcheck warning, which simplifies the logic without
changing it at all.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Ic2f1d5bace5902abb5f347ba859dc588cf1149c3
Dispatch-Trailer: {"type":"trybot","CL":1206355,"patchset":1,"ref":"refs/changes/55/1206355/1","targetBranch":"master"}
  • Loading branch information
mvdan authored and cueckoo committed Dec 26, 2024
1 parent c905888 commit 96eb25e
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 18 deletions.
28 changes: 11 additions & 17 deletions cmd/cue/cmd/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,26 +93,20 @@ func customCommand(c *Command, typ, name string, tools *cue.Instance) (*cobra.Co

// Ensure there is at least one tool file.
// TODO: remove this block to allow commands to be defined in any file.
for _, v := range []cue.Value{tools.Lookup(typ), o} {
_, w := value.ToInternal(v)
hasToolFile := false
w.VisitLeafConjuncts(func(c adt.Conjunct) bool {
src := c.Source()
if src == nil {
return true
}
if strings.HasSuffix(src.Pos().Filename(), "_tool.cue") {
hasToolFile = true
return false
}
_, w := value.ToInternal(tools.Lookup(typ))
hasToolFile := false
w.VisitLeafConjuncts(func(c adt.Conjunct) bool {
src := c.Source()
if src == nil {
return true
})
if hasToolFile {
break
}
if err := v.Err(); err != nil {
return nil, err
if strings.HasSuffix(src.Pos().Filename(), "_tool.cue") {
hasToolFile = true
return false
}
return true
})
if !hasToolFile {
return nil, errors.Newf(token.NoPos, "could not find command %q", name)
}

Expand Down
1 change: 0 additions & 1 deletion staticcheck.conf
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ checks = [
"inherit",
"-SA1019", # use of deprecated APIs
"-SA4000", # identical expressions in && or || logic
"-SA4004", # loop broken unconditionally
"-S1008", # simplify if/else to bool expression
"-S1011", # simplify loop with append
"-U1000", # unused code
Expand Down

0 comments on commit 96eb25e

Please sign in to comment.