Skip to content

fix(action-grammar): support optional rule references #2941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Rio Yu (rioyu123) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(action-grammar): support optional rule references #2941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions ts/docs/architecture/core/actionGrammar.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,26 @@ separated by `|`:

#### Expression types

| Syntax | Meaning |
| ------------------- | -------------------------------------- |
| `word` | Literal token (case-insensitive match) |
| `$(var:wildcard)` | Capture any tokens as string |
| `$(var:number)` | Capture numeric token |
| `$(var:EntityType)` | Capture with entity validation |
| `$(var:<RuleName>)` | Capture via sub-rule match |
| `<RuleName>` | Reference another rule (no capture) |
| `( expr )` | Grouping |
| `expr?` | Optional (zero or one) |
| `expr*` | Zero or more |
| `expr+` | One or more |
| `alt1 \| alt2` | Alternation |
| Syntax | Meaning |
| -------------------- | -------------------------------------- |
| `word` | Literal token (case-insensitive match) |
| `$(var:wildcard)` | Capture any tokens as string |
| `$(var:number)` | Capture numeric token |
| `$(var:EntityType)` | Capture with entity validation |
| `$(var:type)?` | Optional captured variable |
| `$(var:<RuleName>)` | Capture via sub-rule match |
| `$(var:<RuleName>)?` | Optional captured sub-rule |
| `<RuleName>` | Reference another rule (no capture) |
| `<RuleName>?` | Optional rule reference |
| `( expr )` | Grouping |
| `( expr )?` | Optional group (zero or one) |
| `( expr )*` | Zero or more |
| `( expr )+` | One or more |
| `alt1 \| alt2` | Alternation |

The `?` suffix must immediately follow the closing `)` or `>`. A literal
question mark immediately after a rule reference must be escaped as `\?`;
a question mark separated by whitespace remains literal.

#### Value expressions

