Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions docs/design/sysml2-tools-core/filtering.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ flowchart TD
`(as Type).attribute` metadata read is intentionally rejected.
- The evaluator is read-only over `SysmlWorkspace`; it never mutates declarations or resolved
edges.
- `Parse`/`Evaluate` must never throw and must never crash the process, since a planned future GUI
will call them live on every keystroke of a text-editing filter box (see
`docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md`'s Error Handling
section for the hardening this constraint requires beyond ordinary syntax-error diagnostics:
a bounded nesting-depth guard against ANTLR's own recursive-descent stack overflow, a broadened
catch for ANTLR-internal failures such as astral-plane Unicode input, an EOF check rejecting
trailing garbage after a valid expression prefix, and a finite-value check rejecting numeric
literals that overflow `double`).

### Requirements Traceability

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- cspell:ignore istype hastype -->
<!-- cspell:ignore istype hastype parenthesization uncatchable -->

### FilterExpressionEvaluator

Expand Down Expand Up @@ -39,6 +39,22 @@ virtual file path (`[filter-expression]`) so parsing never writes to the console
malformed input. If ANTLR reports no syntax errors, `Parse` delegates to `TryBuild` to adapt the
CST into the Phase 1 AST.

`Parse` guards against four hardening scenarios beyond ordinary syntax errors, all reported as
`SysmlDiagnostic` rather than by throwing or crashing (see Error Handling below for why each is
necessary):

1. It eagerly tokenizes the input (`CommonTokenStream.Fill()`, which only drives the iterative
lexer and never recurses) and rejects input whose parenthesization/bracket-indexing/
body-expression-brace/prefix-unary-operator nesting depth would exceed `MaxNestingDepth` (200)
*before* invoking the recursive-descent parser, which has no depth guard of its own.
2. It catches `Exception` generically (in addition to `RecognitionException`) around the lex/parse
call, converting any other unexpected ANTLR-internal failure into a diagnostic.
3. It checks the token stream is positioned at EOF after `ownedExpression()` returns, reporting an
"unexpected trailing content" diagnostic when the parser only consumed a prefix of the input.
4. `TryBuildLiteral` rejects integer/real literals that parse to a non-finite `double` (overflow),
reporting a "numeric literal out of range" diagnostic instead of producing an
`Infinity`/`NaN`-valued literal that cannot round-trip through `ToString()`.

##### `FilterExpressionParser.TryBuild(OwnedExpressionContext, diagnostics)`

