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í <[email protected]>
Change-Id: Ic2f1d5bace5902abb5f347ba859dc588cf1149c3
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1206355
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Paul Jolly <[email protected]>
  • Loading branch information
mvdan committed Dec 31, 2024
1 parent 46fc54a commit 7da6f24
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 19 deletions.
33 changes: 15 additions & 18 deletions cmd/cue/cmd/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,33 +86,30 @@ func customCommand(c *Command, typ, name string, tools *cue.Instance) (*cobra.Co
}

// TODO: validate allowing incomplete.
o := tools.Lookup(typ, name)
cmds := tools.Lookup(typ)
o := cmds.Lookup(name)
if !o.Exists() {
return nil, o.Err()
}

// 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(cmds)
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 {
// Note that earlier versions of this code checked cmds.Err in this scenario,
// but it isn't clear why that was done, and we had no tests covering it.
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 7da6f24

Please sign in to comment.