From 7da6f24ec3ded804985d333d7a4046961360334d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Thu, 26 Dec 2024 19:04:40 +0000
Subject: [PATCH] cmd/cue: fix the only SA4004 error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1206355
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>
---
 cmd/cue/cmd/custom.go | 33 +++++++++++++++------------------
 staticcheck.conf      |  1 -
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/cmd/cue/cmd/custom.go b/cmd/cue/cmd/custom.go
index f9f7484e6..a71772c22 100644
--- a/cmd/cue/cmd/custom.go
+++ b/cmd/cue/cmd/custom.go
@@ -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)
 	}
 
diff --git a/staticcheck.conf b/staticcheck.conf
index f51215ce0..19d3cc78e 100644
--- a/staticcheck.conf
+++ b/staticcheck.conf
@@ -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