Expand Down
58 changes: 40 additions & 18 deletions ts/packages/actionGrammar/src/grammarCompiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1325,22 +1325,42 @@ function createGrammarRule(
!isLocallyDefined &&
globalPhraseSetRegistry.isPhraseSetName(expr.refName.name)
) {
const partId = allocPartId(
context,
expr.pos,
`<${expr.refName.name}>${expr.optional ? "?" : ""}`,
);
parts.push(
createPhraseSetPart(
expr.refName.name,
undefined,
allocPartId(
context,
expr.pos,
`<${expr.refName.name}>`,
),
),
expr.optional
? createRulesPart(
[
{
parts: [
createPhraseSetPart(
expr.refName.name,
),
],
value: undefined,
spacingMode,
},
],
{
optional: true,
name: expr.refName.name,
partId,
},
)
: createPhraseSetPart(
expr.refName.name,
undefined,
partId,
),
);
// Phrase sets don't produce a captured value on their own.
// Use defaultValue=true so single-part rules using a phrase set
// don't trip the "Start rule does not produce a value" check.
defaultValue = true;
consumedInput(); // phrase sets always consume input
if (!expr.optional) consumedInput();
break;
}
const record = createNamedGrammarRules(
Expand All @@ -1354,22 +1374,24 @@ function createGrammarRule(
defaultValue = record.hasValue;
parts.push(
createRulesPart(record.grammarRules, {
optional: expr.optional,
name: expr.refName.name,
partId: allocPartId(
context,
expr.pos,
`<${expr.refName.name}>`,
`<${expr.refName.name}>${expr.optional ? "?" : ""}`,
),
}),
);
// RuleRefExpr has no optional modifier; it is always non-optional.
// === false: only clear when *definitely* non-nullable (same
// asymmetry as the variable ruleRef case above).
if (record.nullable === false) {
currentEpr = new Set();
if (!expr.optional) {
// === false: only clear when *definitely* non-nullable (same
// asymmetry as the variable ruleRef case above).
if (record.nullable === false) {
currentEpr = new Set();
}
// ?? false: treat undefined (back-ref) as non-nullable.
ruleNullable = ruleNullable && (record.nullable ?? false);
}
// ?? false: treat undefined (back-ref) as non-nullable.
ruleNullable = ruleNullable && (record.nullable ?? false);
break;
}
case "rules": {
Expand Down
22 changes: 15 additions & 7 deletions ts/packages/actionGrammar/src/grammarRuleParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const debugParse = registerDebug("typeagent:grammar:parse");
*
* <VariableSpecifier> ::= <VarName> (":" (<TypeName> | <RuleName>))?
*
* <RuleRefExpr> ::= <RuleName>
* <RuleRefExpr> ::= <RuleName> (immediately followed by "?")?
* <GroupExpr> ::= "(" <Rules> ( ")" | ")?" | ")*" | ")+" )
*
* // ── Value (basic mode: enableValueExpressions=false) ──────────────────────────
Expand Down Expand Up @@ -232,6 +232,7 @@ export type CommentedName = {
export type RuleRefExpr = {
type: "ruleReference";
refName: CommentedName;
optional?: boolean | undefined;
pos?: number | undefined;
leadingComments?: Comment[] | undefined;
};
Expand Down Expand Up @@ -753,7 +754,7 @@ class GrammarRuleParser implements ValueExprParserContext {
refPos = this.pos;
if (this.isAt("<")) {
ruleReference = true;
bracketedName = this.parseRuleName();
bracketedName = this.parseRuleName().name;
} else {
bracketedName = this.parseNameWithComments("Type name");
}
Expand Down Expand Up @@ -798,11 +799,17 @@ class GrammarRuleParser implements ValueExprParserContext {

if (this.isAt("<")) {
const pos = this.pos;
const { name: refName, end } = this.parseRuleName();
const node: RuleRefExpr = {
type: "ruleReference",
refName: this.parseRuleName(),
refName,
pos,
};
if (this.content.startsWith("?", end)) {
node.optional = true;
this.curr = end;
this.skipWhitespace(1);
}
attach(node);
expNodes.push(node);
continue;
Expand Down Expand Up @@ -1171,11 +1178,12 @@ class GrammarRuleParser implements ValueExprParserContext {
return { name, leadingComments, trailingComments };
}

private parseRuleName(): CommentedName {
private parseRuleName(): { name: CommentedName; end: number } {
this.consume("<", "at start of rule name");
const result = this.parseNameWithComments("Rule identifier");
const name = this.parseNameWithComments("Rule identifier");
const end = this.curr + 1;
this.consume(">", "at end of rule name");
return result;
return { name, end };
}

private parseRules(): Rule[] {
Expand Down Expand Up @@ -1238,7 +1246,7 @@ class GrammarRuleParser implements ValueExprParserContext {
afterExportComments?: Comment[],
): RuleDefinition {
const pos = this.pos;
const rn = this.parseRuleName();
const rn = this.parseRuleName().name;
let spacingMode: SpacingMode;
let spacingAnnotationComments: SpacingAnnotationComments | undefined;
let beforeEqualsComments: Comment[] | undefined;
Expand Down
1 change: 1 addition & 0 deletions ts/packages/actionGrammar/src/grammarRuleWriter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ function writeSingleExpr(
}
case "ruleReference":
writeBracketedName(result, expr.refName);
if (expr.optional) result.write("?");
break;
case "rules": {
result.write("(");
Expand Down
31 changes: 31 additions & 0 deletions ts/packages/actionGrammar/test/grammarRuleParser.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,37 @@ describe("Grammar Rule Parser", () => {
value: ["world"],
});
});

it("rule with optional rule reference", () => {
const grammar = "<sentence> = <greeting>? world;";
const result = testParamGrammarRules("test.agr", grammar);

expect(result[0].rules[0].expressions).toHaveLength(2);
expect(result[0].rules[0].expressions[0]).toEqual({
type: "ruleReference",
refName: { name: "greeting" },
optional: true,
});
expect(result[0].rules[0].expressions[1]).toEqual({
type: "string",
value: ["world"],
});
});

it("requires the optional suffix to be adjacent", () => {
const grammar = "<sentence> = <greeting> ? world;";
const result = testParamGrammarRules("test.agr", grammar);

expect(result[0].rules[0].expressions).toHaveLength(2);
expect(result[0].rules[0].expressions[0]).toEqual({
type: "ruleReference",
refName: { name: "greeting" },
});
expect(result[0].rules[0].expressions[1]).toEqual({
type: "string",
value: ["?", "world"],
});
});
});

describe("Expression Parsing", () => {
Expand Down
18 changes: 18 additions & 0 deletions ts/packages/actionGrammar/test/grammarRuleWriter.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,24 @@ function validateRoundTrip(grammar: string) {
}

describe("Formatting layout", () => {
describe("rule references", () => {
it("preserves an optional rule-reference suffix", () => {
const source = "<test> = <greeting>? world;";

expect(fmt(source)).toBe(`${source}\n`);
roundTrip(source);
});

it("does not turn a separated question mark into a suffix", () => {
roundTrip("<test> = <greeting> ? world;");
roundTrip("<test> = <greeting> /* comment */ ? world;");
});

it("keeps an adjacent escaped question mark literal", () => {
roundTrip("<test> = <greeting>\\? world;");
});
});

describe("rule alternatives — flat vs broken", () => {
it("single alt always flat", () => {
// No alternates — always stays on one line
Expand Down
Loading