Skip to content

Report parser errors with line, column, and source caret#87

Merged
mkrueger merged 12 commits into
mainfrom
dev/mkrueger/parser-error-reporting
May 20, 2026
Merged

Report parser errors with line, column, and source caret#87
mkrueger merged 12 commits into
mainfrom
dev/mkrueger/parser-error-reporting

Conversation

@mkrueger
Copy link
Copy Markdown
Contributor

@mkrueger mkrueger commented May 20, 2026

Problem

Parser failures in the interactive shell were silently swallowed, and a number of adjacent error-reporting paths were sub-par:

  • Typing { { ls } returned to the prompt with no output. ParserErrorCommandState flowed into PrintState, which only printed state.Result (null), dropping the error list entirely.
  • A few Fluent messages used \u007B for literal braces, which Linguini interprets as a placeable start — so Expected '{' rendered as Expected 's'.
  • The runtime catch in PrintState dumped e.InnerException.ToString(), leaking full stack traces into user-facing output for ordinary operational failures (CosmosException, RequestFailedException, AuthenticationFailedException).
  • The non-verbose error prefix was the literal class name PrintState:.
  • The parse error: / parse warning: prefix was hardcoded English.
  • Typos like qery were rejected with no hint about the intended command.
  • Cosmos NoSQL BadRequest responses came back as a single wall-of-text Message: blob — no caret, no source line.
  • Long source lines (typed queries) dumped the entire command, pushing the caret off-screen.
  • Diagnostics from inside .csh scripts hid the file name, so editors couldn't jump to the offending line.
  • Unknown command options (--partiton-key) were rejected with no hint about the intended option.

Change

Parser diagnostics

ShellInterpreter.ExecuteCommandAsync now intercepts ParserErrorCommandState and renders a compiler-style diagnostic:

parse error: Expected '{' (1:1)
  > 1 | { { ls   }
        ^
  • Line/column computed from the error offset against the original command text.
  • Source line is echoed with a caret pointing to the column.
  • Tabs in the source line are expanded to a 4-column tab stop and the caret column is shifted to match, so the caret stays aligned with the offending character.
  • Warnings (ErrorLevel.Warning) are still reported in full.
  • Only the first hard error (ErrorLevel.Error) is shown. A shell isn't a compiler — follow-up errors after the first are usually cascade noise (consistent with bash / pwsh / fish behavior).
  • Output is forwarded to ErrOutRedirect when set, respecting AppendErrRedirection. Redirected output is plain text (no Spectre markup).
  • The parse error / parse warning prefix is resolved through MessageService (parser-error-prefix, parser-warning-prefix) so future translations can localize it.

Fluent brace literal fix

statement_error_expected_open_brace, statement_error_expected_close_brace, and statement_error_unexpected_close_brace now use Fluent's string-literal placeable form ({ "{" }) so the rendered text is the literal brace character.

Runtime catch cleanup

The PrintState catch no longer prints e.InnerException.ToString(). Non-verbose output is now:

  • OperationCanceledException → a single localized Canceled. line.
  • All other exceptions → error: <message> followed by → <message> for each InnerException in the chain, no stack traces.
  • --verbose still surfaces the full exception via AnsiConsole.WriteException.
  • The error prefix is resolved through MessageService (runtime-error-prefix).

"Did you mean…?" for commands

CommandNotFoundException now accepts an optional suggestion that the parser computes from shell.App.Commands plus shell.Functions using a Levenshtein distance budget scaled by input length. The exception message becomes 'qery' is not recognized… Did you mean 'query'?. Localized via error-command-not-found-suggestion.

Query error caret + long-line trimming

A new SourceCaretRenderer returns a tab-expanded display string, a caret column, padding, and a caret marker sized to the diagnostic's span. When the line exceeds ~100 visual chars it is trimmed around the caret with ellipses on the trimmed side(s). Both ReportParserErrors and the new query path render through it, so long lines no longer dump the entire command.

A new QueryErrorLocator is a tolerant Cosmos NoSQL error parser that extracts a line / column / length from the most common shapes (structured errors array with location.start/location.end, legacy Errors array of strings, and free-form near '<token>'). QueryCommand now calls ShellInterpreter.TryReportQueryError before throwing on BadRequest; when a location is recognized the shell prints a compiler-style:

query error: Identifier 'FORM' could not be resolved. (1:10)
  > 1 | SELECT * FORM c
                 ^^^^

and throws CommandReportedException so the generic error reporter stays silent. The query error prefix is localized via query-error-prefix.

Script-file origin

When a parser or query diagnostic is raised while a .csh script is executing, the diagnostic prepends the script's file name in cargo / clang style (<file>:<line>:<col>: <prefix>: <msg>) instead of trailing (L:C). Origin is read from ShellInterpreter.CurrentScriptFileName, which RunScriptAsync already maintains, and reduced to Path.GetFileName so absolute paths don't leak. Interactive prompt errors (no script origin) keep the existing (L:C) suffix.

"Did you mean…?" for options

Both CommandStatement and CommandExpression route their Unknown option rejection through a shared UnknownOptionMessage helper that gathers candidate option names from the bound command type and asks CommandNameSuggester for the closest match. The hardcoded English literal is replaced by two new Fluent keys (error-unknown-option, error-unknown-option-suggestion):

error: Unknown option '--fooo'. Did you mean '--foo'?

The dash prefix the user typed is reconstructed from the gap between MinusToken.Start and NameToken.Start so --fooo surfaces --foo (not -foo).

Tests

  • ExecuteCommandExceptionTests — unclosed brace, redirected file, first-error-only, tab-aware caret alignment, no Spectre markup in redirected file, TryReportQueryError happy paths, script-origin prepending for both parser and query paths (and absolute paths not leaked).
  • LocalizationKeyAuditTests — brace literals, runtime prefixes, query-error-prefix, error-unknown-option + error-unknown-option-suggestion.
  • SourceCaretRendererTests — tab expansion, short / long / near-end / near-start lines, multi-char carets.
  • QueryErrorLocatorTests — structured errors, legacy Errors, free-form near '<token>', unrecognized shapes.
  • CommandNameSuggesterTests — common typos, unrelated input, exact match, empty input.
  • CommandStatementTestsqeryquery, unrelated input → no hint.
  • CommandOptionBindingTests — single-dash typo, double-dash typo, unrelated unknown option (no false suggestion).

Full suite: 968 passed / 0 failed (74 integration tests skipped, as expected without a live endpoint).

Example:

image

Previously, parser failures flowed through PrintState which only printed state.Result, silently dropping the error. The shell now intercepts ParserErrorCommandState in ExecuteCommandAsync and renders a compiler-style message:

  parse error: <message> (<line>:<col>)
  > <line> | <source line>
            ^

To avoid follow-up cascade noise (common in a shell), only the first hard error is shown; warnings are still reported in full. Output is also forwarded to ErrOutRedirect when set.

Adds tests covering ParserErrorCommandState detection, the redirected-error format, and the single-error guarantee.
@mkrueger mkrueger requested review from a team and Copilot May 20, 2026 07:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes interactive-shell parser failures being silently dropped by emitting compiler-style diagnostics (message + line/column + source line + caret) when ExecuteCommandAsync returns a ParserErrorCommandState.

Changes:

  • Intercepts ParserErrorCommandState in ShellInterpreter.ExecuteCommandAsync and renders parser diagnostics (with stderr file redirection support).
  • Adds helpers to map absolute offsets to line/column and to render a caret under the offending column.
  • Adds unit tests validating parser error state, stderr redirection output, and “first hard error only” reporting.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Detects parser error states and prints a location-aware diagnostic to console or ErrOutRedirect.
CosmosDBShell.Tests/Shell/ExecuteCommandExceptionTests.cs Adds tests asserting parser errors are surfaced and properly redirected / de-duplicated.

Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Outdated
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Outdated
mkrueger added 2 commits May 20, 2026 10:09
en.ftl: 'Expected ''\u007B''' rendered as 'Expected ''s''' because Linguini Fluent treats the literal text '\u007B' as a brace-opening placeable mid-string. Switch to Fluent's string-literal placeable form ({ "{" }) so the message renders the actual brace character.

ShellInterpreter.PrintState: the catch block was printing 'PrintState:' as a debug prefix on runtime errors, leaking an internal class name to the user. Drop the prefix; print 'error: <message>' to match the parser-error format.

Adds LocalizationKeyAuditTests theory cases asserting the brace messages render as 'Expected ''{''', 'Expected ''}''', and 'Unexpected ''}'''.
Tabs in the source line were emitted verbatim while the caret was placed using character columns, so any tab before the error column caused the caret to drift to the left of the offending token. ReportParserErrors now expands tabs to a 4-column tab stop in the echoed source line and shifts the caret column by the same amount, keeping the caret aligned with the offending character on the user's terminal.

Adds ExecuteCommandAsync_ParserError_ExpandsTabsForCaretAlignment which feeds '\t}', then verifies the echoed source line contains no raw tab and that the caret column points exactly at the '}' on the displayed line.
Copilot AI review requested due to automatic review settings May 20, 2026 08:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Outdated
mkrueger added 3 commits May 20, 2026 10:22
ReportParserErrors now resolves the 'parse error' / 'parse warning' prefix through MessageService so future translations can localize it, with English fallbacks if the Fluent key is missing.

The PrintState catch previously dumped the inner exception via .ToString(), which leaked stack traces into user-facing output for ordinary operational failures (CosmosException, RequestFailedException, AuthenticationFailedException, ShellException). It now prints the message chain only and silences OperationCanceledException with a short localized 'Canceled.' line; verbose mode still surfaces the full exception.

Adds RuntimePrefixMessages_AreLocalized covering the three new Fluent keys.
Adds a Levenshtein-based CommandNameSuggester that picks the nearest known command name (built-ins plus user-defined functions) within an edit-distance budget scaled by input length. CommandNotFoundException accepts the suggestion and appends a localized 'Did you mean ''X''?' sentence to its message; both CommandStatement and CommandExpression compute the suggestion before throwing.

Adds unit tests for the suggester across common typos and end-to-end tests that 'qery' suggests 'query' while unrelated input ('xyzzyfoobarbaz') yields no suggestion.
Adds a shared SourceCaretRenderer that expands tabs, trims the displayed source line around the caret with ''… '' ellipses when it exceeds ~100 visual chars, and returns ready-to-display caret padding plus a caret marker sized to the diagnostic's span. ReportParserErrors now delegates to the renderer, so long parser-error source lines no longer dump the entire command and the caret remains aligned.

Introduces QueryErrorLocator, a tolerant Cosmos NoSQL error parser that extracts a line/column/length from the most common shapes (structured ''errors'' arrays with location.start/end, legacy ''Errors'' string arrays, and free-form ''near ''<token>'''' phrases). QueryCommand calls a new ShellInterpreter.TryReportQueryError before throwing on BadRequest responses; when a location is recognized the shell prints a compiler-style ''query error: <msg> (L:C)'' with the offending span underlined and throws CommandReportedException so the generic error reporter stays silent.

Adds SourceCaretRendererTests, QueryErrorLocatorTests, end-to-end TryReportQueryError tests, a localization theory for the new query-error-prefix key, and a regression test asserting redirected parser-error output never contains Spectre markup.
Copilot AI review requested due to automatic review settings May 20, 2026 08:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/QueryCommand.cs
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs
mkrueger added 2 commits May 20, 2026 10:42
When a parser or query diagnostic is raised while a .csh script is executing, AppendSourceCaretDiagnostic now prepends the script's file name in cargo/clang style ('<file>:<line>:<col>: <prefix>: <msg>') instead of trailing '(L:C)'. Origin is taken from ShellInterpreter.CurrentScriptFileName, which RunScriptAsync already maintains, and is reduced to Path.GetFileName so absolute paths don't leak into the diagnostic. Interactive prompt errors (no script origin) keep the existing '(L:C)' suffix so the output reads naturally without a fake file name. The '  > N | ...' source row and caret row are unchanged in both forms.

Adds ExecuteCommandAsync_ParserError_InScriptContext_PrependsScriptOrigin and TryReportQueryError_InScriptContext_PrependsScriptOrigin covering both diagnostic paths and verifying the absolute path is not leaked.
Both CommandStatement and CommandExpression now route their 'Unknown option' rejection through a shared UnknownOptionMessage helper that gathers the candidate option names from the bound command type, asks CommandNameSuggester for the closest match within its Levenshtein budget, and emits 'Unknown option ''<typed>''. Did you mean ''<suggestion>''?' when a close candidate exists. The hard-coded English literal is replaced by two new Fluent keys ('error-unknown-option' and 'error-unknown-option-suggestion') so the diagnostic can be localized alongside the existing command-not-found suggestion.

The dash prefix the user typed is reconstructed from the gap between MinusToken.Start and NameToken.Start because the parser stores only the first '-' explicitly; that lets '--fooo' surface 'Did you mean ''--foo''?' instead of dropping the second dash.

Adds UnknownOption_WithCloseTypo_SuggestsKnownOption, UnknownOption_WithDoubleDashTypo_PreservesDoubleDashInSuggestion, UnknownOption_WithUnrelatedName_HasNoSuggestion, plus a localization audit theory covering the two new Fluent keys.
Copilot AI review requested due to automatic review settings May 20, 2026 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Util/SourceCaretRenderer.cs Outdated
mkrueger added 2 commits May 20, 2026 11:03
Previously the suggestion sentence ('Did you mean ''query''?') was concatenated onto the base error message and rendered through ReportExecutionError, which wraps the entire string in [red]. That made the actionable hint visually identical to the error itself and crammed it onto the same overflowing line.

Adds an internal IShellExceptionWithHint contract carried by CommandNotFoundException and a new UnknownOptionException : CommandException. The exception Message now contains only the base sentence; the formatted hint lives on a separate Hint property. Both runtime error renderers (ReportExecutionError for the REPL command path and the PrintState fallback) detect the interface and emit the hint on a second, indented line in default color, mirroring cargo's diagnostic shape. UnknownOptionMessage.Build now returns (Message, Hint) so both throw sites can populate the new exception type.

Updates CommandStatementTests and CommandOptionBindingTests to assert on the new Hint property and the more specific UnknownOptionException type.
Both runtime renderers were prefixing the 'Did you mean ''X''?' hint with two spaces to evoke cargo's nested-note layout, but in this shell the hint is the only follow-up line so the indent reads as stray whitespace before the sentence rather than as structure.
Copilot AI review requested due to automatic review settings May 20, 2026 09:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs:1441

  • Non-verbose console output still prints e.InnerException.ToString() when showInner is true, which surfaces stack traces. To keep non-verbose output user-friendly and consistent with the new runtime catch behavior, print only inner.Message (and possibly walk the InnerException chain) instead of ToString().
            if (showInner)
            {
                AnsiConsole.WriteLine(e.InnerException!.ToString());
            }

Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Util/UnknownOptionMessage.cs
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Util/UnknownOptionMessage.cs
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Outdated
Comment thread CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs Outdated
mkrueger added 2 commits May 20, 2026 12:15
Fixes the Linux offline pipeline failure by trimming script diagnostic origins with both Windows and Unix path separators instead of relying on Path.GetFileName, which does not treat backslashes as separators on Linux.

Addresses Copilot diagnostic concerns: query reported exceptions now propagate without being wrapped, ReportExecutionError suppresses already-reported markers anywhere in the exception chain, non-verbose inner exceptions print message chains instead of stack traces, source diagnostics always render gutter/caret context even for blank lines, and numeric diagnostic locations now use source columns while the caret renderer keeps separate displayed columns for trimmed output.

Also preserves EOF caret placement after trimmed source display and removes unreachable MessageService fallback expressions.
Copilot AI review requested due to automatic review settings May 20, 2026 10:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Comment thread CosmosDBShell.Tests/Shell/CompletionTests.cs
@mkrueger mkrueger merged commit cb4446a into main May 20, 2026
9 checks passed
@mkrueger mkrueger deleted the dev/mkrueger/parser-error-reporting branch May 20, 2026 11:01
mkrueger added a commit that referenced this pull request May 21, 2026
Converts the `Unreleased — since v1.0.273` section to `1.1.4 —
2026-05-21` (first release on the 1.1 line) and adds entries for the PRs
that landed after #86 (the last CHANGELOG update):

- **Highlights / New features:** multi-line REPL input with `\`
continuation and parser-driven incomplete-input detection
([#88](#88)); parser & query
diagnostics with line, column, source caret, and "Did you mean…"
suggestions ([#87](#87)).
- **Documentation:** notes that [docs/navigation.md](docs/navigation.md)
and [README](README.md) now document multi-line input.
- **Build & pipeline:** versioning moved to
[Nerdbank.GitVersioning](https://github.com/dotnet/Nerdbank.GitVersioning);
local builds now produce the same versions as CI
([#90](#90),
[#91](#91)).

PR #89 was a stepping stone superseded by the NBGV migration, so it
isn't called out separately.

Docs-only change — no code touched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants