From fa315e8b63c7eac39eaea426ca21974e9861c75b Mon Sep 17 00:00:00 2001 From: Masaori Koshiba Date: Fri, 18 Sep 2026 09:51:08 +0900 Subject: [PATCH] hrw4u: remove the bare operator statement that never compiled The grammar's `op=IDENT SEMICOLON` alternative parsed, but the compiler rejected everything it matched: the emit path was gated on a statement function having no argument validator, and every entry in STATEMENT_FUNCTION_MAP declares one -- `no-op` included, via arg_count(0). Known operators got a "requires an argument" error that is wrong for the zero-arity ones; any other identifier got "Unknown operator or invalid standalone use". The call form is the only spelling the docs, the u4wrh reverse conversion, and the test corpus produce. --- tools/hrw4u/grammar/hrw4u.g4 | 1 - tools/hrw4u/src/ast_visitor.py | 2 -- tools/hrw4u/src/kg_visitor.py | 3 --- tools/hrw4u/src/symbols.py | 7 ------ tools/hrw4u/src/visitor.py | 9 +------ tools/hrw4u/tests/test_ast_visitor.py | 10 ++++---- tools/hrw4u/tests/test_ops.py | 36 +++++++++++++++++++++++++++ 7 files changed, 42 insertions(+), 26 deletions(-) diff --git a/tools/hrw4u/grammar/hrw4u.g4 b/tools/hrw4u/grammar/hrw4u.g4 index 2cb4db5f2ee..5ab457517d8 100644 --- a/tools/hrw4u/grammar/hrw4u.g4 +++ b/tools/hrw4u/grammar/hrw4u.g4 @@ -168,7 +168,6 @@ statement | functionCall SEMICOLON | lhs=IDENT EQUAL value SEMICOLON | lhs=IDENT PLUSEQUAL value SEMICOLON - | op=IDENT SEMICOLON ; conditional diff --git a/tools/hrw4u/src/ast_visitor.py b/tools/hrw4u/src/ast_visitor.py index 4a66ec0a710..242c81d7127 100644 --- a/tools/hrw4u/src/ast_visitor.py +++ b/tools/hrw4u/src/ast_visitor.py @@ -112,8 +112,6 @@ def _visit_statement(self, ctx) -> BodyNode: target = Target.from_dotted(ctx.lhs.text) value = self._extract_value(ctx.value()) return Assignment(target=target, operator="+=", value=value, line=line) - if ctx.op: - return FunctionCall(name=ctx.op.text, args=(), line=line) raise ValueError(f"Unhandled statement alternative at line {line}") def _visit_function_call(self, ctx) -> FunctionCall: diff --git a/tools/hrw4u/src/kg_visitor.py b/tools/hrw4u/src/kg_visitor.py index 4f7ec6c3534..09a32f258d7 100644 --- a/tools/hrw4u/src/kg_visitor.py +++ b/tools/hrw4u/src/kg_visitor.py @@ -352,9 +352,6 @@ def visitStatement(self, ctx) -> None: case _ if ctx.EQUAL(): stmt_id = self._process_assignment_statement(ctx, stmt_properties) - case _ if ctx.op: - stmt_id = self._add_node("Statement", {**stmt_properties, "type": "operator", "operator": ctx.op.text}) - case _: stmt_id = self._add_node("Statement", {**stmt_properties, "type": "unknown"}) diff --git a/tools/hrw4u/src/symbols.py b/tools/hrw4u/src/symbols.py index a57c6f27041..b6c5cd47568 100644 --- a/tools/hrw4u/src/symbols.py +++ b/tools/hrw4u/src/symbols.py @@ -16,7 +16,6 @@ from __future__ import annotations -from typing import Callable from hrw4u.validation import Validator from hrw4u.errors import SymbolResolutionError import hrw4u.types as types @@ -40,12 +39,6 @@ def __init__( def symbol_for(self, name: str) -> types.Symbol | None: return self._symbols.get(name) - def get_statement_spec(self, name: str) -> tuple[str, Callable[[str], None] | None]: - # Use cached lookup from base class - if params := self._lookup_statement_function_cached(name): - return params.target, params.validate - raise SymbolResolutionError(name, "Unknown operator or invalid standalone use") - def declare_variable( self, name: str, type_name: str, explicit_slot: int | None = None, scope: types.VarScope = types.VarScope.TXN) -> str: try: diff --git a/tools/hrw4u/src/visitor.py b/tools/hrw4u/src/visitor.py index 51a62c35894..e0828e75455 100644 --- a/tools/hrw4u/src/visitor.py +++ b/tools/hrw4u/src/visitor.py @@ -919,14 +919,7 @@ def visitStatement(self, ctx) -> None: return case _: - if ctx.op is None: - raise SymbolResolutionError("operator", "Missing operator in statement") - operator = ctx.op.text - self._dbg(f"standalone op: {operator}") - cmd, validator = self.symbol_resolver.get_statement_spec(operator) - if validator: - raise SymbolResolutionError(operator, "This operator requires an argument") - self.emit_statement(cmd) + # Only reachable via parser error recovery, which already reported the error. return def visitVariables(self, ctx) -> None: diff --git a/tools/hrw4u/tests/test_ast_visitor.py b/tools/hrw4u/tests/test_ast_visitor.py index ec919d1f060..76d048ac2d4 100644 --- a/tools/hrw4u/tests/test_ast_visitor.py +++ b/tools/hrw4u/tests/test_ast_visitor.py @@ -80,12 +80,12 @@ def test_with_args(self): assert fc.name == "set-header" assert fc.args == (LiteralStringValue(raw="X-Foo"), LiteralStringValue(raw="bar")) - def test_standalone_operator(self): - ast = _build('REMAP {\n skip-remap;\n}') + def test_bool_arg(self): + ast = _build('REMAP {\n skip-remap(true);\n}') fc = ast.body[0].body[0] assert isinstance(fc, FunctionCall) assert fc.name == "skip-remap" - assert fc.args == () + assert fc.args == (True,) def test_break(self): ast = _build('REMAP {\n if true {\n break;\n }\n}') @@ -494,7 +494,7 @@ class TestLineNumbers: "REMAP {\n" # line 8 ' inbound.req.X-Foo = "val";\n' # line 9 " set-debug();\n" # line 10 - " skip-remap;\n" # line 11 + " skip-remap(true);\n" # line 11 ' if inbound.req.X-A == "a" {\n' # line 12 " break;\n" # line 13 ' } elif inbound.req.X-B == "b" {\n' # line 14 @@ -565,7 +565,7 @@ def test_function_call(self): assert isinstance(fc, FunctionCall) assert fc.line == 10 - def test_standalone_operator(self): + def test_function_call_with_arg(self): fc = self.ast.body[3].body[2] assert isinstance(fc, FunctionCall) assert fc.line == 11 diff --git a/tools/hrw4u/tests/test_ops.py b/tools/hrw4u/tests/test_ops.py index c8d06115f2e..f1c42c57744 100644 --- a/tools/hrw4u/tests/test_ops.py +++ b/tools/hrw4u/tests/test_ops.py @@ -21,6 +21,17 @@ import pytest import utils +from hrw4u.common import create_parse_tree +from hrw4u.hrw4uLexer import hrw4uLexer +from hrw4u.hrw4uParser import hrw4uParser +from hrw4u.visitor import HRW4UVisitor + + +def _compile(source: str): + tree, _, errors = create_parse_tree(source, "", hrw4uLexer, hrw4uParser, "hrw4u", collect_errors=True) + output = HRW4UVisitor(filename="", error_collector=errors).visit(tree) + return output, errors + @pytest.mark.ops @pytest.mark.parametrize("input_file,output_file", utils.collect_output_test_files("ops", "hrw4u")) @@ -42,3 +53,28 @@ def test_ast_matches(input_file: Path, ast_file: Path) -> None: @pytest.mark.parametrize("input_file", utils.collect_failing_inputs("ops")) def test_invalid_inputs_fail(input_file): utils.run_failing_test(input_file) + + +@pytest.mark.ops +@pytest.mark.invalid +@pytest.mark.parametrize("op", ["no-op", "skip-remap", "set-debug"]) +def test_bare_operator_is_a_syntax_error(op: str) -> None: + """Operators only have a call form; `name;` is not a statement.""" + _, errors = _compile(f"REMAP {{\n {op};\n}}\n") + assert errors.has_errors() + assert "no viable alternative" in str(errors.errors[0]) + + +@pytest.mark.ops +@pytest.mark.invalid +def test_bare_operator_reports_once() -> None: + """Visiting the error-recovered node must not pile a second diagnostic onto it.""" + _, errors = _compile("REMAP {\n no-op;\n}\n") + assert len(errors.errors) == 1 + + +@pytest.mark.ops +def test_zero_arg_operator_call_form_compiles() -> None: + output, errors = _compile("REMAP {\n no-op();\n}\n") + assert not errors.has_errors() + assert "no-op" in "\n".join(output)