cue: don't panic on Iterator.Selector.Index with lists

We had tests which covered calling Selector.Index,
but none where the Selector came from a Value.List iterator.

https://cuelang.org/cl/552171 changed Iterator.Selector
to wrap the returned selector with a constraint type,
which is necessary for a struct fields iterator
to properly include any optional or required constraint type.

However, this wrapping was added unconditionally,
even when a selector has no constraint type such as a list element.
Selector.Index would then type assert into indexSelector,
which would no longer work due to the wrapping.

The simplest fix is to skip wrapConstraint if there is no constraint type
to wrap the selector with, which is the case with list elements.
This way, we don't need to change Selector.Index at all,
which somehow still needs to reach for the inner indexSelector value.

Per Marcel, lists will eventually have constraint types too,
so add a note in Selector.Index as a reminder to handle it properly.
The godoc in Selector.Index also still referenced SelIndex.

Rather than adding an entirely new test, expand TestList to check Index.

Fixes #2692.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: I4daf8c23a685c413c88892fb309a83b6c7a76386
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1172105
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
Reviewed-by: Marcel van Lohuizen <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: mvdan

cue/path.go

@@ -198,8 +198,11 @@ func (sel Selector) PkgPath() string {
 }
 
 // Index returns the index of the selector. It panics
-// unless sel.Type is SelIndex.
+// unless sel.Type is IndexLabel.
 func (sel Selector) Index() int {
+	// Note that lists will eventually have constraint types too,
+	// and in that case sel.sel would be of type constraintSelector,
+	// causing the type assertion below to fail.
 	s, ok := sel.sel.(indexSelector)
 	if !ok {
 		panic("Index called on non-index selector")

cue/types.go

@@ -248,7 +248,12 @@ func (i *Iterator) Value() Value {
 
 // Selector reports the field label of this iteration.
 func (i *Iterator) Selector() Selector {
-	return wrapConstraint(featureToSel(i.f, i.idx), fromArcType(i.arcType))
+	sel := featureToSel(i.f, i.idx)
+	// Only call wrapConstraint if there is any constraint type to wrap with.
+	if ctype := fromArcType(i.arcType); ctype != 0 {
+		sel = wrapConstraint(sel, ctype)
+	}
+	return sel
 }
 
 // Label reports the label of the value if i iterates over struct fields and ""

cue/types_test.go

@@ -727,7 +727,11 @@ func TestList(t *testing.T) {
 			checkFatal(t, err, tc.err, "init")
 
 			buf := []byte{'['}
-			for l.Next() {
+			for wantIdx := 0; l.Next(); wantIdx++ {
+				// Ensure that we can get each index as well.
+				if got := l.Selector().Index(); got != wantIdx {
+					t.Errorf("Index got %v; want %v", got, wantIdx)
+				}
 				b, err := l.Value().MarshalJSON()
 				checkFatal(t, err, tc.err, "list.Value")
 				buf = append(buf, b...)