Skip to content

Commit

Permalink
cue/ast: fix IsValidIdent for _0
Browse files Browse the repository at this point in the history
Currently `IsValidIdent` returns false for the identifier `_1`.
This is not correct: the specification defines as valid identifier
as:

	identifier  = [ "#" | "_#" ] letter { letter | unicode_digit } .

where `letter` includes underscore.

This caused an observed panic in `internal/core/export.(*pivotter)` when it calls
`exporter.ident`.

I was wanting to add the test before introducing the fix, but this isn't
possible with the current `IsValidIdentifer` tests because they check
for consistency between `LabelName` and `IsValidIdentifier` and the
current logic is not consistent for this case.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: Icb571edbcac97e7f076497d45a03c7b524d92d20
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/549423
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
  • Loading branch information
rogpeppe committed Apr 6, 2023
1 parent 9e22283 commit 9422314
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
17 changes: 9 additions & 8 deletions cue/ast/ident.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,26 @@ func IsValidIdent(ident string) bool {
return false
}

// TODO: use consumed again to allow #0.
// consumed := false
consumed := false
if strings.HasPrefix(ident, "_") {
ident = ident[1:]
// consumed = true
consumed = true
if len(ident) == 0 {
return true
}
}
if strings.HasPrefix(ident, "#") {
ident = ident[1:]
// consumed = true
// Note: _#0 is not allowed by the spec, although _0 is.
// TODO: set consumed to true here to allow #0.
consumed = false
}

// if !consumed {
if r, _ := utf8.DecodeRuneInString(ident); isDigit(r) {
return false
if !consumed {
if r, _ := utf8.DecodeRuneInString(ident); isDigit(r) {
return false
}
}
// }

for _, r := range ident {
if isLetter(r) || isDigit(r) || r == '_' || r == '$' {
Expand Down
8 changes: 8 additions & 0 deletions cue/ast/ident_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ func TestLabelName(t *testing.T) {
in: &ast.Ident{Name: "_"},
out: "_",
isIdent: true,
}, {
in: &ast.Ident{Name: "_1"},
out: "_1",
isIdent: true,
}, {
in: &ast.Ident{Name: "_#1"},
out: "",
err: true,
}, {
in: &ast.Ident{Name: "8ball"},
out: "",
Expand Down

0 comments on commit 9422314

Please sign in to comment.