diff --git a/CHANGELOG.md b/CHANGELOG.md index 22d8cdf5..4deaf0be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,20 @@ - Added `$sessionRequestCharge` and `$sessionChargedOperationCount` as read-only shell variables. Set `$sessionRequestChargeWarningThreshold` to a positive RU threshold to print one warning when the current connection reaches it; `info` reports it as `session.requestChargeWarningThreshold`. - Destructive MCP confirmations now identify their target. The elicitation prompt adds the connected account endpoint and the current database/container location, and notes that explicit `--db`/`--con` arguments override that location. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) - Import and export no longer hold entire files in memory. CSV imports are parsed incrementally, and CSV exports spool documents to a private temporary file to determine the complete column set, so transfers no longer scale with document count. Allow temporary disk space for the CSV export spool in addition to the destination file. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) +- Script diagnostics now report where a failure happened. Human-readable output shows the innermost source location first followed by the recorded function and script call sites, JSON errors carry the originating file, line, and column, and diagnostic logs retain both through the existing secret-redaction pipeline. Functions keep their defining file's location even when invoked from another file. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- The language server now applies the same validation as script execution: control-flow placement, duplicate function parameters, document-local function names, and commands and built-in options nested inside blocks, branches, loops, pipelines, and command expressions. Variable and function symbols are case-sensitive, so `$value` and `$Value` stay distinct. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Host-requested cancellation now propagates through script files, blocks, loops, and function calls without being turned into a positional runtime error. The shell reports a neutral result, records the cancellation in the diagnostic log, and restores call scopes and source context. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Documented the shell language in [programming](docs/programming.md): operator precedence and associativity, compound assignment, numeric promotion, a statement grammar, validation rules, and resource limits. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) ### Breaking changes - Malformed CSV files are now rejected instead of being silently misread. An unterminated or misplaced quote previously caused the remainder of the file to be absorbed into a single field, so the import reported success while writing corrupted items. Such files now abort with `Invalid CSV record at line `. Imports that previously appeared to succeed may now fail and require the source file to be corrected. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) +- Command text and script files are now fully parsed and validated before any of their statements run, so a syntax or semantic error prevents the entire input from executing rather than failing part-way through. Invalid control flow is rejected: `return` requires an enclosing function or script file, `break` and `continue` require an enclosing loop in the same function or script, and duplicate function parameter names are refused. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Calling a function with too few or too many arguments is now a usage error that exits with code `2`, including calls inside expressions. The function body does not run. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Integer arithmetic now reports overflow as an error instead of wrapping. Integer literal magnitudes must be between `0` and `2147483647`; because the minus sign is a separate unary operator, the minimum integer must be written as `-2147483647 - 1`. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- JSON construction now preserves decimal types, so `$object = {"value":3.0}` stores JSON `3.0` and `$object.value / 2` produces `1.5`. It previously stored `3` and performed integer division, producing `1`. Scripts that relied on the old truncation must be reviewed. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- A failed command expression now propagates its error instead of silently producing an empty result. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Scripts are subject to fixed resource limits: a shared parser nesting budget of 128 entries, a maximum expression tree depth of 128 nodes, and at most 64 active function and script-file calls. Exceeding a limit fails with a diagnostic instead of continuing recursive parsing or execution. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) ### Fixes @@ -21,6 +31,11 @@ - Shell and MCP command execution is serialized, including nested shell calls, so concurrent requests can no longer interleave and corrupt the shared connection and navigation state. Waiting for a destructive confirmation does not hold the execution lock. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) - A destructive MCP command is refused when the connection or navigation context changes while its confirmation is pending, including navigating away and back. It previously ran against the changed context. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) - Echoing an MCP command line no longer fails the command it announces on hosts without an ANSI terminal, which previously reported `Terminal does not support ANSI` instead of running it. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) +- `export` now rejects a directory as its destination and rejects an existing file before running its query instead of after. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- `return` no longer leaves the previous statement's custom renderer and explicit output format active, which could display an earlier command's output in place of the returned value. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Attaching a source location to a runtime failure no longer changes its exit-code category, so authentication, throttling, connectivity, and arithmetic failures are no longer reported as usage errors. Parser errors raised from a script file keep that file's name and source text, including when reached through a command expression. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- Loops and functions preserve JSON `null` values, and numeric conditions use the same zero/nonzero rule for shell values and JSON properties, including fractional numbers. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) +- The language server records a `do` loop's body before its condition and records a `for` binder as the loop variable's definition, so hover and go-to-definition no longer resolve to the wrong occurrence. ([#208](https://github.com/Azure/CosmosDBShell/pull/208)) - MCP command lines now list positional arguments in the order the command binds them. A destructive confirmation and the recorded history entry previously followed the client's argument order, so `rmdb` could display its `force` flag in place of the database name. A call that supplies a positional argument while omitting an earlier one is now rejected, because the shell cannot express that call and the recorded command would bind differently on replay. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) - MCP invocations are now saved to the history file as they run and are bounded by the history size limit. They were previously saved only when a later interactive command was entered. On Linux and macOS, the history file is now restricted to its owner, including an existing file that was previously readable by other users. ([#207](https://github.com/Azure/CosmosDBShell/pull/207)) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f9527b91..0c8a1565 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,6 +89,18 @@ docs/ # User-facing documentation - **Tests** live in `CosmosDBShell.Tests/`. Add or update tests when changing behavior. - Match the existing C# style. Prefer clear names over abbreviations. +### Script Language Tests + +When changing value semantics, extend `ValueOriginCases` in [StatementExecutionTests.cs](CosmosDBShell.Tests/Parser/StatementExecutionTests.cs). The matrix evaluates each operand as a literal, a JSON property, a loop variable, and a function argument/return, checking both result type and value across all combinations. Each combination runs with both raw JSON fixtures and shell-constructed JSON so serialization changes cannot silently alter the input's numeric type. Include repeated object/array reconstruction and integral decimal values when changing JSON conversion. + +Exercise syntax and control flow through `ShellInterpreter.RunCommandAsync` or script-file execution, which includes production parse/semantic validation. Check that invalid statements prevent execution and that returns, failures, and cancellation restore call scopes. For editor changes, check symbol identity, definition/reference locations, and hover ranges as well as diagnostics. + +Run the offline regression suite without a database: + +```bash +dotnet test CosmosDBShell.Tests/CosmosDBShell.Tests.csproj --filter "Category!=Emulator" +``` + ### Updating Localized Text Edit `CosmosDBShell/lang/en.ftl`, then build normally: diff --git a/CosmosDBShell.Tests/CommandTests/CosmosCommandTests.cs b/CosmosDBShell.Tests/CommandTests/CosmosCommandTests.cs index 23948011..316700d6 100644 --- a/CosmosDBShell.Tests/CommandTests/CosmosCommandTests.cs +++ b/CosmosDBShell.Tests/CommandTests/CosmosCommandTests.cs @@ -72,6 +72,18 @@ await Assert.ThrowsAsync(() => shell.ExecuteCosmosCo Assert.Equal(1, shell.SessionChargedOperationCount); } + [Fact] + public async Task ExecuteCosmosCommandAsync_RunsInsideSerializedOperation() + { + using var shell = ShellInterpreter.CreateInstance(); + + var state = await shell.RunSerializedAsync( + () => shell.ExecuteCosmosCommandAsync(new TestCosmosCommand(2.5), new CommandState(), string.Empty, CancellationToken.None), + CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + Assert.Equal(2.5, state.RequestCharge); + } + [Fact] public void CreatePartitionKey_WithHierarchicalIntegerComponents_PreservesIntegerTypes() { diff --git a/CosmosDBShell.Tests/CommandTests/ExportCommandTests.cs b/CosmosDBShell.Tests/CommandTests/ExportCommandTests.cs index a30d647f..fe90cf7c 100644 --- a/CosmosDBShell.Tests/CommandTests/ExportCommandTests.cs +++ b/CosmosDBShell.Tests/CommandTests/ExportCommandTests.cs @@ -10,6 +10,7 @@ namespace CosmosShell.Tests.CommandTests; using System.Threading; using System.Threading.Tasks; using Azure.Data.Cosmos.Shell.Commands; +using Azure.Data.Cosmos.Shell.Util; using Microsoft.Azure.Cosmos; using NSubstitute; @@ -259,7 +260,14 @@ Task ExportAsync() => ExportCommand.WriteFileAsync( } else { - await Assert.ThrowsAsync(ExportAsync); + var exception = await Assert.ThrowsAsync(ExportAsync); + Assert.Equal( + MessageService.GetArgsString("command-export-error-file_exists", "file", Path.GetFullPath(path)), + exception.Message); + var items = Substitute.For>(); + await Assert.ThrowsAsync(() => ExportCommand.WriteFileAsync( + items, ExportFormat.JsonLines, path, false, TestContext.Current.CancellationToken)); + items.DidNotReceive().GetAsyncEnumerator(Arg.Any()); Assert.Equal("previous export", await File.ReadAllTextAsync(path, TestContext.Current.CancellationToken)); } @@ -271,6 +279,32 @@ Task ExportAsync() => ExportCommand.WriteFileAsync( } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task WriteFileAsync_ExistingDirectoryDoesNotEnumerateItems(bool overwrite) + { + var directory = Directory.CreateTempSubdirectory("cosmos-export-test-"); + try + { + var items = Substitute.For>(); + + var exception = await Assert.ThrowsAsync(() => ExportCommand.WriteFileAsync( + items, ExportFormat.JsonLines, directory.FullName, overwrite, TestContext.Current.CancellationToken)); + + Assert.Equal( + MessageService.GetArgsString("command-export-error-destination_directory", "file", Path.GetFullPath(directory.FullName)), + exception.Message); + Assert.Contains("Specify a file path instead", exception.Message, StringComparison.Ordinal); + Assert.DoesNotContain("--force", exception.Message, StringComparison.Ordinal); + items.DidNotReceive().GetAsyncEnumerator(Arg.Any()); + } + finally + { + directory.Delete(); + } + } + [Fact] public async Task WriteFileAsync_CancellationPreservesExistingFile() { diff --git a/CosmosDBShell.Tests/Integration/ScriptArgumentTests.cs b/CosmosDBShell.Tests/Integration/ScriptArgumentTests.cs index ee3ab1d7..ceaea765 100644 --- a/CosmosDBShell.Tests/Integration/ScriptArgumentTests.cs +++ b/CosmosDBShell.Tests/Integration/ScriptArgumentTests.cs @@ -86,7 +86,7 @@ public async Task Script_MultiLineWithControlFlow() var tempDir = Path.Combine(Path.GetTempPath(), "CosmosShellIntTests", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(tempDir); var scriptPath = Path.Combine(tempDir, "control_flow.csh"); - await File.WriteAllTextAsync(scriptPath, "$sum = 0\nfor $i in [1, 2, 3] {\n $sum = ($sum + $i)\n}\nreturn $sum\n", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(scriptPath, "$sum = 0\nfor $i in [1, 2, 3] {\n $sum = ($sum + $i)\n}\nreturn $sum\ndef unreachable { return 0 }\n", TestContext.Current.CancellationToken); try { @@ -94,6 +94,8 @@ public async Task Script_MultiLineWithControlFlow() var state = await cmd.RunAsync(Shell, new CommandState(), CancellationToken.None); Assert.False(state.IsError); + Assert.Equal(6, Assert.IsType(state.Result).Value); + Assert.False(Shell.Functions.ContainsKey("unreachable")); } finally { diff --git a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs index 4670f39b..26288aef 100644 --- a/CosmosDBShell.Tests/Integration/ShellProcessTests.cs +++ b/CosmosDBShell.Tests/Integration/ShellProcessTests.cs @@ -24,6 +24,40 @@ public class ShellProcessTests { private static readonly Regex AnsiEscape = new("\x1b\\[[0-9;?]*[ -/]*[@-~]", RegexOptions.Compiled); + [Theory] + [InlineData("value", "identity", "1 argument, got 0")] + [InlineData("value", "identity 1 2", "1 argument, got 2")] + [InlineData("value", "$result = (identity)", "1 argument, got 0")] + [InlineData("", "identity 1", "0 arguments, got 1")] + [InlineData("first second", "identity", "2 arguments, got 0")] + public async Task WrongFunctionArgumentCount_ReturnsUsageExitCode(string parameters, string invocation, string expectedMessage) + { + var result = await RunShellAsync($"def identity [{parameters}] {{ return }}; {invocation}", cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]); + Assert.Equal(2, result.ExitCode); + Assert.Contains("expects " + expectedMessage, result.StdErr); + } + + [Fact] + public async Task DeepExpression_FailsBeforeExecution_WithoutCrashingProcess() + { + var script = "echo BEFORE_DEPTH_ERROR; $value = " + string.Join(" + ", Enumerable.Repeat("1", 10001)); + var result = await RunShellAsync(script, cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]); + Assert.Equal(2, result.ExitCode); + Assert.DoesNotContain("BEFORE_DEPTH_ERROR", result.StdOut); + Assert.Contains("expression tree depth", result.StdErr); + Assert.DoesNotContain("Stack overflow", result.StdErr); + } + + [Fact] + public async Task ExpressionAtDepthLimit_EvaluatesSuccessfully() + { + var script = "$value = " + string.Join(" + ", Enumerable.Repeat("1", 128)) + "; echo $value"; + var result = await RunShellAsync(script, cancellationToken: TestContext.Current.CancellationToken, extraArgs: ["--quiet"]); + Assert.Equal(0, result.ExitCode); + Assert.Contains("128", result.StdOut); + Assert.Empty(result.StdErr); + } + [Theory] [InlineData("doctor --no-update-check --format json", 0, "PASS")] [InlineData("doctor --database missing --no-update-check --format json", 1, "FAIL")] diff --git a/CosmosDBShell.Tests/Lsp/CosmosShellCompletionHandlerTests.cs b/CosmosDBShell.Tests/Lsp/CosmosShellCompletionHandlerTests.cs index ffab4518..327c249b 100644 --- a/CosmosDBShell.Tests/Lsp/CosmosShellCompletionHandlerTests.cs +++ b/CosmosDBShell.Tests/Lsp/CosmosShellCompletionHandlerTests.cs @@ -98,6 +98,26 @@ public async Task VariableCompletion_SuggestsSessionVariables() Assert.Contains("$sessionRequestChargeWarningThreshold", labels); } + [Fact] + public async Task VariableCompletion_PreservesNamesThatDifferOnlyByCase() + { + var variables = new VariableContainer(); + variables.Set("completionValue", new ShellText("lower")); + variables.Set("CompletionValue", new ShellText("upper")); + ShellInterpreter.Instance.VariableContainers.Push(variables); + try + { + var completions = await GetCompletionsAsync("echo $completion", 0, 16); + + Assert.Single(completions.Items, item => item.Label == "$completionValue" && item.InsertText == "$completionValue"); + Assert.Single(completions.Items, item => item.Label == "$CompletionValue" && item.InsertText == "$CompletionValue"); + } + finally + { + ShellInterpreter.Instance.VariableContainers.Pop(); + } + } + [Fact] public async Task VariableCompletion_IgnoresWhenNotVariableContext() { diff --git a/CosmosDBShell.Tests/Lsp/CosmosShellHoverHandlerTests.cs b/CosmosDBShell.Tests/Lsp/CosmosShellHoverHandlerTests.cs index cc487cc3..8b70bf0b 100644 --- a/CosmosDBShell.Tests/Lsp/CosmosShellHoverHandlerTests.cs +++ b/CosmosDBShell.Tests/Lsp/CosmosShellHoverHandlerTests.cs @@ -133,6 +133,34 @@ public async Task Handle_VariableHover_ReturnsVariableInfo() Assert.NotNull(result); var markup = Assert.IsType(result.Contents.MarkupContent); Assert.Contains("myVar", markup.Value); + Assert.Equal(new Position(1, 5), result.Range!.Start); + Assert.Equal(new Position(1, 11), result.Range.End); + } + + [Theory] + [InlineData("value", 2, 0)] + [InlineData("Value", 3, 1)] + public async Task Handle_VariableHover_UsesCaseSensitiveDefinition(string name, int usageLine, int definitionLine) + { + const string content = "$value = 1\n$Value = 2\necho $value\necho $Value"; + this.workspace.OpenDocument(testUri, content, 1); + var document = this.workspace.GetDocument(testUri)!; + var symbol = document.SemanticModel!.GetSymbolAt(content.LastIndexOf("$" + name, StringComparison.Ordinal) + 1)!; + Assert.Equal(name, symbol.Name); + var definition = Assert.Single(document.SemanticModel.FindReferences(symbol), reference => reference.IsDefinition); + Assert.Equal(definitionLine == 0 ? 0 : content.IndexOf('\n') + 1, definition.Start); + + var result = await this.handler.Handle(new HoverParams + { + TextDocument = new TextDocumentIdentifier { Uri = testUri }, + Position = new Position(usageLine, 6), + }, TestContext.Current.CancellationToken); + + Assert.NotNull(result); + var markup = Assert.IsType(result.Contents.MarkupContent); + Assert.Contains(name, markup.Value); + Assert.Equal(new Position(usageLine, 5), result.Range!.Start); + Assert.Equal(new Position(usageLine, 11), result.Range.End); } [Fact] @@ -451,6 +479,8 @@ public async Task Handle_FunctionCall_ReturnsFunctionInfo() Assert.NotNull(result); var markup = Assert.IsType(result.Contents.MarkupContent); Assert.Contains("myFunc", markup.Value); + Assert.Equal(new Position(3, 0), result.Range!.Start); + Assert.Equal(new Position(3, 6), result.Range.End); } [Fact] diff --git a/CosmosDBShell.Tests/Lsp/CosmosShellWorkspaceTests.cs b/CosmosDBShell.Tests/Lsp/CosmosShellWorkspaceTests.cs index 819ea96c..3c30513a 100644 --- a/CosmosDBShell.Tests/Lsp/CosmosShellWorkspaceTests.cs +++ b/CosmosDBShell.Tests/Lsp/CosmosShellWorkspaceTests.cs @@ -18,6 +18,210 @@ public class CosmosShellWorkspaceTests private readonly CosmosShellWorkspace workspace = new(); private readonly DocumentUri uri = DocumentUri.From("file:///ws.csh"); + [Theory] + [InlineData("return")] + [InlineData("if true { return }")] + [InlineData("def empty { return }")] + [InlineData("def empty { if true { return } }")] + public void BareReturn_AtStatementBoundary_IsValid(string source) + { + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.True(document.LastParseResult!.Success); + Assert.Empty(document.Diagnostics); + } + + [Theory] + [InlineData("def identity [value] { return $value }; identity 1")] + [InlineData("def identity [value] { return $value }; $result = (identity 1)")] + [InlineData("def first { return (second) }; def second { return 1 }; first")] + [InlineData("def recursive { recursive }")] + [InlineData("def outer { def inner { return 1 }; inner }; outer")] + public void LocalFunctionCalls_AreResolvedWithoutUnknownCommandDiagnostics(string source) + { + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.True(document.LastParseResult!.Success); + Assert.Empty(document.Diagnostics); + Assert.Contains(document.SemanticModel!.Symbols, symbol => symbol is Azure.Data.Cosmos.Shell.Lsp.Semantics.FunctionSymbol); + Assert.Contains(document.SemanticModel.References, reference => reference.Symbol is Azure.Data.Cosmos.Shell.Lsp.Semantics.FunctionSymbol && !reference.IsDefinition); + } + + [Theory] + [InlineData("repeat", "repeat")] + [InlineData("$first = (repeat)", "$second = (repeat)")] + public void FunctionRedefinition_BindsCallsToLatestPrecedingDefinition(string firstCall, string secondCall) + { + var source = $"def repeat {{ return 1 }}; {firstCall}; def repeat {{ return 2 }}; {secondCall}"; + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.Empty(document.Diagnostics); + var model = document.SemanticModel!; + var functions = model.Symbols.OfType().OrderBy(symbol => symbol.Start).ToArray(); + Assert.Equal(2, functions.Length); + var calls = model.References.Where(reference => !reference.IsDefinition && reference.Symbol is Azure.Data.Cosmos.Shell.Lsp.Semantics.FunctionSymbol).OrderBy(reference => reference.Start).ToArray(); + Assert.Equal(2, calls.Length); + Assert.Same(functions[0], calls[0].Symbol); + Assert.Same(functions[1], calls[1].Symbol); + Assert.Same(functions[1], model.GetSymbolAt(calls[1].Start + 1)); + foreach (var function in functions) + { + Assert.Single(model.FindReferences(function), reference => reference.IsDefinition); + Assert.Single(model.FindReferences(function), reference => !reference.IsDefinition); + } + } + + [Theory] + [InlineData("ls --not_an_option_xyz")] + [InlineData("$value = (ls --not_an_option_xyz)")] + public void LaterFunctionCollision_DoesNotHideEarlierBuiltinDiagnostics(string call) + { + var source = $"{call}; def ls {{ return 1 }}; {call}"; + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + var diagnostic = Assert.Single(document.Diagnostics); + Assert.Contains("Unknown option '-not_an_option_xyz'", diagnostic.Message); + Assert.True(diagnostic.Range.Start.Character < call.Length); + var calls = document.SemanticModel!.References.Where(reference => !reference.IsDefinition && reference.Symbol is Azure.Data.Cosmos.Shell.Lsp.Semantics.FunctionSymbol).ToArray(); + Assert.Single(calls); + Assert.True(calls[0].Start > source.IndexOf("def ls", System.StringComparison.Ordinal)); + } + + [Theory] + [InlineData("def example { missing_command_xyz }")] + [InlineData("if true { missing_command_xyz }")] + [InlineData("if false {} else { missing_command_xyz }")] + [InlineData("while true { missing_command_xyz }")] + [InlineData("do { missing_command_xyz } while false")] + [InlineData("for $item in [1] { missing_command_xyz }")] + [InlineData("loop { missing_command_xyz }")] + [InlineData("echo 1 | missing_command_xyz")] + [InlineData("$result = (missing_command_xyz)")] + [InlineData("def example { return (missing_command_xyz) }")] + [InlineData("if (missing_command_xyz) {}")] + [InlineData("exec (missing_command_xyz)")] + public void NestedCommands_AreAnalyzedAtTheirSourcePosition(string source) + { + this.workspace.OpenDocument(this.uri, source, 1); + var diagnostic = Assert.Single(this.workspace.GetDocument(this.uri)!.Diagnostics); + Assert.Equal("Unknown command 'missing_command_xyz'.", diagnostic.Message); + var start = source.IndexOf("missing_command_xyz", System.StringComparison.Ordinal); + Assert.Equal(new Position(0, start), diagnostic.Range.Start); + Assert.Equal(new Position(0, start + "missing_command_xyz".Length), diagnostic.Range.End); + } + + [Theory] + [InlineData("def example { ls --not_an_option_xyz }")] + [InlineData("while true { ls --not_an_option_xyz }")] + [InlineData("$value = (ls --not_an_option_xyz)")] + public void NestedBuiltinOptions_StillReceiveValidation(string source) + { + this.workspace.OpenDocument(this.uri, source, 1); + Assert.Contains(this.workspace.GetDocument(this.uri)!.Diagnostics, diagnostic => diagnostic.Message.Contains("Unknown option '-not_an_option_xyz'")); + } + + [Fact] + public void RemovingFunctionDefinition_ClearsSymbolsAndReportsUnresolvedCall() + { + this.workspace.OpenDocument(this.uri, "def local_function_xyz { return 1 }; local_function_xyz", 1); + var document = this.workspace.GetDocument(this.uri)!; + var model = document.SemanticModel!; + var symbol = Assert.Single(model.Symbols.OfType()); + var references = model.References.Where(reference => ReferenceEquals(reference.Symbol, symbol)).ToArray(); + Assert.Equal(2, references.Length); + Assert.Single(references, reference => reference.IsDefinition); + Assert.Single(references, reference => !reference.IsDefinition); + + this.workspace.UpdateDocument(this.uri, "local_function_xyz", 2); + Assert.Empty(document.SemanticModel!.Symbols.OfType()); + Assert.Equal("Unknown command 'local_function_xyz'.", Assert.Single(document.Diagnostics).Message); + Assert.Single(model.Symbols.OfType()); + } + + [Fact] + public void FunctionNames_RemainCaseSensitive() + { + this.workspace.OpenDocument(this.uri, "def lower_function_xyz {} ; LOWER_FUNCTION_XYZ", 1); + Assert.Equal("Unknown command 'LOWER_FUNCTION_XYZ'.", Assert.Single(this.workspace.GetDocument(this.uri)!.Diagnostics).Message); + } + + [Fact] + public void DoLoop_DefinesVariableInBodyBeforeCondition() + { + var source = "do { $value_xyz = 1 } while $value_xyz"; + this.workspace.OpenDocument(this.uri, source, 1); + var model = this.workspace.GetDocument(this.uri)!.SemanticModel!; + var symbol = Assert.Single(model.Symbols.OfType()); + var references = model.FindReferences(symbol).OrderBy(reference => reference.Start).ToArray(); + Assert.Equal(2, references.Length); + Assert.Equal(source.IndexOf("$value_xyz", System.StringComparison.Ordinal), references[0].Start); + Assert.True(references[0].IsDefinition); + Assert.Equal(source.LastIndexOf("$value_xyz", System.StringComparison.Ordinal), references[1].Start); + Assert.False(references[1].IsDefinition); + } + + [Fact] + public void ForLoop_RecordsBinderAsVariableDefinition() + { + var source = "for $item_xyz in [1] { echo $item_xyz }"; + this.workspace.OpenDocument(this.uri, source, 1); + var model = this.workspace.GetDocument(this.uri)!.SemanticModel!; + var symbol = Assert.Single(model.Symbols.OfType()); + var references = model.FindReferences(symbol).OrderBy(reference => reference.Start).ToArray(); + Assert.Equal(2, references.Length); + Assert.Equal(source.IndexOf("$item_xyz", System.StringComparison.Ordinal), references[0].Start); + Assert.True(references[0].IsDefinition); + Assert.False(references[1].IsDefinition); + Assert.Same(symbol, model.GetSymbolAt(references[0].Start + 1)); + } + + [Theory] + [InlineData("value", "Value")] + [InlineData("VALUE", "value")] + [InlineData("item", "ITEM")] + public void VariableReferences_KeepCaseDistinctDefinitions(string first, string second) + { + var source = $"${first} = 1; ${second} = 2; echo ${second}; echo ${first}"; + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.Empty(document.Diagnostics); + var model = document.SemanticModel!; + var variables = model.Symbols.OfType().ToArray(); + Assert.Equal(2, variables.Length); + foreach (var name in new[] { first, second }) + { + var symbol = Assert.Single(variables, variable => variable.Name == name); + var references = model.FindReferences(symbol).ToArray(); + Assert.Equal(2, references.Length); + var definition = Assert.Single(references, reference => reference.IsDefinition); + var usage = Assert.Single(references, reference => !reference.IsDefinition); + Assert.Equal(source.IndexOf("$" + name, System.StringComparison.Ordinal), definition.Start); + Assert.Equal(source.LastIndexOf("$" + name, System.StringComparison.Ordinal), usage.Start); + Assert.Same(symbol, model.GetSymbolAt(usage.Start + 1)); + } + } + + [Theory] + [InlineData("$value = $value + 1", 2)] + [InlineData("$value = 0; $value = $value + 1", 3)] + public void SelfReferentialAssignment_DefinitionRemainsAtFirstAssignmentTarget(string source, int occurrenceCount) + { + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.Empty(document.Diagnostics); + var model = document.SemanticModel!; + var symbol = Assert.Single(model.Symbols.OfType()); + var references = model.FindReferences(symbol).ToArray(); + Assert.Equal(occurrenceCount, references.Length); + var definition = Assert.Single(references, reference => reference.IsDefinition); + Assert.Equal(0, definition.Start); + Assert.Equal("$value".Length, definition.Length); + Assert.Equal(0, symbol.Start); + var usage = Assert.Single(references, reference => reference.Start == source.LastIndexOf("$value", System.StringComparison.Ordinal)); + Assert.False(usage.IsDefinition); + Assert.Same(symbol, model.GetSymbolAt(usage.Start + 1)); + } + [Fact] public void OpenDocument_StoresAndParses() { @@ -161,4 +365,46 @@ public void OpenDocument_WithParseError_ProducesDiagnostics() Assert.NotNull(doc); Assert.NotEmpty(doc!.Diagnostics); } + + [Theory] + [InlineData("break")] + [InlineData("continue")] + [InlineData("def duplicate [value value] { return $value }")] + [InlineData("loop { def invalid { break } }")] + public void RuntimeSemanticErrors_AreReportedByLsp(string source) + { + this.workspace.OpenDocument(this.uri, source, 1); + var document = this.workspace.GetDocument(this.uri)!; + var expected = Azure.Data.Cosmos.Shell.Parser.StatementParser.ScriptParseResult.Parse(source, allowReturn: true); + Assert.False(document.LastParseResult!.Success); + foreach (var error in expected.Errors) + { + Assert.Contains(document.Diagnostics, diagnostic => diagnostic.Message == error.Message); + } + } + + [Fact] + public void SemanticRange_UsesExclusiveEnd_AndClearsAfterFix() + { + this.workspace.OpenDocument(this.uri, "\r\nbreak", 1); + var document = this.workspace.GetDocument(this.uri)!; + var diagnostic = Assert.Single(document.Diagnostics); + Assert.Equal(new Position(1, 0), diagnostic.Range.Start); + Assert.Equal(new Position(1, 5), diagnostic.Range.End); + this.workspace.UpdateDocument(this.uri, "loop { break }\nreturn 1", 2); + Assert.True(document.LastParseResult!.Success); + Assert.Empty(document.Diagnostics); + } + + [Fact] + public void DeepExpression_IsRejectedBeforeSemanticAnalysis() + { + this.workspace.OpenDocument(this.uri, "$value = " + string.Join(" + ", Enumerable.Repeat("1", 10001)), 1); + var document = this.workspace.GetDocument(this.uri)!; + Assert.False(document.LastParseResult!.Success); + Assert.Contains(document.Diagnostics, diagnostic => diagnostic.Message.Contains("expression tree depth")); + this.workspace.UpdateDocument(this.uri, "$value = 1", 2); + Assert.True(document.LastParseResult!.Success); + Assert.Empty(document.Diagnostics); + } } diff --git a/CosmosDBShell.Tests/Parser/CommandStatementTests.cs b/CosmosDBShell.Tests/Parser/CommandStatementTests.cs index 6752845a..3f7a9b47 100644 --- a/CosmosDBShell.Tests/Parser/CommandStatementTests.cs +++ b/CosmosDBShell.Tests/Parser/CommandStatementTests.cs @@ -6,9 +6,12 @@ namespace CosmosShell.Tests.Parser; using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text.Json; using System.Threading.Tasks; +using Azure.Data.Cosmos.Shell.Commands; using Azure.Data.Cosmos.Shell.Core; using Azure.Data.Cosmos.Shell.Parser; using Azure.Data.Cosmos.Shell.Util; @@ -17,6 +20,47 @@ namespace CosmosShell.Tests.Parser; public class CommandStatementTests { + [CosmosCommand("structured-error-test")] + internal sealed class StructuredErrorTestCommand : CosmosCommand + { + public override Task ExecuteAsync(ShellInterpreter shell, CommandState commandState, string commandText, CancellationToken token) + { + var state = new StructuredErrorCommandState( + new CommandException("structured-error-test", "failed"), + new ShellJson(JsonSerializer.SerializeToElement(new { success = false }))) + { + RequestCharge = 2.5, + RenderUser = () => { }, + }; + return Task.FromResult(state); + } + } + + [CosmosCommand("render-failure-test")] + internal sealed class RenderFailureTestCommand : CosmosCommand + { + public override Task ExecuteAsync(ShellInterpreter shell, CommandState commandState, string commandText, CancellationToken token) + { + return Task.FromResult(new CommandState + { + RenderUser = () => throw new IOException("render failure"), + }); + } + } + + [CosmosCommand("touch-marker-test")] + internal sealed class TouchMarkerTestCommand : CosmosCommand + { + [CosmosParameter("file")] + public string? File { get; init; } + + public override Task ExecuteAsync(ShellInterpreter shell, CommandState commandState, string commandText, CancellationToken token) + { + System.IO.File.WriteAllText(this.File!, "executed"); + return Task.FromResult(new CommandState()); + } + } + private static Statement ParseStatement(string input) { var parser = new StatementParser(input); @@ -123,6 +167,221 @@ public async Task CommandStatement_UnknownCommand_ThrowsCommandNotFoundException Assert.Contains("unknowncommand", ex.Message); } + [Fact] + public async Task StructuredError_InScript_PreservesStateAndAddsPosition() + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(StructuredErrorTestCommand), out var factory)); + shell.App.Commands["structured-error-test"] = factory; + shell.CurrentScriptFileName = "test.csh"; + shell.CurrentScriptContent = "structured-error-test"; + + var result = await ParseStatement("structured-error-test").RunAsync(shell, new CommandState(), CancellationToken.None); + + var structured = Assert.IsType(result); + Assert.False(Assert.IsType(structured.Result).Value.GetProperty("success").GetBoolean()); + Assert.NotNull(structured.RenderUser); + Assert.Equal(2.5, structured.RequestCharge); + var positional = Assert.IsType(structured.Exception); + Assert.Equal("test.csh", positional.FileName); + Assert.IsType(positional.InnerException); + } + + [Fact] + public async Task StructuredError_InBlock_IsNotRenderedBeforeCallerCanAddSource() + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(StructuredErrorTestCommand), out var factory)); + shell.App.Commands["structured-error-test"] = factory; + shell.CurrentScriptFileName = "script.csh"; + shell.CurrentScriptContent = "{ structured-error-test }"; + + var result = await ParseStatement(shell.CurrentScriptContent).RunAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + + var error = Assert.IsType(result); + Assert.False(error.OutputRendered); + Assert.Single(PositionalException.GetSourceTrace(error.Exception)); + } + + [Theory] + [InlineData("broken")] + [InlineData("$result = (broken)")] + public async Task StructuredError_InFunction_RetainsDefinitionAndCallerFramesOnce(string invocation) + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(StructuredErrorTestCommand), out var factory)); + shell.App.Commands["structured-error-test"] = factory; + const string definition = "def broken { structured-error-test }"; + shell.CurrentScriptFileName = "definition.csh"; + shell.CurrentScriptContent = definition; + await ParseStatement(definition).RunAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + shell.CurrentScriptFileName = "caller.csh"; + shell.CurrentScriptContent = invocation; + + if (invocation == "broken") + { + var result = await ParseStatement(invocation).RunAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + var error = Assert.IsType(result); + Assert.False(error.OutputRendered); + var frames = PositionalException.GetSourceTrace(error.Exception); + Assert.Equal(2, frames.Count); + Assert.Equal("definition.csh", frames[0].FileName); + Assert.Equal("caller.csh", frames[1].FileName); + } + else + { + var exception = await Assert.ThrowsAsync( + () => ParseStatement(invocation).RunAsync(shell, new CommandState(), TestContext.Current.CancellationToken)); + var frames = PositionalException.GetSourceTrace(exception); + Assert.Equal(2, frames.Count); + Assert.Equal("definition.csh", frames[0].FileName); + Assert.Equal("caller.csh", frames[1].FileName); + Assert.IsType(exception.State); + } + } + + [Fact] + public async Task StructuredError_InNestedScript_RetainsChildAndParentFrames() + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(StructuredErrorTestCommand), out var factory)); + shell.App.Commands["structured-error-test"] = factory; + var child = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(child, "structured-error-test", TestContext.Current.CancellationToken); + shell.CurrentScriptFileName = "parent.csh"; + shell.CurrentScriptContent = child; + var command = new CommandStatement(new Token(TokenType.Identifier, child, 0, child.Length)); + + var result = await command.RunScriptAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + + var error = Assert.IsType(result); + var frames = PositionalException.GetSourceTrace(error.Exception); + Assert.Equal(2, frames.Count); + Assert.Equal(child, frames[0].FileName); + Assert.Equal("parent.csh", frames[1].FileName); + } + finally + { + File.Delete(child); + } + } + + [Fact] + public async Task Script_PrintFailure_DoesNotExecuteNextStatement() + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(RenderFailureTestCommand), out var renderFactory)); + Assert.True(CommandFactory.TryCreateFactory(typeof(TouchMarkerTestCommand), out var markerFactory)); + shell.App.Commands["render-failure-test"] = renderFactory; + shell.App.Commands["touch-marker-test"] = markerFactory; + var script = Path.GetTempFileName(); + var marker = Path.Join(Path.GetTempPath(), $"script-marker-{Guid.NewGuid():N}.txt"); + try + { + await File.WriteAllTextAsync( + script, + $"render-failure-test\ntouch-marker-test {ShellLiteral.Quote(marker.Replace('\\', '/'))}", + TestContext.Current.CancellationToken); + var command = new CommandStatement(new Token(TokenType.Identifier, script, 0, script.Length)); + + var result = await command.RunScriptAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + + Assert.True(result.IsError); + Assert.False(File.Exists(marker)); + } + finally + { + File.Delete(script); + File.Delete(marker); + } + } + + [Theory] + [InlineData(">", "value")] + [InlineData(">>", "before" + "value")] + public async Task ScriptExpression_PreservesEnclosingOutputRedirection(string redirect, string expected) + { + using var shell = ShellInterpreter.CreateInstance(); + var script = Path.Join(Environment.CurrentDirectory, $"script{Guid.NewGuid():N}.csh"); + var output = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(script, "echo value", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(output, "before", TestContext.Current.CancellationToken); + + var result = await shell.ExecuteCommandAsync( + $"echo ({Path.GetFileName(script)}) {redirect} {ShellLiteral.Quote(output.Replace('\\', '/'))}", + TestContext.Current.CancellationToken); + + Assert.False(result.IsError, (result as ErrorCommandState)?.Exception.ToString()); + Assert.Equal(expected + Environment.NewLine, await File.ReadAllTextAsync(output, TestContext.Current.CancellationToken)); + } + finally + { + File.Delete(script); + File.Delete(output); + } + } + + [Fact] + public async Task Script_RespectsStatementOutputRedirection() + { + using var shell = ShellInterpreter.CreateInstance(); + var script = Path.GetTempFileName(); + var output = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync( + script, + $"echo value > {ShellLiteral.Quote(output.Replace('\\', '/'))}", + TestContext.Current.CancellationToken); + var command = new CommandStatement(new Token(TokenType.Identifier, script, 0, script.Length)); + + var result = await command.RunScriptAsync(shell, new CommandState(), TestContext.Current.CancellationToken); + + Assert.False(result.IsError, (result as ErrorCommandState)?.Exception.ToString()); + Assert.Equal("value" + Environment.NewLine, await File.ReadAllTextAsync(output, TestContext.Current.CancellationToken)); + } + finally + { + File.Delete(script); + File.Delete(output); + } + } + + [Fact] + public async Task StructuredError_InScriptExpression_PreservesMachinePayload() + { + using var shell = ShellInterpreter.CreateInstance(); + Assert.True(CommandFactory.TryCreateFactory(typeof(StructuredErrorTestCommand), out var factory)); + shell.App.Commands["structured-error-test"] = factory; + shell.Options = new Program.CosmosShellOptions { Output = "json" }; + var script = Path.GetTempFileName(); + var stderr = Path.GetTempFileName(); + shell.ErrOutRedirect = stderr; + try + { + await File.WriteAllTextAsync(script, "$value = (structured-error-test)", TestContext.Current.CancellationToken); + + var result = await shell.ExecuteCommandAsync($"exec {ShellLiteral.Quote(script)}", TestContext.Current.CancellationToken); + + var structured = Assert.IsType(result); + Assert.False(Assert.IsType(structured.Result).Value.GetProperty("success").GetBoolean()); + Assert.Contains(PositionalException.GetSourceTrace(structured.Exception), frame => frame.FileName == script); + using var document = JsonDocument.Parse(await File.ReadAllTextAsync(stderr, TestContext.Current.CancellationToken)); + Assert.False(document.RootElement.GetProperty("result").GetProperty("success").GetBoolean()); + Assert.StartsWith($"{script}:1:11: ", document.RootElement.GetProperty("error").GetString()); + } + finally + { + shell.ErrOutRedirect = null; + File.Delete(script); + File.Delete(stderr); + } + } + [Fact] public async Task CommandStatement_UnknownCommandWithCloseTypo_SuggestsKnownCommand() { diff --git a/CosmosDBShell.Tests/Parser/ExpressionParserErrorTests.cs b/CosmosDBShell.Tests/Parser/ExpressionParserErrorTests.cs index e639e147..29ac1016 100644 --- a/CosmosDBShell.Tests/Parser/ExpressionParserErrorTests.cs +++ b/CosmosDBShell.Tests/Parser/ExpressionParserErrorTests.cs @@ -14,6 +14,88 @@ namespace CosmosShell.Tests.Parser; /// public class ExpressionParserErrorTests { + [Theory] + [InlineData("+")] + [InlineData("-")] + [InlineData("*")] + [InlineData("/")] + [InlineData("==")] + [InlineData("<")] + [InlineData("|")] + public void FlatOperatorChain_RejectsDeepTree(string operation) + { + var lexer = new Lexer(string.Join($" {operation} ", Enumerable.Repeat("1", 10001))); + var parser = new ExpressionParser(lexer); + var expression = parser.ParseFilterExpression(); + Assert.IsType(expression); + Assert.Contains(lexer.Errors, error => error.Message.Contains("expression tree depth")); + Assert.True(parser.IsAtEnd); + } + + [Theory] + [InlineData("", "")] + [InlineData("(", ")")] + [InlineData("[", "]")] + [InlineData("{value: ", "}")] + [InlineData("$\"$(", ")\"")] + public void ExpressionDepth_IsCheckedAcrossContainingNodes(string prefix, string suffix) + { + var source = prefix + string.Join(" + ", Enumerable.Repeat("1", 140)) + suffix; + var lexer = new Lexer(source); + new ExpressionParser(lexer).ParseExpression(); + Assert.Contains(lexer.Errors, error => error.Message.Contains("expression tree depth")); + } + + [Theory] + [InlineData(128, false)] + [InlineData(129, true)] + public void ExpressionDepth_HasExplicitBoundary(int operands, bool rejected) + { + var lexer = new Lexer(string.Join(" + ", Enumerable.Repeat("1", operands))); + new ExpressionParser(lexer).ParseExpression(); + Assert.Equal(rejected, lexer.Errors.HasErrors); + } + + [Theory] + [InlineData("(", ")")] + [InlineData("[", "]")] + [InlineData("{value: ", "}")] + [InlineData("$\"$(", ")\"")] + public void ContainingNode_CountsTowardsTotalDepth(string prefix, string suffix) + { + var lexer = new Lexer(prefix + string.Join(" + ", Enumerable.Repeat("1", 128)) + suffix); + new ExpressionParser(lexer).ParseExpression(); + Assert.Contains(lexer.Errors, error => error.Message.Contains("expression tree depth")); + } + + [Theory] + [InlineData("(", "1", ")")] + [InlineData("!", "true", "")] + [InlineData("2 ** ", "1", "")] + [InlineData("[", "1", "]")] + [InlineData("{value:", "1", "}")] + public void DeepExpression_ReportsLimitWithoutThrowing(string prefix, string value, string suffix) + { + var source = string.Concat(Enumerable.Repeat(prefix, 300)) + value + string.Concat(Enumerable.Repeat(suffix, 300)); + var lexer = new Lexer(source); + var parser = new ExpressionParser(lexer); + var expression = parser.ParseExpression(); + Assert.NotNull(expression); + Assert.Contains(lexer.Errors, error => error.Message.Contains("nesting budget")); + Assert.True(parser.IsAtEnd); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void DeepBlocks_RejectInStrictAndEditorModes(bool tolerant) + { + var source = new string('{', 300) + "$value = 1" + new string('}', 300); + var parser = new StatementParser(source) { TolerateIncompleteConstructs = tolerant }; + parser.ParseStatements(); + Assert.Contains(parser.Errors, error => error.Message.Contains("nesting budget")); + } + private static (Expression? Expr, int ErrorCount) ParseFilter(string input) { var lexer = new Lexer(input); @@ -39,6 +121,24 @@ public void MissingCloseParenthesis_ReportsError() Assert.True(errors > 0); } + [Fact] + public void DeepInterpolation_UsesSharedBudget() + { + var input = "$\"$(" + new string('(', 300) + "1" + new string(')', 300) + ")\""; + var lexer = new Lexer(input); + new ExpressionParser(lexer).ParseExpression(); + Assert.Contains(lexer.Errors, error => error.Message.Contains("nesting budget")); + } + + [Fact] + public void NormalNesting_AndManySequentialStatements_RemainValid() + { + var source = "$value = " + new string('(', 10) + "1" + new string(')', 10) + ";"; + var parser = new StatementParser(string.Concat(Enumerable.Repeat(source, 200))); + Assert.Equal(200, parser.ParseStatements().Count); + Assert.False(parser.Errors.HasErrors); + } + [Fact] public void MissingCloseBracket_ReportsError() { diff --git a/CosmosDBShell.Tests/Parser/FilterBuiltinsTests.cs b/CosmosDBShell.Tests/Parser/FilterBuiltinsTests.cs index cc865222..ee188ab1 100644 --- a/CosmosDBShell.Tests/Parser/FilterBuiltinsTests.cs +++ b/CosmosDBShell.Tests/Parser/FilterBuiltinsTests.cs @@ -34,6 +34,15 @@ private static async Task EvalAsync(string input, object? value) private static string TypeName(ShellObject result) => Assert.IsType(result).Text; + [Fact] + public async Task Map_PreservesIntegralDecimalThroughPipeline() + { + var result = await EvalAsync(". | map(3.0) | .[0]", new[] { 1 }); + var json = FilterExpressionUtilities.ToJsonElement(result); + Assert.Equal("3.0", json.GetRawText()); + Assert.IsType(ShellNumber.FromJson(json)); + } + [Fact] public async Task Type_Object_ReturnsObject() => Assert.Equal("object", TypeName(await EvalAsync(". | type", new { a = 1 }))); diff --git a/CosmosDBShell.Tests/Parser/FunctionDefinitionTests.cs b/CosmosDBShell.Tests/Parser/FunctionDefinitionTests.cs index 0716cecf..38ea4c5a 100644 --- a/CosmosDBShell.Tests/Parser/FunctionDefinitionTests.cs +++ b/CosmosDBShell.Tests/Parser/FunctionDefinitionTests.cs @@ -10,7 +10,118 @@ namespace CosmosShell.Tests.Parser; -internal class FunctionDefinitionTests +public class FunctionDefinitionTests : TestBase { + [Theory] + [InlineData("return")] + [InlineData("return;")] + [InlineData("return\n")] + [InlineData("if true { return }")] + [InlineData("while true { return }")] + [InlineData("do { return } while true")] + [InlineData("for $item in [1] { return }")] + [InlineData("loop { return }")] + public async Task BareReturn_ExitsFunctionWithoutResult(string body) + { + var scopeCount = Shell.VariableContainers.Count; + var script = $"def empty {{ {body} }}; empty"; + var state = await Shell.RunCommandAsync(new(), script, TestContext.Current.CancellationToken); + Assert.False(state.IsError); + Assert.False(state.ReturnFunc); + Assert.Null(state.Result); + Assert.Null(state.ReturnValue); + Assert.Equal(scopeCount, Shell.VariableContainers.Count); + } + + [Fact] + public async Task BareReturn_InNestedBlock_SkipsRemainingFunctionBody() + { + var state = await Shell.RunCommandAsync(new(), "def empty { if true { return }; unknown_after_return_xyz }; empty", TestContext.Current.CancellationToken); + Assert.False(state.IsError); + Assert.Null(state.Result); + Assert.False(state.ReturnFunc); + } + + [Fact] + public async Task ReturnExpression_CompletesInnerFunction_ThenExitsOuterFunction() + { + var state = await RunScriptAsync("def inner { $local = 1; return $local + 1 }; def outer { return (inner); totallyunknowncmd999 }; $result = (outer)"); + Assert.False(state.IsError); + Assert.False(state.ReturnFunc); + Assert.Equal(2, Assert.IsType(GetVariable("result")).Value); + } + + [Fact] + public async Task Arguments_PreserveNumericTypes() + { + var state = await RunScriptAsync("def add [a b] { return $a + $b }; $result = (add 2 3)"); + Assert.False(state.IsError); + Assert.Equal(5, Assert.IsType(GetVariable("result")).Value); + } + + [Theory] + [InlineData("identity 1")] + [InlineData("identity true")] + [InlineData("identity [1,2]")] + [InlineData("identity {id: 1}")] + public async Task Identity_PreservesArgumentType(string invocation) + { + var state = await RunScriptAsync($"def identity [value] {{ return $value }}; $result = ({invocation})"); + Assert.False(state.IsError); + Assert.IsNotType(GetVariable("result")); + } + + [Theory] + [InlineData("identity")] + [InlineData("identity 1 2")] + [InlineData("$result = (identity)")] + [InlineData("$result = (identity 1 2)")] + public async Task WrongArgumentCount_IsRejected(string invocation) + { + await RunScriptAsync("$value = 99; def identity [value] { return $value }"); + var exception = await Assert.ThrowsAsync(() => RunScriptAsync(invocation)); + Assert.IsType(exception.InnerException); + Assert.Equal(Azure.Data.Cosmos.Shell.Core.ShellExitCode.UsageError, Azure.Data.Cosmos.Shell.Core.ShellExitCode.FromException(exception)); + } + + [Fact] + public async Task Assignment_IsLocalToFunction() + { + var state = await RunScriptAsync("$value = 1; def change { $value = 2; return $value }; $local = (change)"); + Assert.False(state.IsError); + Assert.Equal(1, Assert.IsType(GetVariable("value")).Value); + Assert.Equal(2, Assert.IsType(GetVariable("local")).Value); + } + + [Fact] + public async Task Exception_RestoresCallerScope() + { + await RunScriptAsync("$value = 1; def fail { $value = 2; $invalid = 1 / 0 }"); + await Assert.ThrowsAsync(() => RunScriptAsync("fail")); + Assert.Equal(1, Assert.IsType(GetVariable("value")).Value); + Assert.Single(Shell.VariableContainers); + } + + [Fact] + public async Task NestedCalls_ReadNearestFrame_WithoutChangingCaller() + { + var state = await RunScriptAsync("$value = 1; def inner { return $value }; def outer [value] { return (inner) }; $result = (outer 9)"); + Assert.False(state.IsError); + Assert.Equal(9, Assert.IsType(GetVariable("result")).Value); + Assert.Equal(1, Assert.IsType(GetVariable("value")).Value); + Assert.Single(Shell.VariableContainers); + } + + [Fact] + public async Task RecursiveFunction_StopsAtLimit_AndRestoresInterpreter() + { + await RunScriptAsync("$value = 1; def recurse { recurse }"); + var exception = await Assert.ThrowsAsync(() => RunScriptAsync("recurse")); + Assert.Contains("depth", exception.Message); + Assert.Single(Shell.VariableContainers); + var state = await RunScriptAsync("def valid { return 2 }; $result = (valid)"); + Assert.False(state.IsError); + Assert.Equal(2, Assert.IsType(GetVariable("result")).Value); + } } diff --git a/CosmosDBShell.Tests/Parser/OperatorEvaluationTests.cs b/CosmosDBShell.Tests/Parser/OperatorEvaluationTests.cs index b5242f6b..7e437711 100644 --- a/CosmosDBShell.Tests/Parser/OperatorEvaluationTests.cs +++ b/CosmosDBShell.Tests/Parser/OperatorEvaluationTests.cs @@ -38,6 +38,64 @@ public async Task IntegerArithmetic_ReturnsExpectedNumber(string input, int expe Assert.Equal(expected, number.Value); } + [Theory] + [InlineData("2147483647 + 1")] + [InlineData("(-2147483647 - 1) - 1")] + [InlineData("2147483647 * 2")] + [InlineData("2 ** 31")] + [InlineData("-(-2147483647 - 1)")] + public async Task IntegerOverflow_Throws(string input) + { + await Assert.ThrowsAsync(() => EvalAsync(input)); + } + + [Theory] + [InlineData("3", 1.0)] + [InlineData("3.0", 1.5)] + [InlineData("3.5", 1.75)] + [InlineData("2147483648", 1073741824.0)] + public async Task JsonNumbers_UseConsistentArithmetic(string number, double expected) + { + using var document = JsonDocument.Parse(number); + var expression = new BinaryOperatorExpression( + new ConstantExpression(new(TokenType.Number, number, 0, number.Length), new ShellJson(document.RootElement)), + new(TokenType.Divide, "/", 0, 1), + new ConstantExpression(new(TokenType.Number, "2", 0, 1), new ShellNumber(2))); + var result = await expression.EvaluateAsync(ShellInterpreter.Instance, new(), CancellationToken.None); + Assert.Equal(expected, Assert.IsType(result.ConvertShellObject(DataType.Decimal))); + } + + [Fact] + public async Task NotEqual_EvaluatesEachOperandOnce() + { + var left = new CountingExpression(); + var right = new CountingExpression(); + var expression = new BinaryOperatorExpression(left, new(TokenType.NotEqual, "!=", 0, 2), right); + await expression.EvaluateAsync(ShellInterpreter.Instance, new(), CancellationToken.None); + Assert.Equal(1, left.Count); + Assert.Equal(1, right.Count); + } + + private sealed class CountingExpression : Expression + { + public int Count { get; private set; } + + public override int Start => 0; + + public override int Length => 1; + + public override Task EvaluateAsync(ShellInterpreter interpreter, CommandState currentState, CancellationToken cancellationToken) + { + this.Count++; + return Task.FromResult(new ShellNumber(this.Count)); + } + + public override void Accept(IAstVisitor visitor) + { + throw new NotSupportedException(); + } + } + [Theory] [InlineData("1.5 + 2.5", 4.0)] [InlineData("5.0 - 2.5", 2.5)] diff --git a/CosmosDBShell.Tests/Parser/ShellObjectConversionTests.cs b/CosmosDBShell.Tests/Parser/ShellObjectConversionTests.cs index 0c935881..84d16489 100644 --- a/CosmosDBShell.Tests/Parser/ShellObjectConversionTests.cs +++ b/CosmosDBShell.Tests/Parser/ShellObjectConversionTests.cs @@ -16,6 +16,34 @@ namespace CosmosShell.Tests.Parser; /// public class ShellObjectConversionTests { + [Theory] + [InlineData("3.0")] + [InlineData("-3.0")] + [InlineData("0.0")] + [InlineData("-0.0")] + [InlineData("3.5")] + [InlineData("2147483647.0")] + [InlineData("1e20")] + [InlineData("1e-20")] + [InlineData("5e-324")] + [InlineData("1.7976931348623157e308")] + public void ShellDecimal_JsonRoundTrip_PreservesDecimalTypeAndValue(string source) + { + var value = double.Parse(source, System.Globalization.CultureInfo.InvariantCulture); + var json = Assert.IsType(new ShellDecimal(value).ConvertShellObject(DataType.Json)); + var restored = Assert.IsType(ShellNumber.FromJson(json)); + Assert.Equal(BitConverter.DoubleToInt64Bits(value), BitConverter.DoubleToInt64Bits(restored.Value)); + } + + [Theory] + [InlineData(double.NaN)] + [InlineData(double.PositiveInfinity)] + [InlineData(double.NegativeInfinity)] + public void ShellDecimal_NonFiniteValues_CannotBecomeJsonNumbers(double value) + { + Assert.Throws(() => new ShellDecimal(value).ConvertShellObject(DataType.Json)); + } + [Theory] [InlineData((int)DataType.Boolean, "conversion-error-text-boolean", "conversion-error-identifier-boolean")] [InlineData((int)DataType.Number, "conversion-error-text-number", "conversion-error-identifier-number")] @@ -98,6 +126,20 @@ public void ShellJson_ZeroNumber_IsFalse() Assert.Equal(false, Json("0").ConvertShellObject(DataType.Boolean)); } + [Theory] + [InlineData("1.5", true)] + [InlineData("-0.5", true)] + [InlineData("2147483648", true)] + [InlineData("9007199254740993", true)] + [InlineData("0.0", false)] + [InlineData("-0.0", false)] + public void ShellJson_NumericBooleanConversion_MatchesShellDecimal(string source, bool expected) + { + var json = Json(source); + Assert.Equal(expected, json.ConvertShellObject(DataType.Boolean)); + Assert.Equal(new ShellDecimal(json.Value.GetDouble()).ConvertShellObject(DataType.Boolean), json.ConvertShellObject(DataType.Boolean)); + } + [Fact] public void ShellJson_StringValue_TextReturnsUnquoted() { diff --git a/CosmosDBShell.Tests/Parser/StatementExecutionTests.cs b/CosmosDBShell.Tests/Parser/StatementExecutionTests.cs index 1edd802c..22ade0c4 100644 --- a/CosmosDBShell.Tests/Parser/StatementExecutionTests.cs +++ b/CosmosDBShell.Tests/Parser/StatementExecutionTests.cs @@ -17,12 +17,133 @@ namespace CosmosShell.Tests.Parser; /// public class StatementExecutionTests : TestBase { + public static System.Collections.Generic.IEnumerable ValueOriginCases() + { + var operations = new (string Left, string Operator, string Right, string Type, string Expected)[] + { + ("\"2\"", "+", "\"2\"", "Text", "22"), + ("\"hello\"", "+", "\"world\"", "Text", "helloworld"), + ("\"\"", "+", "\"text\"", "Text", "text"), + ("\"value=\"", "+", "2", "Text", "value=2"), + ("2", "+", "\"px\"", "Text", "2px"), + ("3", "+", "2", "Number", "5"), + ("3.5", "+", "2", "Decimal", "5.5"), + ("3", "/", "2", "Number", "1"), + ("3.0", "/", "2", "Decimal", "1.5"), + ("[1]", "+", "[2]", "Json", "[1,2]"), + ("\"a\"", "==", "\"a\"", "Boolean", "true"), + ("true", "&&", "false", "Boolean", "false"), + }; + + foreach (var operation in operations) + { + var leftOrigins = new[] { operation.Left, "$source.left", "$leftItem", "(identity $source.left)" }; + var rightOrigins = new[] { operation.Right, "$source.right", "$rightItem", "(identity $source.right)" }; + foreach (var left in leftOrigins) + { + foreach (var right in rightOrigins) + { + var script = $"def identity [value] {{ return $value }}; " + + $"for $leftItem in $source.leftItems {{ for $rightItem in $source.rightItems {{ " + + $"$actual = {left} {operation.Operator} {right} }} }}"; + var source = $"{{\"left\":{operation.Left},\"right\":{operation.Right}," + + $"\"leftItems\":[{operation.Left}],\"rightItems\":[{operation.Right}]}}"; + yield return [source, script, operation.Type, operation.Expected]; + yield return [source, $"$source = {source}; " + script, operation.Type, operation.Expected]; + } + } + } + } + + [Theory] + [MemberData(nameof(ValueOriginCases))] + public async Task Operators_PreserveResultsAcrossValueOrigins(string source, string script, string expectedType, string expected) + { + using var document = System.Text.Json.JsonDocument.Parse(source); + SetVariable("source", new ShellJson(document.RootElement.Clone())); + var state = await Shell.RunCommandAsync(new(), script, TestContext.Current.CancellationToken); + Assert.False(state.IsError); + var actual = GetVariable("actual")!; + Assert.Equal(expectedType, actual.DataType.ToString()); + Assert.Equal(expected, actual.ConvertShellObject(DataType.Text)); + } + + [Theory] + [InlineData("3.0", 1.5)] + [InlineData("-3.0", -1.5)] + [InlineData("0.0", 0.0)] + [InlineData("1.5 * 2", 1.5)] + [InlineData("3.5", 1.75)] + public async Task DecimalValues_SurviveRepeatedJsonConstruction(string expression, double expected) + { + var script = $"$initial = {expression}; $object = {{\"value\":$initial}}; " + + "$array = [$object.value]; for $item in $array { " + + "$rebuilt = {\"value\":$item}; $result = $rebuilt.value / 2 }"; + var state = await Shell.RunCommandAsync(new(), script, TestContext.Current.CancellationToken); + Assert.False(state.IsError); + Assert.Equal(expected, Assert.IsType(GetVariable("result")).Value); + var rebuilt = Assert.IsType(GetVariable("rebuilt")); + Assert.IsType(ShellNumber.FromJson(rebuilt.Value.GetProperty("value"))); + } + + [Fact] + public async Task SyntaxError_PreventsEarlierAssignment() + { + SetVariable("value", new ShellNumber(1)); + var state = await Shell.RunCommandAsync(new(), "$value = 2; if true {", System.Threading.CancellationToken.None); + + Assert.True(state.IsError); + Assert.Equal(1, GetInt("value")); + } + + [Theory] + [InlineData("break")] + [InlineData("continue")] + [InlineData("return 1")] + [InlineData("if false { break }")] + [InlineData("def duplicate [value value] { return $value }")] + [InlineData("for $item in [1] { def invalid { break } }")] + public async Task SemanticError_PreventsEarlierAssignment(string invalid) + { + SetVariable("value", new ShellNumber(1)); + var state = await Shell.RunCommandAsync(new(), $"$value = 2; {invalid}", System.Threading.CancellationToken.None); + Assert.True(state.IsError); + Assert.Equal(1, GetInt("value")); + } + private int GetInt(string name) { var value = GetVariable(name); return (int)Assert.IsType(value).Value; } + [Theory] + [InlineData("if true { return 1 }")] + [InlineData("while true { if true { return 1 } }")] + [InlineData("do { if true { return 1 } } while true")] + [InlineData("loop { if true { return 1 } }")] + [InlineData("for $item in [1,2] { if true { return 1 } }")] + public async Task NestedReturn_ExitsFunction(string body) + { + var state = await RunScriptAsync($"def probe {{ {body}; return 2 }}; $result = (probe)"); + Assert.False(state.IsError); + Assert.Equal(1, GetInt("result")); + Assert.False(state.ReturnFunc); + } + + [Theory] + [InlineData("for $item in [1,2] { if true { continue }; $count = $count + 1 }")] + [InlineData("while $index < 2 { $index = $index + 1; if true { continue }; $count = $count + 1 }")] + [InlineData("do { $index = $index + 1; if true { continue }; $count = $count + 1 } while $index < 2")] + [InlineData("loop { $index = $index + 1; if $index > 2 { break }; if true { continue }; $count = $count + 1 }")] + public async Task NestedContinue_SkipsRemainderOfIteration(string body) + { + var state = await RunScriptAsync($"$count = 0; $index = 0; {body}"); + Assert.False(state.IsError); + Assert.Equal(0, GetInt("count")); + Assert.False(state.ContinueBlock); + } + [Fact] public async Task If_TrueCondition_ExecutesThenBranch() { @@ -31,6 +152,15 @@ public async Task If_TrueCondition_ExecutesThenBranch() Assert.Equal(10, GetInt("x")); } + [Theory] + [InlineData("dir \"*.missing-regression-file\" --directory .")] + [InlineData("$files = (dir \"*.missing-regression-file\" --directory .)")] + public async Task ReusedCommand_BindsOptionsWithoutMutatingAst(string command) + { + var state = await RunScriptAsync($"def probe {{ {command} }}; probe; probe"); + Assert.False(state.IsError); + } + [Fact] public async Task If_FalseCondition_ExecutesElseBranch() { @@ -152,6 +282,25 @@ public async Task Pipe_PrintFailure_ReturnsError() Assert.True(state.IsError); } + [Fact] + public async Task Pipe_FinalReturn_DoesNotRenderPreviousResult() + { + var output = Path.GetTempFileName(); + Shell.StdOutRedirect = output; + try + { + var state = await RunScriptAsync("def run { echo value | return }; run"); + + Assert.False(state.IsError); + Assert.Empty(await File.ReadAllTextAsync(output, TestContext.Current.CancellationToken)); + } + finally + { + Shell.StdOutRedirect = null; + File.Delete(output); + } + } + [Fact] public async Task Assignment_ChainedArithmetic_ComputesExpected() { @@ -160,6 +309,43 @@ public async Task Assignment_ChainedArithmetic_ComputesExpected() Assert.Equal(19, GetInt("x")); } + [Theory] + [InlineData("+=", 9)] + [InlineData("-=", 3)] + [InlineData("*=", 18)] + [InlineData("/=", 2)] + public async Task CompoundAssignment_UsesArithmeticRules(string assignment, int expected) + { + var script = $"$value = 6; $value {assignment} 3"; + var parser = new StatementParser(script); + var statements = parser.ParseStatements(); + Assert.False(parser.Errors.HasErrors); + Assert.Equal($"$value {assignment} 3", statements[1].ToString()); + var state = await RunScriptAsync(script); + Assert.False(state.IsError); + Assert.Equal(expected, GetInt("value")); + } + + [Fact] + public async Task CompoundAssignment_InFunction_RemainsLocal() + { + var state = await RunScriptAsync("$value = 1; def increment { $value += 2; return $value }; $local = (increment)"); + Assert.False(state.IsError); + Assert.Equal(1, GetInt("value")); + Assert.Equal(3, GetInt("local")); + } + + [Theory] + [InlineData("while true {}")] + [InlineData("do {} while true")] + [InlineData("loop {}")] + [InlineData("for $value in [1] {}")] + public async Task PureLoop_ObservesCancellation(string script) + { + var statement = Assert.Single(new StatementParser(script).ParseStatements()); + await Assert.ThrowsAnyAsync(() => statement.RunAsync(Shell, new(), new System.Threading.CancellationToken(true))); + } + [Fact] public async Task For_OverStrings_BindsTextElements() { @@ -177,11 +363,27 @@ public async Task For_OverBooleans_BindsBoolElements() } [Fact] - public async Task For_OverNull_BindsNullAsText() + public async Task For_OverNull_PreservesNullThroughFunctionAndArray() { - var state = await RunScriptAsync("for $x in [null] { }"); + var state = await RunScriptAsync("def identity [value] { return $value }; for $x in [null] { $result = [(identity $x)] }"); Assert.False(state.IsError); - Assert.Equal("null", Assert.IsType(GetVariable("x")).Text); + Assert.Equal(System.Text.Json.JsonValueKind.Null, Assert.IsType(GetVariable("x")).Value.ValueKind); + Assert.Equal("[null]", Assert.IsType(GetVariable("result")).Value.GetRawText()); + } + + [Theory] + [InlineData("1.5", true)] + [InlineData("-0.5", true)] + [InlineData("2147483648.0", true)] + [InlineData("0.0", false)] + public async Task NumericTruthiness_IsIndependentOfValueOrigin(string number, bool expected) + { + var state = await RunScriptAsync($"$direct = false; if {number} {{ $direct = true }}; $obj = {{value: {number}}}; $json = false; if $obj.value {{ $json = true }}; def truth [value] {{ if $value {{ return true }}; return false }}; $function = (truth $obj.value); for $item in [{number}] {{ $loop = (truth $item) }}"); + Assert.False(state.IsError); + foreach (var name in new[] { "direct", "json", "function", "loop" }) + { + Assert.Equal(expected, Assert.IsType(GetVariable(name)).Value); + } } [Fact] @@ -206,4 +408,14 @@ public async Task For_OverNonArray_Throws() await Assert.ThrowsAsync( () => RunScriptAsync("for $x in 5 { }")); } + + [Theory] + [InlineData("[1.5,2.5]", 4.0)] + [InlineData("[2147483648.0,1]", 2147483649.0)] + public async Task For_OverDecimalAndLargeNumbers_PreservesValues(string values, double expected) + { + var state = await RunScriptAsync($"$sum = 0; for $value in {values} {{ $sum = $sum + $value }}"); + Assert.False(state.IsError); + Assert.Equal(expected, Assert.IsType(GetVariable("sum")).Value); + } } diff --git a/CosmosDBShell.Tests/Parser/StatementParserStructureTests.cs b/CosmosDBShell.Tests/Parser/StatementParserStructureTests.cs index ff5730c3..87596045 100644 --- a/CosmosDBShell.Tests/Parser/StatementParserStructureTests.cs +++ b/CosmosDBShell.Tests/Parser/StatementParserStructureTests.cs @@ -31,6 +31,18 @@ private static (List Statements, IReadOnlyList Errors) Pa return (parser.ParseStatements(), lexer.Errors); } + [Theory] + [InlineData("def duplicate [value value] { return $value }")] + [InlineData("def duplicate [value $value] { return $value }")] + public void DuplicateParameters_HighlightFunctionDefinition(string source) + { + var result = StatementParser.ScriptParseResult.Parse(source); + + var error = Assert.Single(result.Errors); + Assert.Equal(0, error.Start); + Assert.Equal(source.Length, error.Length); + } + [Fact] public void AppendOutputRedirect_IsRecognized() { diff --git a/CosmosDBShell.Tests/Parser/StatementPositionalErrorTests.cs b/CosmosDBShell.Tests/Parser/StatementPositionalErrorTests.cs index 0378f0b2..b5dcf161 100644 --- a/CosmosDBShell.Tests/Parser/StatementPositionalErrorTests.cs +++ b/CosmosDBShell.Tests/Parser/StatementPositionalErrorTests.cs @@ -18,6 +18,148 @@ namespace CosmosShell.Tests.Parser; /// public class StatementPositionalErrorTests : TestBase { + [Theory] + [InlineData("$value = (totallyunknowncmd999)")] + [InlineData("if (totallyunknowncmd999) {}")] + [InlineData("for $item in (totallyunknowncmd999) {}")] + public async Task ThrownExpressionError_UsesCommandLocation(string source) + { + Shell.CurrentScriptFileName = "expression.csh"; + Shell.CurrentScriptContent = source; + var statement = new StatementParser(source).ParseStatement()!; + var exception = await Assert.ThrowsAsync(() => statement.RunAsync(Shell, new(), TestContext.Current.CancellationToken)); + var frame = PositionalException.GetSourceTrace(exception)[0]; + Assert.Equal("expression.csh", frame.FileName); + Assert.Equal(source.IndexOf("totallyunknowncmd999", StringComparison.Ordinal) + 1, frame.Column); + Assert.IsType(frame.InnerException); + } + + [Theory] + [InlineData("$value = (help totallyunknowncmd999)")] + [InlineData("if (help totallyunknowncmd999) {}")] + [InlineData("for $item in (help totallyunknowncmd999) {}")] + public async Task ReturnedExpressionError_UsesCommandLocation(string source) + { + Shell.CurrentScriptFileName = "expression.csh"; + Shell.CurrentScriptContent = source; + var statement = new StatementParser(source).ParseStatement()!; + var exception = await Assert.ThrowsAsync(() => statement.RunAsync(Shell, new(), TestContext.Current.CancellationToken)); + var frame = PositionalException.GetSourceTrace(exception)[0]; + Assert.Equal("expression.csh", frame.FileName); + Assert.Equal(source.IndexOf("help", StringComparison.Ordinal) + 1, frame.Column); + var failure = Assert.IsType(frame.InnerException); + Assert.IsType(failure.State); + } + + [Fact] + public async Task InteractiveFunction_RetainsScriptModeWithoutBorrowingCallerOffsets() + { + var definition = new DefStatement(new Token(TokenType.Identifier, "def", 0, 3), new Token(TokenType.Identifier, "probe", 4, 5), [], new ScriptContextProbe()); + await definition.RunAsync(Shell, new(), TestContext.Current.CancellationToken); + Shell.CurrentScriptFileName = "caller.csh"; + Shell.CurrentScriptContent = "\nprobe"; + + var exception = await Assert.ThrowsAsync(() => definition.ExecuteCallAsync(Shell, new(), TestContext.Current.CancellationToken, 1)); + var frame = Assert.Single(PositionalException.GetSourceTrace(exception)); + Assert.Equal("caller.csh", frame.FileName); + Assert.Equal(2, frame.Line); + Assert.Equal("\nprobe", Shell.CurrentScriptContent); + } + + private sealed class ScriptContextProbe : Statement + { + public override int Start => 100; + + public override int Length => 1; + + public override Task RunAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) + { + Assert.Equal("caller.csh", shell.CurrentScriptFileName); + Assert.Null(shell.CurrentScriptContent); + throw new InvalidOperationException("probe failure"); + } + + internal override void Accept(IAstVisitor visitor) + { + } + } + + [Theory] + [InlineData(false, "totallyunknowncmd999")] + [InlineData(true, "totallyunknowncmd999")] + [InlineData(false, "help totallyunknowncmd999")] + [InlineData(true, "help totallyunknowncmd999")] + public async Task ScriptFunctionFailure_RecordsEachCallerOnce(bool nestedScript, string failure) + { + var script = Path.GetTempFileName().Replace('\\', '/'); + var caller = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(script, $"def broken {{\n {failure}\n}}\nbroken", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(caller, $"exec \"{script.Replace('\\', '/')}\"", TestContext.Current.CancellationToken); + var path = nestedScript ? caller : script; + var command = new CommandStatement(new Token(TokenType.Identifier, path, 0, path.Length)); + var exception = await Assert.ThrowsAsync(() => command.RunScriptAsync(Shell, new(), TestContext.Current.CancellationToken)); + var frames = PositionalException.GetSourceTrace(exception); + Assert.Single(frames, frame => frame.FileName == script && frame.Line == 2); + Assert.Single(frames, frame => frame.FileName == script && frame.Line == 4); + if (nestedScript) + { + Assert.Single(frames, frame => frame.FileName == caller && frame.Line == 1); + } + + Assert.Equal(nestedScript ? 3 : 2, frames.Count); + } + finally + { + File.Delete(script); + File.Delete(caller); + } + } + + [Fact] + public async Task ReturnedScriptError_PreservesStatementLocationAndCause() + { + var script = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(script, "\nhelp totallyunknowncmd999", TestContext.Current.CancellationToken); + var command = new CommandStatement(new Token(TokenType.Identifier, script, 0, script.Length)); + var exception = await Assert.ThrowsAsync(() => command.RunScriptAsync(Shell, new(), TestContext.Current.CancellationToken)); + var frame = Assert.Single(PositionalException.GetSourceTrace(exception)); + Assert.Equal(script, frame.FileName); + Assert.Equal(2, frame.Line); + Assert.IsType(frame.InnerException); + Assert.Equal(ShellExitCode.FromException(frame.InnerException!), ShellExitCode.FromException(exception)); + } + finally + { + File.Delete(script); + } + } + + [Theory] + [InlineData("broken")] + [InlineData("$result = (broken)")] + public async Task FunctionFailure_PreservesDefinitionAndCallerSources(string invocation) + { + const string definition = "def broken {\n totallyunknowncmd999\n}"; + Shell.CurrentScriptFileName = "definition.csh"; + Shell.CurrentScriptContent = definition; + await new StatementParser(definition).ParseStatement()!.RunAsync(Shell, new(), CancellationToken.None); + Shell.CurrentScriptFileName = "caller.csh"; + Shell.CurrentScriptContent = invocation; + + var exception = await Assert.ThrowsAsync(() => new StatementParser(invocation).ParseStatement()!.RunAsync(Shell, new(), CancellationToken.None)); + var frames = PositionalException.GetSourceTrace(exception); + Assert.Equal("definition.csh", frames[0].FileName); + Assert.Equal(2, frames[0].Line); + Assert.Equal("caller.csh", frames[^1].FileName); + Assert.Equal("caller.csh", Shell.CurrentScriptFileName); + Assert.Equal(invocation, Shell.CurrentScriptContent); + Assert.Equal(ShellExitCode.UsageError, ShellExitCode.FromException(exception)); + } + private async Task RunWithScriptContextAsync(string script) { Shell.CurrentScriptFileName = "script.csh"; diff --git a/CosmosDBShell.Tests/Runtime/DiagnosticLogTests.cs b/CosmosDBShell.Tests/Runtime/DiagnosticLogTests.cs index 5894d176..fce26bfa 100644 --- a/CosmosDBShell.Tests/Runtime/DiagnosticLogTests.cs +++ b/CosmosDBShell.Tests/Runtime/DiagnosticLogTests.cs @@ -129,6 +129,35 @@ public void LogParserErrors_WritesErrorMessages() Assert.Contains("[ERROR ] dir | -> error: Unexpected token; warning: Missing argument", line); } + [Fact] + public void LogError_PreservesSourceChain_AndRedactsEveryEntry() + { + using (var log = DiagnosticLog.Create(this.path)) + { + log.AddSecret("private-source"); + var cause = new InvalidOperationException("private-source failed"); + var child = new Azure.Data.Cosmos.Shell.Parser.PositionalException("private-source.csh", cause, 3, 2); + log.LogError("invoke", new Azure.Data.Cosmos.Shell.Parser.PositionalException("caller.csh", child, 1, 1)); + } + + var text = File.ReadAllText(this.path); + Assert.Contains(".csh:3:2", text); + Assert.Contains("caller.csh:1:1", text); + Assert.Contains("InvalidOperationException:", text); + Assert.DoesNotContain("private-source", text); + } + + [Fact] + public void LogParserErrors_ResolvesFileCoordinates() + { + using (var log = DiagnosticLog.Create(this.path)) + { + log.LogParserErrors("exec child", new[] { new Azure.Data.Cosmos.Shell.Parser.ParseError(2, 1, "Unexpected token") }, "child.csh", "\r\n}"); + } + + Assert.Contains("child.csh:2:1: Unexpected token", LastEntry()); + } + [Fact] public void LogParserErrors_NoErrors_WritesNothing() { diff --git a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs index 1daeac06..3927b0f1 100644 --- a/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs +++ b/CosmosDBShell.Tests/Runtime/SerializedExecutionTests.cs @@ -125,7 +125,45 @@ await Assert.ThrowsAsync(() => shell.RunSerializedAsync } [Fact] - public async Task RunSerializedAsync_WaitsForOtherExecutionButAllowsNestedCalls() + public async Task RunSerializedAsync_SerializesWorkStartedInsideAnOwnedOperation() + { + using var shell = ShellInterpreter.CreateInstance(); + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var childEntered = false; + Task child = Task.FromResult(0); + var first = shell.RunSerializedAsync( + async () => + { + child = Task.Run(() => shell.RunSerializedAsync( + () => + { + childEntered = true; + return Task.FromResult(7); + }, + CancellationToken.None)); + entered.SetResult(); + await release.Task; + return 42; + }, + CancellationToken.None); + await entered.Task; + try + { + Assert.False(childEntered); + Assert.False(child.IsCompleted); + } + finally + { + release.SetResult(); + } + + Assert.Equal(42, await first.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + Assert.Equal(7, await child.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken)); + } + + [Fact] + public async Task RunSerializedAsync_WaitsForOtherExecution() { using var shell = ShellInterpreter.CreateInstance(); var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -134,7 +172,7 @@ public async Task RunSerializedAsync_WaitsForOtherExecutionButAllowsNestedCalls( { entered.SetResult(); await release.Task; - return await shell.RunSerializedAsync(() => Task.FromResult(42), CancellationToken.None); + return 42; }, CancellationToken.None); await entered.Task; var secondEntered = false; diff --git a/CosmosDBShell.Tests/Runtime/ShellExitCodeTests.cs b/CosmosDBShell.Tests/Runtime/ShellExitCodeTests.cs index 4468e8e7..b6528f11 100644 --- a/CosmosDBShell.Tests/Runtime/ShellExitCodeTests.cs +++ b/CosmosDBShell.Tests/Runtime/ShellExitCodeTests.cs @@ -19,6 +19,37 @@ namespace CosmosShell.Tests.Runtime; public class ShellExitCodeTests { + [Theory] + [InlineData(HttpStatusCode.Unauthorized, ShellExitCode.AuthFailure)] + [InlineData(HttpStatusCode.NotFound, ShellExitCode.NotFound)] + [InlineData(HttpStatusCode.TooManyRequests, ShellExitCode.Throttled)] + [InlineData(HttpStatusCode.ServiceUnavailable, ShellExitCode.ConnectionError)] + public void NestedSourceWrappers_PreserveFailureCategory(HttpStatusCode status, int expected) + { + var cause = new CosmosException("failure", status, 0, "test", 0); + var inner = new PositionalException("child.csh", cause, 3, 5, "query"); + var outer = new PositionalException("parent.csh", inner, 1, 1, "child.csh"); + Assert.Equal(expected, ShellExitCode.FromException(outer)); + Assert.Equal(expected, new ErrorCommandState(outer).ExitCode); + } + + [Fact] + public void PositionalRuntimeFailure_RemainsRuntimeFailure() + { + Assert.Equal(ShellExitCode.GeneralFailure, ShellExitCode.FromException(new PositionalException("script.csh", new DivideByZeroException(), 1, 1))); + } + + [Fact] + public void ExpressionFailure_PreservesOriginalStateAndCause() + { + var cause = new RequestFailedException(429, "throttled"); + var state = new ErrorCommandState(cause); + var failure = new CommandState.FailureException(state); + Assert.Same(state, failure.State); + Assert.Same(cause, failure.InnerException); + Assert.Equal(ShellExitCode.Throttled, ShellExitCode.FromException(failure)); + } + [Fact] public void FromException_Null_ReturnsGeneralFailure() { @@ -247,4 +278,20 @@ public void FromCommandState_StructuredErrorWithHttpStatus_ReturnsClassifiedExit Assert.Equal(expected, ShellExitCode.FromCommandState(state)); } + + [Fact] + public void FromCommandState_StructuredErrorReattachedToItsOwnFailure_ClassifiesOriginalCause() + { + var cause = new CommandException( + "batch", + "Batch failed.", + new RequestFailedException((int)HttpStatusCode.NotFound, "Batch failed.")); + var state = new StructuredErrorCommandState( + cause, + new ShellJson(JsonSerializer.SerializeToElement(new { success = false }))); + var failure = new CommandState.FailureException(state); + state.Exception = new PositionalException("script.csh", failure, 1, 1); + + Assert.Equal(ShellExitCode.NotFound, ShellExitCode.FromCommandState(state)); + } } diff --git a/CosmosDBShell.Tests/ScriptExecutionScopeTests.cs b/CosmosDBShell.Tests/ScriptExecutionScopeTests.cs index 95ea9c61..2cb759b5 100644 --- a/CosmosDBShell.Tests/ScriptExecutionScopeTests.cs +++ b/CosmosDBShell.Tests/ScriptExecutionScopeTests.cs @@ -16,6 +16,271 @@ namespace CosmosShell.Tests; public class ScriptExecutionScopeTests { + [Fact] + public async Task ScriptFrame_ShadowsNearestScopeBeforeOuterScopes() + { + using var shell = ShellInterpreter.CreateInstance(); + var globals = new VariableContainer(); + globals.Set("value", new ShellText("global")); + shell.VariableContainers.Push(globals); + var caller = new VariableContainer(); + caller.Set("value", new ShellText("caller")); + shell.VariableContainers.Push(caller); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "return $value", TestContext.Current.CancellationToken); + var command = new CommandStatement(new Token(TokenType.Identifier, path, 0, path.Length)); + var state = await command.RunScriptAsync(shell, new(), TestContext.Current.CancellationToken); + Assert.Equal("caller", Assert.IsType(state.Result).Text); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData("if $cancel {}")] + [InlineData("{ if $cancel {} }")] + [InlineData("while true { if $cancel {} }")] + [InlineData("do { if $cancel {} } while true")] + [InlineData("for $item in [1] { if $cancel {} }")] + [InlineData("loop { if $cancel {} }")] + [InlineData("def run { if $cancel {} }; run")] + [InlineData("def run { if $cancel {} }; $result = (run)")] + [InlineData("def run { if $cancel {} }; exec \"run\"")] + public async Task InternalCancellation_PreservesScriptSource(string body) + { + using var shell = ShellInterpreter.CreateInstance(); + using var cancellation = new CancellationTokenSource(); + var globals = new VariableContainer(); + globals.Set("cancel", new CancelOnConversion(cancellation, cancelToken: false)); + shell.VariableContainers.Push(globals); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, body, TestContext.Current.CancellationToken); + var command = new CommandStatement(new Token(TokenType.Identifier, path, 0, path.Length)); + var exception = await Assert.ThrowsAsync(() => command.RunScriptAsync(shell, new(), cancellation.Token)); + Assert.Contains(PositionalException.GetSourceTrace(exception), frame => frame.FileName == path); + Assert.IsType(exception.GetBaseException()); + Assert.False(cancellation.IsCancellationRequested); + Assert.Single(shell.VariableContainers); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task CancellationBetweenStatements_PropagatesAndRestoresScope(bool expression) + { + using var shell = ShellInterpreter.CreateInstance(); + using var cancellation = new CancellationTokenSource(); + var trigger = new CancelOnConversion(cancellation, throwOnConversion: false); + var globals = new VariableContainer(); + globals.Set("cancel", trigger); + shell.VariableContainers.Push(globals); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "$value = !$cancel\nreturn 123", TestContext.Current.CancellationToken); + var token = new Token(TokenType.Identifier, path, 0, path.Length); + await Assert.ThrowsAnyAsync(() => expression + ? new CommandExpression(token).EvaluateAsync(shell, new(), cancellation.Token) + : (Task)new CommandStatement(token).RunScriptAsync(shell, new(), cancellation.Token)); + Assert.True(trigger.WasEvaluated); + Assert.Single(shell.VariableContainers); + Assert.Null(shell.CurrentScriptFileName); + Assert.Null(shell.CurrentScriptContent); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BareReturn_BeforeBlockEnd_ExitsFileAndRestoresScope(bool expression) + { + using var shell = ShellInterpreter.CreateInstance(); + shell.SetVariable("value", new ShellNumber(1)); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "$value = 2; if true { return }; unknown_after_return_xyz", TestContext.Current.CancellationToken); + var token = new Token(TokenType.Identifier, path, 0, path.Length); + var state = expression + ? await new CommandExpression(token).RunScriptAsync(shell, new(), TestContext.Current.CancellationToken) + : await new CommandStatement(token).RunScriptAsync(shell, new(), TestContext.Current.CancellationToken); + Assert.False(state.IsError); + Assert.False(state.ReturnFunc); + Assert.Null(state.Result); + Assert.Null(state.ReturnValue); + Assert.Single(shell.VariableContainers); + Assert.Equal(1, Assert.IsType(shell.GetVariable("value")).Value); + Assert.Null(shell.CurrentScriptFileName); + Assert.Null(shell.CurrentScriptContent); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData("if $cancel {}")] + [InlineData("{ if $cancel {} }")] + [InlineData("while true { if $cancel {} }")] + [InlineData("do { if $cancel {} } while true")] + [InlineData("for $item in [1] { if $cancel {} }")] + [InlineData("loop { if $cancel {} }")] + [InlineData("def run { if $cancel {} }; run")] + [InlineData("def run { if $cancel {} }; $result = (run)")] + [InlineData("def run { if $cancel {} }; exec \"run\"")] + public async Task CancellationDuringFileExecution_IsNotReportedAsRuntimeFailure(string body) + { + using var shell = ShellInterpreter.CreateInstance(); + using var cancellation = new CancellationTokenSource(); + var trigger = new CancelOnConversion(cancellation); + var globals = new VariableContainer(); + globals.Set("cancel", trigger); + shell.VariableContainers.Push(globals); + var script = Path.GetTempFileName().Replace('\\', '/'); + var output = Path.GetTempFileName(); + var log = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(script, body, TestContext.Current.CancellationToken); + shell.EnableDiagnostics(log); + shell.ErrOutRedirect = output; + var state = await shell.ExecuteCommandAsync($"exec \"{script}\"", cancellation.Token); + + Assert.True(trigger.WasEvaluated); + Assert.True(cancellation.IsCancellationRequested); + Assert.False(state.IsError); + Assert.Equal(ShellExitCode.Success, state.ExitCode); + Assert.Single(shell.VariableContainers); + Assert.Same(trigger, shell.GetVariable("cancel")); + Assert.Null(shell.CurrentScriptFileName); + Assert.Null(shell.CurrentScriptContent); + Assert.Empty(File.ReadAllText(output)); + shell.Diagnostics!.Dispose(); + var entries = File.ReadAllText(log); + Assert.Contains("[CANCELLED]", entries); + Assert.DoesNotContain("[ERROR", entries); + Assert.DoesNotContain("[FAIL]", entries); + var next = await shell.RunCommandAsync(new(), "$value = 1", CancellationToken.None); + Assert.False(next.IsError); + } + finally + { + shell.ErrOutRedirect = null; + shell.Dispose(); + File.Delete(script); + File.Delete(output); + File.Delete(log); + } + } + + [Fact] + public async Task RecursiveScript_StopsAtCallLimit_AndRestoresScope() + { + var shell = ShellInterpreter.CreateInstance(); + shell.SetVariable("value", new ShellNumber(1)); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "exec $0", TestContext.Current.CancellationToken); + var command = new CommandStatement(new(TokenType.Identifier, path, 0, path.Length)); + var exception = await Assert.ThrowsAnyAsync(() => command.RunScriptAsync(shell, new(), CancellationToken.None)); + Assert.Contains("call depth", exception.ToString()); + Assert.Single(shell.VariableContainers); + Assert.Null(shell.CurrentScriptFileName); + var state = await shell.RunCommandAsync(new(), "$value = 2", CancellationToken.None); + Assert.False(state.IsError); + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task InvalidScript_DoesNotRegisterFunctions_AndRestoresScope(bool expression) + { + var shell = ShellInterpreter.CreateInstance(); + shell.SetVariable("value", new ShellNumber(1)); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "def mustNotRegister { return 1 }; if true {", TestContext.Current.CancellationToken); + var token = new Token(TokenType.Identifier, path, 0, path.Length); + var state = expression + ? await new CommandExpression(token).RunScriptAsync(shell, new(), CancellationToken.None) + : await new CommandStatement(token).RunScriptAsync(shell, new(), CancellationToken.None); + Assert.True(state.IsError); + Assert.False(shell.Functions.ContainsKey("mustNotRegister")); + Assert.Single(shell.VariableContainers); + Assert.Equal(1, Assert.IsType(shell.GetVariable("value")).Value); + Assert.Null(shell.CurrentScriptFileName); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public async Task ExpressionScript_DoesNotModifyCaller_AndRestoresFrame() + { + var shell = ShellInterpreter.CreateInstance(); + shell.SetVariable("value", new ShellNumber(1)); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "$value = 2", TestContext.Current.CancellationToken); + var expression = new CommandExpression(new(TokenType.Identifier, path, 0, path.Length)); + var state = await expression.RunScriptAsync(shell, new(), CancellationToken.None); + Assert.False(state.IsError); + Assert.Single(shell.VariableContainers); + Assert.Equal(1, Assert.IsType(shell.GetVariable("value")).Value); + } + finally + { + File.Delete(path); + } + } + + [Fact] + public async Task InvalidExpressionScript_PropagatesError() + { + var shell = ShellInterpreter.CreateInstance(); + var path = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(path, "if true {", TestContext.Current.CancellationToken); + var expression = new CommandExpression(new(TokenType.Identifier, path, 0, path.Length)); + var failure = await Assert.ThrowsAsync(() => expression.EvaluateAsync(shell, new(), CancellationToken.None)); + var state = Assert.IsType(failure.State); + Assert.Equal(path, state.SourceName); + Assert.Equal("if true {", state.SourceText); + Assert.Equal(ShellExitCode.UsageError, ShellExitCode.FromException(failure)); + } + finally + { + File.Delete(path); + } + } + [Fact] public async Task RunScriptAsync_CreatesIsolatedFrame_ForNewVariables() { @@ -80,4 +345,29 @@ public async Task RunScriptAsync_PrintFailure_ReturnsError() Assert.True(state.IsError); } + private sealed class CancelOnConversion(CancellationTokenSource cancellation, bool throwOnConversion = true, bool cancelToken = true) : ShellObject(DataType.Boolean) + { + public bool WasEvaluated { get; private set; } + + public override object? ConvertShellObject(DataType type) + { + if (type != DataType.Boolean) + { + return new ShellBool(true).ConvertShellObject(type); + } + + this.WasEvaluated = true; + if (cancelToken) + { + cancellation.Cancel(); + } + + if (throwOnConversion) + { + throw new OperationCanceledException(cancellation.Token); + } + + return true; + } + } } diff --git a/CosmosDBShell.Tests/Shell/ExecuteCommandExceptionTests.cs b/CosmosDBShell.Tests/Shell/ExecuteCommandExceptionTests.cs index 0827e916..870a61e8 100644 --- a/CosmosDBShell.Tests/Shell/ExecuteCommandExceptionTests.cs +++ b/CosmosDBShell.Tests/Shell/ExecuteCommandExceptionTests.cs @@ -10,6 +10,99 @@ namespace CosmosShell.Tests.Shell; public class ExecuteCommandExceptionTests { + [Theory] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(true, true)] + public async Task FileParserError_ReportsItsOwnSource(bool expression, bool machineMode) + { + using var interpreter = CreateInterpreter(); + var script = Path.GetTempFileName().Replace('\\', '/'); + var output = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(script, "\n}", TestContext.Current.CancellationToken); + interpreter.ErrOutRedirect = output; + if (machineMode) + { + interpreter.Options = new Program.CosmosShellOptions { Output = "json" }; + } + + var command = expression ? $"def invoke {{ exec \"{script}\" }}; $value = (invoke)" : $"exec \"{script}\""; + var state = await interpreter.ExecuteCommandAsync(command, CancellationToken.None); + Assert.Equal(ShellExitCode.UsageError, state.ExitCode); + var content = File.ReadAllText(output); + if (machineMode) + { + using var json = JsonDocument.Parse(content); + content = json.RootElement.GetProperty("error").GetString()!; + Assert.Contains(":2:1:", content); + } + else + { + Assert.Contains(":2:1:", content); + Assert.Contains("}", content); + } + + Assert.Contains(Path.GetFileName(script), content); + } + finally + { + interpreter.ErrOutRedirect = null; + File.Delete(script); + File.Delete(output); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task NestedScriptError_ReportsOriginAndRetainsCaller(bool machineMode) + { + using var interpreter = CreateInterpreter(); + var child = Path.GetTempFileName().Replace('\\', '/'); + var parent = Path.GetTempFileName().Replace('\\', '/'); + var output = Path.GetTempFileName(); + try + { + await File.WriteAllTextAsync(child, "\n$invalid = 1 / 0", TestContext.Current.CancellationToken); + await File.WriteAllTextAsync(parent, $"exec \"{child}\"", TestContext.Current.CancellationToken); + interpreter.ErrOutRedirect = output; + if (machineMode) + { + interpreter.Options = new Program.CosmosShellOptions { Output = "json" }; + } + + var state = Assert.IsType(await interpreter.ExecuteCommandAsync($"exec \"{parent}\"", CancellationToken.None)); + Assert.Equal(ShellExitCode.GeneralFailure, state.ExitCode); + var frames = PositionalException.GetSourceTrace(state.Exception); + Assert.Equal(child, frames[0].FileName); + Assert.Equal(2, frames[0].Line); + Assert.Contains(frames, frame => frame.FileName == parent); + var content = File.ReadAllText(output); + if (machineMode) + { + using var json = JsonDocument.Parse(content); + content = json.RootElement.GetProperty("error").GetString()!; + } + else + { + Assert.Contains($"at {parent}:1:1", content); + } + + Assert.Contains($"{Path.GetFileName(child)}:2:1", content); + Assert.Null(interpreter.CurrentScriptFileName); + } + finally + { + interpreter.ErrOutRedirect = null; + File.Delete(child); + File.Delete(parent); + File.Delete(output); + } + } + private ShellInterpreter CreateInterpreter() { return new ShellInterpreter(); @@ -48,6 +141,36 @@ public void PrintState_StructuredErrorInMachineMode_WritesSingleEnvelopeToStderr } } + [Fact] + public void PrintState_StructuredErrorWithScriptLocationInUserMode_ReportsLocationAndKeepsRenderer() + { + using var interpreter = CreateInterpreter(); + var stderrFile = Path.GetTempFileName(); + interpreter.ErrOutRedirect = stderrFile; + try + { + var rendered = false; + var state = new StructuredErrorCommandState( + new PositionalException("script.csh", new CommandException("batch", "Batch failed."), 4, 7, "batch items.json"), + new ShellJson(JsonSerializer.SerializeToElement(new { success = false }))) + { + RenderUser = () => rendered = true, + }; + + interpreter.PrintState(state); + + Assert.True(rendered); + var content = File.ReadAllText(stderrFile); + Assert.Contains("script.csh:4:7", content); + Assert.Contains("Batch failed.", content); + } + finally + { + interpreter.ErrOutRedirect = null; + File.Delete(stderrFile); + } + } + [Fact] public async Task ExecuteCommandAsync_ShellException_ReturnsErrorState() { diff --git a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs index 406ab92f..531e6836 100644 --- a/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs +++ b/CosmosDBShell.Tests/ToolOperationsCallToolTests.cs @@ -506,6 +506,7 @@ public async Task CallTool_InvalidContinuationType_ReturnsError(string continuat public async Task CallTool_EchoCommand_ReturnsSuccessResult() { var tool = CreateToolOperations(); + var history = ShellInterpreter.Instance.History.ToArray(); using var output = new StringWriter(); var arguments = new Dictionary { @@ -526,6 +527,7 @@ public async Task CallTool_EchoCommand_ReturnsSuccessResult() Assert.Contains("echo", output.ToString(), StringComparison.Ordinal); var recorded = ShellInterpreter.Instance.History.ToArray(); + Assert.Equal(history.Length + 1, recorded.Length); Assert.Equal("echo \"hello\" \"world\"", recorded[^1]); Assert.Single(recorded, entry => entry == recorded[^1]); diff --git a/CosmosDBShell.Tests/UtilTest/OutputFormatTests.cs b/CosmosDBShell.Tests/UtilTest/OutputFormatTests.cs index 2224f048..0da08889 100644 --- a/CosmosDBShell.Tests/UtilTest/OutputFormatTests.cs +++ b/CosmosDBShell.Tests/UtilTest/OutputFormatTests.cs @@ -233,6 +233,28 @@ async Task AssignmentStatement_ClearsRenderTabularFromPriorStatement() Assert.False(state.OutputFormatExplicitlySet); } + [Fact] + async Task ReturnStatement_ClearsRenderersFromPriorStatement() + { + using var shell = ShellInterpreter.CreateInstance(); + var lexer = new Lexer("return 1"); + var parser = new StatementParser(lexer); + var statements = parser.ParseStatements(); + + var state = new CommandState + { + RenderTabular = () => new TabularData("Leaked"), + RenderUser = () => { }, + OutputFormat = OutputFormat.CSV, + }; + + state = await statements[0].RunAsync(shell, state, TestContext.Current.CancellationToken); + + Assert.Null(state.RenderTabular); + Assert.Null(state.RenderUser); + Assert.False(state.OutputFormatExplicitlySet); + } + private string StripWS(string input) { var sb = new StringBuilder(); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ExportCommand.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ExportCommand.cs index 57152c45..1d1707da 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ExportCommand.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Commands/ExportCommand.cs @@ -326,6 +326,18 @@ internal static async Task WriteFileAsync( CancellationToken token) { var destination = Path.GetFullPath(filePath); + + // A directory destination can never be overwritten by File.Move, so reject it before spending RU on the query. + if (Directory.Exists(destination)) + { + throw new IOException(MessageService.GetArgsString("command-export-error-destination_directory", "file", destination)); + } + + if (!overwrite && System.IO.File.Exists(destination)) + { + throw new IOException(MessageService.GetArgsString("command-export-error-file_exists", "file", destination)); + } + var directory = Path.GetDirectoryName(destination)!; Directory.CreateDirectory(directory); var temporary = Path.Join(directory, $".cosmos-export-{Guid.NewGuid():N}.tmp"); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/CommandState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/CommandState.cs index a5ad0fa2..f6bd0f58 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/CommandState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/CommandState.cs @@ -186,4 +186,17 @@ internal string GenerateOutputText() throw new InvalidOperationException("OutputFormat invalid " + this.OutputFormat); } } + + internal sealed class FailureException : Exception + { + public FailureException(CommandState state) + : base( + state is ParserErrorCommandState parserError ? string.Join("; ", parserError.Errors.Select(error => error.Message)) : (state as ErrorCommandState)?.Exception.Message ?? MessageService.GetString("script-error-expression-failed"), + (state as ErrorCommandState)?.Exception) + { + this.State = state; + } + + public CommandState State { get; } + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/DiagnosticLog.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/DiagnosticLog.cs index e94572b6..83fb8751 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/DiagnosticLog.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/DiagnosticLog.cs @@ -129,6 +129,27 @@ public void LogCancelled(double elapsedMilliseconds, string command) public void LogError(string command, Exception exception) { this.WriteLine("ERROR", $"{this.Flatten(command)} -> {exception.GetType().Name}: {this.Flatten(exception.Message)}"); + foreach (var frame in PositionalException.GetSourceTrace(exception)) + { + this.WriteLine("ERROR", this.Flatten($"at {frame.FileName}:{frame.Line}:{frame.Column}")); + } + + for (var inner = exception.InnerException; inner != null; inner = inner.InnerException) + { + if (inner is not PositionalException) + { + this.WriteLine("ERROR", $"{inner.GetType().Name}: {this.Flatten(inner.Message)}"); + } + } + + for (Exception? current = exception; current != null; current = current.InnerException) + { + if (current is CommandState.FailureException { State: ParserErrorCommandState parserError }) + { + this.LogParserErrors(command, parserError.Errors, parserError.SourceName, parserError.SourceText); + break; + } + } } /// @@ -136,17 +157,19 @@ public void LogError(string command, Exception exception) /// /// The command text. /// The parser errors to record. - public void LogParserErrors(string command, IEnumerable errors) + /// The script file containing the errors. + /// The script text used to resolve source positions. + public void LogParserErrors(string command, IEnumerable errors, string? sourceName = null, string? sourceText = null) { if (errors is null) { return; } + var sourceErrors = errors.Where(static error => error is not null).ToArray(); var detail = string.Join( "; ", - errors - .Where(static error => error is not null) + sourceErrors .Select(static error => $"{(error.ErrorLevel == ErrorLevel.Warning ? "warning" : "error")}: {error.Message}")); if (string.IsNullOrEmpty(detail)) @@ -155,6 +178,14 @@ public void LogParserErrors(string command, IEnumerable errors) } this.WriteLine("ERROR", $"{this.Flatten(command)} -> {this.Flatten(detail)}"); + if (sourceName != null && sourceText != null) + { + foreach (var error in sourceErrors) + { + var (line, column, _) = PositionalErrorHelper.GetLineAndColumn(sourceText, error.Start); + this.WriteLine("ERROR", this.Flatten($"{sourceName}:{line}:{column}: {error.Message}")); + } + } } /// diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ErrorCommandState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ErrorCommandState.cs index 364fb569..cd7d8dc6 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ErrorCommandState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ErrorCommandState.cs @@ -8,7 +8,7 @@ namespace Azure.Data.Cosmos.Shell.Core; internal class ErrorCommandState(Exception exception) : CommandState { - public Exception Exception { get; init; } = exception; + public Exception Exception { get; internal set; } = exception; public override bool IsError => true; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ParserErrorCommandState.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ParserErrorCommandState.cs index 3516fcd1..ab8f9726 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ParserErrorCommandState.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ParserErrorCommandState.cs @@ -8,13 +8,19 @@ namespace Azure.Data.Cosmos.Shell.Core; internal class ParserErrorCommandState : CommandState { - public ParserErrorCommandState(ErrorList errors) + public ParserErrorCommandState(ErrorList errors, string? sourceName = null, string? sourceText = null) { this.Errors = errors; + this.SourceName = sourceName; + this.SourceText = sourceText; } public ErrorList Errors { get; init; } + public string? SourceName { get; } + + public string? SourceText { get; } + public override bool IsError => true; public override int ExitCode => ShellExitCode.UsageError; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellExitCode.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellExitCode.cs index e992cfdd..aaf64199 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellExitCode.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellExitCode.cs @@ -69,8 +69,20 @@ public static int FromException(Exception? exception) { for (var ex = exception; ex is not null; ex = ex.InnerException) { + if (ex is CommandState.FailureException failure) + { + // A structured failure can be re-attached to the state it came from; asking that state + // for its ExitCode would recurse forever, so classify the original inner exception instead. + if (IsSelfReferential(failure)) + { + continue; + } + + return failure.State.ExitCode; + } + // Peel our own wrappers so the underlying SDK/identity failure classifies. - if ((ex is CommandException || ex is ShellException) && ex.InnerException is not null) + if ((ex is CommandException || ex is ShellException || ex is PositionalException) && ex.InnerException is not null) { continue; } @@ -104,10 +116,27 @@ public static int FromException(Exception? exception) return GeneralFailure; } + private static bool IsSelfReferential(CommandState.FailureException failure) + { + if (failure.State is not ErrorCommandState errorState) + { + return false; + } + + for (var ex = errorState.Exception; ex is not null; ex = ex.InnerException) + { + if (ReferenceEquals(ex, failure)) + { + return true; + } + } + + return false; + } + private static bool IsUsage(Exception ex) { return ex is CommandNotFoundException - or PositionalException or JsonException or ArgumentException; } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs index f1640927..50e56321 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Core/ShellInterpreter.cs @@ -40,6 +40,8 @@ public partial class ShellInterpreter : IDisposable private const int OptionalArmDiscoveryTimeoutSeconds = 3; + internal const int MaximumCallDepth = 64; + private const string EncodedHistoryLinePrefix = "CosmosDBShellHistoryV1:"; // Sentinel written immediately after the prefix by EncodeHistoryLine so that @@ -63,10 +65,10 @@ public partial class ShellInterpreter : IDisposable private readonly SemaphoreSlim executionGate = new(1, 1); - private readonly AsyncLocal ownsExecutionGate = new(); - private long stateVersion; + private int callDepth; + private TokenCredential? activeCredential; private LineEditor? lineEditor; @@ -299,7 +301,7 @@ internal State State internal PendingBatchState? CurrentBatch { get; set; } - internal Queue VariableContainers { get; } = new(); + internal Stack VariableContainers { get; } = new(); /// /// Gets a value indicating whether the shell is running in machine mode, where @@ -555,22 +557,18 @@ public async Task ExecuteCommandAsync(string command, Cancellation } } + /// + /// Callers must not already hold the gate; nested work runs through the ungated core methods. + /// internal async Task RunSerializedAsync(Func> operation, CancellationToken token) { - if (this.ownsExecutionGate.Value) - { - return await operation(); - } - await this.executionGate.WaitAsync(token); try { - this.ownsExecutionGate.Value = true; return await operation(); } finally { - this.ownsExecutionGate.Value = false; this.executionGate.Release(); } } @@ -632,10 +630,20 @@ private async Task ExecuteCommandCoreAsync(string command, Cancell } catch (Exception e) { + if (FindException(e)?.State is StructuredErrorCommandState structuredError) + { + if (e is PositionalException) + { + structuredError.Exception = e; + } + + result = this.PrintState(structuredError); + return result; + } + this.ReportExecutionError(e, command); this.DisconnectLocalEmulatorAfterConnectivityFailure(e); - var inner = e is PositionalException pe ? (pe.InnerException ?? pe) : e; - result = new ErrorCommandState(inner) + result = new ErrorCommandState(e) { RequestCharge = RequestChargeContext.GetExceptionCharge(e), }; @@ -651,7 +659,7 @@ private async Task ExecuteCommandCoreAsync(string command, Cancell if (state is ParserErrorCommandState parserErrorState) { - this.ReportParserErrors(parserErrorState.Errors, command); + this.ReportParserErrors(parserErrorState.Errors, parserErrorState.SourceText ?? command, parserErrorState.SourceName); result = state; return result; } @@ -684,7 +692,7 @@ private async Task ExecuteCommandCoreAsync(string command, Cancell } else if (result is ParserErrorCommandState parserErrorResult) { - diagnostics.LogParserErrors(command, parserErrorResult.Errors); + diagnostics.LogParserErrors(command, parserErrorResult.Errors, parserErrorResult.SourceName, parserErrorResult.SourceText); } } @@ -981,10 +989,18 @@ internal async Task RunAsync() internal async Task RunCommandAsync(CommandState currentState, string commandText, CancellationToken token) { - var lexer = new Lexer(commandText); - var parser = new StatementParser(lexer); + var parser = StatementParser.ScriptParseResult.Parse(commandText); + if (parser.Errors.HasErrors) + { + if (LooksLikeConnectionStringLine(commandText)) + { + parser.Errors.Add(new ParseError(0, 1, MessageService.GetString("error-command-not-found-connection-string"), ErrorLevel.Warning)); + } - foreach (var statements in parser.ParseStatements()) + return new ParserErrorCommandState(parser.Errors); + } + + foreach (var statements in parser.Statements) { if (token.IsCancellationRequested) { @@ -1061,7 +1077,7 @@ internal Task ExecuteCosmosCommandAsync( CommandState commandState, string commandText, CancellationToken token) - => this.RunSerializedAsync(() => this.ExecuteCosmosCommandCoreAsync(command, commandState, commandText, token), token); + => this.ExecuteCosmosCommandCoreAsync(command, commandState, commandText, token); private async Task ExecuteCosmosCommandCoreAsync( CosmosCommand command, @@ -1884,6 +1900,16 @@ internal CommandState PrintState(CommandState state, bool markAsRendered = false var redirected = !string.IsNullOrEmpty(this.StdOutRedirect); var inMachineMode = this.IsMachineMode; + // Structured failures bypass ReportExecutionError, so the script location the + // statement attached would otherwise be dropped from the human-readable output. + // Emit it here and let the structured result render itself below. + if (!inMachineMode + && state is StructuredErrorCommandState humanStructuredError + && humanStructuredError.Exception is PositionalException positionalError) + { + this.ReportPositionalError(positionalError); + } + // Interactive, user-facing view: when the command supplied a custom renderer and // the effective format is User, let it draw. Redirection, piping, and machine // mode always fall through to the structured (JSON/CSV/Table) path below. @@ -1898,7 +1924,14 @@ internal CommandState PrintState(CommandState state, bool markAsRendered = false if (inMachineMode && state is StructuredErrorCommandState structuredError) { - this.WriteMachineError(structuredError.Exception.Message, structuredError.Result); + // PositionalException.Message carries only the inner message, so prepend the + // location here the same way ReportExecutionError does for ordinary failures. + var errorLocation = PositionalException.GetSourceTrace(structuredError.Exception).FirstOrDefault(); + this.WriteMachineError( + errorLocation == null + ? structuredError.Exception.Message + : $"{errorLocation.FileName}:{errorLocation.Line}:{errorLocation.Column}: {structuredError.Exception.Message}", + structuredError.Result); return state; } @@ -2022,6 +2055,24 @@ internal void DeclareFunction(DefStatement defStatement) this.Functions[defStatement.Name] = defStatement; } + internal void PushCallScope(VariableContainer frame, CancellationToken token) + { + token.ThrowIfCancellationRequested(); + if (this.callDepth >= MaximumCallDepth) + { + throw new ShellException(MessageService.GetArgsString("script-error-call-depth", "limit", MaximumCallDepth)); + } + + this.VariableContainers.Push(frame); + this.callDepth++; + } + + internal void PopCallScope() + { + this.VariableContainers.Pop(); + this.callDepth--; + } + internal void SetVariable(string variableName, ShellObject value) { if (string.Equals(variableName, SessionRequestChargeVariable, StringComparison.OrdinalIgnoreCase) @@ -2039,23 +2090,10 @@ internal void SetVariable(string variableName, ShellObject value) // Ensure we have at least one variable container (global scope) if (this.VariableContainers.Count == 0) { - this.VariableContainers.Enqueue(new VariableContainer()); + this.VariableContainers.Push(new VariableContainer()); } - // When running inside a script, always write to the current (script) frame. - // This ensures script-local assignments don't modify variables in caller scopes. - // Outside of scripts, search for existing variable to maintain back-compat. - VariableContainer currentScope; - if (!string.IsNullOrEmpty(this.CurrentScriptFileName)) - { - // Script execution: always use current frame (script-local by default) - currentScope = this.VariableContainers.Last(); - } - else - { - // Interactive/global: update existing variable if found, else use current frame - currentScope = this.GetScope(variableName) ?? this.VariableContainers.Last(); - } + var currentScope = this.VariableContainers.Peek(); var targetType = value.DataType; @@ -2242,7 +2280,7 @@ private LineEditor CreateLineEditor() private VariableContainer? GetScope(string name) { - foreach (var container in this.VariableContainers.Reverse()) + foreach (var container in this.VariableContainers) { if (container.Variables.ContainsKey(name)) { @@ -2641,9 +2679,17 @@ private void ReportExecutionError(Exception e, string? sourceText = null) return; } + if (FindException(e)?.State is ParserErrorCommandState parserError) + { + this.ReportParserErrors(parserError.Errors, parserError.SourceText ?? sourceText ?? string.Empty, parserError.SourceName); + return; + } + if (this.IsMachineMode) { - this.WriteMachineError(e.Message); + var sourceTrace = PositionalException.GetSourceTrace(e); + var location = sourceTrace.FirstOrDefault(); + this.WriteMachineError(location == null ? e.Message : $"{location.FileName}:{location.Line}:{location.Column}: {e.Message}"); return; } @@ -2796,6 +2842,9 @@ private static bool LooksLikeConnectionStringLine(string lineText) private void ReportPositionalError(PositionalException pe) { + var frames = PositionalException.GetSourceTrace(pe); + pe = frames[0]; + var callTrace = frames.Skip(1).Select(frame => $" at {frame.FileName}:{frame.Line}:{frame.Column}").ToArray(); if (this.ErrOutRedirect != null) { var errorMessage = $"[{Path.GetFileName(pe.FileName)}:{pe.Line}:{pe.Column}]: {MessageService.GetString("runtime-error-prefix")}: {pe.Message}"; @@ -2805,6 +2854,11 @@ private void ReportPositionalError(PositionalException pe) errorMessage += Environment.NewLine + new string(' ', Math.Max(0, pe.Column - 1)) + "^"; } + if (callTrace.Length > 0) + { + errorMessage += Environment.NewLine + string.Join(Environment.NewLine, callTrace); + } + if (this.AppendErrRedirection) { File.AppendAllText(this.ErrOutRedirect, errorMessage); @@ -2822,6 +2876,11 @@ private void ReportPositionalError(PositionalException pe) AnsiConsole.MarkupLine(" " + Theme.FormatMuted(pe.LineText)); AnsiConsole.MarkupLine(" " + Theme.FormatError(new string(' ', Math.Max(0, pe.Column - 1)) + "^")); } + + foreach (var frame in callTrace) + { + AnsiConsole.MarkupLine(Theme.FormatMuted(frame)); + } } } @@ -2869,7 +2928,7 @@ private string[] SplitIntoLines(string text) return (line, column); } - private void ReportParserErrors(ErrorList errors, string commandText) + private void ReportParserErrors(ErrorList errors, string commandText, string? sourceName = null) { if (this.IsMachineMode && errors != null && errors.Count > 0) @@ -2879,7 +2938,8 @@ private void ReportParserErrors(ErrorList errors, string commandText) { if (err != null && err.ErrorLevel == ErrorLevel.Error) { - errorStrings.Add(err.Message ?? "Parser error"); + var (line, column) = this.OffsetToLineColumn(commandText, err.Start); + errorStrings.Add(sourceName == null ? err.Message : $"{sourceName}:{line + 1}:{column + 1}: {err.Message}"); } } @@ -2935,7 +2995,7 @@ private void ReportParserErrors(ErrorList errors, string commandText) error.Message, lineNumber, rendered, - origin: this.GetDiagnosticOrigin(this.CurrentScriptFileName)); + origin: this.GetDiagnosticOrigin(sourceName ?? this.CurrentScriptFileName)); } if (redirected && fileBuffer != null) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/FunctionSymbol.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/FunctionSymbol.cs index 48a482e1..a78fb396 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/FunctionSymbol.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/FunctionSymbol.cs @@ -4,6 +4,6 @@ namespace Azure.Data.Cosmos.Shell.Lsp.Semantics; /// -/// Symbol representing a user-defined function (reserved for future expansion of the language). +/// Symbol representing a user-defined function in the analyzed document. /// public sealed class FunctionSymbol(string name, int start, int length) : Symbol(name, SymbolKind.Function, start, length); \ No newline at end of file diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticAnalyzer.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticAnalyzer.cs index 3fec6330..505097e7 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticAnalyzer.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticAnalyzer.cs @@ -28,7 +28,9 @@ public sealed class SemanticAnalyzer private readonly List symbols = new(); private readonly List references = new(); private readonly List diagnostics = new(); - private readonly HashSet declaredVariables = new(StringComparer.OrdinalIgnoreCase); + private readonly HashSet declaredVariables = new(StringComparer.Ordinal); + private readonly Dictionary functions = new(StringComparer.Ordinal); + private readonly Dictionary functionDefinitions = new(); /// /// Analyzes a collection of parsed statements to produce a semantic model. @@ -38,43 +40,143 @@ public sealed class SemanticAnalyzer /// A containing symbols, references, and diagnostics. public SemanticModel Analyze(IEnumerable statements, string source) { - foreach (var st in statements) + this.symbols.Clear(); + this.references.Clear(); + this.diagnostics.Clear(); + this.declaredVariables.Clear(); + this.functions.Clear(); + this.functionDefinitions.Clear(); + var statementList = statements.ToList(); + foreach (var statement in statementList) + { + this.CollectFunctions(statement); + } + + foreach (var st in statementList) { this.VisitStatement(st); } return new SemanticModel { - Symbols = this.symbols, - References = this.references, - Diagnostics = this.diagnostics, + Symbols = this.symbols.ToArray(), + References = this.references.ToArray(), + Diagnostics = this.diagnostics.ToArray(), }; } - /// - /// Helper method to extract expression properties via reflection. - /// - /// The object containing the property. - /// The property name to extract. - /// The extracted expression. - private static Expression GetExpression(object owner, string prop) - => (Expression)(owner.GetType().GetProperty(prop)!.GetValue(owner)!); + private static IEnumerable GetChildStatements(Statement statement) + { + return statement switch + { + BlockStatement block => block.Statements, + PipeStatement pipe => pipe.Statements, + DefStatement function => [function.Statement], + IfStatement conditional when conditional.ElseStatement != null => [conditional.Statement, conditional.ElseStatement], + IfStatement conditional => [conditional.Statement], + ForStatement forLoop => [forLoop.Statement], + WhileStatement whileLoop => [whileLoop.Statement], + DoWhileStatement doLoop => [doLoop.Statement], + LoopStatement loop => [loop.Statement], + _ => [], + }; + } + + private void CollectFunctions(Statement statement) + { + if (statement is DefStatement definition) + { + var symbol = new FunctionSymbol(definition.Name, definition.NameToken.Start, definition.NameToken.Length); + this.functionDefinitions[definition] = symbol; + this.functions.TryAdd(definition.Name, symbol); + this.symbols.Add(symbol); + } + + foreach (var child in GetChildStatements(statement)) + { + this.CollectFunctions(child); + } + } /// /// Visits a statement node in the AST to extract semantic information. /// /// The statement to analyze. /// - /// Currently handles: - /// - CommandStatement: Validates command existence and options - /// - Other statements: Extracts expressions via reflection for variable analysis. + /// Visits expressions and child statements without executing declarations or commands. /// private void VisitStatement(Statement st) { - if (st is CommandStatement cmd) + switch (st) { - var name = cmd.Name ?? string.Empty; - if (name.Length > 0) + case CommandStatement command: + this.VisitCommand(command); + break; + case DefStatement definition: + this.functions[definition.Name] = this.functionDefinitions[definition]; + this.references.Add(new ReferenceInfo + { + Symbol = this.functionDefinitions[definition], + Start = definition.NameToken.Start, + Length = definition.NameToken.Length, + IsDefinition = true, + }); + break; + case AssignmentStatement assignment: + this.RecordVariableReference(assignment.Variable); + this.VisitExpression(assignment.Value); + break; + case ReturnStatement returned when returned.Value != null: + this.VisitExpression(returned.Value); + break; + case IfStatement conditional: + this.VisitExpression(conditional.Condition); + break; + case WhileStatement whileLoop: + this.VisitExpression(whileLoop.Condition); + break; + case DoWhileStatement doLoop: + // The body runs before the condition, so visit it first to keep definitions in execution order. + this.VisitStatement(doLoop.Statement); + this.VisitExpression(doLoop.Condition); + return; + case ForStatement forLoop: + this.RecordVariableReference(new VariableExpression(forLoop.VariableToken, forLoop.VariableName)); + this.VisitExpression(forLoop.Collection); + break; + case ExecStatement executed: + this.VisitExpression(executed.CommandExpression); + foreach (var argument in executed.Arguments) + { + this.VisitExpression(argument); + } + + break; + } + + foreach (var child in GetChildStatements(st)) + { + this.VisitStatement(child); + } + } + + private void VisitCommand(CommandStatement cmd) + { + var name = cmd.Name ?? string.Empty; + if (name.Length > 0) + { + if (this.functions.TryGetValue(name, out var function) + && (function.Start <= cmd.Start || !ShellInterpreter.Instance.App.Commands.ContainsKey(name))) + { + this.references.Add(new ReferenceInfo + { + Symbol = function, + Start = cmd.Start, + Length = cmd.CommandToken.Length, + IsDefinition = false, + }); + } + else { var sym = new CommandSymbol(name, cmd.Start, Math.Max(1, name.Length)); this.symbols.Add(sym); @@ -102,19 +204,11 @@ private void VisitStatement(Statement st) this.ValidateCommandOptions(cmd, factory); } } - - foreach (var arg in cmd.Arguments) - { - this.VisitExpression(arg); - } - - return; } - var expProp = st.GetType().GetProperty("Expression"); - if (expProp?.GetValue(st) is Expression expr) + foreach (var arg in cmd.Arguments) { - this.VisitExpression(expr); + this.VisitExpression(arg); } } @@ -229,11 +323,30 @@ private void VisitExpression(Expression expr) this.RecordVariableReference(ve); break; case BinaryOperatorExpression be: - this.VisitExpression(GetExpression(be, "Left")); - this.VisitExpression(GetExpression(be, "Right")); + this.VisitExpression(be.Left); + this.VisitExpression(be.Right); break; case UnaryOperatorExpression ue: - this.VisitExpression(GetExpression(ue, "Expression")); + this.VisitExpression(ue.Expression); + break; + case CommandExpression command: + var statement = new CommandStatement(command.CommandToken); + statement.Arguments.AddRange(command.Arguments); + this.VisitCommand(statement); + break; + case CommandOption option when option.Value != null: + this.VisitExpression(option.Value); + break; + case FilterPipeExpression pipe: + this.VisitExpression(pipe.Left); + this.VisitExpression(pipe.Right); + break; + case FilterCallExpression call: + foreach (var argument in call.Arguments) + { + this.VisitExpression(argument); + } + break; case ParensExpression pe: this.VisitExpression(pe.InnerExpression); @@ -296,7 +409,7 @@ private void RecordVariableReference(VariableExpression ve) else { // Subsequent occurrence - reference to existing symbol - var sym = this.symbols.OfType().First(v => v.Name.Equals(name, StringComparison.OrdinalIgnoreCase)); + var sym = this.symbols.OfType().First(v => v.Name.Equals(name, StringComparison.Ordinal)); this.references.Add(new ReferenceInfo { Symbol = sym, diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticModel.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticModel.cs index 51012fad..a18120c6 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticModel.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/SemanticModel.cs @@ -29,12 +29,13 @@ public sealed class SemanticModel public IReadOnlyList Diagnostics { get; init; } = Array.Empty(); /// - /// Returns the symbol whose defining span contains the specified absolute position, - /// or null if no symbol definition covers that point. + /// Returns the symbol whose definition or reference contains the specified absolute position, + /// or null if no symbol occurrence covers that point. /// /// Zero-based absolute character offset in the source text. public Symbol? GetSymbolAt(int position) - => this.Symbols.FirstOrDefault(s => position >= s.Start && position < s.Start + s.Length); + => this.References.FirstOrDefault(reference => position >= reference.Start && position < reference.Start + reference.Length)?.Symbol + ?? this.Symbols.FirstOrDefault(s => position >= s.Start && position < s.Start + s.Length); /// /// Finds all recorded references associated with the given symbol instance. diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/Symbol.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/Symbol.cs index 92fe02d0..072ffad4 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/Symbol.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp.Semantics/Symbol.cs @@ -19,7 +19,7 @@ public enum SymbolKind Command, /// - /// A user-defined function (future extension / placeholder). + /// A user-defined function. /// Function, diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellCompletionHandler.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellCompletionHandler.cs index 379d6442..c5907d92 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellCompletionHandler.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellCompletionHandler.cs @@ -141,7 +141,7 @@ private static void AddVariableCompletions(List items, string pa return; } - var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + var seen = new HashSet(StringComparer.Ordinal); foreach (var name in ShellInterpreter.SessionVariableNames) { string variableName = "$" + name; @@ -157,7 +157,7 @@ private static void AddVariableCompletions(List items, string pa } } - foreach (var container in ShellInterpreter.Instance.VariableContainers.Reverse()) + foreach (var container in ShellInterpreter.Instance.VariableContainers) { foreach (var name in container.Variables.Keys) { diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellHoverHandler.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellHoverHandler.cs index 63d947e9..30666ac9 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellHoverHandler.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/CosmosShellHoverHandler.cs @@ -90,6 +90,16 @@ public HoverRegistrationOptions GetRegistrationOptions( var hover = CreateSymbolHover(symbol, doc); if (hover != null) { + var occurrence = sem.FindReferences(symbol).FirstOrDefault(reference => offset >= reference.Start && offset < reference.Start + reference.Length); + if (occurrence != null) + { + hover = new Hover + { + Contents = hover.Contents, + Range = ToRange(doc.Content, occurrence.Start, occurrence.Start + occurrence.Length), + }; + } + return Task.FromResult(hover); } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/WorkspaceDocument.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/WorkspaceDocument.cs index 6a01b842..d5613647 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/WorkspaceDocument.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Lsp/WorkspaceDocument.cs @@ -69,6 +69,7 @@ public void Parse() lock (this.parseLock) { this.Diagnostics.Clear(); + this.SemanticModel = null; try { @@ -76,11 +77,16 @@ public void Parse() var parser = new StatementParser(lexer); var statements = parser.ParseStatements(); + if (!parser.Errors.HasErrors) + { + StatementParser.ScriptParseResult.ValidateStatements(statements, parser.Errors, allowReturn: true); + } + this.LastParseResult = new ParseResult { Statements = statements, Comments = lexer.Comments, - Success = true, + Success = !parser.Errors.HasErrors, Errors = lexer.Errors, }; @@ -95,7 +101,7 @@ public void Parse() foreach (var sdiag in this.SemanticModel.Diagnostics) { var (sl, sc) = ToLineColumn(this.Content, sdiag.Start); - var (el, ec) = ToLineColumn(this.Content, sdiag.Start + Math.Max(0, sdiag.Length - 1)); + var (el, ec) = ToLineColumn(this.Content, sdiag.Start + Math.Max(0, sdiag.Length)); if (sdiag.Length == 0) { el = sl; @@ -142,7 +148,7 @@ public void Parse() private static Diagnostic ToDiagnostic(ParseError error, string content) { var (startLine, startCol) = ToLineColumn(content, error.Start); - var (endLine, endCol) = ToLineColumn(content, error.Start + Math.Max(0, error.Length - 1)); + var (endLine, endCol) = ToLineColumn(content, error.Start + Math.Max(0, error.Length)); if (error.Length == 0) { endLine = startLine; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs index 3336f747..8f75c5ae 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Mcp/ToolOperations.cs @@ -625,7 +625,6 @@ private async ValueTask OnCallToolsAsync( // MCP argument order is not semantic, so render positionals in the order the shell binds them. sb.Append(FormatPositionalsForHistory(command.Parameters, positionalValues)); sb.Append(optionText); - var server = parameters.Server; Func>? elicit = server?.ClientCapabilities?.Elicitation != null ? server.ElicitAsync : null; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/BinaryOperatorExpression.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/BinaryOperatorExpression.cs index d642098b..b3415f2c 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/BinaryOperatorExpression.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/BinaryOperatorExpression.cs @@ -72,14 +72,8 @@ public override async Task EvaluateAsync(ShellInterpreter interpret var leftResult = await this.Left.EvaluateAsync(interpreter, currentState, cancellationToken); var rightResult = await this.Right.EvaluateAsync(interpreter, currentState, cancellationToken); - // JSON numbers carry DataType.Json, so without normalization the numeric - // operator paths below fall through to the Int32 branch and throw or - // truncate for decimals and values outside the Int32 range. Promote JSON - // number operands to double-backed decimals (via GetDouble so large - // magnitudes such as 1e308 do not overflow GetDecimal) so comparisons and - // arithmetic operate on the JSON number model. - leftResult = NormalizeJsonNumber(leftResult); - rightResult = NormalizeJsonNumber(rightResult); + leftResult = ShellNumber.Normalize(leftResult); + rightResult = ShellNumber.Normalize(rightResult); // Handle arithmetic operators // Handle arithmetic operators @@ -127,7 +121,9 @@ public override async Task EvaluateAsync(ShellInterpreter interpret } // For strings, concatenate - if (leftResult.DataType == DataType.Text || rightResult.DataType == DataType.Text) + if (leftResult.DataType == DataType.Text || rightResult.DataType == DataType.Text || + leftResult is ShellJson { Value.ValueKind: JsonValueKind.String } || + rightResult is ShellJson { Value.ValueKind: JsonValueKind.String }) { var leftStrObj = leftResult.ConvertShellObject(DataType.Text); var rightStrObj = rightResult.ConvertShellObject(DataType.Text); @@ -157,7 +153,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret throw new InvalidOperationException(MessageService.GetString("expression_error_null_numeric_add")); } - return new ShellNumber((int)leftNumObj1 + (int)rightNumObj1); + return new ShellNumber(checked((int)leftNumObj1 + (int)rightNumObj1)); case TokenType.Minus: // Check if either operand is decimal @@ -180,7 +176,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret throw new InvalidOperationException(MessageService.GetString("expression_error_null_numeric_subtract")); } - return new ShellNumber((int)leftNumObj2 - (int)rightNumObj2); + return new ShellNumber(checked((int)leftNumObj2 - (int)rightNumObj2)); case TokenType.Multiply: // Check if either operand is decimal @@ -203,7 +199,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret throw new InvalidOperationException(MessageService.GetString("expression_error_null_numeric_multiply")); } - return new ShellNumber((int)leftNumObj3 * (int)rightNumObj3); + return new ShellNumber(checked((int)leftNumObj3 * (int)rightNumObj3)); case TokenType.Divide: // Check if either operand is decimal @@ -302,7 +298,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret throw new NotSupportedException(MessageService.GetString("expression_error_negative_exponent_integer")); } - return new ShellNumber((int)Math.Pow((int)leftNumObj6, rightNum6)); + return new ShellNumber(checked((int)Math.Pow((int)leftNumObj6, rightNum6))); // Comparison operators case TokenType.Equal: @@ -359,7 +355,10 @@ public override async Task EvaluateAsync(ShellInterpreter interpret case TokenType.NotEqual: // Evaluate equality and negate var equalToken = new Token(TokenType.Equal, "==", this.OperatorToken.Start, this.OperatorToken.Length); - var equalResult = await new BinaryOperatorExpression(this.Left, equalToken, this.Right) + var equalResult = await new BinaryOperatorExpression( + new ConstantExpression(this.OperatorToken, leftResult), + equalToken, + new ConstantExpression(this.OperatorToken, rightResult)) .EvaluateAsync(interpreter, currentState, cancellationToken); var isEqualObj = equalResult.ConvertShellObject(DataType.Boolean); if (isEqualObj == null) @@ -486,14 +485,4 @@ public override string ToString() { return $"({this.Left} {this.OperatorToken.Value} {this.Right})"; } - - private static ShellObject NormalizeJsonNumber(ShellObject value) - { - if (value is ShellJson json && json.Value.ValueKind == JsonValueKind.Number) - { - return new ShellDecimal(json.Value.GetDouble()); - } - - return value; - } } \ No newline at end of file diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/CommandExpression.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/CommandExpression.cs index 1d9c3e73..b1632883 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/CommandExpression.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/CommandExpression.cs @@ -82,7 +82,38 @@ public override int Length public override async Task EvaluateAsync(ShellInterpreter interpreter, CommandState currentState, CancellationToken cancellationToken) { // Execute the command asynchronously and return the result - var resultState = await this.ExecuteCommandAsync(interpreter, currentState, cancellationToken); + CommandState resultState; + try + { + resultState = await this.ExecuteCommandAsync(interpreter, currentState, cancellationToken); + } + catch (Exception exception) when (exception is not PositionalException + && (exception is not OperationCanceledException || !cancellationToken.IsCancellationRequested) + && interpreter.CurrentScriptFileName != null && interpreter.CurrentScriptContent != null) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(interpreter.CurrentScriptContent, this.Start); + throw new PositionalException(interpreter.CurrentScriptFileName, exception, line, column, lineText); + } + + if (resultState.IsError) + { + var failure = new CommandState.FailureException(resultState); + if (resultState is ErrorCommandState error && interpreter.CurrentScriptFileName is { } sourceName && interpreter.CurrentScriptContent is { } sourceText) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(sourceText, this.Start); + if (error.Exception is PositionalException positional + && positional.FileName == sourceName + && positional.Line == line + && positional.Column == column) + { + throw failure; + } + + throw new PositionalException(sourceName, failure, line, column, lineText); + } + + throw failure; + } // Return the result from the command state if (resultState.Result != null) @@ -123,14 +154,14 @@ internal async Task ExecuteCommandAsync(ShellInterpreter shell, Co // Check for user-defined functions first if (shell.Functions.TryGetValue(this.Name, out var function)) { - var args = new List(); + var args = new List(); foreach (var a in this.Arguments) { var evaluated = await a.EvaluateAsync(shell, commandState, token); - args.Add(evaluated?.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty); + args.Add(evaluated); } - return await function.ExecuteFunctionAsync(shell, commandState, token, args.ToArray()); + return await function.ExecuteCallAsync(shell, commandState, token, this.Start, args.ToArray()); } // Check for built-in commands @@ -163,215 +194,20 @@ internal async Task ExecuteCommandAsync(ShellInterpreter shell, Co /// /// Creates a command instance with bound parameters and options. /// - internal async Task CreateCommandAsync(CommandFactory factory, ShellInterpreter shell, CommandState commandState, CancellationToken token) + internal Task CreateCommandAsync(CommandFactory factory, ShellInterpreter shell, CommandState commandState, CancellationToken token) { - var cmd = factory.CreateCommand(); - - // Build a map of option properties on the command type for quick lookup. - var optionProperties = cmd.GetType().GetProperties() - .Select(p => new { Prop = p, Attr = p.GetCustomAttribute() }) - .Where(x => x.Attr != null) - .ToList(); - - bool IsBoolean(System.Reflection.PropertyInfo pi) - => (Nullable.GetUnderlyingType(pi.PropertyType) ?? pi.PropertyType) == typeof(bool); - - // First pass: bind option values (including space-separated values) and record which argument indices are consumed. - var consumedArgumentIndices = new HashSet(); - - for (int i = 0; i < this.Arguments.Count; i++) - { - if (this.Arguments[i] is not CommandOption opt) - { - continue; - } - - var rawName = opt.Name.TrimStart('-'); - var matchingProperty = optionProperties - .FirstOrDefault(x => x.Attr!.Names.Contains(rawName, StringComparer.OrdinalIgnoreCase)); - - if (matchingProperty == null) - { - var knownNames = optionProperties - .Where(x => x.Attr != null) - .SelectMany(x => x.Attr!.Names) - .Where(n => !string.IsNullOrEmpty(n)); - - // The parser stores only the first '-' in MinusToken; a second - // '-' for '--option' is consumed but discarded. Reconstruct the - // exact dash prefix from the gap between the two tokens so the - // suggestion echoes back what the user typed. - var dashCount = Math.Max(1, opt.NameToken.Start - opt.MinusToken.Start); - var typedPrefix = new string('-', dashCount); - var (msg, hint) = Azure.Data.Cosmos.Shell.Util.UnknownOptionMessage.Build(typedPrefix, rawName, knownNames); - throw new UnknownOptionException(this.Name, msg, hint); - } - - var pi = matchingProperty.Prop; - var attr = matchingProperty.Attr; - - // For boolean options, the presence of the option is enough. Do not consume the next - // positional argument as a value (e.g. `dir "*.csh" -l` should not treat -l as taking - // the filter as its value). - if (IsBoolean(pi)) - { - if (opt.Value == null) - { - opt.Value = new ConstantExpression(new Token(TokenType.Identifier, "true", 0, 0), new ShellText("true")); - } - } - else if (opt.Value == null) - { - int nextIndex = i + 1; - if (nextIndex < this.Arguments.Count && - this.Arguments[nextIndex] is not CommandOption && - !consumedArgumentIndices.Contains(nextIndex)) - { - opt.Value = this.Arguments[nextIndex]; - consumedArgumentIndices.Add(nextIndex); - } - } - - if (opt.Value != null) - { - var evaluatedValue = await opt.Value.EvaluateAsync(shell, commandState, token); - var stringValue = evaluatedValue.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty; - - var targetType = Nullable.GetUnderlyingType(pi.PropertyType) ?? pi.PropertyType; - pi.SetValue(cmd, CommandOptionBinder.ConvertOptionValue(this.Name, rawName, stringValue, targetType)); - } - else - { - if (attr?.DefaultValue != null) - { - pi.SetValue(cmd, attr.DefaultValue); - } - else if (IsBoolean(pi)) - { - pi.SetValue(cmd, true); - } - else - { - throw new CommandException(this.Name, $"Option '{rawName}' requires a value."); - } - } - } - - // Collect parameter properties - var parameters = cmd.GetType().GetProperties() - .Select(p => new { Prop = p, Attr = p.GetCustomAttribute() }) - .Where(x => x.Attr != null) - .ToList(); - - // Remaining positional arguments - var positional = new List(); - for (int i = 0; i < this.Arguments.Count; i++) - { - if (this.Arguments[i] is CommandOption) - { - continue; - } - - if (consumedArgumentIndices.Contains(i)) - { - continue; - } - - positional.Add(this.Arguments[i]); - } - - // Bind positional parameters - int argIndex = 0; - foreach (var param in parameters) - { - var prop = param.Prop; - var attr = param.Attr!; - if (argIndex >= positional.Count) - { - if (attr.IsRequired) - { - var message = !string.IsNullOrEmpty(attr.RequiredErrorKey) - ? MessageService.GetString(attr.RequiredErrorKey) - : $"Missing required parameter: {prop.Name}"; - throw new CommandException(this.Name, message); - } - - break; - } - - if (prop.PropertyType.IsArray) - { - var arr = new List(); - while (argIndex < positional.Count) - { - var evaluatedArg = await positional[argIndex].EvaluateAsync(shell, commandState, token); - var stringValue = evaluatedArg.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty; - arr.Add(stringValue); - argIndex++; - } - - prop.SetValue(cmd, arr.ToArray()); - } - else - { - var evaluatedArg = await positional[argIndex].EvaluateAsync(shell, commandState, token); - var stringValue = evaluatedArg.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty; - Parameter.SetValue(cmd, prop, stringValue); - argIndex++; - } - } - - if (argIndex < positional.Count && parameters.All(p => !p.Prop.PropertyType.IsArray)) - { - throw new CommandException(this.Name, $"Too many arguments. Expected {parameters.Count}, got {positional.Count}"); - } - - return cmd; + var statement = new CommandStatement(this.CommandToken); + statement.Arguments.AddRange(this.Arguments); + return statement.CreateCommandAsync(factory, shell, commandState, token); } /// /// Runs a script file. /// - internal async Task RunScriptAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) + internal Task RunScriptAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) { - var fileName = this.Name; - - var arguments = new VariableContainer(); - arguments.Set("0", new ShellText(fileName)); - - for (int i = 0; i < this.Arguments.Count; i++) - { - var evaluated = await this.Arguments[i].EvaluateAsync(shell, commandState, token); - arguments.Set((i + 1).ToString(), new ShellText(evaluated.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty)); - } - - shell.VariableContainers.Enqueue(arguments); - var currentState = commandState; - - try - { - var scriptContent = File.ReadAllText(fileName); - var lexer = new Lexer(scriptContent); - var parser = new StatementParser(lexer); - foreach (var statement in parser.ParseStatements()) - { - if (token.IsCancellationRequested) - { - break; - } - - currentState = await statement.RunAsync(shell, currentState, token); - if (currentState.IsError) - { - break; - } - } - } - finally - { - shell.VariableContainers.Dequeue(); - } - - return currentState; + var statement = new CommandStatement(this.CommandToken); + statement.Arguments.AddRange(this.Arguments); + return statement.RunScriptAsync(shell, commandState, token, renderOutput: false); } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/FilterExpressionUtilities.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/FilterExpressionUtilities.cs index 8690bf71..ed0d8961 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/FilterExpressionUtilities.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/FilterExpressionUtilities.cs @@ -43,7 +43,7 @@ public static JsonElement ToJsonElement(ShellObject shellObject) case ShellNumber shellNumber: return JsonSerializer.SerializeToElement(shellNumber.Value); case ShellDecimal shellDecimal: - return JsonSerializer.SerializeToElement(shellDecimal.Value); + return JsonSerializer.SerializeToElement(shellDecimal.Value, ShellDecimal.JsonSerializationOptions); case ShellBool shellBool: return JsonSerializer.SerializeToElement(shellBool.Value); case ShellSequence shellSequence: diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JSonArrayExpression.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JSonArrayExpression.cs index 5e0f33cf..3374bacc 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JSonArrayExpression.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JSonArrayExpression.cs @@ -145,7 +145,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret } } - var element = JsonSerializer.SerializeToElement(items); + var element = JsonSerializer.SerializeToElement(items, ShellDecimal.JsonSerializationOptions); return new ShellJson(element); } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JsonExpression.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JsonExpression.cs index a865c8c0..57ca0fc9 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JsonExpression.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/JsonExpression.cs @@ -161,7 +161,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret } // Serialize the dictionary to a JsonElement - var element = JsonSerializer.SerializeToElement(dict); + var element = JsonSerializer.SerializeToElement(dict, ShellDecimal.JsonSerializationOptions); return new ShellJson(element); } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/UnaryOperatorExpression.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/UnaryOperatorExpression.cs index a9c58250..c130f2d6 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/UnaryOperatorExpression.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Expression/UnaryOperatorExpression.cs @@ -29,6 +29,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret { // Evaluate the operand expression var operandResult = await this.Expression.EvaluateAsync(interpreter, currentState, cancellationToken); + operandResult = ShellNumber.Normalize(operandResult); switch (this.Operator) { @@ -68,7 +69,7 @@ public override async Task EvaluateAsync(ShellInterpreter interpret } var operandNum = (int)operandNumObj; - return new ShellNumber(-operandNum); + return new ShellNumber(checked(-operandNum)); } case TokenType.Plus: diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ExpressionParser.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ExpressionParser.cs index aa078768..04e85bbf 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ExpressionParser.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ExpressionParser.cs @@ -26,6 +26,12 @@ public Token? Current { get { + if (this.lexer.Budget.Exceeded) + { + this.aborted = true; + return null; + } + this.Initialize(); if (this.currentToken == null) { @@ -37,7 +43,7 @@ public Token? Current } } - public bool IsAtEnd => this.currentToken == null || this.aborted; + public bool IsAtEnd => this.currentToken == null || this.aborted || this.lexer.Budget.Exceeded; /// /// Returns the token immediately following without consuming it. @@ -173,7 +179,7 @@ public Expression ParseExpression() return this.CreateAbortExpression(); } - return this.ParseOr(); + return this.ValidateDepth(this.ParseOr()); } /// @@ -194,7 +200,7 @@ public Expression ParseFilterExpression() this.inFilterMode = true; try { - return this.ParsePipeExpression(); + return this.ValidateDepth(this.ParsePipeExpression()); } finally { @@ -210,7 +216,43 @@ public Expression ParsePrimaryExpression() return this.CreateAbortExpression(); } - return this.ParsePrimary(); + return this.ValidateDepth(this.ParsePrimary()); + } + + private Expression ValidateDepth(Expression expression) + { + var pending = new Stack<(Expression Expression, int Depth)>(); + pending.Push((expression, 1)); + while (pending.TryPop(out var current)) + { + if (current.Depth > Lexer.ParsingBudget.MaximumExpressionDepth) + { + this.lexer.Budget.RejectExpressionDepth(this.lexer.Errors, this.currentToken ?? this.lastNonNullToken); + this.aborted = true; + return this.CreateAbortExpression(); + } + + IEnumerable children = current.Expression switch + { + BinaryOperatorExpression binary => [binary.Left, binary.Right], + FilterPipeExpression pipe => [pipe.Left, pipe.Right], + UnaryOperatorExpression unary => [unary.Expression], + ParensExpression parens => [parens.InnerExpression], + JsonExpression json => json.Properties.Values, + JsonArrayExpression array => array.Expressions, + InterpolatedStringExpression interpolated => interpolated.Expressions, + FilterCallExpression call => call.Arguments, + CommandExpression command => command.Arguments, + CommandOption option when option.Value != null => [option.Value], + _ => [], + }; + foreach (var child in children) + { + pending.Push((child, current.Depth + 1)); + } + } + + return expression; } private Expression ParsePipeExpression() @@ -454,6 +496,11 @@ private Expression ParseMultiplication() // Power operator (right associative) private Expression ParsePower() + { + return this.ParseNested(this.ParsePowerCore); + } + + private Expression ParsePowerCore() { if (this.aborted) { @@ -483,6 +530,29 @@ private Expression ParsePower() // Unary operators (!, -, +) private Expression ParseUnary() + { + return this.ParseNested(this.ParseUnaryCore); + } + + private Expression ParseNested(Func parse) + { + if (!this.lexer.Budget.TryEnter(this.lexer.Errors, this.currentToken)) + { + this.aborted = true; + return this.CreateAbortExpression(); + } + + try + { + return this.ValidateDepth(parse()); + } + finally + { + this.lexer.Budget.Exit(); + } + } + + private Expression ParseUnaryCore() { if (this.aborted) { @@ -510,6 +580,11 @@ private Expression ParseUnary() // Primary expressions (literals, variables, parentheses) private Expression ParsePrimary() + { + return this.ParseNested(this.ParsePrimaryCore); + } + + private Expression ParsePrimaryCore() { if (this.aborted) { @@ -894,7 +969,7 @@ Token MakeOuterToken(string value, int contentStart, int contentEnd) if (rawStart >= 0 && rawEnd >= rawStart && rawEnd <= this.lexer.RawInput.Length) { var rawSlice = this.lexer.RawInput.Substring(rawStart, rawEnd - rawStart); - var rawLexer = new Lexer(rawSlice, rawStartOuter); + var rawLexer = new Lexer(rawSlice, rawStartOuter, this.lexer.Budget); var rawParser = new ExpressionParser(rawLexer); var expr = this.ValidateInterpolatedExpression(rawParser, rawParser.ParseExpression()); @@ -913,7 +988,7 @@ Token MakeOuterToken(string value, int contentStart, int contentEnd) // Token positions may drift through escape sequences but stay correct // for escape-free interpolations, which covers the common case. var innerOffset = OuterPos(startExprPos); - var exprLexer = new Lexer(exprContent, innerOffset); + var exprLexer = new Lexer(exprContent, innerOffset, this.lexer.Budget); var exprParser = new ExpressionParser(exprLexer); var expr2 = this.ValidateInterpolatedExpression(exprParser, exprParser.ParseExpression()); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Lexer.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Lexer.cs index 4a06957d..a55f3f50 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Lexer.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Lexer.cs @@ -246,15 +246,18 @@ public Lexer(string input) /// a $(...) interpolation inside an interpolated string) so that the produced /// tokens carry positions relative to the outer buffer. /// - public Lexer(string input, int positionOffset) + public Lexer(string input, int positionOffset, ParsingBudget? budget = null) { this.input = input ?? string.Empty; this.position = 0; this.positionOffset = positionOffset; this.putBackTokens = new Stack(); this.lastToken = null; + this.Budget = budget ?? new ParsingBudget(); } + internal ParsingBudget Budget { get; } + public List Comments { get; } = new(); public ErrorList Errors { get; } = new ErrorList(); @@ -299,6 +302,11 @@ public IEnumerable Tokenize() public Token? NextToken() { + if (this.Budget.Exceeded) + { + return null; + } + // If we have tokens that were put back, return them first if (this.putBackTokens.Count > 0) { @@ -978,4 +986,46 @@ private Token ReadInterpolatedString(int startPosition) return token; } + + internal sealed class ParsingBudget + { + internal const int MaximumDepth = 128; + internal const int MaximumExpressionDepth = 128; + + private int depth; + + public bool Exceeded { get; private set; } + + public bool TryEnter(ErrorList errors, Token? token) + { + if (this.Exceeded) + { + return false; + } + + if (this.depth >= MaximumDepth) + { + this.Exceeded = true; + errors.Add(new ParseError(token?.Start ?? 0, token?.Length ?? 1, MessageService.GetArgsString("script-error-parse-depth", "limit", MaximumDepth))); + return false; + } + + this.depth++; + return true; + } + + public void Exit() + { + this.depth--; + } + + public void RejectExpressionDepth(ErrorList errors, Token? token) + { + if (!this.Exceeded) + { + this.Exceeded = true; + errors.Add(new ParseError(token?.Start ?? 0, token?.Length ?? 1, MessageService.GetArgsString("script-error-expression-depth", "limit", MaximumExpressionDepth))); + } + } + } } \ No newline at end of file diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellDecimal.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellDecimal.cs index beb3bdb6..faa90621 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellDecimal.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellDecimal.cs @@ -6,6 +6,7 @@ namespace Azure.Data.Cosmos.Shell.Parser; using System.Globalization; using System.Text.Json; +using System.Text.Json.Serialization; using Azure.Data.Cosmos.Shell.Core; using Azure.Data.Cosmos.Shell.Util; @@ -20,6 +21,11 @@ public ShellDecimal(double value) public double Value { get; } + internal static JsonSerializerOptions JsonSerializationOptions { get; } = new() + { + Converters = { new DecimalJsonConverter() }, + }; + public override object ConvertShellObject(DataType type) { switch (type) @@ -33,13 +39,29 @@ public override object ConvertShellObject(DataType type) case DataType.Boolean: return this.Value != 0; case DataType.Json: - using (JsonDocument document = JsonDocument.Parse(this.Value.ToString(CultureInfo.InvariantCulture))) - { - return document.RootElement.Clone(); - } + return JsonSerializer.SerializeToElement(this.Value, JsonSerializationOptions); default: throw new InvalidOperationException(MessageService.GetArgsString("conversion-error-decimal-type", "type", type)); } } + + private sealed class DecimalJsonConverter : JsonConverter + { + public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return reader.GetDouble(); + } + + public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options) + { + var number = JsonSerializer.Serialize(value); + if (number.IndexOfAny(['.', 'e', 'E']) < 0) + { + number += ".0"; + } + + writer.WriteRawValue(number); + } + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellJson.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellJson.cs index 08f9e2a2..fcde1ad0 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellJson.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellJson.cs @@ -67,7 +67,7 @@ public override object ConvertShellObject(DataType type) if (this.Value.ValueKind == JsonValueKind.Number) { - return this.Value.GetInt32() != 0; + return this.Value.GetDouble() != 0; } throw new InvalidOperationException(MessageService.GetArgsString("conversion-error-json-boolean", "kind", this.Value.ValueKind)); diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellNumber.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellNumber.cs index 5c509782..bbffce8e 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellNumber.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/ShellObject/ShellNumber.cs @@ -19,6 +19,16 @@ public ShellNumber(int value) public int Value { get; } + internal static ShellObject FromJson(JsonElement value) + { + return value.TryGetInt32(out var number) ? new ShellNumber(number) : new ShellDecimal(value.GetDouble()); + } + + internal static ShellObject Normalize(ShellObject value) + { + return value is ShellJson json && json.Value.ValueKind == JsonValueKind.Number ? FromJson(json.Value) : value; + } + public override object ConvertShellObject(DataType type) { switch (type) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/AssignmentStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/AssignmentStatement.cs index 51688ba4..da897da3 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/AssignmentStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/AssignmentStatement.cs @@ -35,7 +35,22 @@ public AssignmentStatement(VariableExpression variable, Token assignmentToken, E public override async Task RunAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) { - var value = await this.Value.EvaluateAsync(shell, commandState, token); + Expression expression = this.Value; + if (this.AssignmentToken.Value != "=") + { + var operatorType = this.AssignmentToken.Value switch + { + "+=" => TokenType.Plus, + "-=" => TokenType.Minus, + "*=" => TokenType.Multiply, + "/=" => TokenType.Divide, + _ => throw new InvalidOperationException($"Unsupported assignment operator '{this.AssignmentToken.Value}'."), + }; + var operatorToken = new Token(operatorType, this.AssignmentToken.Value[..1], this.AssignmentToken.Start, 1); + expression = new BinaryOperatorExpression(this.Variable, operatorToken, this.Value); + } + + var value = await expression.EvaluateAsync(shell, commandState, token); shell.SetVariable(this.Variable.Name, value); commandState.Result = null; commandState.RenderUser = null; @@ -47,7 +62,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman public override string ToString() { - return $"{this.Variable} = {this.Value}"; + return $"{this.Variable} {this.AssignmentToken.Value} {this.Value}"; } internal override void Accept(IAstVisitor visitor) diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/BlockStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/BlockStatement.cs index 7fd0bdd5..5c01bc4c 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/BlockStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/BlockStatement.cs @@ -84,6 +84,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { foreach (var statement in this.Statements) { + token.ThrowIfCancellationRequested(); try { commandState = await statement.RunAsync(shell, commandState, token); @@ -92,7 +93,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; @@ -105,34 +106,19 @@ public override async Task RunAsync(ShellInterpreter shell, Comman throw; } - commandState = shell.PrintState(commandState, markAsRendered: true); - if (commandState.IsError) + if (commandState.BreakBlock || commandState.ContinueBlock || commandState.ReturnFunc) { return commandState; } - // Propagate break out of this block (do not clear it here) - if (commandState.BreakBlock) - { - return commandState; - } - - if (commandState.ContinueBlock) + if (commandState.IsError) { - commandState.ContinueBlock = false; // Reset continue state return commandState; } - if (commandState.ReturnFunc) + commandState = shell.PrintState(commandState, markAsRendered: true); + if (commandState.IsError) { - commandState.ReturnFunc = false; // Reset return state - - if (commandState.ReturnValue != null) - { - commandState.Result = commandState.ReturnValue; - commandState.ReturnValue = null; // Reset return value - } - return commandState; } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/CommandStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/CommandStatement.cs index fae24f8a..55bdc1a2 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/CommandStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/CommandStatement.cs @@ -167,14 +167,14 @@ public override async Task RunAsync(ShellInterpreter shell, Comman if (shell.Functions.TryGetValue(this.Name, out var function)) { - var args = new List(); + var args = new List(); foreach (var a in this.Arguments) { var evaluated = await a.EvaluateAsync(shell, commandState, token); - args.Add(evaluated?.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty); + args.Add(evaluated); } - return await function.ExecuteFunctionAsync(shell, commandState, token, args.ToArray()); + return await function.ExecuteCallAsync(shell, commandState, token, this.Start, args.ToArray()); } if (shell.App.Commands.TryGetValue(this.Name, out var factory)) @@ -186,7 +186,21 @@ public override async Task RunAsync(ShellInterpreter shell, Comman } var cmd = await this.CreateCommandAsync(factory, shell, commandState, token); - return await shell.ExecuteCosmosCommandAsync(cmd, commandState, string.Empty, token); + var result = await shell.ExecuteCosmosCommandAsync(cmd, commandState, string.Empty, token); + if (result is ErrorCommandState error && shell.CurrentScriptFileName is { } sourceName && shell.CurrentScriptContent is { } sourceText) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(sourceText, this.Start); + var positionalException = new PositionalException(sourceName, error.Exception, line, column, lineText); + if (result is StructuredErrorCommandState structuredError) + { + structuredError.Exception = positionalException; + return structuredError; + } + + throw positionalException; + } + + return result; } if (File.Exists(this.Name)) @@ -254,9 +268,10 @@ bool IsBoolean(PropertyInfo pi) var pi = matchingProperty.Prop; var attr = matchingProperty.Attr; + var optionValue = opt.Value; // If option already has an inline value (e.g. -opt:VAL parsed earlier) leave it. - if (!IsBoolean(pi) && opt.Value == null) + if (!IsBoolean(pi) && optionValue == null) { int nextIndex = i + 1; if (nextIndex < this.Arguments.Count && @@ -264,15 +279,15 @@ this.Arguments[nextIndex] is not CommandOption && !consumedArgumentIndices.Contains(nextIndex)) { // Treat next expression as the value of this non-boolean option. - opt.Value = this.Arguments[nextIndex]; + optionValue = this.Arguments[nextIndex]; consumedArgumentIndices.Add(nextIndex); } } // Now assign the option value to the command instance. - if (opt.Value != null) + if (optionValue != null) { - var evaluatedValue = await opt.Value.EvaluateAsync(shell, commandState, token); + var evaluatedValue = await optionValue.EvaluateAsync(shell, commandState, token); var stringValue = evaluatedValue.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty; var targetType = Nullable.GetUnderlyingType(pi.PropertyType) ?? pi.PropertyType; @@ -371,7 +386,7 @@ this.Arguments[nextIndex] is not CommandOption && return cmd; } - public async Task RunScriptAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) + public async Task RunScriptAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token, bool renderOutput = true) { var fileName = this.Name; @@ -387,7 +402,7 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt { foreach (var kvp in container.Variables) { - frame.Variables[kvp.Key] = kvp.Value; + frame.Variables.TryAdd(kvp.Key, kvp.Value); } } @@ -399,7 +414,7 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt frame.Set((i + 1).ToString(), new ShellText(evaluated.ConvertShellObject(DataType.Text)?.ToString() ?? string.Empty)); } - shell.VariableContainers.Enqueue(frame); + shell.PushCallScope(frame, token); var currentState = commandState; string scriptContent = string.Empty; var priorFileName = shell.CurrentScriptFileName; @@ -410,14 +425,19 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt scriptContent = File.ReadAllText(fileName); shell.CurrentScriptFileName = fileName; shell.CurrentScriptContent = scriptContent; - var lexer = new Lexer(scriptContent); - var parser = new StatementParser(lexer); - foreach (var statement in parser.ParseStatements()) + var parser = StatementParser.ScriptParseResult.Parse(scriptContent, allowReturn: true); + if (parser.Errors.HasErrors) { - if (token.IsCancellationRequested) - { - break; - } + return new ParserErrorCommandState(parser.Errors, fileName, scriptContent); + } + + foreach (var statement in parser.Statements) + { + token.ThrowIfCancellationRequested(); + var savedStdOut = shell.StdOutRedirect; + var savedAppendOut = shell.AppendOutRedirection; + var savedErrOut = shell.ErrOutRedirect; + var savedAppendErr = shell.AppendErrRedirection; try { @@ -428,27 +448,60 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt break; } - shell.StdOutRedirect = this.OutputRedirect; - shell.AppendOutRedirection = this.AppendOutput; - - shell.ErrOutRedirect = this.ErrorRedirect; - shell.AppendErrRedirection = this.AppendError; + if (currentState.ReturnFunc) + { + currentState.ReturnFunc = false; + currentState.Result = currentState.ReturnValue; + currentState.ReturnValue = null; + currentState.OutputRendered = false; + return currentState; + } - try + if (renderOutput) { + if (this.OutRedirectToken != null) + { + shell.StdOutRedirect = this.OutputRedirect; + shell.AppendOutRedirection = this.AppendOutput; + } + + if (this.ErrRedirectToken != null) + { + shell.ErrOutRedirect = this.ErrorRedirect; + shell.AppendErrRedirection = this.AppendError; + } + currentState = shell.PrintState(currentState, markAsRendered: true); } - finally + + if (currentState.IsError) { - shell.StdOutRedirect = null; - shell.ErrOutRedirect = null; + if (currentState is ErrorCommandState renderError && renderError.Exception is not PositionalException) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(scriptContent, statement.Start); + renderError.Exception = new PositionalException(fileName, renderError.Exception, line, column, lineText); + } + + break; } } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(scriptContent, statement.Start); + if (e is PositionalException positional && positional.FileName == fileName && positional.Line == line && positional.Column == column) + { + throw; + } + throw new PositionalException(fileName, e, line, column, lineText); } + finally + { + shell.StdOutRedirect = savedStdOut; + shell.AppendOutRedirection = savedAppendOut; + shell.ErrOutRedirect = savedErrOut; + shell.AppendErrRedirection = savedAppendErr; + } } } finally @@ -456,22 +509,7 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt shell.CurrentScriptFileName = priorFileName; shell.CurrentScriptContent = priorContent; - // Remove the script frame we pushed. Since VariableContainers is a Queue (FIFO), - // we need to rotate all elements except the last one to the back, then dequeue the last one. - // Example: [A, B, C] where C (script frame) needs to be removed: - // Rotate A: [B, C, A], Rotate B: [C, A, B], Dequeue C: [A, B] - var count = shell.VariableContainers.Count; - if (count > 0) - { - // Rotate (count - 1) elements to the back - for (int i = 0; i < count - 1; i++) - { - shell.VariableContainers.Enqueue(shell.VariableContainers.Dequeue()); - } - - // Now the script frame is at the front, dequeue it - shell.VariableContainers.Dequeue(); - } + shell.PopCallScope(); } /* @@ -520,6 +558,12 @@ public async Task RunScriptAsync(ShellInterpreter shell, CommandSt } }*/ + if (currentState is ErrorCommandState error && priorFileName is not null && priorContent is not null) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(priorContent, this.Start); + error.Exception = new PositionalException(priorFileName, error.Exception, line, column, lineText); + } + return currentState; } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DefStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DefStatement.cs index 02a70cd4..c5cff961 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DefStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DefStatement.cs @@ -7,6 +7,7 @@ namespace Azure.Data.Cosmos.Shell.Parser; using System; using Azure.Data.Cosmos.Shell.Core; +using Azure.Data.Cosmos.Shell.Util; /// /// Represents a function definition statement that declares a reusable function. @@ -18,6 +19,9 @@ namespace Azure.Data.Cosmos.Shell.Parser; [AstHelp("statement-def")] internal class DefStatement : Statement { + private string? sourceName; + private string? sourceText; + public DefStatement(Token defToken, Token nameToken, string[] parameters, Statement statement) { this.DefToken = defToken ?? throw new ArgumentNullException(nameof(defToken)); @@ -70,6 +74,8 @@ public DefStatement(Token defToken, Token nameToken, string[] parameters, Statem /// public override Task RunAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) { + this.sourceName = shell.CurrentScriptFileName; + this.sourceText = shell.CurrentScriptContent; shell.DeclareFunction(this); return Task.FromResult(commandState); } @@ -86,36 +92,75 @@ public override Task RunAsync(ShellInterpreter shell, CommandState /// Creates a new variable scope for parameters, executes the function body, /// then restores the previous scope. /// - public async Task ExecuteFunctionAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token, params string[] args) + public async Task ExecuteFunctionAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token, params ShellObject[] args) { + if (args.Length != this.Parameters.Length) + { + var message = MessageService.GetString("script-error-argument-count", new Dictionary + { + ["name"] = this.Name, + ["expected"] = this.Parameters.Length, + ["actual"] = args.Length, + }); + throw new CommandException(this.Name, message, new ArgumentException(message)); + } + var arguments = new VariableContainer(); - for (int i = 0; i < this.Parameters.Length && i < args.Length; i++) + for (int i = 0; i < this.Parameters.Length; i++) { - arguments.Set(this.Parameters[i], new ShellText(args[i])); + var argument = args[i] is ShellIdentifier identifier ? new ShellText(identifier.Value) : args[i]; + arguments.Set(this.Parameters[i].TrimStart('$'), argument); } - shell.VariableContainers.Enqueue(arguments); + shell.PushCallScope(arguments, token); + var callerName = shell.CurrentScriptFileName; + var callerText = shell.CurrentScriptContent; try { - return await this.Statement.RunAsync(shell, commandState, token); + shell.CurrentScriptFileName = this.sourceName ?? callerName; + shell.CurrentScriptContent = this.sourceText; + var result = await this.Statement.RunAsync(shell, commandState, token); + if (result.ReturnFunc) + { + result.ReturnFunc = false; + result.Result = result.ReturnValue; + result.ReturnValue = null; + result.OutputRendered = false; + } + + return result; + } + catch (Exception exception) when (exception is not PositionalException && (exception is not OperationCanceledException || !token.IsCancellationRequested) && this.sourceName != null && this.sourceText != null) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(this.sourceText, this.Statement.Start); + throw new PositionalException(this.sourceName, exception, line, column, lineText); } finally { - // Remove the function frame we pushed. Since VariableContainers is a Queue (FIFO), - // we need to rotate all elements except the last one to the back, then dequeue the last one. - var count = shell.VariableContainers.Count; - if (count > 0) + shell.CurrentScriptFileName = callerName; + shell.CurrentScriptContent = callerText; + shell.PopCallScope(); + } + } + + internal async Task ExecuteCallAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token, int start, params ShellObject[] args) + { + try + { + var result = await this.ExecuteFunctionAsync(shell, commandState, token, args); + if (result is ErrorCommandState error && shell.CurrentScriptFileName is { } currentSourceName && shell.CurrentScriptContent is { } currentScriptContent) { - // Rotate (count - 1) elements to the back - for (int i = 0; i < count - 1; i++) - { - shell.VariableContainers.Enqueue(shell.VariableContainers.Dequeue()); - } - - // Now the function frame is at the front, dequeue it - shell.VariableContainers.Dequeue(); + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(currentScriptContent, start); + error.Exception = new PositionalException(currentSourceName, error.Exception, line, column, lineText); } + + return result; + } + catch (Exception exception) when ((exception is not OperationCanceledException || !token.IsCancellationRequested) && shell.CurrentScriptFileName != null && shell.CurrentScriptContent != null) + { + var (line, column, lineText) = PositionalErrorHelper.GetLineAndColumn(shell.CurrentScriptContent, start); + throw new PositionalException(shell.CurrentScriptFileName, exception, line, column, lineText); } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DoWhileStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DoWhileStatement.cs index a22d01e8..a170f606 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DoWhileStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/DoWhileStatement.cs @@ -70,6 +70,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { do { + token.ThrowIfCancellationRequested(); try { commandState = await this.Statement.RunAsync(shell, commandState, token); @@ -78,7 +79,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; @@ -91,6 +92,12 @@ public override async Task RunAsync(ShellInterpreter shell, Comman throw; } + if (commandState.IsError || commandState.ReturnFunc) + { + return commandState; + } + + commandState.ContinueBlock = false; if (commandState.BreakBlock) { commandState.BreakBlock = false; // Reset break state diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ExecStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ExecStatement.cs index abe4054f..6125a355 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ExecStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ExecStatement.cs @@ -122,7 +122,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ForStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ForStatement.cs index 1618a360..693544e4 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ForStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ForStatement.cs @@ -102,13 +102,14 @@ public override async Task RunAsync(ShellInterpreter shell, Comman foreach (var arr in collection.EnumerateArray()) { + token.ThrowIfCancellationRequested(); ShellObject elementValue = arr.ValueKind switch { - JsonValueKind.Number => new ShellNumber(arr.GetInt32()), + JsonValueKind.Number => ShellNumber.FromJson(arr), JsonValueKind.String => new ShellText(arr.GetString() ?? string.Empty), JsonValueKind.True => new ShellBool(true), JsonValueKind.False => new ShellBool(false), - JsonValueKind.Null => new ShellText("null"), + JsonValueKind.Null => new ShellJson(arr), JsonValueKind.Object or JsonValueKind.Array => new ShellJson(arr), _ => new ShellText(arr.ToString()), }; @@ -122,7 +123,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; @@ -135,6 +136,12 @@ public override async Task RunAsync(ShellInterpreter shell, Comman throw; } + if (commandState.IsError || commandState.ReturnFunc) + { + return commandState; + } + + commandState.ContinueBlock = false; if (commandState.BreakBlock) { commandState.BreakBlock = false; // Reset break state diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/LoopStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/LoopStatement.cs index c7cf1621..9bcb25ae 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/LoopStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/LoopStatement.cs @@ -73,6 +73,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { while (true) { + token.ThrowIfCancellationRequested(); try { commandState = await this.Statement.RunAsync(shell, commandState, token); @@ -81,7 +82,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; @@ -94,6 +95,12 @@ public override async Task RunAsync(ShellInterpreter shell, Comman throw; } + if (commandState.IsError || commandState.ReturnFunc) + { + return commandState; + } + + commandState.ContinueBlock = false; if (commandState.BreakBlock) { commandState.BreakBlock = false; // Reset break state diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PipeStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PipeStatement.cs index fb3b9493..61dea54b 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PipeStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PipeStatement.cs @@ -84,7 +84,8 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { foreach (var statement in this.Statements) { - if (commandState.IsError) + token.ThrowIfCancellationRequested(); + if (commandState.IsError || commandState.BreakBlock || commandState.ContinueBlock || commandState.ReturnFunc) { return commandState; } @@ -92,6 +93,11 @@ public override async Task RunAsync(ShellInterpreter shell, Comman commandState = await statement.RunAsync(shell, commandState, token); } + if (commandState.IsError || commandState.BreakBlock || commandState.ContinueBlock || commandState.ReturnFunc) + { + return commandState; + } + commandState = shell.PrintState(commandState, markAsRendered: true); return commandState; } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PositionalException.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PositionalException.cs index 5a33c142..e564917c 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PositionalException.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/PositionalException.cs @@ -34,4 +34,19 @@ public PositionalException(string fileName, Exception innerException, int line, /// Gets the text of the line where the exception occurred. /// public string? LineText { get; } + + internal static IReadOnlyList GetSourceTrace(Exception exception) + { + var frames = new List(); + for (Exception? current = exception; current != null; current = current.InnerException) + { + if (current is PositionalException frame) + { + frames.Add(frame); + } + } + + frames.Reverse(); + return frames; + } } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ReturnStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ReturnStatement.cs index c92f0ba0..94bd10e3 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ReturnStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/ReturnStatement.cs @@ -78,11 +78,14 @@ public override int Length /// public override async Task RunAsync(ShellInterpreter shell, CommandState commandState, CancellationToken token) { + var value = this.Value != null ? await this.Value.EvaluateAsync(shell, commandState, token) : null; + commandState.ReturnValue = value; commandState.ReturnFunc = true; - if (this.Value != null) - { - commandState.ReturnValue = await this.Value.EvaluateAsync(shell, commandState, token); - } + + // The caller replaces Result with the return value, so renderers from earlier statements must not survive. + commandState.RenderUser = null; + commandState.RenderTabular = null; + commandState.ResetOutputFormat(); return commandState; } diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/WhileStatement.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/WhileStatement.cs index fef4d095..f9ab75b6 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/WhileStatement.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/Statement/WhileStatement.cs @@ -66,6 +66,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { while (!commandState.IsError && await this.EvaluateConditionAsync(shell, commandState, token)) { + token.ThrowIfCancellationRequested(); try { commandState = await this.Statement.RunAsync(shell, commandState, token); @@ -74,7 +75,7 @@ public override async Task RunAsync(ShellInterpreter shell, Comman { throw; } - catch (Exception e) + catch (Exception e) when (e is not OperationCanceledException || !token.IsCancellationRequested) { var content = shell.CurrentScriptContent; var fileName = shell.CurrentScriptFileName; @@ -87,6 +88,12 @@ public override async Task RunAsync(ShellInterpreter shell, Comman throw; } + if (commandState.IsError || commandState.ReturnFunc) + { + return commandState; + } + + commandState.ContinueBlock = false; if (commandState.BreakBlock) { commandState.BreakBlock = false; // Reset break state diff --git a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/StatementParser.cs b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/StatementParser.cs index c7846007..b2299056 100644 --- a/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/StatementParser.cs +++ b/CosmosDBShell/Azure.Data.Cosmos.Shell.Parser/StatementParser.cs @@ -81,6 +81,44 @@ public List ParseStatements() } public Statement? ParseStatement() + { + if (this.expressionParser.IsAtEnd || !this.lexer.Budget.TryEnter(this.Errors, this.expressionParser.Current)) + { + return null; + } + + try + { + return this.ParseStatementCore(); + } + finally + { + this.lexer.Budget.Exit(); + } + } + + private static string RedirectLabel(Token redirectToken) + => redirectToken.Type switch + { + TokenType.RedirectOutput => ">", + TokenType.RedirectAppendOutput => ">>", + TokenType.RedirectError => "2>", + TokenType.RedirectAppendError => "2>>", + _ => redirectToken.Value, + }; + + private static bool IsCommandTerminator(Token token) + => token.Type == TokenType.Semicolon || + token.Type == TokenType.Eol || + token.Type == TokenType.CloseBrace || + token.Type == TokenType.Pipe || + token.Type == TokenType.GreaterThan || + token.Type == TokenType.RedirectOutput || + token.Type == TokenType.RedirectAppendOutput || + token.Type == TokenType.RedirectError || + token.Type == TokenType.RedirectAppendError; + + private Statement? ParseStatementCore() { if (this.expressionParser.IsAtEnd) { @@ -148,27 +186,6 @@ public List ParseStatements() return segments[0]; } - private static string RedirectLabel(Token redirectToken) - => redirectToken.Type switch - { - TokenType.RedirectOutput => ">", - TokenType.RedirectAppendOutput => ">>", - TokenType.RedirectError => "2>", - TokenType.RedirectAppendError => "2>>", - _ => redirectToken.Value, - }; - - private static bool IsCommandTerminator(Token token) - => token.Type == TokenType.Semicolon || - token.Type == TokenType.Eol || - token.Type == TokenType.CloseBrace || - token.Type == TokenType.Pipe || - token.Type == TokenType.GreaterThan || - token.Type == TokenType.RedirectOutput || - token.Type == TokenType.RedirectAppendOutput || - token.Type == TokenType.RedirectError || - token.Type == TokenType.RedirectAppendError; - /// /// Detects the start of a '2>' or '2>>' stderr redirect in command context. /// True when is a Number token with value "2" @@ -702,6 +719,7 @@ private void SkipWs() if (!this.expressionParser.IsAtEnd && this.expressionParser.Current != null && this.expressionParser.Current.Type != TokenType.Semicolon && + this.expressionParser.Current.Type != TokenType.CloseBrace && this.expressionParser.Current.Type != TokenType.Eol) { value = this.expressionParser.ParseExpression(); @@ -851,6 +869,19 @@ private void SkipWs() var current = this.expressionParser.Current; this.expressionParser.Advance(); + var compoundOperator = this.expressionParser.IsAtEnd ? null : this.expressionParser.Current; + if (compoundOperator?.Type is TokenType.Plus or TokenType.Minus or TokenType.Multiply or TokenType.Divide && + this.expressionParser.Peek() is { Type: TokenType.Assignment } equals && + compoundOperator.Start + compoundOperator.Length == equals.Start) + { + this.expressionParser.Advance(); + this.expressionParser.Advance(); + var value = this.expressionParser.ParseExpression(); + var variable = new VariableExpression(current, current.Value.StartsWith('$') ? current.Value[1..] : current.Value); + var assignmentToken = new Token(TokenType.Assignment, compoundOperator.Value + "=", compoundOperator.Start, compoundOperator.Length + equals.Length); + return new AssignmentStatement(variable, assignmentToken, value); + } + if (!this.expressionParser.IsAtEnd && this.expressionParser.Current != null && this.expressionParser.Current.Type == TokenType.Assignment) @@ -1189,4 +1220,96 @@ private void Synchronize() this.expressionParser.Advance(); } } + + internal sealed class ScriptParseResult + { + private ScriptParseResult(IReadOnlyList statements, ErrorList errors) + { + this.Statements = statements; + this.Errors = errors; + } + + public IReadOnlyList Statements { get; } + + public ErrorList Errors { get; } + + public static ScriptParseResult Parse(string source, bool allowReturn = false) + { + var parser = new StatementParser(source); + var statements = parser.ParseStatements(); + if (!parser.Errors.HasErrors) + { + ValidateStatements(statements, parser.Errors, allowReturn); + } + + return new ScriptParseResult(statements.AsReadOnly(), parser.Errors); + } + + internal static void ValidateStatements(IEnumerable statements, ErrorList errors, bool allowReturn) + { + foreach (var statement in statements) + { + Validate(statement, errors, inFunction: allowReturn, loopDepth: 0); + } + } + + private static void Validate(Statement statement, ErrorList errors, bool inFunction, int loopDepth) + { + switch (statement) + { + case BreakStatement or ContinueStatement when loopDepth == 0: + errors.Add(new ParseError(statement.Start, statement.Length, MessageService.GetString("script-error-loop-control"))); + break; + case ReturnStatement when !inFunction: + errors.Add(new ParseError(statement.Start, statement.Length, MessageService.GetString("script-error-return-context"))); + break; + case DefStatement function: + var names = new HashSet(StringComparer.Ordinal); + foreach (var parameter in function.Parameters) + { + if (!names.Add(parameter.TrimStart('$'))) + { + errors.Add(new ParseError(function.Start, function.Length, MessageService.GetArgsString("script-error-duplicate-parameter", "name", parameter))); + } + } + + Validate(function.Statement, errors, inFunction: true, loopDepth: 0); + break; + case BlockStatement block: + foreach (var child in block.Statements) + { + Validate(child, errors, inFunction, loopDepth); + } + + break; + case PipeStatement pipe: + foreach (var child in pipe.Statements) + { + Validate(child, errors, inFunction, loopDepth); + } + + break; + case IfStatement conditional: + Validate(conditional.Statement, errors, inFunction, loopDepth); + if (conditional.ElseStatement != null) + { + Validate(conditional.ElseStatement, errors, inFunction, loopDepth); + } + + break; + case ForStatement forLoop: + Validate(forLoop.Statement, errors, inFunction, loopDepth + 1); + break; + case WhileStatement whileLoop: + Validate(whileLoop.Statement, errors, inFunction, loopDepth + 1); + break; + case DoWhileStatement doLoop: + Validate(doLoop.Statement, errors, inFunction, loopDepth + 1); + break; + case LoopStatement loop: + Validate(loop.Statement, errors, inFunction, loopDepth + 1); + break; + } + } + } } \ No newline at end of file diff --git a/CosmosDBShell/lang/en.ftl b/CosmosDBShell/lang/en.ftl index 92b86425..f7b0c4db 100644 --- a/CosmosDBShell/lang/en.ftl +++ b/CosmosDBShell/lang/en.ftl @@ -451,6 +451,7 @@ command-export-success = Exported { $count } { $count -> } to { $file } (RU charge: { $charge }) command-export-error-missing_file = A destination file path is required. command-export-error-file_exists = File '{ $file }' already exists. Use --force to overwrite. +command-export-error-destination_directory = Destination '{ $file }' is a directory. Specify a file path instead. command-export-error-query_failed = Export query failed: { $status } - { $message } command-import-description = Imports items into a container from a JSON Lines, JSON array, or CSV file. @@ -477,6 +478,18 @@ command-import-dry-run-success = Dry run: { $count } valid { $count -> } command-import-error-missing_file = A source file path is required. command-import-error-invalid_csv = Invalid CSV record at line { $line }. +script-error-argument-count = Function '{ $name }' expects { $expected } { $expected -> + [one] argument + *[other] arguments + }, got { $actual }. +script-error-loop-control = break and continue require an enclosing loop in the same function or script. +script-error-return-context = return requires an enclosing function or script file. +script-error-duplicate-parameter = Duplicate function parameter '{ $name }'. +script-error-command-failed = Command expression '{ $name }' failed. +script-error-expression-failed = Command expression failed. +script-error-call-depth = Maximum function/script call depth of { $limit } exceeded. +script-error-parse-depth = Maximum parser nesting budget of { $limit } exceeded. Simplify the expression or split the script. +script-error-expression-depth = Maximum expression tree depth of { $limit } exceeded. Simplify the expression or split the script. command-import-error-file_not_found = File '{ $file }' was not found. command-import-error-blank_line = Line { $line } is blank. command-import-error-not_object = Line { $line } is not a JSON object. diff --git a/README.md b/README.md index 5ad63169..0fd37a00 100644 --- a/README.md +++ b/README.md @@ -230,6 +230,18 @@ cosmosdbshell --connect "AccountEndpoint=...;AccountKey=..." -c seed.csh mydb my echo "seed.csh mydb mycontainer" | cosmosdbshell --connect "AccountEndpoint=...;AccountKey=..." ``` +Scripts are parsed and validated before their statements execute. Functions preserve argument types and keep assignments local; `return`, `break`, and `continue` propagate through nested blocks to their owning function, script, or loop. Integer overflow is reported as an error. See the [language rules and compatibility notes](docs/programming.md#operators), including operator precedence, compound assignment, numeric promotion, and runtime errors. Runtime failures do not roll back earlier successful operations. + +Parser nesting, expression tree depth, and active function/script calls have fixed [resource limits](docs/programming.md#resource-limits). Limit violations fail with diagnostics rather than continuing recursive parsing or execution. + +Script diagnostics preserve source files, runtime failure categories, and function/script call sites. The language server shares the runtime's control-flow and duplicate-parameter checks, recognizes document-local functions, and checks nested commands and options. Incorrect function argument counts produce usage exit code `2`. See [validation and errors](docs/programming.md#validation-and-errors). + +Loops and functions preserve JSON `null` values. Numeric conditions use the same zero/nonzero rule for shell values and JSON properties, including fractional numbers. See [value conversion rules](docs/programming.md#numbers). + +JSON strings use the same `+` concatenation rules as shell strings. A bare `return` is valid immediately before a closing block brace. Editor variable symbols distinguish case-sensitive names such as `$value` and `$Value`. + +JSON objects and arrays preserve decimal number types across roundtrips: constructing `{"value":3.0}` no longer changes subsequent division into integer arithmetic. + ## Deterministic Exit Codes When running scripts or automation, Cosmos DB Shell maps execution failures to a set of stable exit codes (accessible via `$?`, `%ERRORLEVEL%`, or `$LASTEXITCODE`): diff --git a/docs/mcp.md b/docs/mcp.md index 8f8d1b62..559c6d95 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -170,4 +170,3 @@ Because `max` bounds a single page, a call can return fewer items than requested "continuation": "" } ``` - diff --git a/docs/programming.md b/docs/programming.md index c92c38e4..73f178cb 100644 --- a/docs/programming.md +++ b/docs/programming.md @@ -12,7 +12,7 @@ This document covers scripts and custom commands in Cosmos Shell. ### Identifiers - Allowed: letters, digits, `_`, `-`, `.`, `\`, `$` -- Keywords (case-insensitive): `if`, `while`, `for`, `do`, `loop`, `def`, `return`, `break`, `continue` +- Keywords (case-insensitive): `if`, `else`, `while`, `for`, `in`, `do`, `loop`, `def`, `return`, `break`, `continue`, `exec` ### Variables @@ -20,13 +20,28 @@ This document covers scripts and custom commands in Cosmos Shell. - Script args: `$0` = path, `$1`, `$2`... = positional arguments - Assign: `$name = ` +Variable names are case-sensitive. For compatibility, the lexer also accepts hyphens in variable names: `$name-1` refers to a variable named `name-1`, not subtraction. Use spaces around arithmetic and assignment operators, for example `$name - 1` and `$name -= 1`. + ### Numbers - Integers: `42`, `314` - Negatives: `-1` +- Decimal literals: `3.14`, `3.0` + +Integer values use signed 32-bit storage. Integer literal magnitudes must be between `0` and `2147483647`; the minus sign is a separate unary operator, so `-2147483648` is not accepted as a literal. The minimum integer value can be computed as `-2147483647 - 1`. Arithmetic between integers stays integer arithmetic, including truncating division (`3 / 2` is `1`). Integer overflow raises an error instead of wrapping. Use a decimal operand for floating-point arithmetic (`3.0 / 2` is `1.5`). Decimal values use IEEE 754 `double`, not exact base-10 decimal arithmetic. + +JSON numbers use the same rules in expressions and `for` loops: integer-form values within the `Int32` range become integers; fractional, exponent-form, or larger values use `double`. Large JSON integers can therefore lose precision beyond the exact range of `double`. For example, a JSON property containing `3` divided by `2` produces `1`, while a property containing `3.0` produces `1.5`. + +JSON construction preserves the distinction between integers and shell decimals. Integral decimal values retain a fractional suffix or exponent, so `$object = {"value":3.0}` stores JSON `3.0` and `$object.value / 2` produces `1.5`. This also applies to arrays, computed decimal values, and repeated reconstruction after loop or function calls. Integer values remain JSON integers. The original spelling and number of trailing zeros are not preserved; values still use IEEE 754 `double` precision. + +Numeric Boolean conversion uses zero versus nonzero, including for fractional and large JSON numbers. JSON numbers use the same `double` conversion as decimal shell values for this check, so `if 1.5` and `if $object.value` behave alike when the property contains `1.5`. + +JSON `null` remains JSON `null` when bound by a `for` loop or passed through a function. Rebuilding an array from that value produces `[null]`, not `["null"]`. Text conversion remains explicit and separate from JSON type preservation. ### Strings +The `+` operator concatenates when either operand is a shell string or a JSON string, including values read through JSON paths or passed as function arguments. Numeric-looking strings remain text: two JSON properties containing `"2"` concatenate to `"22"`, not `4`. + | Type | Syntax | Notes | | ------ | ------ | ----- | | Single-quoted | `'text'` | Literal, no escapes. Double `'` for quote: `'it''s'` | @@ -63,6 +78,72 @@ $.values[0].id # property and array access | Grouping | `( ... )` | | Assignment | `=` `+=` `-=` `*=` `/=` | +Precedence, from lowest to highest: + +| Level | Operators | Associativity | +| --- | --- | --- | +| 1 | `\|\|` | Left | +| 2 | `&&` | Left | +| 3 | `^` | Left | +| 4 | `==`, `!=` | Left | +| 5 | `<`, `<=`, `>`, `>=` | Left | +| 6 | `+`, `-` | Left | +| 7 | `*`, `/`, `%` | Left | +| 8 | `**` | Right | +| 9 | Unary `!`, `+`, `-` | Right | + +Parentheses override precedence. Unary operators bind more tightly than power: `-2 ** 2` is `4`; use `-(2 ** 2)` for `-4`. `&&` and `||` short-circuit; other binary operators evaluate each operand exactly once. Assignment is a statement, not an expression. Compound assignments use the same arithmetic rules as their corresponding binary operators and evaluate the right-hand side once. + +### Statement Grammar + +The following EBNF summarizes statement structure; command arguments retain shell-word quoting and option syntax. Expressions follow the precedence table above and include literals, variables, JSON construction, paths, and parenthesized command calls. + +```ebnf +script = { statement, [ separator ] } ; +separator = ";" | newline ; +statement = simple, { "|", simple } ; +simple = assignment | command | block + | "if", expression, statement, [ "else", statement ] + | "while", expression, statement + | "do", statement, "while", expression + | "for", variable, "in", expression, statement + | "loop", statement + | "def", name, [ parameters ], statement + | "return", [ expression ] | "break" | "continue" + | "exec", expression, { argument } ; +block = "{", script, "}" ; +assignment = variable, ( "=" | "+=" | "-=" | "*=" | "/=" ), expression ; +parameters = "[", { name }, "]" | "(", [ name, { ",", name } ], ")" ; +``` + +### Validation and Errors + +Each command text or script file is fully parsed and checked for invalid control-flow placement and duplicate function parameters before any of its statements execute. Syntax or semantic errors prevent execution of that entire input. Script files are checked when invoked, including calls through `exec` and command expressions; callers are not recursively preflighted against dynamically selected files. + +Runtime failures stop execution but do not roll back earlier successful commands. A failed command expression propagates an error rather than silently producing an empty result. Cancellation is checked between block statements and loop iterations, including loops without database commands. + +Host-requested cancellation propagates through script files, blocks, loops, and function calls without being converted into a positional runtime error. The shell reports a neutral result and records cancellation in the diagnostic log; call scopes and source context are restored. Cancellation exceptions without a canceled host token still follow the existing error/timeout handling. + +Parser errors from script files retain their own filename and source text, including when reached through a command expression. Runtime exceptions retain their original cause and exit-code category: attaching a source location does not turn authentication, throttling, connectivity, or arithmetic failures into usage errors. + +Calling a function with too few or too many arguments is a usage error (exit code `2`), including calls within expressions. The function body is not executed. + +Functions defined in a script retain the definition's source location even when invoked later from another file. Runtime diagnostics show the innermost source location first, followed by the recorded function/script call sites in human-readable output. JSON error messages include the originating file, line, and column. Diagnostic logs retain source locations and underlying exception details through the existing secret-redaction pipeline. + +The language server applies the same control-flow and duplicate-parameter validation as script execution. File-level `return` is valid; `break` and `continue` require an enclosing loop in the same function. Diagnostics use exclusive-end editor ranges and are refreshed when a document changes. + +The language server also recognizes case-sensitive function names declared in the current document, including recursive calls and calls from other function bodies. It checks commands and built-in options inside blocks, branches, loops, pipelines, and command expressions. Function-name discovery is document-wide: it does not prove that a definition has executed before a call, or resolve functions loaded dynamically from other files. Runtime registration and execution order are unchanged. + +Variable symbols and hover lookups are also case-sensitive: `$value` and `$Value` remain distinct. Variable analysis still treats the first occurrence of each name as its definition and does not model the runtime's call scopes. + +### Resource Limits + +The parser has a shared nesting budget of 128 recursive parsing entries. Statements, expression operators, primary expressions, and interpolation subparsers share this budget, so the allowed number of source-level parentheses depends on the surrounding syntax. Exceeding it produces a parser diagnostic before execution, including in editor/highlighter parsing. Sequential statements do not accumulate nesting depth. + +Expression trees also have a maximum depth of 128 nodes, including operators and containing expressions such as parentheses, arrays, objects, and interpolation. This independent check rejects long flat operator chains that do not require deep parser recursion. Overdeep expressions are replaced with error nodes before execution or editor analysis; the containing input is not executed. Split a long expression into intermediate assignments when needed. + +At most 64 function and script-file calls may be active at once, including mixed or indirect recursion. Exceeding this limit produces a runtime error, unwinds call scopes, and leaves the interpreter usable. Calls also check cancellation before entering a new scope. These are fixed safety limits, not a sandbox or a wall-clock timeout; long-running valid scripts still require host cancellation. + ## Variable Usage ```bash @@ -173,6 +254,8 @@ Piping the contents of a script file directly runs those statements as standard Each script run gets its own variable scope. Variables from the caller are readable at script start, but assignments inside the script stay local to that script run and do not leak back to the caller. +This also applies when a script is called as an expression. `return [expression]` exits the current script file and supplies its result, including from nested blocks or loops. A return inside a function exits only that function. Script positional arguments remain text values, unlike typed function parameters. + ## Control Flow ### if/else @@ -250,12 +333,14 @@ loop { ### break / continue ```bash -while $true { +while true { if $skip { continue } if $done { break } } ``` +`break` exits the nearest enclosing loop; `continue` skips the rest of its current iteration. Nested blocks and conditionals preserve these signals. Neither may cross a function or script-file boundary. Bare blocks do not introduce variable scopes. + ## Custom Commands (def) Define reusable commands invoked like built-ins. @@ -275,9 +360,11 @@ greet "Cosmos" ### Parameters and Scope -- Arguments available as `$param1`, `$param2` inside body -- Functions have own variable scope (don't leak to caller) -- Globals remain readable +- Arguments are available as `$param1`, `$param2` inside the body and retain their numeric, boolean, text, or JSON types. Unquoted shell words are text. +- The argument count must exactly match the parameter count. Duplicate parameter names are rejected. +- Functions have their own variable scope. Assignments, including compound assignments to existing outer variables, stay local. +- Variables are read from the nearest active call frame, then outer caller frames and globals. Functions do not capture lexical closures. Frames are removed on success, failure, and cancellation. +- Return a value to update a caller variable explicitly, for example `$total = (add $total 1)`. Session settings such as `$sessionRequestChargeWarningThreshold` remain session-wide. ### Returning Values @@ -286,7 +373,7 @@ def add [a b] { return ($a + $b) } add 2 3 | echo $"sum=$." ``` -- `return` stops execution and sets result +- `return` stops the current function even inside nested blocks or loops and sets its result. A bare `return` has no result and can appear immediately before `}`, as in `def empty { return }`; no semicolon is required there. Outside a function or script file, `return` is rejected. - Returned JSON can be accessed with paths downstream - Without `return`, function completes with last state diff --git a/l10n/CosmosDBShell.json b/l10n/CosmosDBShell.json index 05904bc7..02f0502a 100644 --- a/l10n/CosmosDBShell.json +++ b/l10n/CosmosDBShell.json @@ -346,6 +346,7 @@ "command-export-description-format": "Output format: jsonl (default), array, or csv.", "command-export-description-max": "Maximum number of items to export. 0 means no limit.", "command-export-description-query": "SELECT query whose results are exported (default: SELECT * FROM c).", + "command-export-error-destination_directory": "Destination \u0027{0}\u0027 is a directory. Specify a file path instead.", "command-export-error-file_exists": "File \u0027{0}\u0027 already exists. Use --force to overwrite.", "command-export-error-missing_file": "A destination file path is required.", "command-export-error-query_failed": "Export query failed: {0} - {1}", @@ -1392,6 +1393,17 @@ "query-error-prefix": "query error", "runtime-error-canceled": "Canceled.", "runtime-error-prefix": "error", + "script-error-argument-count": "Function \u0027{0}\u0027 expects {1} {2}, got {3}.", + "script-error-argument-count.__p2.one": "argument", + "script-error-argument-count.__p2.other": "arguments", + "script-error-call-depth": "Maximum function/script call depth of {0} exceeded.", + "script-error-command-failed": "Command expression \u0027{0}\u0027 failed.", + "script-error-duplicate-parameter": "Duplicate function parameter \u0027{0}\u0027.", + "script-error-expression-depth": "Maximum expression tree depth of {0} exceeded. Simplify the expression or split the script.", + "script-error-expression-failed": "Command expression failed.", + "script-error-loop-control": "break and continue require an enclosing loop in the same function or script.", + "script-error-parse-depth": "Maximum parser nesting budget of {0} exceeded. Simplify the expression or split the script.", + "script-error-return-context": "return requires an enclosing function or script file.", "shell-connect-arm-discovery-ambiguous": "Multiple ARM Cosmos DB accounts match the connected endpoint. Reconnect with --subscription and --resource-group, or use --connect-subscription and --connect-resource-group at startup, to specify which account to use. Using Cosmos DB data plane for now.", "shell-connect-arm-discovery-failed": "Using Cosmos DB data plane.", "shell-connect-azure-cli-auth": "Connecting with Azure CLI credential (az login)...",