Performs a shape-driven CST walk restricted to the supported subset:
Expand Down Expand Up @@ -88,6 +104,55 @@ candidates, missing metadata, and missing attributes. Diagnostics in `FilterEval
currently always empty because evaluation of an already-supported AST cannot fail, but the result
shape leaves room for future evaluation-time diagnostics without a breaking API change.

`Parse`'s "never throws" contract is non-negotiable: a planned future GUI will call `Parse`/
`Evaluate` live on every keystroke of a text-editing filter box, so any uncaught exception —
recognized syntax error or not — would be user-visible and disruptive, and an uncatchable process
crash would be far worse. A retroactive robustness review found four ways ANTLR's own
lexer/recursive-descent-parser internals could violate that contract despite `Parse`'s existing
`RecognitionException` handling, all now hardened against:

- **Uncatchable stack overflow on deep nesting**: ANTLR's recursive-descent
`ownedExpression()`/`baseExpression()`/`bodyExpression()` parse recurses once per nesting
level — each `(`, each `[` (the `ownedExpression LBRACK sequenceExpressionList? RBRACK`
sequence-indexing production), each `{` (the `bodyExpression : LBRACE functionBodyPart RBRACE`
production, reachable via `ownedExpression DOT_QUESTION bodyExpression` and directly from
`baseExpression`), or each prefix unary operator such as `not` — with no depth guard. Every one
of these three balanced-delimiter productions recurses back into `ownedExpression` for its
enclosed contents exactly like parenthesization does; per `SysMLv2Parser.g4`'s
`ownedExpression`/`baseExpression`/`bodyExpression`/`argumentList` productions, `(`/`)`, `[`/`]`,
and `{`/`}` are the *complete* set of delimiter pairs that can drive this recursion — see
`ExceedsMaxNestingDepth`'s remarks in source for the full derivation. Beyond roughly 4000-5000
levels for parens (far fewer — around 500 — for bracket indexing and body-expression braces,
since their recursion signature involves an extra `sequenceExpressionList()`/`functionBodyPart()`
frame per level) this overflows the native call stack with a `StackOverflowException`, which —
unlike every other .NET exception — cannot be caught and terminates the entire process
immediately, not just the `Parse` call. `Parse` now pre-scans the already-lexed (non-recursive)
token stream and rejects input whose simulated recursion depth — tracking `(`/`[`/`{` and prefix
unary operators identically as "pending frame" pushes, popped by a matching `)`/`]`/`}` or by
reaching the next atom — would exceed 200 levels, well before the recursive parser runs. Two
follow-up reviews each found the guard missing one of the three delimiter pairs in turn
(`[`/`]` first, then `{`/`}`); all three are now handled identically and the source comment
explicitly records that these are the complete set, to catch a future grammar change that adds a
fourth.
- **Uncaught `ArgumentException` on astral-plane Unicode input**: `Antlr4.Runtime.Lexer.GetErrorDisplay`
calls `Char.ConvertToUtf32` while formatting a lexer error message, which throws `ArgumentException`
(not `RecognitionException`) for input containing an unpaired UTF-16 surrogate — including a
*paired* surrogate that the lexer itself cannot otherwise tokenize. `Parse`'s catch clause was
broadened to `catch (Exception ex)` around the lex/parse call (a deliberate, documented generic
catch, consistent with this repository's established pattern for boundary code that must never
propagate an unexpected failure) so this and any other ANTLR-internal failure mode become a
diagnostic instead of an unhandled exception.
- **Silent trailing-garbage acceptance**: `parser.ownedExpression()` only requires a syntactically
valid expression *prefix*; it returns successfully without consuming any trailing tokens, and
previously nothing checked for that. `Parse` now verifies the token stream is positioned at EOF
after the parse and reports an "unexpected trailing content" diagnostic otherwise.
- **Non-lossless round-trip for numeric literal overflow**: `double.TryParse` silently accepts
literal text whose magnitude overflows `double` (e.g. `3.14e400`), returning
`double.PositiveInfinity` rather than failing. `LiteralFilterExpression.ToString()` would then
print the non-SysML-syntax text `"Infinity"`, which fails to re-parse. `TryBuildLiteral` now
checks `double.IsFinite` after parsing an integer/real literal and reports a "numeric literal out
of range" diagnostic instead of building a non-finite-valued literal.

#### Dependencies

- `SysMLv2Lexer` / `SysMLv2Parser` (Language Parser subsystem) — reusable grammar implementation
Expand Down Expand Up @@ -126,6 +191,10 @@ shape leaves room for future evaluation-time diagnostics without a breaking API
`FilterExpressionEvaluator.EvaluateComparison`
- `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-UnsupportedConstructDiagnostics` —
`CollectingErrorListener`, `FilterExpressionParser.Unsupported`,
`FilterExpressionParser.TryBuildLiteral`, `FilterExpressionEvaluator.Evaluate`
`FilterExpressionParser.TryBuildLiteral`, `FilterExpressionEvaluator.Evaluate`,
`FilterExpressionParser.ExceedsMaxNestingDepth` (deep-nesting guard), `FilterExpressionParser.Parse`'s
broadened `catch (Exception)` (astral Unicode / other ANTLR-internal failures), and `Parse`'s
post-parse EOF check (trailing-content diagnostic)
- `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-RoundTripPrettyPrinting` —
`FilterExpression.ToString()` overrides and `FilterExpressionParser.Parse`
`FilterExpression.ToString()` overrides, `FilterExpressionParser.Parse`, and
`FilterExpressionParser.TryBuildLiteral`'s `double.IsFinite` check (numeric-literal-overflow guard)
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,18 @@ sections:
- id: SysML2Tools-Core-Filtering-FilterExpressionEvaluator-UnsupportedConstructDiagnostics
title: >-
FilterExpressionEvaluator shall report malformed syntax and every construct outside the
Phase 1 subset as explicit diagnostics and shall never throw while parsing or evaluating
them.
Phase 1 subset as explicit diagnostics and shall never throw — or crash the process —
while parsing or evaluating them, including for adversarial/malformed input shapes such
as pathologically deep nesting, non-BMP Unicode characters, and syntactically-valid
expression prefixes followed by trailing content.
justification: |
Unsupported constructs are an expected Phase 1 boundary, not an exceptional condition.
Reporting them as diagnostics allows layout to fall back to the unfiltered scope with a
visible warning instead of crashing or silently dropping the filter.
visible warning instead of crashing or silently dropping the filter. Because a planned
future GUI will call `Parse`/`Evaluate` live on every keystroke of a text-editing filter
box, this "never throws / never crashes" contract must also hold for adversarial or
malformed input a user could easily produce by accident (a stray paste, an editor
autocomplete bug, an emoji typed mid-edit) — not just recognized syntax errors.
tests:
- Parse_Istype_ReturnsUnsupportedConstructDiagnostic
- Parse_Hastype_ReturnsUnsupportedConstructDiagnostic
Expand All @@ -87,15 +93,34 @@ sections:
- Parse_Conditional_ReturnsUnsupportedConstructDiagnostic
- Parse_GeneralFeatureChainNavigation_ReturnsUnsupportedConstructDiagnostic
- Parse_MalformedSyntax_NeverThrows_ReturnsDiagnostic
- Parse_DeeplyNestedParentheses_ReturnsDiagnosticInsteadOfCrashing
- Parse_ModeratelyNestedParentheses_StillParsesSuccessfully
- Parse_DeeplyNestedBracketIndexing_ReturnsDiagnosticInsteadOfCrashing
- Parse_ShallowBracketIndexing_ReturnsUnsupportedConstructNotDeepNestingDiagnostic
- Parse_DeeplyNestedBodyExpressionBraces_ReturnsDiagnosticInsteadOfCrashing
- Parse_ShallowBodyExpressionBraces_ReturnsUnsupportedConstructNotDeepNestingDiagnostic
- Parse_AstralPlaneUnicodeCharacter_NeverThrows_ReturnsDiagnostic
- Parse_AstralPlaneUnicodeCharacterAsTrailingToken_NeverThrows_ReturnsDiagnostic
- Parse_TrailingGarbageAfterValidExpression_ReturnsDiagnostic
- Parse_TrailingCloseParen_ReturnsDiagnostic
- Parse_TrailingSemicolon_ReturnsDiagnostic
- Evaluate_UnknownCandidate_SkipsGracefully

- id: SysML2Tools-Core-Filtering-FilterExpressionEvaluator-RoundTripPrettyPrinting
title: >-
Every supported Phase 1 filter-expression tree shall pretty-print to canonical SysML v2
filter syntax whose re-parse yields a semantically-equivalent tree.
filter syntax whose re-parse yields a semantically-equivalent tree, and numeric literals
that would overflow that lossless round-trip (i.e. parse to a non-finite `double`) shall
be rejected as an invalid literal at parse time rather than silently accepted.
justification: |
Canonical pretty-printing keeps diagnostics, debugging output, and future persisted
filter-expression scenarios stable. Re-parsing the printed form proves the AST shape and
printer stay aligned with the accepted grammar subset.
printer stay aligned with the accepted grammar subset. A numeric literal that overflows
`double` during parsing (e.g. `3.14e400`) would otherwise silently become
`double.PositiveInfinity`, whose pretty-printed form (`"Infinity"`) is not valid SysML v2
syntax and cannot be re-parsed — violating this requirement's round-trip guarantee at its
source rather than only at its symptom.
tests:
- Parse_RoundTrip_PrettyPrintedTextReparsesToEquivalentTree
- Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity
- Parse_LargeButFiniteRealLiteral_StillParsesSuccessfully
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- cspell:ignore istype hastype reparses -->
<!-- cspell:ignore istype hastype reparses parenthesization -->

### FilterExpressionEvaluator Verification

Expand Down Expand Up @@ -27,6 +27,14 @@ SDK and the repository's committed SysML fixtures.
- Absent metadata attributes evaluate conservatively as false.
- Unsupported constructs and malformed syntax report diagnostics and never throw.
- Pretty-printing a supported AST re-parses to an equivalent tree.
- Pathologically deep nesting (thousands of levels of parenthesization, or hundreds of levels of
sequence-indexing brackets or body-expression braces) reports a diagnostic instead of
overflowing the native call stack and crashing the process.
- Filter text containing non-BMP (astral-plane) Unicode characters never throws.
- A syntactically valid expression prefix followed by trailing content reports a diagnostic
instead of silently discarding the trailing tokens.
- A numeric literal that overflows `double` during parsing (producing a non-finite value) reports
a diagnostic instead of silently round-tripping to unparsable `"Infinity"`/`"NaN"` text.

#### Requirement-to-Test Mapping

Expand Down Expand Up @@ -65,9 +73,22 @@ SDK and the repository's committed SysML fixtures.
- `Parse_Conditional_ReturnsUnsupportedConstructDiagnostic`
- `Parse_GeneralFeatureChainNavigation_ReturnsUnsupportedConstructDiagnostic`
- `Parse_MalformedSyntax_NeverThrows_ReturnsDiagnostic`
- `Parse_DeeplyNestedParentheses_ReturnsDiagnosticInsteadOfCrashing`
- `Parse_ModeratelyNestedParentheses_StillParsesSuccessfully`
- `Parse_DeeplyNestedBracketIndexing_ReturnsDiagnosticInsteadOfCrashing`
- `Parse_ShallowBracketIndexing_ReturnsUnsupportedConstructNotDeepNestingDiagnostic`
- `Parse_DeeplyNestedBodyExpressionBraces_ReturnsDiagnosticInsteadOfCrashing`
- `Parse_ShallowBodyExpressionBraces_ReturnsUnsupportedConstructNotDeepNestingDiagnostic`
- `Parse_AstralPlaneUnicodeCharacter_NeverThrows_ReturnsDiagnostic`
- `Parse_AstralPlaneUnicodeCharacterAsTrailingToken_NeverThrows_ReturnsDiagnostic`
- `Parse_TrailingGarbageAfterValidExpression_ReturnsDiagnostic`
- `Parse_TrailingCloseParen_ReturnsDiagnostic`
- `Parse_TrailingSemicolon_ReturnsDiagnostic`
- `Evaluate_UnknownCandidate_SkipsGracefully`
- `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-RoundTripPrettyPrinting`
- `Parse_RoundTrip_PrettyPrintedTextReparsesToEquivalentTree`
- `Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity`
- `Parse_LargeButFiniteRealLiteral_StillParsesSuccessfully`

#### Test Scenarios

Expand All @@ -87,3 +108,18 @@ SDK and the repository's committed SysML fixtures.
failure
- `Parse_RoundTrip_PrettyPrintedTextReparsesToEquivalentTree` — pretty-printer output remains
accepted by the parser
- `Parse_DeeplyNestedParentheses_ReturnsDiagnosticInsteadOfCrashing` — 5000 levels of nested
parentheses report a diagnostic instead of overflowing the native call stack
- `Parse_DeeplyNestedBracketIndexing_ReturnsDiagnosticInsteadOfCrashing` — 500 levels of nested
sequence-indexing brackets (`a[a[a[...0...]]]`) report a diagnostic instead of overflowing the
native call stack, closing the gap a follow-up review found in the initial paren-only guard
- `Parse_DeeplyNestedBodyExpressionBraces_ReturnsDiagnosticInsteadOfCrashing` — 500 levels of
nested body-expression braces (`a.?{a.?{...0...}}`) report a diagnostic instead of overflowing
the native call stack, closing a second follow-up gap in the paren/bracket-only guard
- `Parse_AstralPlaneUnicodeCharacter_NeverThrows_ReturnsDiagnostic` — an astral-plane Unicode
character (surrogate pair) is reported as a diagnostic instead of throwing `ArgumentException`
- `Parse_TrailingGarbageAfterValidExpression_ReturnsDiagnostic` — a valid expression prefix
followed by extra tokens is reported as a diagnostic instead of silently truncating
- `Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity` — a numeric literal that
overflows `double` (`3.14e400`) is reported as a diagnostic instead of silently becoming
`Infinity`
Loading
Loading