From 9b42239327a92420245b224124350c5bd75c1273 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:55:28 -0500 Subject: [PATCH] Expand keyword-in-ref coverage for complex parser edge cases (interpreter + RVM) (#744) * Initial plan * Add keywords_in_refs: allow reserved keywords as dot-notation field names * Address review feedback: improve parse_ref_field doc comment and clean up test comment * Add complex keyword-in-ref test cases * Polish keyword-ref test expectations and validate coverage --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- src/parser.rs | 40 ++- .../cases/refr/keywords_in_refs.yaml | 268 ++++++++++++++++++ tests/rvm/rego/cases/chained_access.yaml | 106 +++++++ 3 files changed, 409 insertions(+), 5 deletions(-) create mode 100644 tests/interpreter/cases/refr/keywords_in_refs.yaml diff --git a/src/parser.rs b/src/parser.rs index 9b0532f..1f96464 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -354,6 +354,33 @@ impl<'source> Parser<'source> { } } + /// Parse a field name after `.` in a ref expression. + /// + /// Unlike [`Self::parse_var`] and [`Self::parse_ident`], this method accepts **any** + /// `TokenKind::Ident` token, including reserved keywords (e.g. `as`, `default`, `else`, + /// `false`, `if`, `import`, `in`, `not`, `null`, `package`, `some`, `true`, `with`). + /// + /// The position immediately after `.` is unambiguously a field name, so there is no + /// syntactic ambiguity with statement-level keywords. This matches OPA's + /// `keywords_in_refs` capability, which is enabled by default in standard OPA builds. + /// + /// # Example + /// ```rego + /// allow if { input.v0.package.format == "npm" } # `package` is a keyword but valid here + /// ``` + fn parse_ref_field(&mut self) -> Result { + let span = self.tok.1.clone(); + match self.tok.0 { + TokenKind::Ident => { + self.next_token()?; + Ok(span) + } + _ => Err(self + .source + .error(self.tok.1.line, self.tok.1.col, "expecting identifier")), + } + } + fn read_number(&mut self, span: Span) -> Result { match Number::from_str(span.text()) { Ok(v) => Ok(Expr::Number { @@ -743,9 +770,10 @@ impl<'source> Parser<'source> { ); } "." => { - // Read identifier. + // Read identifier. Keywords are allowed as field names in + // dot-notation refs (e.g. `input.package.name`). self.next_token()?; - let field = self.parse_var()?; + let field = self.parse_ref_field()?; span.end = self.end; // Disallow any whitespace between . and identifier. @@ -1418,9 +1446,10 @@ impl<'source> Parser<'source> { ); } "." => { - // Read identifier. + // Read identifier. Keywords are allowed as field names in + // dot-notation refs (e.g. `import data.my.package`). self.next_token()?; - let field = self.parse_ident()?; + let field = self.parse_ref_field()?; span.end = self.end; // Disallow any whitespace between . and identifier. @@ -1523,7 +1552,8 @@ impl<'source> Parser<'source> { "." => { let sep_pos = self.tok.1.start; self.next_token()?; - let field = self.parse_var()?; + // Keywords are allowed as field names in dot-notation refs. + let field = self.parse_ref_field()?; span.end = self.end; // Disallow any whitespace between . and identifier. diff --git a/tests/interpreter/cases/refr/keywords_in_refs.yaml b/tests/interpreter/cases/refr/keywords_in_refs.yaml new file mode 100644 index 0000000..536fe5a --- /dev/null +++ b/tests/interpreter/cases/refr/keywords_in_refs.yaml @@ -0,0 +1,268 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +# +# Tests for keywords-as-field-names in dot-notation refs. +# Matches OPA's `keywords_in_refs` behavior, enabled by default. +cases: + - note: keywords_in_refs/package field + modules: + - | + package test + allow if { + input.v0.package.format == "npm" + } + input: + v0: + package: + format: npm + query: data.test.allow + want_result: true + + - note: keywords_in_refs/as field + modules: + - | + package test + x = input.as.type + input: + as: + type: string + query: data.test.x + want_result: string + + - note: keywords_in_refs/default field + modules: + - | + package test + x = input.default.value + input: + default: + value: 42 + query: data.test.x + want_result: 42 + + - note: keywords_in_refs/else field + modules: + - | + package test + x = input.else.value + input: + else: + value: hello + query: data.test.x + want_result: hello + + - note: keywords_in_refs/import field + modules: + - | + package test + x = input.import.name + input: + import: + name: foo + query: data.test.x + want_result: foo + + - note: keywords_in_refs/not field + modules: + - | + package test + x = input.not.allowed + input: + not: + allowed: false + query: data.test.x + want_result: false + + - note: keywords_in_refs/null field + modules: + - | + package test + x = input.null.value + input: + "null": + value: 1 + query: data.test.x + want_result: 1 + + - note: keywords_in_refs/some field + modules: + - | + package test + x = input.some.field + input: + some: + field: bar + query: data.test.x + want_result: bar + + - note: keywords_in_refs/true field + modules: + - | + package test + x = input.true.x + input: + "true": + x: 2 + query: data.test.x + want_result: 2 + + - note: keywords_in_refs/false field + modules: + - | + package test + x = input.false.x + input: + "false": + x: 3 + query: data.test.x + want_result: 3 + + - note: keywords_in_refs/with field + modules: + - | + package test + x = input.with.config + input: + with: + config: test + query: data.test.x + want_result: test + + - note: keywords_in_refs/future keywords (if, in, every, contains) + modules: + - | + package test + import future.keywords + x if { + input.if.condition == true + input.in.set == "member" + input.every.item == "x" + input.contains.key == "val" + } + input: + if: + condition: true + in: + set: member + every: + item: x + contains: + key: val + query: data.test.x + want_result: true + + - note: keywords_in_refs/chained keywords + modules: + - | + package test + x = input.package.import.default + input: + package: + import: + default: chained + query: data.test.x + want_result: chained + + - note: keywords_in_refs/data path with keyword + modules: + - | + package test + x = data.mydata.package.name + data: + mydata: + package: + name: mypackage + query: data.test.x + want_result: mypackage + + - note: keywords_in_refs/rego v1 all keywords + modules: + - | + package test + import rego.v1 + allow if { + input.package.format == "npm" + input.default.value == 1 + input.if.enabled == true + input.in.set == "member" + input.not.flag == false + input.with.config == "ok" + } + input: + package: + format: npm + default: + value: 1 + if: + enabled: true + in: + set: member + not: + flag: false + with: + config: ok + query: data.test.allow + want_result: true + + - note: keywords_in_refs/future keywords without import + modules: + - | + package test + x = [input.if.flag, input.in.value, input.every.item, input.contains.key] + input: + if: + flag: true + in: + value: member + every: + item: each + contains: + key: present + query: data.test.x + want_result: [true, "member", "each", "present"] + + - note: keywords_in_refs/package path keywords + modules: + - | + package words.if.default + value = 7 + - | + package test + x = data.words.if.default.value + query: data.test.x + want_result: 7 + + - note: keywords_in_refs/import path keywords + modules: + - | + package test + import data.catalog.if.default as kw + x = kw.value + data: + catalog: + if: + default: + value: 99 + query: data.test.x + want_result: 99 + + - note: keywords_in_refs/rule head keyword path + modules: + - | + package test + policy.default.level := 3 + query: data.test.policy.default.level + want_result: 3 + + - note: keywords_in_refs/mixed dot keyword and dynamic bracket + modules: + - | + package test + x = input.package[segment].value + segment = "import" + input: + package: + import: + value: from_dynamic + query: data.test.x + want_result: from_dynamic diff --git a/tests/rvm/rego/cases/chained_access.yaml b/tests/rvm/rego/cases/chained_access.yaml index 7b734be..e8ef5f9 100644 --- a/tests/rvm/rego/cases/chained_access.yaml +++ b/tests/rvm/rego/cases/chained_access.yaml @@ -315,3 +315,109 @@ cases: } query: data.test.main want_result: "/api/v1/users" + + - note: keywords_in_refs/package_field + data: {} + input: + v0: + package: + format: npm + modules: + - | + package test + allow := true if { + input.v0.package.format == "npm" + } + query: data.test.allow + want_result: true + + - note: keywords_in_refs/multiple_keywords + data: {} + input: + default: + value: 42 + import: + name: foo + not: + allowed: false + with: + config: ok + modules: + - | + package test + import rego.v1 + result if { + input.default.value == 42 + input.import.name == "foo" + input.not.allowed == false + input.with.config == "ok" + } + query: data.test.result + want_result: true + + - note: keywords_in_refs/future_keywords_without_import + data: {} + input: + if: + flag: true + in: + value: member + every: + item: each + contains: + key: present + modules: + - | + package test + result := [input.if.flag, input.in.value, input.every.item, input.contains.key] + query: data.test.result + want_result: [true, "member", "each", "present"] + + - note: keywords_in_refs/package_path_keywords + data: {} + modules: + - | + package words.if.default + value := 7 + - | + package test + result := data.words.if.default.value + query: data.test.result + want_result: 7 + + - note: keywords_in_refs/import_path_keywords + data: + catalog: + if: + default: + value: 99 + modules: + - | + package test + import data.catalog.if.default as kw + result := kw.value + query: data.test.result + want_result: 99 + + - note: keywords_in_refs/rule_head_keyword_path + data: {} + modules: + - | + package test + policy.default.level := 3 + query: data.test.policy.default.level + want_result: 3 + + - note: keywords_in_refs/mixed_dot_keyword_and_dynamic_bracket + data: {} + input: + package: + import: + value: from_dynamic + modules: + - | + package test + segment := "import" + result := input.package[segment].value + query: data.test.result + want_result: "from_dynamic"