Skip to content

Commit

Permalink
ast: change ident token string (#6435)
Browse files Browse the repository at this point in the history
This commit changes the string representation of the ident token from
'ident' to 'identifier' as this shows up in error messages and should be
a bit friendlier for users.

Signed-off-by: Torin Sandall <[email protected]>
  • Loading branch information
tsandall authored Nov 25, 2023
1 parent 85886a5 commit 3c6e079
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 28 deletions.
2 changes: 1 addition & 1 deletion ast/capabilities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestParserCapabilitiesWithWildcardOptInAndOlderOPA(t *testing.T) {
t.Fatal("expected error")
} else if errs, ok := err.(Errors); !ok || len(errs) != 1 {
t.Fatal("expected exactly one error but got:", err)
} else if errs[0].Code != ParseErr || errs[0].Location.Row != 7 || errs[0].Message != "unexpected ident token: expected \\n or ; or }" {
} else if errs[0].Code != ParseErr || errs[0].Location.Row != 7 || errs[0].Message != "unexpected identifier token: expected \\n or ; or }" {
t.Fatal("unexpected error:", err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion ast/internal/tokens/tokens.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ var strings = [...]string{
EOF: "eof",
Whitespace: "whitespace",
Comment: "comment",
Ident: "ident",
Ident: "identifier",
Package: "package",
Import: "import",
As: "as",
Expand Down
10 changes: 5 additions & 5 deletions ast/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -752,13 +752,13 @@ func TestSomeDeclExpr(t *testing.T) {
p[x] {
some x in {"foo": "bar"}
}`,
"unexpected ident token: expected \\n or ; or } (hint: `import future.keywords.in` for `some x in xs` expressions)")
"unexpected identifier token: expected \\n or ; or } (hint: `import future.keywords.in` for `some x in xs` expressions)")

assertParseErrorContains(t, "some x, y in ... usage is hinted properly", `
p[y] = x {
some x, y in {"foo": "bar"}
}`,
"unexpected ident token: expected \\n or ; or } (hint: `import future.keywords.in` for `some x in xs` expressions)")
"unexpected identifier token: expected \\n or ; or } (hint: `import future.keywords.in` for `some x in xs` expressions)")

assertParseRule(t, "whitespace terminated", `
Expand Down Expand Up @@ -848,13 +848,13 @@ func TestEvery(t *testing.T) {
p {
every x, y in {"foo": "bar"} { is_string(x); is_string(y) }
}`,
"unexpected ident token: expected \\n or ; or } (hint: `import future.keywords.every` for `every x in xs { ... }` expressions)")
"unexpected identifier token: expected \\n or ; or } (hint: `import future.keywords.every` for `every x in xs { ... }` expressions)")

assertParseErrorContains(t, "not every 'every' gets a hint", `
p {
every x
}`,
"unexpected ident token: expected \\n or ; or }\n\tevery x\n", // this asserts that the tail of the error message doesn't contain a hint
"unexpected identifier token: expected \\n or ; or }\n\tevery x\n", // this asserts that the tail of the error message doesn't contain a hint
)

assertParseErrorContains(t, "invalid domain (internal.member_2)", "every internal.member_2()", "illegal domain", opts)
Expand Down Expand Up @@ -5194,7 +5194,7 @@ p { input = "str" }`,
# - Tyrell Corp.
# related_resources:
# - https://example.com
# -
# -
# ref: http://john:[email protected]/mi?foo=bar#baz
# description: foo bar
# authors:
Expand Down
42 changes: 21 additions & 21 deletions cmd/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ package test
# METADATA
# schemas:
# - input: schema["input"]
p {
p {
rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation
input.foo == 42 # type mismatch with schema that should be ignored
}`
Expand All @@ -530,7 +530,7 @@ package test
# schemas:
# - input.foo: {"type": "boolean"}
p {
rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation
rego.metadata.rule() # presence of rego.metadata.* calls must not trigger unwanted schema evaluation
input.foo == 42 # type mismatch with schema that should NOT be ignored since it is an inlined schema format
}`

Expand Down Expand Up @@ -1432,9 +1432,9 @@ func TestPolicyWithStrictFlag(t *testing.T) {
}{
{
note: "strict mode should error on duplicate imports",
policy: `package x
import future.keywords.if
import future.keywords.if
policy: `package x
import future.keywords.if
import future.keywords.if
foo = 2`,
query: "data.foo",
expectedCode: "rego_compile_error",
Expand All @@ -1444,7 +1444,7 @@ func TestPolicyWithStrictFlag(t *testing.T) {
note: "strict mode should error on unused imports",
policy: `package x
import future.keywords.if
import data.foo
import data.foo
foo = 2`,
query: "data.foo",
expectedCode: "rego_compile_error",
Expand Down Expand Up @@ -1501,15 +1501,15 @@ func TestPolicyWithStrictFlag(t *testing.T) {
}{
{
note: "This should not error as it is valid",
policy: `package x
policy: `package x
import future.keywords.if
foo = 2`,
query: "data.foo",
},
{
note: "Strict mode should not validate the query, only the policy, this should not error",
policy: `package x
import future.keywords.if
policy: `package x
import future.keywords.if
foo = 2`,
query: "x := data.x.foo",
},
Expand Down Expand Up @@ -1546,9 +1546,9 @@ func TestBundleWithStrictFlag(t *testing.T) {
}{
{
note: "strict mode should error on duplicate imports in this bundle",
policy: `package x
import future.keywords.if
import future.keywords.if
policy: `package x
import future.keywords.if
import future.keywords.if
foo = 2`,
query: "data.foo",
expectedCode: "rego_compile_error",
Expand All @@ -1557,8 +1557,8 @@ func TestBundleWithStrictFlag(t *testing.T) {
{
note: "strict mode should error on unused imports in this bundle",
policy: `package x
import future.keywords.if
import data.foo
import future.keywords.if
import data.foo
foo = 2`,
query: "data.foo",
expectedCode: "rego_compile_error",
Expand All @@ -1567,7 +1567,7 @@ func TestBundleWithStrictFlag(t *testing.T) {
{
note: "strict mode should error when reserved vars data or input is used in this bundle",
policy: `package x
import future.keywords.if
import future.keywords.if
data if { x = 1}`,
query: "data.foo",
expectedCode: "rego_compile_error",
Expand Down Expand Up @@ -1616,15 +1616,15 @@ func TestBundleWithStrictFlag(t *testing.T) {
}{
{
note: "This bundle should not error as it is valid",
policy: `package x
import future.keywords.if
policy: `package x
import future.keywords.if
foo = 2`,
query: "data.foo",
},
{
note: "Strict mode should not validate the query, only the policy, this bundle should not error",
policy: `package x
import future.keywords.if
policy: `package x
import future.keywords.if
foo = 2`,
query: "x := data.x.foo",
},
Expand Down Expand Up @@ -1817,7 +1817,7 @@ func TestUnexpectedElseIfElseErr(t *testing.T) {
else := x if {
x=2
1==2
} else
} else
x=3
`,
}
Expand All @@ -1840,7 +1840,7 @@ func TestUnexpectedElseIfElseErr(t *testing.T) {

// Check the error message
errorMessage := err.Error()
expectedErrorMessage := "rego_parse_error: unexpected ident token: expected else value term or rule body"
expectedErrorMessage := "rego_parse_error: unexpected identifier token: expected else value term or rule body"
if !strings.Contains(errorMessage, expectedErrorMessage) {
t.Fatalf("expected error message to contain '%s', but got '%s'", expectedErrorMessage, errorMessage)
}
Expand Down

0 comments on commit 3c6e079

Please sign in to comment.