diff --git a/AGENTS.md b/AGENTS.md index e34547d..fcb2c07 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,7 +43,7 @@ It is deliberately **thin**: Microsoft's `almcp` already compiles, runs diagnost ### Key layers - **Tools/** — MCP tool endpoints, annotated `[McpServerToolType]` / `[McpServerTool]`, auto-discovered via `WithToolsFromAssembly()`. Five native tools: `list_rules`, `get_fixes`, `apply_fix`, `apply_fix_all`, `analyze`. The proxied `al_*` tools are served by the dynamic list/call handlers in `McpHost`, not by classes here. -- **Services/**, all singletons registered in `McpHost`: +- **Services/**, singletons registered in `McpHost` unless noted: - `BcToolsLocator` — the single runtime lookup. Finds the one directory holding both `Microsoft.Dynamics.Nav.*.dll` and `almcp` (they ship side by side). Probe order: `--devtools-path` → `BCDEVELOPMENTTOOLSPATH` → dotnet tool store → hard error naming every probe and the install command. The AL VS Code extension is no longer probed; the dotnet tool store is the primary channel on every OS. No auto-install by design. `AlMcpLaunch` describes how to start the child: native `almcp.exe` on Windows, `dotnet almcp.dll` on Linux/macOS (the nupkg ships no extension-less launcher). - `WorkspaceStartupResolver` — discovers AL projects (mirrors `almcp`'s own `DiscoverProjectPaths`: downward scan for `app.json`, depth 4, standard exclusions) and composes the child `almcp`'s `--projects` / `--codeanalyzers` / `--rulesetpath` / `--packagecachepath` args. `almcp` in MCP mode never reads `.vscode/settings.json` and has no per-call analyzer, ruleset or package-cache parameter, so this bridge at launch is the only thing keeping `al_compile` and our fix tools in agreement (`ProjectLoader` reads the same `al.packageCachePath` for the in-process compilation). - `AlMcpProxy` — child process lifecycle plus generic tool forwarding over a single long-lived MCP client that reconnects on session expiry. `ForwardAsync` is a passthrough with **no per-tool argument rewriting**; configuration is conveyed at launch instead. @@ -51,6 +51,8 @@ It is deliberately **thin**: Microsoft's `almcp` already compiles, runs diagnost - `AlcopsAnalyzerProvisioner` — cache-first: when a valid cached version exists, `Task Ready` completes immediately with it and a background task checks NuGet for a newer version (for the next start). `internal Task BackgroundRefresh` is that background task, drained by `ProvisionAsync` and cancelled by `AlcopsAnalyzerProvisionerStartup.StopAsync`. On a cold cache or with a pinned version, the provisioner fetches from NuGet before completing. Internal HTTP timeouts surface as `TimeoutException` and fall back to cache; only the caller's cancellation propagates. Version ordering is SemVer 2 via `SemanticVersion`. The `.in-use` lock (`FileShare.None`) is a cross-process lock via `flock` on Unix, reliable on local file systems and advisory on network mounts such as NFS home directories. Configured via `--alcops-analyzers` / `ALCOPS_ANALYZERS` / `ALCOPS_ANALYZERS_CACHE`. - `ExternalAnalyzerLoader` — loads analyzer DLLs through `AnalyzerAssemblyLoadContext`, which resolves shared types by simple name from the default context. That type sharing is what makes `typeof(DiagnosticAnalyzer).IsAssignableFrom` work, and therefore what makes in-process code fixes possible at all. - `ProjectSessionManager` / `ProjectLoader` — caches AL project workspaces keyed by path; `GetOrLoadProjectAsync` is the entry point tools use. On a cache hit it calls `ProjectSession.RefreshFromDiskAsync`: length+mtime gate, then ordinal content compare; changed files get `OnDocumentTextChanged`, new/deleted files are added/removed; untouched documents keep `DocumentId` and compilation state. Mutations go through `Workspace.OnDocument*` (like almcp's `ProjectWatcher`), never `TryApplyChanges` on a forked solution. + - `ToolResults` / `ToolErrors` — static helpers, not registered. `ToolResults.Ok` serializes a success payload; `ToolErrors` is the only place a tool error is built (see Tool patterns). + - `ProjectScope` — static helper, not registered. `Resolve` validates `projectPath` against the startup projects for `analyze` and `list_rules`; `RequireProjectFolder` checks that the fix tools got a folder that exists and holds `app.json`. A failure is an `Invalid` error. - `GuardedFileWriter` — atomic, encoding-preserving `.al` writes; the only code that writes `.al` files. Used by `apply_fix` (`WriteIfUnchangedAsync`) and `apply_fix_all` (`WriteAllIfUnchangedAsync`). Its internal `Action` constructor is a test seam for injecting move failures. - **Models/** — record types for tool return values, serialized with `JsonDefaults.Options` (camelCase, not indented). @@ -64,13 +66,14 @@ When passing analyzers to the child `almcp`, their sibling dependencies must tra ### Tool patterns -- All tool methods are `static async Task`, receiving DI services as parameters. -- Tools return JSON-serialized results. Errors are caught and returned as `{ error, message }` JSON, not thrown. -- `list_rules` is read-only. `get_fixes`, `apply_fix` and `apply_fix_all` re-read changed `.al` files from disk before computing a fix (via `RefreshFromDiskAsync`); the write only proceeds if the file still equals the text the fix was computed from. `apply_fix` returns `{ error: "StaleFile" }` on a conflict; `apply_fix_all` writes the non-conflicting files and lists the rest in `conflicts`. There is no explicit reload after writing — the next `GetOrLoadProjectAsync` call picks the written file up via the same refresh. -- Writes go through `GuardedFileWriter`: read the file as bytes → decode with BOM detection → ordinal compare with the expected text → write preamble + new text to a sibling `..alcops.tmp` → `File.Move(temp, path, overwrite: true)`; the temp is deleted in `finally`. The encoding is detected at write time from that read, not stored on `TrackedDocument`. The BOM is sniffed explicitly (UTF-32 LE before UTF-16 LE) and every branch, BOM-less UTF-8 included, uses a strict (throwing) decoder: bytes invalid in the detected encoding (e.g. a Windows-1252 file, a stray Windows-1252 byte behind a UTF-8 BOM, a lone surrogate in UTF-16; the loader reads all of these with U+FFFD replacement, so the stale check alone would pass) are refused as `UnsupportedEncoding` instead of being re-encoded. Never decode through `StreamReader` with BOM detection: it swaps in its own lossy encoding when a BOM is present. `File.Move` replaces a symlink at the target with a regular file and drops Unix mode bits; both accepted, out of scope. **Never `File.Replace` and never a backup file**: almcp's `ProjectWatcher` (`*.al` filter, Renamed handler checks `EndsWith(".al")`) treats a move-over as an in-place update that keeps the `DocumentId`, while replace-with-backup makes it remove and re-add the document. Every `FileWriteConflict` has a `Kind` (serialized as `kind`): `StaleFile`, `UnsupportedEncoding`, `ReadFailed`, `WriteFailed`, `RolledBack`, `NotWritten`; `apply_fix` returns it as `error`. `apply_fix_all` is two-phase: stale/deleted/unreadable/invalidly-encoded files, and files whose fixed text the strict encoder cannot encode (also `UnsupportedEncoding`), drop out as conflicts first, then the rest are committed one by one; a commit failure (any exception; a cancellation is rethrown after the rollback) restores every file already committed in that call from the original bytes held in memory (same temp + move path) and reports the whole staged set in `conflicts` with `applied: false`. A rollback that itself fails, or a file modified after this call wrote it (rollback compares current bytes with what was written and skips the restore), is named in `message` and that file stays in `filesChanged`. `LoadProjectAsync` and `RefreshFromDiskAsync` sweep stray `*.alcops.tmp` files outside `.alpackages` (`ProjectLoader.SweepTempFiles`); the refresh only removes files older than 30 s because tool calls run concurrently and a write may be between temp-create and move. +- All tool methods are `static async Task`, receiving DI services as parameters. Success goes through `ToolResults.Ok(payload)` (`IsError` left unset); errors only through `ToolErrors`. No tool calls `JsonSerializer.Serialize` itself or builds an anonymous `error` object. +- Every error sets `IsError = true` and carries one `ToolError` envelope `{ error, message, reason?, candidates?, filePath?, diagnosticId?, detail? }` (optional fields omitted when null via per-property `JsonIgnore`; `JsonDefaults.Options` is not changed). `error` is one of `Invalid`, `NotFound`, `Ambiguous`, `Stale`, `Unavailable`, `Faulted`; `NotFound` reasons are the `FixNotFoundReason` names, `Unavailable` reasons the `UnavailableReason` constants. The README `## Errors` section is the public table. Every tool body ends in `catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; } catch (Exception ex) { return ToolErrors.Faulted(ex); }`: a cancellation propagates, anything else becomes `Faulted` with `detail` = the exception's full type name. +- `get_fixes` returns `{ diagnosticId, filePath, line, column, fixes }` with a non-empty `fixes`; every no-match case is `NotFound` with a reason (`CodeFixRunner` returns `FixLookupResult` / `FixApplyResult` / `FixAllResult` with a `FixNotFoundReason`). Ruleset suppression is checked before analysis; pragma suppression is told apart from no diagnostic because `GetEffectiveDiagnostics` keeps pragma-suppressed diagnostics with `IsSuppressed = true`. Equivalence keys are compared through `KeyOf(action) => action.EquivalenceKey ?? ""` on both the advertising and the matching side, so every advertised key round-trips. `candidates` are de-duplicated by key, first occurrence wins. +- `list_rules` is read-only. `get_fixes`, `apply_fix` and `apply_fix_all` re-read changed `.al` files from disk before computing a fix (via `RefreshFromDiskAsync`); the write only proceeds if the file still equals the text the fix was computed from. `apply_fix` returns the error `Stale` on a conflict; `apply_fix_all` writes the non-conflicting files and lists the rest in `conflicts`. There is no explicit reload after writing — the next `GetOrLoadProjectAsync` call picks the written file up via the same refresh. +- Writes go through `GuardedFileWriter`: read the file as bytes → decode with BOM detection → ordinal compare with the expected text → write preamble + new text to a sibling `..alcops.tmp` → `File.Move(temp, path, overwrite: true)`; the temp is deleted in `finally`. The encoding is detected at write time from that read, not stored on `TrackedDocument`. The BOM is sniffed explicitly (UTF-32 LE before UTF-16 LE) and every branch, BOM-less UTF-8 included, uses a strict (throwing) decoder: bytes invalid in the detected encoding (e.g. a Windows-1252 file, a stray Windows-1252 byte behind a UTF-8 BOM, a lone surrogate in UTF-16; the loader reads all of these with U+FFFD replacement, so the stale check alone would pass) are refused as `UnsupportedEncoding` instead of being re-encoded. Never decode through `StreamReader` with BOM detection: it swaps in its own lossy encoding when a BOM is present. `File.Move` replaces a symlink at the target with a regular file and drops Unix mode bits; both accepted, out of scope. **Never `File.Replace` and never a backup file**: almcp's `ProjectWatcher` (`*.al` filter, Renamed handler checks `EndsWith(".al")`) treats a move-over as an in-place update that keeps the `DocumentId`, while replace-with-backup makes it remove and re-add the document. Every `FileWriteConflict` has a `Kind` (serialized as `kind`): `StaleFile`, `UnsupportedEncoding`, `ReadFailed`, `WriteFailed`, `RolledBack`, `NotWritten`; `apply_fix_all` reports it verbatim in `conflicts[].kind`, `apply_fix` maps `StaleFile` to the error `Stale` and every other kind to `Faulted` with `reason` = the kind (an exception thrown by the write itself is `Faulted` / `WriteFailed`). `apply_fix_all` is two-phase: stale/deleted/unreadable/invalidly-encoded files, and files whose fixed text the strict encoder cannot encode (also `UnsupportedEncoding`), drop out as conflicts first, then the rest are committed one by one; a commit failure (any exception; a cancellation is rethrown after the rollback) restores every file already committed in that call from the original bytes held in memory (same temp + move path) and reports the whole staged set in `conflicts` with `applied: false`. That rolled-back batch is a normal (non-`isError`) result with the failure in `message`, because the per-file outcome is in the body; only argument, lookup and single-file write failures use the error envelope. A rollback that itself fails, or a file modified after this call wrote it (rollback compares current bytes with what was written and skips the restore), is named in `message` and that file stays in `filesChanged`. `LoadProjectAsync` and `RefreshFromDiskAsync` sweep stray `*.alcops.tmp` files outside `.alpackages` (`ProjectLoader.SweepTempFiles`); the refresh only removes files older than 30 s because tool calls run concurrently and a write may be between temp-create and move. - `al_compile` defaults to `onlyErrors: true` while nearly every ALCops rule is a warning — callers must pass `options.onlyErrors: false` (the flag lives inside almcp's `options` object; a top-level `onlyErrors` is ignored, and omitting `options` entirely also yields `onlyErrors: false` with no diagnostics cap). This is documented rather than patched, because `ForwardAsync` stays a generic passthrough. - After `apply_fix` / `apply_fix_all`, verify with `analyze` (preferred) or `al_compile` (`options.onlyErrors: false`), not `al_getdiagnostics`. almcp's `ProjectWatcher` (`FileSystemWatcher`) re-reads changed `.al` files, and `al_compile` awaits `WaitForProcessingAsync` before compiling, so it normally picks up on-disk changes before the compile starts. The gate starts signalled and has no debounce, so on slow file systems or right after a large `apply_fix_all` a second `al_compile` may be needed if the watcher has not yet delivered the change notification. `al_getdiagnostics` returns cached compilation results without re-analyzing and will report stale diagnostics. `al_build` does not await the watcher at all. -- `analyze` is the one native tool that goes through `AlMcpProxy.ForwardAsync` (`al_compile` with nested `options.onlyErrors=false`, `enableCodeAnalysis=true`, `maxDiagnosticsPerCompilation=int.MaxValue`, never `codeAnalyzers`); it resolves the proxy via `IServiceProvider` because under `--no-proxy` the type is unregistered and the SDK would otherwise expose it as a tool argument; it returns `{error:"ProxyUnavailable"}` in that case. +- `analyze` is the one native tool that goes through `AlMcpProxy.ForwardAsync` (`al_compile` with nested `options.onlyErrors=false`, `enableCodeAnalysis=true`, `maxDiagnosticsPerCompilation=int.MaxValue`, never `codeAnalyzers`); it resolves the proxy via `IServiceProvider` because under `--no-proxy` the type is unregistered and the SDK would otherwise expose it as a tool argument; it returns `Unavailable` / `NoProxy` in that case, `AlmcpNotFound` when almcp is not installed, `AlmcpNotReady` when it failed to start, and `AlmcpCallFailed` (almcp text in `detail`) when the proxied call itself reports an error. - `analyze` inherits the FileSystemWatcher staleness caveat after `apply_fix` / `apply_fix_all`. ## Conventions diff --git a/README.md b/README.md index eba7e4a..e55adf5 100644 --- a/README.md +++ b/README.md @@ -52,9 +52,9 @@ The native tools (`list_rules`, `get_fixes`, `apply_fix`, `apply_fix_all`) work | Tool | Description | |------|-------------| | `list_rules` | List analyzer rules with metadata (ID, title, severity, category, cop). | -| `get_fixes` | Get available code fixes for a specific diagnostic at a location. | -| `apply_fix` | Apply a code fix to resolve a diagnostic. Writes the fixed content to disk unless the file changed after the fix was computed (`StaleFile`). The write is atomic (sibling temp file renamed over the target) and keeps the file's encoding, BOM and line endings. | -| `apply_fix_all` | Apply a code fix to every occurrence of a diagnostic rule across a project or a single file (like VS Code's "Fix all in workspace"). Writes to disk unless `dryRun` is set. Files that changed on disk mid-operation are skipped and listed in `conflicts`; the rest are written as one batch — if any write fails, every file already written in that call is restored and nothing from the batch is kept. | +| `get_fixes` | Get available code fixes for a specific diagnostic at a location. Returns `{ diagnosticId, filePath, line, column, fixes: [{ equivalenceKey, title, providerName }] }`; when nothing matches, the error `NotFound` with a `reason` (see [Errors](#errors)). | +| `apply_fix` | Apply a code fix to resolve a diagnostic. Writes the fixed content to disk unless the file changed after the fix was computed (error `Stale`); a file that cannot be read, decoded or written is `Faulted` with `reason` `ReadFailed`, `UnsupportedEncoding` or `WriteFailed`. The write is atomic (sibling temp file renamed over the target) and keeps the file's encoding, BOM and line endings. | +| `apply_fix_all` | Apply a code fix to every occurrence of a diagnostic rule across a project or a single file (like VS Code's "Fix all in workspace"). Writes to disk unless `dryRun` is set. A rule with several distinct fixes and no `equivalenceKey` is the error `Ambiguous` with `candidates`. Zero occurrences is a success (`applied: false`, `diagnosticsFound: 0`), except a rule the project ruleset suppresses (`NotFound` with `reason: SuppressedByRuleset`) or one that no loaded analyzer reports (`NotFound` with `reason: NoAnalyzerForRule`). Files that changed on disk mid-operation are skipped and listed in `conflicts`; the rest are written as one batch — if any write fails, every file already written in that call is restored and nothing from the batch is kept. A batch that fails and is rolled back is a normal (non-`isError`) result with `applied: false`, the failure in `message` and every staged file in `conflicts`, because the per-file outcome is in the body; only argument, lookup and single-file write failures use the error envelope. | | `analyze` | Compile with all configured analyzers and return structured cop + compiler diagnostics (analyzer, hasFix, filters, summary). Wraps `al_compile` with `onlyErrors: false`; needs `almcp`. | ### Proxied from Microsoft's `almcp` @@ -73,7 +73,35 @@ After `apply_fix` or `apply_fix_all`, call `analyze` (preferred) or `al_compile` **Editing between calls:** `get_fixes`, `apply_fix` and `apply_fix_all` re-read `.al` files that changed on disk before every call (only changed files are re-parsed), so you can edit files between `get_fixes` and `apply_fix`, and neither tool will overwrite a file that no longer matches the text its fix was computed from. -**How files are written:** each fixed file is written to a sibling `.al..alcops.tmp` and then renamed over the original, so a crash or a full disk never leaves a truncated `.al` file. The file's encoding is detected from its byte-order mark and kept: UTF-8 with or without BOM, UTF-16 and UTF-32 come back exactly as they were, and line endings are untouched. Each file is decoded strictly in its detected encoding (UTF-8 when there is no BOM): a file holding bytes that are invalid in that encoding, such as a legacy Windows-1252 file or a stray Windows-1252 byte in a UTF-8 file with BOM, is never re-encoded; the fix is refused with `UnsupportedEncoding` instead. Every entry in `apply_fix_all`'s `conflicts` carries a `kind` (`StaleFile`, `UnsupportedEncoding`, `ReadFailed`, `WriteFailed`, `RolledBack`, `NotWritten`); `apply_fix` returns the same value as its `error`. No backup files are created. A stray `*.alcops.tmp` left by an interrupted write is removed the next time the project is loaded or refreshed. +**How files are written:** each fixed file is written to a sibling `.al..alcops.tmp` and then renamed over the original, so a crash or a full disk never leaves a truncated `.al` file. The file's encoding is detected from its byte-order mark and kept: UTF-8 with or without BOM, UTF-16 and UTF-32 come back exactly as they were, and line endings are untouched. Each file is decoded strictly in its detected encoding (UTF-8 when there is no BOM): a file holding bytes that are invalid in that encoding, such as a legacy Windows-1252 file or a stray Windows-1252 byte in a UTF-8 file with BOM, is never re-encoded; the fix is refused with `UnsupportedEncoding` instead. Every entry in `apply_fix_all`'s `conflicts` carries a `kind` (`StaleFile`, `UnsupportedEncoding`, `ReadFailed`, `WriteFailed`, `RolledBack`, `NotWritten`); `apply_fix` reports `StaleFile` as the error `Stale` and the other kinds as `Faulted` with that kind as `reason`. No backup files are created. A stray `*.alcops.tmp` left by an interrupted write is removed the next time the project is loaded or refreshed. + +## Errors + +Every native tool reports a failure the same way: the MCP result has `isError: true`, and its single text block holds one JSON envelope. A successful result never sets `isError`. The full JSON stays in the text block, so a client that hides error results still has the message. + +```json +{ "error": "NotFound", "message": "No LC0020 at C:\\src\\MyPage.al:12:17; re-run analyze and use its line/column.", "reason": "NoDiagnosticAtPosition", "filePath": "C:\\src\\MyPage.al", "diagnosticId": "LC0020" } +``` + +| Field | Present | +|-------|---------| +| `error` | Always; one of the codes below. | +| `message` | Always; human-readable. | +| `reason` | For `NotFound`, `Unavailable`, and a `Faulted` write. | +| `candidates` | For `Ambiguous` and `NotFound` / `NoFixForEquivalenceKey`: `[{ equivalenceKey, title, providerName }]`. | +| `filePath`, `diagnosticId` | When the error concerns one file or rule. | +| `detail` | For `Faulted` from an exception (its full type name) and `Unavailable` / `AlmcpCallFailed` (the text almcp returned). | + +| `error` | Meaning | What to do | `reason` | +|---------|---------|------------|----------| +| `Invalid` | The arguments are unusable as given: unknown `scope`, missing `filePath`, `limit` not positive, a `projectPath` that is not an AL project folder, or (for `analyze` and `list_rules`) not one of the projects the server was started with. | Fix the call. | – | +| `NotFound` | Nothing matched. | `NoDiagnosticAtPosition`: re-run `analyze` and use its line/column. `NoFixForEquivalenceKey`: pick a key from `candidates`. Otherwise stop; there is nothing to fix. | `NoFixProvider`, `FileNotInProject`, `NoAnalyzerForRule`, `SuppressedByRuleset`, `SuppressedByPragma`, `NoDiagnosticAtPosition`, `NoFixForDiagnostic`, `NoFixForEquivalenceKey` | +| `Ambiguous` | The rule offers several distinct fixes; `candidates` lists them. | Pass one `equivalenceKey` verbatim, or ask the user. | – | +| `Stale` | The file changed or was deleted after the fix was computed; nothing was written. | Re-run the call. | – | +| `Unavailable` | A dependency is down. | Report it; do not retry in a loop. | `NoProxy` (`--no-proxy`), `AlmcpNotFound`, `AlmcpNotReady`, `AlmcpCallFailed` (also covers errors `al_compile` itself reports) | +| `Faulted` | An unexpected exception, or a write that was refused or failed. | Report it, including `message`. | `UnsupportedEncoding`, `ReadFailed`, `WriteFailed` for writes; none for exceptions | + +`candidates` holds one entry per distinct `equivalenceKey`: when two providers offer the same key, the first one wins, and that is also the one a key match applies. A key is `""` when the provider sets none; pass it back as is. The proxied `al_*` tools are not covered by this envelope: their errors pass through from `almcp` unchanged. ## Analyzers diff --git a/src/ALCops.Mcp/Models/CodeFixInfo.cs b/src/ALCops.Mcp/Models/CodeFixInfo.cs index 7541c56..a310fac 100644 --- a/src/ALCops.Mcp/Models/CodeFixInfo.cs +++ b/src/ALCops.Mcp/Models/CodeFixInfo.cs @@ -1,7 +1,13 @@ namespace ALCops.Mcp.Models; +/// +/// One code fix offered for a diagnostic: the element type of both the fixes of get_fixes and +/// the candidates of an error. candidates holds one entry per distinct key (first wins); +/// fixes lists every registered action. +/// is the action's key, or "" when the provider set none; pass it +/// back verbatim. +/// public record CodeFixInfo( - string Title, string EquivalenceKey, - string DiagnosticId, + string Title, string ProviderName); diff --git a/src/ALCops.Mcp/Models/FileWriteConflict.cs b/src/ALCops.Mcp/Models/FileWriteConflict.cs index af39a9a..ca6d9b0 100644 --- a/src/ALCops.Mcp/Models/FileWriteConflict.cs +++ b/src/ALCops.Mcp/Models/FileWriteConflict.cs @@ -5,7 +5,9 @@ namespace ALCops.Mcp.Models; /// StaleFile (changed on disk or deleted), UnsupportedEncoding (the file is not valid in its detected encoding, or the fixed text cannot be encoded in it), /// ReadFailed (could not be read for the stale check), WriteFailed (the file whose commit threw), /// RolledBack (committed, then restored) or NotWritten (staged but never attempted). -/// Serialized as kind; this vocabulary is additive and may be renamed by issue #41. +/// Serialized as kind. apply_fix_all reports these kinds verbatim in conflicts[].kind; +/// apply_fix maps StaleFile to the Stale error and every other kind to Faulted +/// with reason = the kind. /// public record FileWriteConflict(string FilePath, string Message, string Kind = FileWriteConflictKind.StaleFile); diff --git a/src/ALCops.Mcp/Models/FixAllResult.cs b/src/ALCops.Mcp/Models/FixAllResult.cs index fb692ae..01d5cce 100644 --- a/src/ALCops.Mcp/Models/FixAllResult.cs +++ b/src/ALCops.Mcp/Models/FixAllResult.cs @@ -9,8 +9,10 @@ public enum FixAllStatus /// The fix-all pass ran; see Changes/Unfixed for what happened (possibly zero changes). Completed, NoDiagnosticsFound, - NoFixAvailable, - AmbiguousFix + /// No fix could be chosen; says why. + NotFound, + /// Several distinct fixes apply and no equivalence key was given; see . + Ambiguous } /// One file whose content changed (or would change, if dryRun) as a result of the fix-all pass. @@ -19,6 +21,11 @@ public record FixAllFileChange(string FilePath, string OriginalContent, string M /// A diagnostic matching the requested rule that no available fix could resolve. public record FixAllUnfixedDiagnostic(string FilePath, int Line, int Column); +/// +/// The distinct fixes offered for the probe diagnostic, filled for +/// and for ; empty otherwise. +/// +/// Set when is . public record FixAllResult( FixAllStatus Status, string DiagnosticId, @@ -26,5 +33,6 @@ public record FixAllResult( string? FixTitle, string? EquivalenceKey, IReadOnlyList Changes, - IReadOnlyList AvailableEquivalenceKeys, - IReadOnlyList Unfixed); + IReadOnlyList Candidates, + IReadOnlyList Unfixed, + FixNotFoundReason? NotFoundReason = null); diff --git a/src/ALCops.Mcp/Models/FixLookupResult.cs b/src/ALCops.Mcp/Models/FixLookupResult.cs new file mode 100644 index 0000000..e27d267 --- /dev/null +++ b/src/ALCops.Mcp/Models/FixLookupResult.cs @@ -0,0 +1,52 @@ +using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics; + +namespace ALCops.Mcp.Models; + +/// +/// Why a fix lookup found nothing. Serialized by name as the reason of a NotFound error, +/// so the member names are part of the tool contract. +/// +public enum FixNotFoundReason +{ + /// No loaded analyzer assembly registers a code fix provider for the rule. + NoFixProvider, + /// The file is not a document of the project. + FileNotInProject, + /// A fix provider exists, but no loaded analyzer reports the rule. + NoAnalyzerForRule, + /// The project's ruleset sets the rule to None. + SuppressedByRuleset, + /// The diagnostic exists at the position but a #pragma warning disable suppresses it. + SuppressedByPragma, + /// The rule is not reported at that line/column (nor anywhere on that line). + NoDiagnosticAtPosition, + /// The diagnostic was found, but no provider registered an applicable fix for it. + NoFixForDiagnostic, + /// Fixes exist, but none has the requested equivalence key. + NoFixForEquivalenceKey, +} + +/// Outcome of CodeFixRunner.GetFixesAsync: either fixes, or the reason there are none. +public record FixLookupResult(IReadOnlyList Fixes, FixNotFoundReason? NotFoundReason) +{ + public static FixLookupResult Found(IReadOnlyList fixes) => new(fixes, null); + public static FixLookupResult NotFound(FixNotFoundReason reason) => new([], reason); +} + +/// +/// Outcome of CodeFixRunner.ApplyFixAsync: the computed fix, or the reason there is none. +/// is filled for . +/// , when set, replaces the shared per-reason message of the NotFound error. +/// +public record FixApplyResult(CodeFixResult? Fix, FixNotFoundReason? NotFoundReason, IReadOnlyList Candidates) +{ + public string? MessageOverride { get; init; } + + public static FixApplyResult Applied(CodeFixResult fix) => new(fix, null, []); + + public static FixApplyResult NotFound(FixNotFoundReason reason, IReadOnlyList? candidates = null) => + new(null, reason, candidates ?? []); +} + +/// A diagnostic found at a position, or the reason none was. +internal readonly record struct DiagnosticLookup(Diagnostic? Diagnostic, FixNotFoundReason? Reason); diff --git a/src/ALCops.Mcp/Models/GetFixesResult.cs b/src/ALCops.Mcp/Models/GetFixesResult.cs new file mode 100644 index 0000000..0d8c978 --- /dev/null +++ b/src/ALCops.Mcp/Models/GetFixesResult.cs @@ -0,0 +1,9 @@ +namespace ALCops.Mcp.Models; + +/// Success result of get_fixes. is never empty. +public record GetFixesResult( + string DiagnosticId, + string FilePath, + int Line, + int Column, + IReadOnlyList Fixes); diff --git a/src/ALCops.Mcp/Models/ToolError.cs b/src/ALCops.Mcp/Models/ToolError.cs new file mode 100644 index 0000000..bd2ce9c --- /dev/null +++ b/src/ALCops.Mcp/Models/ToolError.cs @@ -0,0 +1,47 @@ +using System.Text.Json.Serialization; + +namespace ALCops.Mcp.Models; + +/// +/// The single error envelope every native tool returns (with isError: true). Only +/// and are always present; the optional fields are omitted +/// when null. The parameter order is the JSON key order. +/// +public sealed record ToolError( + string Error, + string Message, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? Reason = null, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] IReadOnlyList? Candidates = null, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? FilePath = null, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? DiagnosticId = null, + [property: JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? Detail = null); + +/// The fixed vocabulary of . +public static class ToolErrorCode +{ + /// The arguments are unusable as given; fix the call. + public const string Invalid = "Invalid"; + /// Nothing matched; reason is a name. + public const string NotFound = "NotFound"; + /// Several distinct fixes apply; candidates lists them. + public const string Ambiguous = "Ambiguous"; + /// The file changed or was deleted after the fix was computed; nothing was written. + public const string Stale = "Stale"; + /// A dependency is down; reason is an value. + public const string Unavailable = "Unavailable"; + /// An unexpected exception, or a refused or failed write (reason = the write kind). + public const string Faulted = "Faulted"; +} + +/// The reason values of an error. +public static class UnavailableReason +{ + /// The server runs with --no-proxy. + public const string NoProxy = "NoProxy"; + /// almcp is not in the DevTools directory. + public const string AlmcpNotFound = "AlmcpNotFound"; + /// almcp failed to start, or has been stopped. + public const string AlmcpNotReady = "AlmcpNotReady"; + /// The proxied call failed or almcp reported an error; detail carries the almcp text. + public const string AlmcpCallFailed = "AlmcpCallFailed"; +} diff --git a/src/ALCops.Mcp/Services/CodeFixRunner.cs b/src/ALCops.Mcp/Services/CodeFixRunner.cs index 20286cd..e62ff71 100644 --- a/src/ALCops.Mcp/Services/CodeFixRunner.cs +++ b/src/ALCops.Mcp/Services/CodeFixRunner.cs @@ -12,9 +12,9 @@ namespace ALCops.Mcp.Services; public sealed class CodeFixRunner { /// - /// Gets available code fixes for a specific diagnostic at a location. + /// Gets available code fixes for a specific diagnostic at a location, or the reason there are none. /// - public async Task> GetFixesAsync( + public async Task GetFixesAsync( ProjectSession session, string filePath, string diagnosticId, @@ -23,75 +23,145 @@ public async Task> GetFixesAsync( IAnalyzerProvider analyzerProvider, CancellationToken ct = default) { - var providers = analyzerProvider.GetCodeFixProvidersForDiagnostic(diagnosticId); - if (providers.IsEmpty) - return []; + var (located, notFound) = await LocateAsync(session, filePath, diagnosticId, line, column, analyzerProvider, ct); + if (notFound is { } reason) + return FixLookupResult.NotFound(reason); - var document = session.GetDocument(filePath); - if (document is null) - return []; + var (document, diagnostic, providers) = located!.Value; + var pairs = await CollectActionsAsync(providers, document, diagnostic, ct); + if (pairs.Count == 0) + return FixLookupResult.NotFound(FixNotFoundReason.NoFixForDiagnostic); - // Find the diagnostic at the specified location - var diagnostic = await FindDiagnosticAsync(session, document, diagnosticId, line, column, ct, analyzerProvider); - if (diagnostic is null) - return []; + return FixLookupResult.Found([.. pairs.Select(p => ToInfo(p.Provider, p.Action))]); + } - // Collect code actions from all providers - var fixes = new List(); + /// + /// Applies a specific code fix and returns the modified content without writing to disk, or the + /// reason no fix applies. + /// + public async Task ApplyFixAsync( + ProjectSession session, + string filePath, + string diagnosticId, + int line, + int column, + string equivalenceKey, + IAnalyzerProvider analyzerProvider, + CancellationToken ct = default) + { + var (located, notFound) = await LocateAsync(session, filePath, diagnosticId, line, column, analyzerProvider, ct); + if (notFound is { } reason) + return FixApplyResult.NotFound(reason); - foreach (var fixProvider in providers) - { - var actions = new List(); + var (document, diagnostic, providers) = located!.Value; + var pairs = await CollectActionsAsync(providers, document, diagnostic, ct); + if (pairs.Count == 0) + return FixApplyResult.NotFound(FixNotFoundReason.NoFixForDiagnostic); - var context = new CodeFixContext( - document, - diagnostic.Location.SourceSpan, - ImmutableArray.Create(diagnostic), - (action, _) => actions.Add(action), - ct); + var matching = pairs.Where(p => KeyOf(p.Action) == equivalenceKey).ToList(); + if (matching.Count == 0) + return FixApplyResult.NotFound(FixNotFoundReason.NoFixForEquivalenceKey, ToCandidates(pairs)); - await fixProvider.RegisterCodeFixesAsync(context); + var originalText = (await document.GetTextAsync(ct)).ToString(); - foreach (var action in actions) + foreach (var (_, matchingAction) in matching) + { + // Apply the code action to get the modified document + var operations = await matchingAction.GetOperationsAsync(ct); + + foreach (var operation in operations) { - fixes.Add(new CodeFixInfo( - Title: action.Title, - EquivalenceKey: action.EquivalenceKey ?? "", - DiagnosticId: diagnosticId, - ProviderName: fixProvider.GetType().Name)); + if (operation is ApplyChangesOperation applyChanges) + { + var changedSolution = applyChanges.ChangedSolution; + var changedDocument = changedSolution.GetDocument(document.Id); + if (changedDocument is null) + continue; + + var newText = await changedDocument.GetTextAsync(ct); + var modifiedContent = newText?.ToString() ?? ""; + + return FixApplyResult.Applied(new CodeFixResult( + FilePath: filePath, + OriginalContent: originalText, + ModifiedContent: modifiedContent, + FixTitle: matchingAction.Title)); + } } } - return fixes; + // The key matched, but no matching action changes this document. Same reason as "no fix at + // all", but get_fixes did advertise this key, so the message says what actually happened. + return FixApplyResult.NotFound(FixNotFoundReason.NoFixForDiagnostic) with + { + MessageOverride = + $"A fix with equivalence key '{equivalenceKey}' exists for {diagnosticId} at {filePath}:{line}:{column} " + + "but produced no change in this document (it may edit another file, which apply_fix does not support yet)." + }; } /// - /// Applies a specific code fix and returns the modified content without writing to disk. + /// The equivalence key a caller sees and passes back. A null key is advertised as "", so + /// every comparison goes through this too; otherwise a null-key action could never be selected. + /// + private static string KeyOf(CodeAction action) => action.EquivalenceKey ?? ""; + + private static CodeFixInfo ToInfo(CodeFixProvider provider, CodeAction action) => + new(KeyOf(action), action.Title, provider.GetType().Name); + + /// + /// One candidate per distinct equivalence key, in registration order. When two providers (or two + /// actions) share a key, the first occurrence wins: that is also the one a key match selects. + /// + private static IReadOnlyList ToCandidates(IEnumerable<(CodeFixProvider Provider, CodeAction Action)> pairs) => + [.. pairs + .GroupBy(p => KeyOf(p.Action), StringComparer.Ordinal) + .Select(g => ToInfo(g.First().Provider, g.First().Action))]; + + /// True when the project ruleset sets to None. + private static bool IsRulesetSuppressed(IAnalyzerProvider provider, string diagnosticId) => + provider is AnalyzerSet { RuleActions: var ruleActions } && RulesetFilter.IsSuppressed(ruleActions, diagnosticId, out _); + + /// + /// The shared front half of get_fixes and apply_fix: a fix provider, the document, no ruleset + /// suppression, and the diagnostic at the position, in that order. Returns the first failing reason. /// - public async Task ApplyFixAsync( + private static async Task<((Document Document, Diagnostic Diagnostic, ImmutableArray Providers)? Located, FixNotFoundReason? NotFound)> LocateAsync( ProjectSession session, string filePath, string diagnosticId, int line, int column, - string equivalenceKey, IAnalyzerProvider analyzerProvider, - CancellationToken ct = default) + CancellationToken ct) { var providers = analyzerProvider.GetCodeFixProvidersForDiagnostic(diagnosticId); if (providers.IsEmpty) - return null; + return (null, FixNotFoundReason.NoFixProvider); var document = session.GetDocument(filePath); if (document is null) - return null; + return (null, FixNotFoundReason.FileNotInProject); + + if (IsRulesetSuppressed(analyzerProvider, diagnosticId)) + return (null, FixNotFoundReason.SuppressedByRuleset); - // Find the diagnostic - var diagnostic = await FindDiagnosticAsync(session, document, diagnosticId, line, column, ct, analyzerProvider); - if (diagnostic is null) - return null; + var lookup = await FindDiagnosticAsync(session, document, diagnosticId, line, column, ct, analyzerProvider); + if (lookup.Reason is { } reason) + return (null, reason); + + return ((document, lookup.Diagnostic!, providers), null); + } + + /// Every code action every provider registers for , in provider order. + private static async Task> CollectActionsAsync( + ImmutableArray providers, + Document document, + Diagnostic diagnostic, + CancellationToken ct) + { + var pairs = new List<(CodeFixProvider Provider, CodeAction Action)>(); - // Find the matching code action foreach (var fixProvider in providers) { var actions = new List(); @@ -105,39 +175,11 @@ public async Task> GetFixesAsync( await fixProvider.RegisterCodeFixesAsync(context); - var matchingAction = actions.FirstOrDefault(a => - string.Equals(a.EquivalenceKey, equivalenceKey, StringComparison.Ordinal)); - - if (matchingAction is null) - continue; - - var originalText = (await document.GetTextAsync(ct)).ToString(); - - // Apply the code action to get the modified document - var operations = await matchingAction.GetOperationsAsync(ct); - - foreach (var operation in operations) - { - if (operation is ApplyChangesOperation applyChanges) - { - var changedSolution = applyChanges.ChangedSolution; - var changedDocument = changedSolution.GetDocument(document.Id); - if (changedDocument is null) - continue; - - var newText = await changedDocument.GetTextAsync(ct); - var modifiedContent = newText?.ToString() ?? ""; - - return new CodeFixResult( - FilePath: filePath, - OriginalContent: originalText, - ModifiedContent: modifiedContent, - FixTitle: matchingAction.Title); - } - } + foreach (var action in actions) + pairs.Add((fixProvider, action)); } - return null; + return pairs; } /// @@ -158,10 +200,22 @@ public async Task ApplyFixAllAsync( { var providers = analyzerProvider.GetCodeFixProvidersForDiagnostic(diagnosticId); if (providers.IsEmpty) - return NoFixAvailable(diagnosticId); + return NotFound(diagnosticId, FixNotFoundReason.NoFixProvider); + + // Checked up front so a suppressed rule is reported as such rather than as zero occurrences. + if (IsRulesetSuppressed(analyzerProvider, diagnosticId)) + return NotFound(diagnosticId, FixNotFoundReason.SuppressedByRuleset); + + // Same for a rule no loaded analyzer reports: get_fixes and apply_fix say NoAnalyzerForRule, and + // "zero occurrences" would wrongly tell the caller the code is clean. + if (!analyzerProvider.GetAllAnalyzers().Any(a => a.SupportedDiagnostics.Any(d => d.Id == diagnosticId))) + return NotFound(diagnosticId, FixNotFoundReason.NoAnalyzerForRule); var normalizedFilePath = filePath is null ? null : Path.GetFullPath(filePath); + if (scope == FixAllScope.Document && session.GetDocument(normalizedFilePath!) is null) + return NotFound(diagnosticId, FixNotFoundReason.FileNotInProject); + var diagnostics = await CollectDiagnosticsForRuleAsync(session, diagnosticId, normalizedFilePath, ct, analyzerProvider); if (diagnostics.IsEmpty) return new FixAllResult(FixAllStatus.NoDiagnosticsFound, diagnosticId, 0, null, null, [], [], []); @@ -184,34 +238,30 @@ public async Task ApplyFixAllAsync( } if (candidateActions.Count == 0) - return NoFixAvailable(diagnosticId, diagnostics.Length); + return NotFound(diagnosticId, FixNotFoundReason.NoFixForDiagnostic, diagnostics.Length); (CodeFixProvider Provider, CodeAction Action) chosen; if (equivalenceKey is not null) { - var match = candidateActions.FirstOrDefault(c => - string.Equals(c.Action.EquivalenceKey, equivalenceKey, StringComparison.Ordinal)); + var match = candidateActions.FirstOrDefault(c => KeyOf(c.Action) == equivalenceKey); if (match.Action is null) - return NoFixAvailable(diagnosticId, diagnostics.Length); + return NotFound(diagnosticId, FixNotFoundReason.NoFixForEquivalenceKey, diagnostics.Length, + ToCandidates(candidateActions)); chosen = match; } else { - var distinctKeys = candidateActions - .Select(c => c.Action.EquivalenceKey ?? "") - .Distinct(StringComparer.Ordinal) - .ToList(); - - if (distinctKeys.Count > 1) + var candidates = ToCandidates(candidateActions); + if (candidates.Count > 1) return new FixAllResult( - FixAllStatus.AmbiguousFix, diagnosticId, diagnostics.Length, - null, null, [], distinctKeys, []); + FixAllStatus.Ambiguous, diagnosticId, diagnostics.Length, + null, null, [], candidates, []); chosen = candidateActions[0]; } var fixProviderInstance = chosen.Provider; - var chosenKey = chosen.Action.EquivalenceKey ?? ""; + var chosenKey = KeyOf(chosen.Action); var fixTitle = chosen.Action.Title; var project = session.GetProject(); @@ -304,8 +354,9 @@ public async Task ApplyFixAllAsync( changes, [], unfixed); } - private static FixAllResult NoFixAvailable(string diagnosticId, int diagnosticsFound = 0) => - new(FixAllStatus.NoFixAvailable, diagnosticId, diagnosticsFound, null, null, [], [], []); + private static FixAllResult NotFound( + string diagnosticId, FixNotFoundReason reason, int diagnosticsFound = 0, IReadOnlyList? candidates = null) => + new(FixAllStatus.NotFound, diagnosticId, diagnosticsFound, null, null, [], candidates ?? [], [], reason); /// /// Applies fixes to a single document one diagnostic at a time, in descending source-position @@ -330,8 +381,7 @@ private static async Task ApplyIterativeFixesAsync( await fixProvider.RegisterCodeFixesAsync(context); - var match = actions.FirstOrDefault(a => - string.Equals(a.EquivalenceKey, equivalenceKey, StringComparison.Ordinal)); + var match = actions.FirstOrDefault(a => KeyOf(a) == equivalenceKey); if (match is null) continue; @@ -467,7 +517,13 @@ public override Task> GetAllDiagnosticsAsync( Task.FromResult(_all.Where(d => diagnosticIds.Contains(d.Id))); } - private static async Task FindDiagnosticAsync( + /// + /// Finds the diagnostic at a position, or says why there is none. Ruleset suppression is checked by + /// the caller before analysis; pragma suppression is detected here, which relies on + /// GetEffectiveDiagnostics keeping pragma-suppressed diagnostics with IsSuppressed = true + /// rather than dropping them. + /// + private static async Task FindDiagnosticAsync( ProjectSession session, Document document, string diagnosticId, @@ -484,7 +540,7 @@ public override Task> GetAllDiagnosticsAsync( .ToImmutableArray(); if (analyzers.IsEmpty) - return null; + return new DiagnosticLookup(null, FixNotFoundReason.NoAnalyzerForRule); var compilationWithAnalyzers = new CompilationWithAnalyzers( compilation, analyzers, null!, ct); @@ -495,32 +551,67 @@ public override Task> GetAllDiagnosticsAsync( var effectiveDiagnostics = CompilationWithAnalyzers .GetEffectiveDiagnostics(rawDiagnostics, compilation); - // Apply ruleset suppression (RuleAction.None) - var ruleActions = provider is AnalyzerSet analyzerSet ? analyzerSet.RuleActions : null; - - // Filter to the target file and diagnostic ID + // Filter to the target file and diagnostic ID. Suppressed diagnostics are kept here so a + // pragma-suppressed hit can be told apart from no hit at all. var documentPath = document.FilePath ?? ""; var diagnostics = effectiveDiagnostics - .Where(d => !d.IsSuppressed && !RulesetFilter.IsSuppressed(ruleActions, d.Id, out _)) .Where(d => d.Id == diagnosticId && d.Location.SourceTree?.FilePath is string fp && Path.GetFullPath(fp).Equals(Path.GetFullPath(documentPath), StringComparison.OrdinalIgnoreCase)) .ToImmutableArray(); - // Find the diagnostic at or near the specified line/column (1-based input) - return diagnostics.FirstOrDefault(d => - { - var lineSpan = d.Location.GetLineSpan(); - var startLine = lineSpan.StartLinePosition.Line + 1; - var startCol = lineSpan.StartLinePosition.Character + 1; - - return startLine == line && startCol == column; - }) - // Fallback: find any diagnostic with matching ID on the same line - ?? diagnostics.FirstOrDefault(d => - { - var lineSpan = d.Location.GetLineSpan(); - return lineSpan.StartLinePosition.Line + 1 == line; - }); + var (hit, suppressed) = MatchPosition(diagnostics, StartOf, d => d.IsSuppressed, line, column); + if (hit is null) + return new DiagnosticLookup(null, FixNotFoundReason.NoDiagnosticAtPosition); + + return suppressed + ? new DiagnosticLookup(null, FixNotFoundReason.SuppressedByPragma) + : new DiagnosticLookup(hit, null); + } + + /// 1-based start line/column of a diagnostic. + private static (int Line, int Column) StartOf(Diagnostic d) + { + var start = d.Location.GetLineSpan().StartLinePosition; + return (start.Line + 1, start.Character + 1); + } + + /// + /// Picks the candidate at the 1-based /. An exact + /// match wins over a same-line fallback in either set, so a pragma-suppressed diagnostic at the exact + /// position is not answered with a live neighbour on the same line. Order: exact live, exact + /// suppressed, same-line live, same-line suppressed. Suppressed says which set the hit came from. + /// + internal static (T? Hit, bool Suppressed) MatchPosition( + IEnumerable candidates, + Func startOf, + Func isSuppressed, + int line, + int column) + where T : class + { + var all = candidates.ToList(); + var live = all.Where(d => !isSuppressed(d)).ToList(); + var suppressed = all.Where(isSuppressed).ToList(); + + if (ExactlyAt(live, startOf, line, column) is { } exactLive) + return (exactLive, false); + if (ExactlyAt(suppressed, startOf, line, column) is { } exactSuppressed) + return (exactSuppressed, true); + if (OnLine(live, startOf, line) is { } lineLive) + return (lineLive, false); + if (OnLine(suppressed, startOf, line) is { } lineSuppressed) + return (lineSuppressed, true); + return (null, false); } + + /// The first candidate starting exactly at /. + private static T? ExactlyAt(IEnumerable candidates, Func startOf, int line, int column) + where T : class => + candidates.FirstOrDefault(d => startOf(d) == (line, column)); + + /// The first candidate starting anywhere on . + private static T? OnLine(IEnumerable candidates, Func startOf, int line) + where T : class => + candidates.FirstOrDefault(d => startOf(d).Line == line); } diff --git a/src/ALCops.Mcp/Services/ProjectScope.cs b/src/ALCops.Mcp/Services/ProjectScope.cs new file mode 100644 index 0000000..59a4921 --- /dev/null +++ b/src/ALCops.Mcp/Services/ProjectScope.cs @@ -0,0 +1,115 @@ +namespace ALCops.Mcp.Services; + +/// +/// Project argument validation shared by the native tools. A failure is an Invalid error with +/// the returned message. +/// +internal static class ProjectScope +{ + private const string EmptyProjectListMessage = + "No AL project available. Pass projectPath, or start the server from a folder " + + "containing app.json (or use --projects)."; + + /// + /// For tools scoped to the projects this server was started with (analyze, list_rules): + /// resolves , or the primary project when it is null, and rejects a + /// path that is not one of . Returns the + /// normalized project folder, or null with set. + /// + internal static string? Resolve(WorkspaceStartupConfig config, string? projectPath, out string? invalidMessage) + { + var knownProjects = config.ProjectDirectories.Select(Normalize).ToList(); + + if (knownProjects.Count == 0) + { + invalidMessage = EmptyProjectListMessage; + return null; + } + + if (projectPath is null) + { + invalidMessage = null; + return knownProjects[0]; + } + + if (!TryNormalizePath(projectPath, trimTrailingSeparator: true, out var normalized, out invalidMessage, "projectPath")) + return null; + + if (!knownProjects.Contains(normalized, StringComparer.OrdinalIgnoreCase)) + { + invalidMessage = + $"'{projectPath}' is not one of the AL projects this server was started with: " + + $"{string.Join(", ", knownProjects)}. Pass one of those, or restart the server with --projects."; + return null; + } + + invalidMessage = null; + return normalized; + } + + /// + /// For the fix tools, which accept any AL project: the folder must exist and contain app.json. + /// + internal static bool RequireProjectFolder(string projectPath, out string? invalidMessage) + { + if (string.IsNullOrWhiteSpace(projectPath)) + { + invalidMessage = "projectPath is required: the absolute path of the AL project folder (contains app.json)."; + return false; + } + + if (!TryNormalizePath(projectPath, trimTrailingSeparator: false, out var full, out invalidMessage, "projectPath")) + return false; + + if (!Directory.Exists(full)) + { + invalidMessage = $"projectPath '{projectPath}' does not exist. Pass the absolute path of the AL project folder (contains app.json)."; + return false; + } + + if (!File.Exists(Path.Combine(full, "app.json"))) + { + invalidMessage = $"projectPath '{projectPath}' contains no app.json. Pass the AL project folder itself, not a parent or child folder."; + return false; + } + + invalidMessage = null; + return true; + } + + /// + /// (optionally without a trailing separator) for a + /// caller-supplied path. An empty or whitespace-only string, or one + /// rejects (embedded NUL, unsupported format, too long), is an argument error: returns false with + /// set, so the tool reports Invalid rather than Faulted. + /// + internal static bool TryNormalizePath( + string path, + bool trimTrailingSeparator, + out string normalized, + out string? invalidMessage, + string parameterName = "path") + { + normalized = ""; + if (string.IsNullOrWhiteSpace(path)) + { + invalidMessage = $"{parameterName} '{path}' is not a valid path."; + return false; + } + + try + { + var full = Path.GetFullPath(path); + normalized = trimTrailingSeparator ? Path.TrimEndingDirectorySeparator(full) : full; + invalidMessage = null; + return true; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + invalidMessage = $"{parameterName} '{path}' is not a valid path."; + return false; + } + } + + private static string Normalize(string path) => Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)); +} diff --git a/src/ALCops.Mcp/Services/ToolErrors.cs b/src/ALCops.Mcp/Services/ToolErrors.cs new file mode 100644 index 0000000..7318bed --- /dev/null +++ b/src/ALCops.Mcp/Services/ToolErrors.cs @@ -0,0 +1,100 @@ +using System.Text.Json; +using ALCops.Mcp.Models; +using ModelContextProtocol.Protocol; + +namespace ALCops.Mcp.Services; + +/// +/// The only place a native tool error is built. Every result has +/// set and carries a envelope as its single text block, so a client that hides +/// error results still has the full message. +/// +internal static class ToolErrors +{ + /// The arguments are unusable as given. + internal static CallToolResult Invalid(string message) => + Build(new ToolError(ToolErrorCode.Invalid, message)); + + /// Nothing matched; is written by name. + internal static CallToolResult NotFound( + FixNotFoundReason reason, + string message, + string? filePath = null, + string? diagnosticId = null, + IReadOnlyList? candidates = null) => + Build(new ToolError(ToolErrorCode.NotFound, message, reason.ToString(), candidates, filePath, diagnosticId)); + + /// Several distinct fixes apply; the caller has to pick one of . + internal static CallToolResult Ambiguous( + string message, + IReadOnlyList candidates, + string? filePath = null, + string? diagnosticId = null) => + Build(new ToolError(ToolErrorCode.Ambiguous, message, Candidates: candidates, FilePath: filePath, DiagnosticId: diagnosticId)); + + /// The file changed or was deleted after the fix was computed; nothing was written. + internal static CallToolResult Stale(string filePath, string? diagnosticId, string message) => + Build(new ToolError(ToolErrorCode.Stale, message, FilePath: filePath, DiagnosticId: diagnosticId)); + + /// A dependency is down; is an value. + internal static CallToolResult Unavailable(string reason, string message, string? detail = null) => + Build(new ToolError(ToolErrorCode.Unavailable, message, reason, Detail: detail)); + + /// An unexpected exception: message is its message, detail its full type name. + internal static CallToolResult Faulted(Exception ex) => + Build(new ToolError(ToolErrorCode.Faulted, ex.Message, Detail: ex.GetType().FullName)); + + /// A refused or failed write; is the . + internal static CallToolResult Faulted( + string reason, + string message, + string? filePath = null, + string? diagnosticId = null, + string? detail = null) => + Build(new ToolError(ToolErrorCode.Faulted, message, reason, FilePath: filePath, DiagnosticId: diagnosticId, Detail: detail)); + + /// + /// The shared wording of a message, so the three fix tools + /// say the same thing for the same reason. / are + /// null for apply_fix_all, which has no position. + /// + internal static string NotFoundMessage( + FixNotFoundReason reason, + string diagnosticId, + string? filePath, + int? line = null, + int? column = null, + string? equivalenceKey = null) + { + var at = line is null + ? (filePath is null ? "in the project" : $"in {filePath}") + : $"at {filePath}:{line}:{column}"; + + return reason switch + { + FixNotFoundReason.NoFixProvider => + $"No code fix provider is registered for {diagnosticId} by the configured analyzers.", + FixNotFoundReason.FileNotInProject => + $"{filePath} is not an .al file of this project.", + FixNotFoundReason.NoAnalyzerForRule => + $"No loaded analyzer reports {diagnosticId}, so the diagnostic cannot be located. Check al.codeAnalyzers.", + FixNotFoundReason.SuppressedByRuleset => + $"{diagnosticId} is suppressed by the project ruleset (action None); there is nothing to fix.", + FixNotFoundReason.SuppressedByPragma => + $"{diagnosticId} {at} is suppressed by #pragma warning disable; there is nothing to fix.", + FixNotFoundReason.NoDiagnosticAtPosition => + $"No {diagnosticId} {at}; re-run analyze and use its line/column.", + FixNotFoundReason.NoFixForDiagnostic => + $"{diagnosticId} {at} was found, but no code fix applies to it.", + FixNotFoundReason.NoFixForEquivalenceKey => + $"No fix for {diagnosticId} {at} has equivalence key '{equivalenceKey}'. Pass one candidate's equivalenceKey verbatim.", + _ => $"No fix found for {diagnosticId} {at}.", + }; + } + + private static CallToolResult Build(ToolError error) => new() + { + IsError = true, + Content = [new TextContentBlock { Text = JsonSerializer.Serialize(error, JsonDefaults.Options) }] + }; +} diff --git a/src/ALCops.Mcp/Services/ToolResults.cs b/src/ALCops.Mcp/Services/ToolResults.cs new file mode 100644 index 0000000..b0ffb54 --- /dev/null +++ b/src/ALCops.Mcp/Services/ToolResults.cs @@ -0,0 +1,19 @@ +using System.Text.Json; +using ModelContextProtocol.Protocol; + +namespace ALCops.Mcp.Services; + +/// +/// Success results of the native tools. Errors go through only. +/// +internal static class ToolResults +{ + /// + /// The payload serialized with as a single text block. + /// is left unset: only errors set it. + /// + internal static CallToolResult Ok(T payload) => new() + { + Content = [new TextContentBlock { Text = JsonSerializer.Serialize(payload, JsonDefaults.Options) }] + }; +} diff --git a/src/ALCops.Mcp/Tools/AnalyzeTool.cs b/src/ALCops.Mcp/Tools/AnalyzeTool.cs index e55a687..fd5f54c 100644 --- a/src/ALCops.Mcp/Tools/AnalyzeTool.cs +++ b/src/ALCops.Mcp/Tools/AnalyzeTool.cs @@ -14,7 +14,7 @@ public sealed class AnalyzeTool [McpServerTool(Name = "analyze", ReadOnly = true), Description("Compile the AL workspace with all configured analyzers and return cop + compiler diagnostics as structured JSON. Wraps Microsoft's al_compile (onlyErrors=false, enableCodeAnalysis=true, no diagnostic cap) using the analyzers and ruleset the server passed to almcp at startup, then enriches each diagnostic with the owning analyzer ('CodeCop', 'ALCops.LinterCop', ..., or 'Compiler' for AL#### errors) and whether a native code fix exists (hasFix). Prefer this over al_compile or al_getdiagnostics whenever you want cop diagnostics: al_compile hides warnings unless you remember onlyErrors=false, and al_getdiagnostics never runs analyzers. Scope with filePath, folderPath or projectPath (combined with AND); filter with severities, analyzers, ruleIds; cap with limit (default 500). totalCount, truncated and summary always describe the full filtered set. Results are sorted by filePath, line, column. Scoping: without any scope argument, results are limited to the startup project. With filePath or folderPath and no projectPath, the file/folder is the only scope and analyzer/hasFix come from the analyzer configuration of the project that contains it (falling back to the startup project). The 'project' field names that project. Next steps: for a diagnostic with hasFix=true call get_fixes (then apply_fix) at its filePath/line/column/id, or apply_fix_all for every occurrence of one rule. After apply_fix / apply_fix_all, call analyze again to verify; almcp's file watcher normally sees the write first, but on slow file systems or right after a large apply_fix_all a second call may be needed before the fixed diagnostic disappears.")] - public static async Task Analyze( + public static async Task Analyze( IServiceProvider services, ProjectAnalyzerResolver analyzerResolver, WorkspaceStartupResolver workspaceResolver, @@ -29,59 +29,61 @@ public static async Task Analyze( { try { + if (limit <= 0) + return ToolErrors.Invalid("limit must be a positive integer."); + + // Normalized before the proxy checks so a malformed path is Invalid whether or not almcp runs. + string? normalizedFilePath = null; + if (filePath is not null) + { + if (!ProjectScope.TryNormalizePath(filePath, trimTrailingSeparator: false, out var full, out var pathMessage, "filePath")) + return ToolErrors.Invalid(pathMessage!); + normalizedFilePath = full; + } + + string? normalizedFolderPath = null; + if (folderPath is not null) + { + if (!ProjectScope.TryNormalizePath(folderPath, trimTrailingSeparator: true, out var full, out var pathMessage, "folderPath")) + return ToolErrors.Invalid(pathMessage!); + normalizedFolderPath = full; + } + var proxy = services.GetService(typeof(AlMcpProxy)) as AlMcpProxy; if (proxy is null) - return Error("ProxyUnavailable", + return ToolErrors.Unavailable(UnavailableReason.NoProxy, "analyze wraps the proxied al_compile, but this server runs with --no-proxy. " + "Restart without --no-proxy to use analyze."); - if (!proxy.IsAvailable || !await proxy.Ready.WaitAsync(cancellationToken)) - return Error("ProxyUnavailable", - "MS AL MCP Server (almcp) is not available (not found in the DevTools directory, or it failed to start). " + - "See the server log on stderr."); + // IsAvailable never changes and Ready never regresses from false to true, so these two + // checks are not racy; they are what tells "not installed" apart from "failed to start". + if (!proxy.IsAvailable) + return ToolErrors.Unavailable(UnavailableReason.AlmcpNotFound, + "almcp was not found in the DevTools directory, so analyze (which wraps al_compile) is unavailable. " + + "Install BC DevTools 17.0 or later; see the server log on stderr."); + + if (!await proxy.Ready.WaitAsync(cancellationToken)) + return ToolErrors.Unavailable(UnavailableReason.AlmcpNotReady, + "almcp failed to start; see the server log on stderr."); var callerPassedProjectPath = projectPath is not null; var callerPassedFileOrFolder = filePath is not null || folderPath is not null; var callerPassedAnyScope = callerPassedProjectPath || callerPassedFileOrFolder; - var normalizedFilePath = filePath is not null ? Path.GetFullPath(filePath) : null; - var normalizedFolderPath = folderPath is not null ? Path.TrimEndingDirectorySeparator(Path.GetFullPath(folderPath)) : null; - var scopePath = normalizedFilePath ?? normalizedFolderPath; - string? enrichmentProject; - if (callerPassedProjectPath) - { - enrichmentProject = Path.TrimEndingDirectorySeparator(Path.GetFullPath(projectPath!)); - - var knownProjects = workspaceResolver.Config.ProjectDirectories - .Select(p => Path.TrimEndingDirectorySeparator(Path.GetFullPath(p))).ToList(); - if (!knownProjects.Contains(enrichmentProject, StringComparer.OrdinalIgnoreCase)) - return Error("UnknownProject", - $"'{projectPath}' is not one of the AL projects this server was started with: " + - $"{string.Join(", ", knownProjects)}. Pass one of those, or restart the server with --projects."); - } - else if (scopePath is not null) - { - enrichmentProject = CompileDiagnosticsParser.FindContainingProject( - scopePath, workspaceResolver.Config.ProjectDirectories) - ?? workspaceResolver.Config.PrimaryProject; - } - else - { - enrichmentProject = workspaceResolver.Config.PrimaryProject; - } + var config = workspaceResolver.Config; + string? invalidMessage = null; + var enrichmentProject = !callerPassedProjectPath && scopePath is not null + ? CompileDiagnosticsParser.FindContainingProject(scopePath, config.ProjectDirectories) + ?? ProjectScope.Resolve(config, null, out invalidMessage) + : ProjectScope.Resolve(config, projectPath, out invalidMessage); if (enrichmentProject is null) - return Error("NoProject", - "No AL project available. Pass projectPath, or start the server from a folder " + - "containing app.json (or use --projects)."); + return ToolErrors.Invalid(invalidMessage!); // Project filter applies when projectPath is explicit or when no file/folder scope was given. var projectScopeFilter = callerPassedFileOrFolder && !callerPassedProjectPath ? null : enrichmentProject; - if (limit <= 0) - return Error("InvalidLimit", "limit must be a positive integer."); - HashSet? severitySet = severities is { Length: > 0 } ? new HashSet(severities, StringComparer.OrdinalIgnoreCase) : null; HashSet? analyzerSet = analyzers is { Length: > 0 } @@ -105,11 +107,7 @@ public static async Task Analyze( var result = await proxy.ForwardAsync("al_compile", args, cancellationToken); if (result.IsError == true) - { - var errorMessage = string.Join('\n', result.Content.OfType().Select(b => b.Text)); - return Error("ProxyCallFailed", - "The proxied al_compile call failed (almcp may have exited, or its session was lost): " + errorMessage); - } + return MapProxyFailure(result); var (raw, message, succeeded) = CompileDiagnosticsParser.Parse(result); warnings.AddRange(CompileDiagnosticsParser.ExtractWarnings(message)); @@ -125,14 +123,27 @@ public static async Task Analyze( var analyzeResult = CompileDiagnosticsParser.Build(enrichmentProject, sorted, limit, warnings); - return JsonSerializer.Serialize(analyzeResult, JsonDefaults.Options); + return ToolResults.Ok(analyzeResult); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; } catch (Exception ex) { - return Error(ex.GetType().Name, ex.Message); + return ToolErrors.Faulted(ex); } } - private static string Error(string code, string message) => - JsonSerializer.Serialize(new { error = code, message }, JsonDefaults.Options); + /// + /// Maps a failed proxied al_compile (almcp gone, session lost, or an error al_compile itself + /// reported) to Unavailable/AlmcpCallFailed, with the almcp text in message and detail. + /// + internal static CallToolResult MapProxyFailure(CallToolResult result) + { + var almcpText = string.Join('\n', result.Content.OfType().Select(b => b.Text)); + return ToolErrors.Unavailable(UnavailableReason.AlmcpCallFailed, + $"The proxied al_compile call failed (almcp may have exited, its session was lost, or al_compile itself reported an error): {almcpText}", + almcpText); + } } diff --git a/src/ALCops.Mcp/Tools/ApplyFixAllTool.cs b/src/ALCops.Mcp/Tools/ApplyFixAllTool.cs index e5a9589..4daa39b 100644 --- a/src/ALCops.Mcp/Tools/ApplyFixAllTool.cs +++ b/src/ALCops.Mcp/Tools/ApplyFixAllTool.cs @@ -1,8 +1,8 @@ using System.ComponentModel; -using System.Text.Json; using ALCops.Mcp.Models; using ALCops.Mcp.Services; using Microsoft.Dynamics.Nav.CodeAnalysis.CodeFixes; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; namespace ALCops.Mcp.Tools; @@ -13,13 +13,16 @@ public sealed class ApplyFixAllTool [McpServerTool(Name = "apply_fix_all", ReadOnly = false, Destructive = false), Description("Apply a code fix to every occurrence of a diagnostic rule across a project (or a single file). " + "Runs analysis once, then fixes all matches for that rule ID in one pass — like VS Code's 'Fix all in workspace'. " + - "Writes changed files directly to disk unless dryRun is true. Use get_fixes first to discover equivalenceKey options. " + + "Writes changed files directly to disk unless dryRun is true. " + + "If the rule offers more than one distinct fix and no equivalenceKey is given, returns the error Ambiguous whose candidates " + + "list each fix's equivalenceKey, title and providerName; pass one equivalenceKey verbatim. " + + "Zero occurrences is a success (applied: false, diagnosticsFound: 0), except a rule the project ruleset suppresses (NotFound with reason SuppressedByRuleset) or one no loaded analyzer reports (NotFound with reason NoAnalyzerForRule). " + "Changed project files are re-read from disk first. Files that change on disk while the fix is being computed " + "(or that cannot be read, or are not valid in their detected encoding) " + "are left untouched and listed in 'conflicts' (each with a 'kind') and their diagnostics remain in 'unfixedDiagnostics' (positions as analysed, so they may have shifted if the file was edited); the other files are still written. " + "If a write fails, every file written in this call is restored (unless it was edited since) and all of them are listed in 'conflicts'. " + "Verify with analyze or al_compile (options.onlyErrors: false).")] - public static async Task ApplyFixAll( + public static async Task ApplyFixAll( ProjectSessionManager sessionManager, CodeFixRunner codeFixRunner, ProjectAnalyzerResolver analyzerResolver, @@ -42,14 +45,20 @@ public static async Task ApplyFixAll( else if (string.Equals(scope, "document", StringComparison.OrdinalIgnoreCase)) fixAllScope = FixAllScope.Document; else - return JsonSerializer.Serialize( - new { error = "InvalidScope", message = $"Unknown scope '{scope}'. Use 'project' or 'document'." }, - JsonDefaults.Options); + return ToolErrors.Invalid($"Unknown scope '{scope}'. Use 'project' or 'document'."); if (fixAllScope == FixAllScope.Document && string.IsNullOrWhiteSpace(filePath)) - return JsonSerializer.Serialize( - new { error = "MissingFilePath", message = "filePath is required when scope='document'." }, - JsonDefaults.Options); + return ToolErrors.Invalid("filePath is required when scope='document'."); + + if (fixAllScope == FixAllScope.Document) + { + if (!ProjectScope.TryNormalizePath(filePath!, trimTrailingSeparator: false, out var fullFilePath, out var invalidFilePath, "filePath")) + return ToolErrors.Invalid(invalidFilePath!); + filePath = fullFilePath; + } + + if (!ProjectScope.RequireProjectFolder(projectPath, out var invalidMessage)) + return ToolErrors.Invalid(invalidMessage!); string? warning = null; if (fixAllScope == FixAllScope.Project && !string.IsNullOrWhiteSpace(filePath)) @@ -69,7 +78,7 @@ public static async Task ApplyFixAll( switch (result.Status) { case FixAllStatus.NoDiagnosticsFound: - return JsonSerializer.Serialize(new + return ToolResults.Ok(new { applied = false, dryRun, @@ -77,26 +86,19 @@ public static async Task ApplyFixAll( diagnosticsFound = 0, message = $"No occurrences of {diagnosticId} were found in the given scope.", warning - }, JsonDefaults.Options); - - case FixAllStatus.NoFixAvailable: - return JsonSerializer.Serialize(new - { - error = "NoFixAvailable", - message = $"No applicable code fix found for {diagnosticId}" + - (equivalenceKey is not null ? $" with equivalence key '{equivalenceKey}'." : "."), - diagnosticsFound = result.DiagnosticsFound - }, JsonDefaults.Options); - - case FixAllStatus.AmbiguousFix: - return JsonSerializer.Serialize(new - { - error = "AmbiguousFix", - message = $"{diagnosticId} has multiple distinct fixes available. " + - "Call get_fixes on one occurrence to see titles, then pass the desired equivalenceKey.", - diagnosticsFound = result.DiagnosticsFound, - availableEquivalenceKeys = result.AvailableEquivalenceKeys - }, JsonDefaults.Options); + }); + + case FixAllStatus.NotFound: + var reason = result.NotFoundReason!.Value; + return ToolErrors.NotFound(reason, + ToolErrors.NotFoundMessage(reason, diagnosticId, filePath, equivalenceKey: equivalenceKey), + filePath, diagnosticId, + reason == FixNotFoundReason.NoFixForEquivalenceKey ? result.Candidates : null); + + case FixAllStatus.Ambiguous: + return ToolErrors.Ambiguous( + $"{diagnosticId} has {result.Candidates.Count} distinct fixes. Pass one candidate's equivalenceKey verbatim.", + result.Candidates, filePath, diagnosticId); } // Completed @@ -128,7 +130,7 @@ public static async Task ApplyFixAll( var unfixed = MergeUnfixed(result, conflicts); - return JsonSerializer.Serialize(new + return ToolResults.Ok(new { applied = !dryRun && written.Count > 0, dryRun, @@ -141,11 +143,15 @@ public static async Task ApplyFixAll( message = conflictMessage, unfixedDiagnostics = unfixed, warning - }, JsonDefaults.Options); + }); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; } catch (Exception ex) { - return JsonSerializer.Serialize(new { error = ex.GetType().Name, message = ex.Message }, JsonDefaults.Options); + return ToolErrors.Faulted(ex); } } diff --git a/src/ALCops.Mcp/Tools/ApplyFixTool.cs b/src/ALCops.Mcp/Tools/ApplyFixTool.cs index 012f895..88a5d3e 100644 --- a/src/ALCops.Mcp/Tools/ApplyFixTool.cs +++ b/src/ALCops.Mcp/Tools/ApplyFixTool.cs @@ -1,7 +1,7 @@ using System.ComponentModel; -using System.Text.Json; using ALCops.Mcp.Models; using ALCops.Mcp.Services; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; namespace ALCops.Mcp.Tools; @@ -12,12 +12,13 @@ public sealed class ApplyFixTool [McpServerTool(Name = "apply_fix", ReadOnly = false, Destructive = false), Description("Apply a code fix to resolve a diagnostic. Changed project files are re-read from disk first. " + "Writes the fixed content to the file on disk unless the file changed after the fix was computed, " + - "in which case nothing is written and { error: 'StaleFile' } is returned. " + + "in which case nothing is written and the error Stale is returned (re-run). " + "A file that is not valid in its detected encoding (e.g. a Windows-1252 file read as UTF-8) is never re-encoded: " + - "nothing is written and { error: 'UnsupportedEncoding' } is returned ({ error: 'ReadFailed' } if the file cannot be read). " + + "nothing is written and the error Faulted with reason UnsupportedEncoding is returned (reason ReadFailed if the file cannot be read, WriteFailed if the write fails). " + + "When no fix matches, returns NotFound with a reason, the same as get_fixes; for NoFixForEquivalenceKey, candidates lists the keys that do apply. " + "The write is atomic and preserves the file's encoding and line endings. " + "Verify with analyze or al_compile (options.onlyErrors: false).")] - public static async Task ApplyFix( + public static async Task ApplyFix( ProjectSessionManager sessionManager, CodeFixRunner codeFixRunner, ProjectAnalyzerResolver analyzerResolver, @@ -27,51 +28,73 @@ public static async Task ApplyFix( [Description("The diagnostic rule ID (e.g., 'AC0018', 'LC0001').")] string diagnosticId, [Description("Line number of the diagnostic (1-based).")] int line, [Description("Column number of the diagnostic (1-based).")] int column, - [Description("Equivalence key of the fix to apply (from get_fixes results).")] string equivalenceKey, + [Description("Equivalence key of the fix to apply (from get_fixes results), verbatim.")] string equivalenceKey, [Description("Optional: JSON array of analyzer specs (e.g., '[\"${CodeCop}\",\"${UICop}\"]'). If omitted, auto-discovers from .vscode/settings.json.")] string? analyzers = null, CancellationToken cancellationToken = default) { try { + if (!ProjectScope.RequireProjectFolder(projectPath, out var invalidMessage)) + return ToolErrors.Invalid(invalidMessage!); + + if (!ProjectScope.TryNormalizePath(filePath, trimTrailingSeparator: false, out var fullFilePath, out var invalidFilePath, "filePath")) + return ToolErrors.Invalid(invalidFilePath!); + filePath = fullFilePath; + var session = await sessionManager.GetOrLoadProjectAsync(projectPath, cancellationToken); var analyzerSpecs = AnalyzerSpec.ParseJsonArray(analyzers); var analyzerSet = await analyzerResolver.ResolveAsync(projectPath, analyzerSpecs, cancellationToken); - var result = await codeFixRunner.ApplyFixAsync( + var outcome = await codeFixRunner.ApplyFixAsync( session, filePath, diagnosticId, line, column, equivalenceKey, analyzerSet, cancellationToken); - if (result is null) - return JsonSerializer.Serialize( - new { error = "NoFixFound", message = $"No code fix with equivalence key '{equivalenceKey}' found for {diagnosticId} at {filePath}:{line}:{column}." }, - JsonDefaults.Options); + if (outcome.NotFoundReason is { } reason) + return ToolErrors.NotFound(reason, + outcome.MessageOverride + ?? ToolErrors.NotFoundMessage(reason, diagnosticId, filePath, line, column, equivalenceKey), + filePath, diagnosticId, + reason == FixNotFoundReason.NoFixForEquivalenceKey ? outcome.Candidates : null); - var conflict = await fileWriter.WriteIfUnchangedAsync( - filePath, result.OriginalContent, result.ModifiedContent, cancellationToken); + var fix = outcome.Fix!; + + FileWriteConflict? conflict; + try + { + conflict = await fileWriter.WriteIfUnchangedAsync( + filePath, fix.OriginalContent, fix.ModifiedContent, cancellationToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + Console.Error.WriteLine($"Warning: {filePath} could not be written: {ex.Message}"); + return ToolErrors.Faulted(FileWriteConflictKind.WriteFailed, + $"{filePath} could not be written ({ex.GetType().Name}: {ex.Message}); the file is unchanged.", + filePath, diagnosticId, detail: ex.GetType().FullName); + } if (conflict is not null) { Console.Error.WriteLine($"Warning: {conflict.Message}"); - return JsonSerializer.Serialize(new - { - error = conflict.Kind, - message = conflict.Message, - filePath = conflict.FilePath, - diagnosticId - }, JsonDefaults.Options); + return conflict.Kind == FileWriteConflictKind.StaleFile + ? ToolErrors.Stale(conflict.FilePath, diagnosticId, conflict.Message) + : ToolErrors.Faulted(conflict.Kind, conflict.Message, conflict.FilePath, diagnosticId); } - return JsonSerializer.Serialize(new + return ToolResults.Ok(new { applied = true, filePath, - fixTitle = result.FixTitle, + fixTitle = fix.FixTitle, diagnosticId - }, JsonDefaults.Options); + }); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; } catch (Exception ex) { - return JsonSerializer.Serialize(new { error = ex.GetType().Name, message = ex.Message }, JsonDefaults.Options); + return ToolErrors.Faulted(ex); } } } diff --git a/src/ALCops.Mcp/Tools/GetFixesTool.cs b/src/ALCops.Mcp/Tools/GetFixesTool.cs index faa3f9f..1f8def4 100644 --- a/src/ALCops.Mcp/Tools/GetFixesTool.cs +++ b/src/ALCops.Mcp/Tools/GetFixesTool.cs @@ -1,6 +1,7 @@ using System.ComponentModel; -using System.Text.Json; +using ALCops.Mcp.Models; using ALCops.Mcp.Services; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; namespace ALCops.Mcp.Tools; @@ -9,8 +10,12 @@ namespace ALCops.Mcp.Tools; public sealed class GetFixesTool { [McpServerTool(Name = "get_fixes", ReadOnly = true), - Description("Get available code fixes for a specific diagnostic at a location. Returns fix titles and equivalence keys needed by apply_fix.")] - public static async Task GetFixes( + Description("Get available code fixes for a specific diagnostic at a location. " + + "Returns { diagnosticId, filePath, line, column, fixes: [{ equivalenceKey, title, providerName }] }; " + + "pass a fix's equivalenceKey verbatim to apply_fix (or apply_fix_all). fixes is never empty. " + + "When nothing matches, returns the error NotFound with a reason: NoDiagnosticAtPosition (re-run analyze and use its line/column), " + + "SuppressedByRuleset or SuppressedByPragma (nothing to fix), NoFixProvider, NoAnalyzerForRule, FileNotInProject or NoFixForDiagnostic.")] + public static async Task GetFixes( ProjectSessionManager sessionManager, CodeFixRunner codeFixRunner, ProjectAnalyzerResolver analyzerResolver, @@ -24,19 +29,36 @@ public static async Task GetFixes( { try { + if (!ProjectScope.RequireProjectFolder(projectPath, out var invalidMessage)) + return ToolErrors.Invalid(invalidMessage!); + + if (!ProjectScope.TryNormalizePath(filePath, trimTrailingSeparator: false, out var fullFilePath, out var invalidFilePath, "filePath")) + return ToolErrors.Invalid(invalidFilePath!); + filePath = fullFilePath; + var session = await sessionManager.GetOrLoadProjectAsync(projectPath, cancellationToken); var analyzerSpecs = AnalyzerSpec.ParseJsonArray(analyzers); var analyzerSet = await analyzerResolver.ResolveAsync(projectPath, analyzerSpecs, cancellationToken); - var fixes = await codeFixRunner.GetFixesAsync( + var lookup = await codeFixRunner.GetFixesAsync( session, filePath, diagnosticId, line, column, analyzerSet, cancellationToken); - return JsonSerializer.Serialize(fixes, JsonDefaults.Options); + if (lookup.NotFoundReason is { } reason) + return ToolErrors.NotFound(reason, + ToolErrors.NotFoundMessage(reason, diagnosticId, filePath, line, column), + filePath, diagnosticId); + + return ToolResults.Ok(new GetFixesResult( + diagnosticId, filePath, line, column, lookup.Fixes)); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; } catch (Exception ex) { - return JsonSerializer.Serialize(new { error = ex.GetType().Name, message = ex.Message }, JsonDefaults.Options); + return ToolErrors.Faulted(ex); } } } diff --git a/src/ALCops.Mcp/Tools/ListRulesTool.cs b/src/ALCops.Mcp/Tools/ListRulesTool.cs index e520dac..22953e7 100644 --- a/src/ALCops.Mcp/Tools/ListRulesTool.cs +++ b/src/ALCops.Mcp/Tools/ListRulesTool.cs @@ -1,7 +1,7 @@ using System.ComponentModel; -using System.Text.Json; using ALCops.Mcp.Models; using ALCops.Mcp.Services; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; namespace ALCops.Mcp.Tools; @@ -11,60 +11,67 @@ public sealed class ListRulesTool { [McpServerTool(Name = "list_rules", ReadOnly = true), Description("List available analyzer rules. Rules come from the analyzers the project configures via al.codeAnalyzers — nothing is bundled. By default returns a compact list (ID, title, cop name). Use verbose=true for full metadata including description, severity, category, help URI, and code fix availability.")] - public static async Task ListRules( + public static async Task ListRules( ProjectAnalyzerResolver analyzerResolver, WorkspaceStartupResolver workspaceResolver, - [Description("Optional: absolute path to the AL project folder. Defaults to the project discovered at startup.")] string? projectPath = null, + [Description("Optional: absolute path to one of the AL project folders this server was started with. Defaults to the project discovered at startup.")] string? projectPath = null, [Description("Filter rules by cop name (e.g., 'LinterCop', 'ApplicationCop', 'CodeCop'). Leave empty for all cops.")] string? copFilter = null, [Description("Optional: JSON array of analyzer specs (e.g., '[\"${CodeCop}\",\"${UICop}\"]'). If omitted, auto-discovers from .vscode/settings.json.")] string? analyzers = null, [Description("Return full rule metadata (description, severity, category, helpUri, hasCodeFix). Default: false.")] bool verbose = false, CancellationToken cancellationToken = default) { - // No projectPath: fall back to the project discovered at startup, the same one the proxied - // MS tools operate on. There is no project-independent rule list any more — analyzers are - // whatever the project configures. - projectPath ??= workspaceResolver.Config.PrimaryProject; - if (projectPath is null) - return JsonSerializer.Serialize(new - { - error = "NoProject", - message = "No AL project available. Pass projectPath, or start the server from a folder " + - "containing app.json (or use --projects)." - }, JsonDefaults.Options); + try + { + // No projectPath: fall back to the project discovered at startup, the same one the proxied + // MS tools operate on. There is no project-independent rule list any more — analyzers are + // whatever the project configures. An explicit projectPath must be one of the startup + // projects, exactly as for analyze. + var project = ProjectScope.Resolve(workspaceResolver.Config, projectPath, out var invalidMessage); + if (project is null) + return ToolErrors.Invalid(invalidMessage!); - var analyzerSpecs = AnalyzerSpec.ParseJsonArray(analyzers); - var provider = await analyzerResolver.ResolveAsync(projectPath, analyzerSpecs, cancellationToken); - var warnings = provider.Warnings.Count > 0 ? provider.Warnings : null; + var analyzerSpecs = AnalyzerSpec.ParseJsonArray(analyzers); + var provider = await analyzerResolver.ResolveAsync(project, analyzerSpecs, cancellationToken); + var warnings = provider.Warnings.Count > 0 ? provider.Warnings : null; - var descriptors = provider.GetAllDescriptors(); + var descriptors = provider.GetAllDescriptors(); - var filtered = descriptors.Values - .Select(d => (Descriptor: d, CopName: provider.GetCopName(d.Id))) - .Where(r => copFilter is null || r.CopName.Equals(copFilter, StringComparison.OrdinalIgnoreCase)) - .OrderBy(r => r.Descriptor.Id); + var filtered = descriptors.Values + .Select(d => (Descriptor: d, CopName: provider.GetCopName(d.Id))) + .Where(r => copFilter is null || r.CopName.Equals(copFilter, StringComparison.OrdinalIgnoreCase)) + .OrderBy(r => r.Descriptor.Id); - if (verbose) + if (verbose) + { + var rules = filtered.Select(r => new RuleInfo( + Id: r.Descriptor.Id, + Title: r.Descriptor.Title.ToString(), + Description: r.Descriptor.Description.ToString(), + Severity: r.Descriptor.DefaultSeverity.ToString(), + Category: r.Descriptor.Category, + CopName: r.CopName, + HasCodeFix: provider.HasCodeFix(r.Descriptor.Id), + HelpUri: r.Descriptor.HelpLinkUri)).ToList(); + return ToolResults.Ok(new { rules, warnings }); + } + else + { + var rules = filtered.Select(r => new + { + id = r.Descriptor.Id, + title = r.Descriptor.Title.ToString(), + cop = r.CopName + }).ToList(); + return ToolResults.Ok(new { rules, warnings }); + } + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - var rules = filtered.Select(r => new RuleInfo( - Id: r.Descriptor.Id, - Title: r.Descriptor.Title.ToString(), - Description: r.Descriptor.Description.ToString(), - Severity: r.Descriptor.DefaultSeverity.ToString(), - Category: r.Descriptor.Category, - CopName: r.CopName, - HasCodeFix: provider.HasCodeFix(r.Descriptor.Id), - HelpUri: r.Descriptor.HelpLinkUri)).ToList(); - return JsonSerializer.Serialize(new { rules, warnings }, JsonDefaults.Options); + throw; } - else + catch (Exception ex) { - var rules = filtered.Select(r => new - { - id = r.Descriptor.Id, - title = r.Descriptor.Title.ToString(), - cop = r.CopName - }).ToList(); - return JsonSerializer.Serialize(new { rules, warnings }, JsonDefaults.Options); + return ToolErrors.Faulted(ex); } } } diff --git a/tests/ALCops.Mcp.Tests/AnalyzeToolTests.cs b/tests/ALCops.Mcp.Tests/AnalyzeToolTests.cs index 556d83b..09d0e8b 100644 --- a/tests/ALCops.Mcp.Tests/AnalyzeToolTests.cs +++ b/tests/ALCops.Mcp.Tests/AnalyzeToolTests.cs @@ -4,6 +4,7 @@ using ALCops.Mcp.Tools; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; +using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using Xunit; using Xunit.Abstractions; @@ -12,26 +13,45 @@ namespace ALCops.Mcp.Tests; public sealed class AnalyzeToolTests { + private static WorkspaceStartupResolver DummyResolver(ProjectAnalyzerResolver analyzerResolver) => + new(analyzerResolver, + new ExternalAnalyzerLoader(TestAnalyzers.ToolsLocator), + NullLogger.Instance, + ["C:\\dummy"]); + [Fact] - public async Task ProxyUnavailable_WhenNoProxy() + public async Task Unavailable_NoProxy_WhenServerRunsWithNoProxy() { var services = new ServiceCollection().BuildServiceProvider(); var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); - var resolver = new WorkspaceStartupResolver( - analyzerResolver, - new ExternalAnalyzerLoader(TestAnalyzers.ToolsLocator), - NullLogger.Instance, - ["C:\\dummy"]); - var json = await AnalyzeTool.Analyze(services, analyzerResolver, resolver); - var doc = JsonDocument.Parse(json); + var result = await AnalyzeTool.Analyze(services, analyzerResolver, DummyResolver(analyzerResolver)); + + var root = ToolResultAssert.Error(result, "Unavailable", "NoProxy"); + Assert.Contains("--no-proxy", root.GetProperty("message").GetString()); + } + + [Theory] + [InlineData("C:\\bad\0file.al", null)] + [InlineData(" ", null)] + [InlineData(null, "C:\\bad\0folder")] + [InlineData(null, "")] + public async Task Invalid_WhenFileOrFolderPathIsMalformed_EvenWithoutAlmcp(string? filePath, string? folderPath) + { + // No proxy registered: the path check must run first, so this is Invalid, not Unavailable. + var services = new ServiceCollection().BuildServiceProvider(); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); - Assert.Equal("ProxyUnavailable", doc.RootElement.GetProperty("error").GetString()); - Assert.Contains("--no-proxy", doc.RootElement.GetProperty("message").GetString()); + var result = await AnalyzeTool.Analyze(services, analyzerResolver, DummyResolver(analyzerResolver), + filePath: filePath, folderPath: folderPath); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains(filePath is not null ? "filePath" : "folderPath", root.GetProperty("message").GetString()); + Assert.Contains("is not a valid path", root.GetProperty("message").GetString()); } [Fact] - public async Task ProxyUnavailable_WhenAlmcpNotFound() + public async Task Unavailable_AlmcpNotFound_WhenToolsDirHasNoAlmcp() { var toolsDir = Path.Combine(Path.GetTempPath(), $"alcops-noalmcp-analyze-{Guid.NewGuid():N}"); Directory.CreateDirectory(toolsDir); @@ -55,11 +75,10 @@ public async Task ProxyUnavailable_WhenAlmcpNotFound() sc.AddSingleton(proxy); var sp = sc.BuildServiceProvider(); - var json = await AnalyzeTool.Analyze(sp, analyzerResolver, resolver); - var doc = JsonDocument.Parse(json); + var result = await AnalyzeTool.Analyze(sp, analyzerResolver, resolver); - Assert.Equal("ProxyUnavailable", doc.RootElement.GetProperty("error").GetString()); - Assert.Contains("not available", doc.RootElement.GetProperty("message").GetString()); + var root = ToolResultAssert.Error(result, "Unavailable", "AlmcpNotFound"); + Assert.Contains("not found", root.GetProperty("message").GetString()); } finally { @@ -67,6 +86,106 @@ public async Task ProxyUnavailable_WhenAlmcpNotFound() } } + [Fact] + public async Task Unavailable_AlmcpNotReady_WhenAlmcpStopped() + { + var toolsDir = CreateStubToolsDirWithAlmcp(); + + try + { + var locator = new BcToolsLocator(toolsDir); + Assert.True(locator.HasAlMcp); + + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + var resolver = DummyResolver(analyzerResolver); + + // Never started; disposing completes Ready with false, as a failed start does. + var proxy = new AlMcpProxy(locator, resolver, new CapturingLogger()); + await proxy.DisposeAsync(); + + var sc = new ServiceCollection(); + sc.AddSingleton(proxy); + + var result = await AnalyzeTool.Analyze(sc.BuildServiceProvider(), analyzerResolver, resolver); + + ToolResultAssert.Error(result, "Unavailable", "AlmcpNotReady"); + } + finally + { + TestAnalyzers.TryDeleteDirectory(toolsDir); + } + } + + [Fact] + public async Task Cancellation_Propagates_InsteadOfFaulted() + { + var toolsDir = CreateStubToolsDirWithAlmcp(); + + try + { + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + var resolver = DummyResolver(analyzerResolver); + + // Never started and not disposed: Ready stays pending, so the cancelled wait throws. + var proxy = new AlMcpProxy(new BcToolsLocator(toolsDir), resolver, new CapturingLogger()); + + var sc = new ServiceCollection(); + sc.AddSingleton(proxy); + + using var cts = new CancellationTokenSource(); + await cts.CancelAsync(); + + await Assert.ThrowsAnyAsync(() => + AnalyzeTool.Analyze(sc.BuildServiceProvider(), analyzerResolver, resolver, cancellationToken: cts.Token)); + + await proxy.DisposeAsync(); + } + finally + { + TestAnalyzers.TryDeleteDirectory(toolsDir); + } + } + + [Fact] + public void MapProxyFailure_IsUnavailableAlmcpCallFailed_WithAlmcpTextAsDetail() + { + var failed = new CallToolResult + { + IsError = true, + Content = [new TextContentBlock { Text = "Session not found" }] + }; + + var result = AnalyzeTool.MapProxyFailure(failed); + + var root = ToolResultAssert.Error(result, "Unavailable", "AlmcpCallFailed"); + Assert.Equal("Session not found", root.GetProperty("detail").GetString()); + Assert.EndsWith(": Session not found", root.GetProperty("message").GetString()); + } + + [Fact] + public async Task Invalid_WhenLimitIsNotPositive() + { + // The limit check runs before the proxy checks, so this needs no almcp. + var services = new ServiceCollection().BuildServiceProvider(); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await AnalyzeTool.Analyze(services, analyzerResolver, DummyResolver(analyzerResolver), limit: 0); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("limit", root.GetProperty("message").GetString()); + } + + /// A tools directory that BcToolsLocator accepts as having almcp; nothing in it can run. + private static string CreateStubToolsDirWithAlmcp() + { + var toolsDir = Path.Combine(Path.GetTempPath(), $"alcops-stubalmcp-analyze-{Guid.NewGuid():N}"); + Directory.CreateDirectory(toolsDir); + File.WriteAllText(Path.Combine(toolsDir, "Microsoft.Dynamics.Nav.CodeAnalysis.dll"), "stub"); + File.WriteAllText(Path.Combine(toolsDir, "almcp.exe"), "stub"); + File.WriteAllText(Path.Combine(toolsDir, "almcp.dll"), "stub"); + return toolsDir; + } + [Fact] public void Schema_ExposesUserParameters_HidesServicesAndCancellationToken() { @@ -104,20 +223,16 @@ private async Task RunAnalyze( string[]? ruleIds = null, int limit = AnalyzeTool.DefaultLimit) { - var json = await AnalyzeTool.Analyze( + var result = await AnalyzeTool.Analyze( fixture.ServiceProvider, fixture.AnalyzerResolver, fixture.WorkspaceResolver, filePath, folderPath, projectPath, severities, analyzers, ruleIds, limit, Cts.Token); - output.WriteLine(json); + output.WriteLine(ToolResultAssert.Text(result)); - var doc = JsonDocument.Parse(json); - Assert.False(doc.RootElement.TryGetProperty("error", out var err), - $"Unexpected error: {err}"); - - return JsonSerializer.Deserialize(json, JsonDefaults.Options)!; + return ToolResultAssert.OkAs(result); } [AlMcpFact] @@ -265,35 +380,20 @@ public async Task FolderPathProjB_NoProjectPath_ReturnsProjBDiagnostics() } [AlMcpFact] - public async Task InvalidLimit_ReturnsError() - { - var json = await AnalyzeTool.Analyze( - fixture.ServiceProvider, - fixture.AnalyzerResolver, - fixture.WorkspaceResolver, - limit: 0, - cancellationToken: Cts.Token); - - var doc = JsonDocument.Parse(json); - Assert.Equal("InvalidLimit", doc.RootElement.GetProperty("error").GetString()); - } - - [AlMcpFact] - public async Task UnknownProject_ReturnsError() + public async Task UnknownProject_ReturnsInvalid() { var bogus = Path.Combine(Path.GetDirectoryName(fixture.ProjA)!, "NonExistent"); - var json = await AnalyzeTool.Analyze( + var result = await AnalyzeTool.Analyze( fixture.ServiceProvider, fixture.AnalyzerResolver, fixture.WorkspaceResolver, projectPath: bogus, cancellationToken: Cts.Token); - output.WriteLine(json); - var doc = JsonDocument.Parse(json); - Assert.Equal("UnknownProject", doc.RootElement.GetProperty("error").GetString()); + output.WriteLine(ToolResultAssert.Text(result)); + var root = ToolResultAssert.Error(result, "Invalid"); - var message = doc.RootElement.GetProperty("message").GetString()!; + var message = root.GetProperty("message").GetString()!; Assert.Contains(fixture.ProjA, message, StringComparison.OrdinalIgnoreCase); Assert.Contains(fixture.ProjB, message, StringComparison.OrdinalIgnoreCase); } diff --git a/tests/ALCops.Mcp.Tests/ApplyFixAllToolTests.cs b/tests/ALCops.Mcp.Tests/ApplyFixAllToolTests.cs index c2b92e7..e856427 100644 --- a/tests/ALCops.Mcp.Tests/ApplyFixAllToolTests.cs +++ b/tests/ALCops.Mcp.Tests/ApplyFixAllToolTests.cs @@ -47,14 +47,13 @@ public async Task ApplyFixAll_ProjectScope_FixesAllOccurrencesAcrossFiles() { using var ctx = new TestContext(); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Ok(result); - Assert.True(root.GetProperty("applied").GetBoolean(), resultJson); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(2, root.GetProperty("diagnosticsFound").GetInt32()); Assert.Empty(root.GetProperty("conflicts").EnumerateArray()); @@ -90,14 +89,13 @@ public async Task ApplyFixAll_DocumentScope_OnlyFixesTargetFile() using var ctx = new TestContext(); var pageAPath = Path.Combine(ctx.ProjectPath, "PageA.al"); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020", scope: "document", filePath: pageAPath); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Ok(result); - Assert.True(root.GetProperty("applied").GetBoolean(), resultJson); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(1, root.GetProperty("diagnosticsFound").GetInt32()); var filesChanged = root.GetProperty("filesChanged").EnumerateArray() @@ -116,14 +114,13 @@ public async Task ApplyFixAll_DryRun_ReportsChangesWithoutWriting() var originalPageA = ctx.ReadFile("PageA.al"); var originalPageB = ctx.ReadFile("PageB.al"); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020", dryRun: true); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Ok(result); - Assert.False(root.GetProperty("applied").GetBoolean(), resultJson); + Assert.False(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.True(root.GetProperty("dryRun").GetBoolean()); Assert.Equal(2, root.GetProperty("diagnosticsFound").GetInt32()); Assert.Equal(2, root.GetProperty("filesChanged").GetArrayLength()); @@ -138,35 +135,125 @@ public async Task ApplyFixAll_MultipleDistinctFixesWithoutEquivalenceKey_Returns { using var ctx = new TestContext(); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "AC0012"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Error(result, "Ambiguous"); + Assert.False(root.TryGetProperty("reason", out _)); + Assert.Equal("AC0012", root.GetProperty("diagnosticId").GetString()); - Assert.Equal("AmbiguousFix", root.GetProperty("error").GetString()); - var keys = root.GetProperty("availableEquivalenceKeys").EnumerateArray().Select(e => e.GetString()).ToArray(); - Assert.True(keys.Length > 1, resultJson); + var candidates = root.GetProperty("candidates").EnumerateArray().ToArray(); + Assert.True(candidates.Length > 1, ToolResultAssert.Text(result)); + Assert.All(candidates, c => + { + Assert.Equal(JsonValueKind.String, c.GetProperty("equivalenceKey").ValueKind); + Assert.False(string.IsNullOrEmpty(c.GetProperty("title").GetString())); + Assert.False(string.IsNullOrEmpty(c.GetProperty("providerName").GetString())); + }); + + var keys = candidates.Select(c => c.GetProperty("equivalenceKey").GetString()).ToArray(); + Assert.Equal(keys.Length, keys.Distinct(StringComparer.Ordinal).Count()); // The codeunit must be untouched since no fix was chosen or applied. Assert.Contains("Access = Internal;", ctx.ReadFile("Codeunit.al")); } + [Fact] + public async Task ApplyFixAll_UnknownEquivalenceKey_ReturnsNotFoundWithCandidates() + { + using var ctx = new TestContext(); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "AC0012", equivalenceKey: "no-such-key"); + + var root = ToolResultAssert.Error(result, "NotFound", "NoFixForEquivalenceKey"); + Assert.True(root.GetProperty("candidates").GetArrayLength() > 1, ToolResultAssert.Text(result)); + Assert.False(root.TryGetProperty("diagnosticsFound", out _)); + Assert.Contains("Access = Internal;", ctx.ReadFile("Codeunit.al")); + } + + [Fact] + public async Task ApplyFixAll_RuleWithoutFixProvider_ReturnsNotFoundNoFixProvider() + { + using var ctx = new TestContext(); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "ZZ9999"); + + ToolResultAssert.Error(result, "NotFound", "NoFixProvider"); + } + + [Fact] + public async Task ApplyFixAll_UnknownScope_ReturnsInvalid() + { + using var ctx = new TestContext(); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "LC0020", scope: "workspace"); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("workspace", root.GetProperty("message").GetString()); + } + + [Fact] + public async Task ApplyFixAll_DocumentScopeWithoutFilePath_ReturnsInvalid() + { + using var ctx = new TestContext(); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "LC0020", scope: "document"); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("filePath", root.GetProperty("message").GetString()); + } + + [Theory] + [InlineData("C:\\bad\0file.al")] + [InlineData("")] + public async Task ApplyFixAll_DocumentScopeMalformedFilePath_ReturnsInvalid(string filePath) + { + using var ctx = new TestContext(); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "LC0020", scope: "document", filePath: filePath); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("filePath", root.GetProperty("message").GetString()); + } + + [Fact] + public async Task ApplyFixAll_DocumentScopeFileOutsideProject_ReturnsNotFoundFileNotInProject() + { + using var ctx = new TestContext(); + var nope = Path.Combine(ctx.ProjectPath, "Nope.al"); + + var result = await ApplyFixAllTool.ApplyFixAll( + ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, + ctx.ProjectPath, "LC0020", scope: "document", filePath: nope); + + var root = ToolResultAssert.Error(result, "NotFound", "FileNotInProject"); + Assert.Equal(Path.GetFullPath(nope), root.GetProperty("filePath").GetString()); + } + [Fact] public async Task ApplyFixAll_NoDiagnosticsFound_ReturnsNotAppliedWithZeroCount() { using var ctx = new TestContext(); var pageCPath = Path.Combine(ctx.ProjectPath, "PageC.al"); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020", scope: "document", filePath: pageCPath); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Ok(result); - Assert.False(root.GetProperty("applied").GetBoolean(), resultJson); + Assert.False(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(0, root.GetProperty("diagnosticsFound").GetInt32()); } @@ -187,13 +274,12 @@ await ApplyFixAllTool.ApplyFixAll( await File.WriteAllTextAsync(pageBPath, pageBContent.Replace("OtherField", "RenamedField")); // Apply for real — the refresh must pick up the rename. - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; - Assert.True(root.GetProperty("applied").GetBoolean(), resultJson); + var root = ToolResultAssert.Ok(result); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); // PageB must still contain the externally-renamed field AND have exactly one ApplicationArea. var pageBFinal = ctx.ReadFile("PageB.al"); @@ -217,13 +303,12 @@ await ApplyFixAllTool.ApplyFixAll( .Replace("PageB", "PageD"); await File.WriteAllTextAsync(Path.Combine(ctx.ProjectPath, "PageD.al"), pageDContent); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; - Assert.True(root.GetProperty("applied").GetBoolean(), resultJson); + var root = ToolResultAssert.Ok(result); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(3, root.GetProperty("diagnosticsFound").GetInt32()); var filesChanged = root.GetProperty("filesChanged").EnumerateArray() @@ -244,13 +329,12 @@ await ApplyFixAllTool.ApplyFixAll( // Delete PageB.al — only PageA still has LC0020. File.Delete(Path.Combine(ctx.ProjectPath, "PageB.al")); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; - Assert.True(root.GetProperty("applied").GetBoolean(), resultJson); + var root = ToolResultAssert.Ok(result); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(1, root.GetProperty("diagnosticsFound").GetInt32()); var filesChanged = root.GetProperty("filesChanged").EnumerateArray() @@ -260,7 +344,7 @@ await ApplyFixAllTool.ApplyFixAll( } [Fact] - public async Task ApplyFixAll_RulesetSuppressesRule_TreatsItAsZeroDiagnostics() + public async Task ApplyFixAll_RulesetSuppressesRule_ReturnsNotFoundSuppressedByRuleset() { // FixAllRulesetProject ships a custom.ruleset.json setting LC0020 to "None", // even though PageA.al contains a redundant ApplicationArea occurrence. @@ -268,15 +352,12 @@ public async Task ApplyFixAll_RulesetSuppressesRule_TreatsItAsZeroDiagnostics() var pageAPath = Path.Combine(ctx.ProjectPath, "PageA.al"); var originalPageA = ctx.ReadFile("PageA.al"); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020", scope: "document", filePath: pageAPath); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; - - Assert.False(root.GetProperty("applied").GetBoolean(), resultJson); - Assert.Equal(0, root.GetProperty("diagnosticsFound").GetInt32()); + var root = ToolResultAssert.Error(result, "NotFound", "SuppressedByRuleset"); + Assert.Equal("LC0020", root.GetProperty("diagnosticId").GetString()); Assert.Equal(originalPageA, ctx.ReadFile("PageA.al")); } @@ -311,13 +392,13 @@ public async Task ApplyFixAll_MixedEncodings_EachFileKeepsItsOwn() var pageBPath = Path.Combine(ctx.ProjectPath, "PageB.al"); await File.WriteAllBytesAsync(pageBPath, [0xEF, 0xBB, 0xBF, .. await File.ReadAllBytesAsync(pageBPath)]); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - Assert.True(doc.RootElement.GetProperty("applied").GetBoolean(), resultJson); - Assert.Equal(2, doc.RootElement.GetProperty("filesChanged").GetArrayLength()); + var root = ToolResultAssert.Ok(result); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); + Assert.Equal(2, root.GetProperty("filesChanged").GetArrayLength()); var pageB = await File.ReadAllBytesAsync(pageBPath); Assert.Equal([0xEF, 0xBB, 0xBF], pageB.Take(3)); @@ -344,14 +425,13 @@ public async Task ApplyFixAll_SecondCommitFails_RollsBackAndReportsAllThree() ctx.Writer = GuardedFileWriterTests.FailingOnCalls(2); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - var root = doc.RootElement; + var root = ToolResultAssert.Ok(result); - Assert.False(root.GetProperty("applied").GetBoolean(), resultJson); + Assert.False(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Equal(3, root.GetProperty("diagnosticsFound").GetInt32()); Assert.Empty(root.GetProperty("filesChanged").EnumerateArray()); @@ -385,12 +465,12 @@ public async Task ApplyFixAll_NoTempFilesLeftBehind() { using var ctx = new TestContext(); - var resultJson = await ApplyFixAllTool.ApplyFixAll( + var result = await ApplyFixAllTool.ApplyFixAll( ctx.SessionManager, ctx.CodeFixRunner, ctx.AnalyzerResolver, ctx.Writer, ctx.ProjectPath, "LC0020"); - using var doc = JsonDocument.Parse(resultJson); - Assert.True(doc.RootElement.GetProperty("applied").GetBoolean(), resultJson); + var root = ToolResultAssert.Ok(result); + Assert.True(root.GetProperty("applied").GetBoolean(), ToolResultAssert.Text(result)); Assert.Empty(TempFiles(ctx.ProjectPath)); } diff --git a/tests/ALCops.Mcp.Tests/ApplyFixThenCompileTests.cs b/tests/ALCops.Mcp.Tests/ApplyFixThenCompileTests.cs index d56af1a..6f7e644 100644 --- a/tests/ALCops.Mcp.Tests/ApplyFixThenCompileTests.cs +++ b/tests/ALCops.Mcp.Tests/ApplyFixThenCompileTests.cs @@ -57,15 +57,15 @@ public async Task ApplyFix_ThenAlCompile_NoLongerReportsFixedDiagnostic() var analyzerSet = await analyzerResolver.ResolveAsync(projectDir, null, Cts.Token); const int line = 11, column = 17; - var fixes = await codeFixRunner.GetFixesAsync(session, filePath, "LC0020", line, column, analyzerSet, Cts.Token); - Assert.True(fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17."); + var lookup = await codeFixRunner.GetFixesAsync(session, filePath, "LC0020", line, column, analyzerSet, Cts.Token); + Assert.True(lookup.Fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17."); var applyResult = await ApplyFixTool.ApplyFix( sessionManager, codeFixRunner, analyzerResolver, new GuardedFileWriter(), projectDir, filePath, "LC0020", line, column, - fixes[0].EquivalenceKey, analyzers: null, Cts.Token); + lookup.Fixes[0].EquivalenceKey, analyzers: null, Cts.Token); - Assert.Contains("\"applied\":true", applyResult); + Assert.True(ToolResultAssert.Ok(applyResult).GetProperty("applied").GetBoolean()); // 3. Compile again — LC0020 must be gone fixture.Logger.Lines.Clear(); diff --git a/tests/ALCops.Mcp.Tests/ApplyFixToolTests.cs b/tests/ALCops.Mcp.Tests/ApplyFixToolTests.cs index 30934b6..09be44b 100644 --- a/tests/ALCops.Mcp.Tests/ApplyFixToolTests.cs +++ b/tests/ALCops.Mcp.Tests/ApplyFixToolTests.cs @@ -1,3 +1,4 @@ +using ALCops.Mcp.Models; using ALCops.Mcp.Services; using ALCops.Mcp.Tools; using Xunit; @@ -31,22 +32,22 @@ public async Task ApplyFix_WritesModifiedContentToDisk() var analyzerSet = await analyzerResolver.ResolveAsync(tempProjectPath, null); // LC0020 (ApplicationAreaRedundancy) on the field-level ApplicationArea in MyPage.al. - // Hardcoded rather than discovered, matching GetFixes_RulesetSuppressesRule_ReturnsNoFixes. + // Hardcoded rather than discovered, matching GetFixes_RulesetSuppressesRule_ReturnsSuppressedByRuleset. const int line = 11, column = 17; - var fixes = await codeFixRunner.GetFixesAsync( + var lookup = await codeFixRunner.GetFixesAsync( session, filePath, "LC0020", line, column, analyzerSet); - Assert.True(fixes.Count > 0, - $"Expected a fixable LC0020 at line {line}, column {column}. Loaded {analyzerSet.GetAllAnalyzers().Length} " + + Assert.True(lookup.NotFoundReason is null && lookup.Fixes.Count > 0, + $"Expected a fixable LC0020 at line {line}, column {column}, got {lookup.NotFoundReason}. Loaded {analyzerSet.GetAllAnalyzers().Length} " + $"analyzer(s); warnings: {string.Join("; ", analyzerSet.Warnings)}. " + "The fixture, the location, or the analyzer configuration may have changed."); var result = await ApplyFixTool.ApplyFix( sessionManager, codeFixRunner, analyzerResolver, new GuardedFileWriter(), tempProjectPath, filePath, "LC0020", line, column, - fixes[0].EquivalenceKey); + lookup.Fixes[0].EquivalenceKey); - Assert.Contains("\"applied\":true", result); + Assert.True(ToolResultAssert.Ok(result).GetProperty("applied").GetBoolean()); // The actual regression assertion: the file on disk must have changed. var updatedContent = await File.ReadAllTextAsync(filePath); @@ -77,9 +78,9 @@ public async Task ApplyFix_ExternalEditBetweenCalls_PreservesEditAndAppliesFix() var analyzerSet = await analyzerResolver.ResolveAsync(tempProjectPath, null); const int line = 11, column = 17; - var fixes = await codeFixRunner.GetFixesAsync( + var lookup = await codeFixRunner.GetFixesAsync( session, filePath, "LC0020", line, column, analyzerSet); - Assert.True(fixes.Count > 0, "Expected a fixable LC0020."); + Assert.True(lookup.Fixes.Count > 0, "Expected a fixable LC0020."); // External edit: append a comment as the final line. Line 11 stays valid. var content = await File.ReadAllTextAsync(filePath); @@ -90,9 +91,9 @@ public async Task ApplyFix_ExternalEditBetweenCalls_PreservesEditAndAppliesFix() var result = await ApplyFixTool.ApplyFix( sessionManager, codeFixRunner, analyzerResolver, new GuardedFileWriter(), tempProjectPath, filePath, "LC0020", line, column, - fixes[0].EquivalenceKey); + lookup.Fixes[0].EquivalenceKey); - Assert.Contains("\"applied\":true", result); + Assert.True(ToolResultAssert.Ok(result).GetProperty("applied").GetBoolean()); var finalContent = await File.ReadAllTextAsync(filePath); Assert.Contains("// edited", finalContent); @@ -130,7 +131,7 @@ public async Task ApplyFix_FileWithUtf8Bom_KeepsBom() var (result, _) = await ApplyLc0020Async(tempProjectPath, filePath, new GuardedFileWriter()); - Assert.Contains("\"applied\":true", result); + Assert.True(ToolResultAssert.Ok(result).GetProperty("applied").GetBoolean()); var written = await File.ReadAllBytesAsync(filePath); Assert.Equal([0xEF, 0xBB, 0xBF], written.Take(3)); @@ -145,7 +146,7 @@ public async Task ApplyFix_FileWithUtf8Bom_KeepsBom() } [Fact] - public async Task ApplyFix_WriteFails_ReturnsIOExceptionAndLeavesFileIntact() + public async Task ApplyFix_WriteFails_ReturnsFaultedWriteFailedAndLeavesFileIntact() { var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-ioerr-test"); @@ -157,8 +158,11 @@ public async Task ApplyFix_WriteFails_ReturnsIOExceptionAndLeavesFileIntact() var writer = new GuardedFileWriter((_, _) => throw new IOException("injected move failure")); var (result, _) = await ApplyLc0020Async(tempProjectPath, filePath, writer); - Assert.Contains("\"error\":\"IOException\"", result); - Assert.Contains("injected move failure", result); + var root = ToolResultAssert.Error(result, "Faulted", "WriteFailed"); + Assert.Equal("System.IO.IOException", root.GetProperty("detail").GetString()); + Assert.Contains("injected move failure", root.GetProperty("message").GetString()); + Assert.Equal(filePath, root.GetProperty("filePath").GetString()); + Assert.Equal("LC0020", root.GetProperty("diagnosticId").GetString()); Assert.Equal(originalBytes, await File.ReadAllBytesAsync(filePath)); Assert.Empty(Directory.GetFiles(tempProjectPath, "*" + GuardedFileWriter.TempSuffix, SearchOption.AllDirectories)); } @@ -169,7 +173,7 @@ public async Task ApplyFix_WriteFails_ReturnsIOExceptionAndLeavesFileIntact() } [Fact] - public async Task ApplyFix_FileNotValidUtf8_ReturnsUnsupportedEncodingAndLeavesFileIntact() + public async Task ApplyFix_FileNotValidUtf8_ReturnsFaultedUnsupportedEncodingAndLeavesFileIntact() { var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-cp1252-test"); @@ -184,8 +188,8 @@ public async Task ApplyFix_FileNotValidUtf8_ReturnsUnsupportedEncodingAndLeavesF var (result, _) = await ApplyLc0020Async(tempProjectPath, filePath, new GuardedFileWriter()); - Assert.Contains("\"error\":\"UnsupportedEncoding\"", result); - Assert.Contains("not valid in its detected encoding (UTF-8;", result); + var root = ToolResultAssert.Error(result, "Faulted", "UnsupportedEncoding"); + Assert.Contains("not valid in its detected encoding (UTF-8;", root.GetProperty("message").GetString()); Assert.Equal(originalBytes, await File.ReadAllBytesAsync(filePath)); Assert.Empty(Directory.GetFiles(tempProjectPath, "*" + GuardedFileWriter.TempSuffix, SearchOption.AllDirectories)); } @@ -195,8 +199,160 @@ public async Task ApplyFix_FileNotValidUtf8_ReturnsUnsupportedEncodingAndLeavesF } } + [Fact] + public async Task ApplyFix_FileChangedButRefreshSkipsIt_ReturnsStaleAndWritesNothing() + { + var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-stale-test"); + + try + { + var filePath = Path.Combine(tempProjectPath, "MyPage.al"); + + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var codeFixRunner = new CodeFixRunner(); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + // Prime the session so the cached text is the original. + var session = await sessionManager.GetOrLoadProjectAsync(tempProjectPath); + var analyzerSet = await analyzerResolver.ResolveAsync(tempProjectPath, null); + var lookup = await codeFixRunner.GetFixesAsync(session, filePath, "LC0020", 11, 17, analyzerSet); + Assert.True(lookup.Fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17."); + + // A same-length edit with the old timestamp: the refresh's length+mtime gate skips the + // file, so the fix is computed from the cached text and the guarded write must refuse it. + var stamp = File.GetLastWriteTimeUtc(filePath); + var content = await File.ReadAllTextAsync(filePath); + Assert.Contains("50100", content); + await File.WriteAllTextAsync(filePath, content.Replace("50100", "50109")); + File.SetLastWriteTimeUtc(filePath, stamp); + var editedBytes = await File.ReadAllBytesAsync(filePath); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, codeFixRunner, analyzerResolver, new GuardedFileWriter(), + tempProjectPath, filePath, "LC0020", 11, 17, lookup.Fixes[0].EquivalenceKey); + + var root = ToolResultAssert.Error(result, "Stale"); + Assert.Equal(filePath, root.GetProperty("filePath").GetString()); + Assert.Equal("LC0020", root.GetProperty("diagnosticId").GetString()); + Assert.False(root.TryGetProperty("reason", out _)); + Assert.Equal(editedBytes, await File.ReadAllBytesAsync(filePath)); + Assert.Empty(Directory.GetFiles(tempProjectPath, "*" + GuardedFileWriter.TempSuffix, SearchOption.AllDirectories)); + } + finally + { + TestAnalyzers.TryDeleteDirectory(tempProjectPath); + } + } + + [Fact] + public async Task ApplyFix_UnknownEquivalenceKey_ReturnsNotFoundWithCandidates() + { + var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-badkey-test"); + + try + { + var filePath = Path.Combine(tempProjectPath, "MyPage.al"); + var originalBytes = await File.ReadAllBytesAsync(filePath); + + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, new CodeFixRunner(), analyzerResolver, new GuardedFileWriter(), + tempProjectPath, filePath, "LC0020", 11, 17, "no-such-key"); + + var root = ToolResultAssert.Error(result, "NotFound", "NoFixForEquivalenceKey"); + var candidates = root.GetProperty("candidates").EnumerateArray().ToList(); + Assert.NotEmpty(candidates); + Assert.All(candidates, c => + { + Assert.True(c.TryGetProperty("equivalenceKey", out _)); + Assert.False(string.IsNullOrEmpty(c.GetProperty("title").GetString())); + Assert.False(string.IsNullOrEmpty(c.GetProperty("providerName").GetString())); + }); + Assert.Contains("no-such-key", root.GetProperty("message").GetString()); + Assert.Equal(originalBytes, await File.ReadAllBytesAsync(filePath)); + } + finally + { + TestAnalyzers.TryDeleteDirectory(tempProjectPath); + } + } + + [Fact] + public async Task ApplyFix_RuleWithoutFixProvider_ReturnsNotFoundNoFixProvider() + { + var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-noprovider-test"); + + try + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, new CodeFixRunner(), analyzerResolver, new GuardedFileWriter(), + tempProjectPath, Path.Combine(tempProjectPath, "MyPage.al"), "ZZ9999", 11, 17, "any"); + + var root = ToolResultAssert.Error(result, "NotFound", "NoFixProvider"); + Assert.False(root.TryGetProperty("candidates", out _)); + } + finally + { + TestAnalyzers.TryDeleteDirectory(tempProjectPath); + } + } + + [Fact] + public async Task ApplyFix_FolderWithoutAppJson_ReturnsInvalid() + { + var folder = Path.Combine(Path.GetTempPath(), $"alcops-applyfix-noappjson-{Guid.NewGuid():N}"); + Directory.CreateDirectory(folder); + + try + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, new CodeFixRunner(), analyzerResolver, new GuardedFileWriter(), + folder, Path.Combine(folder, "MyPage.al"), "LC0020", 11, 17, "any"); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("app.json", root.GetProperty("message").GetString()); + } + finally + { + TestAnalyzers.TryDeleteDirectory(folder); + } + } + + [Theory] + [InlineData("C:\\bad\0file.al")] + [InlineData("")] + public async Task ApplyFix_MalformedFilePath_ReturnsInvalid(string filePath) + { + var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-applyfix-badfile-test"); + + try + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, new CodeFixRunner(), analyzerResolver, new GuardedFileWriter(), + tempProjectPath, filePath, "LC0020", 11, 17, "any"); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("filePath", root.GetProperty("message").GetString()); + } + finally + { + TestAnalyzers.TryDeleteDirectory(tempProjectPath); + } + } + /// Loads the project, finds the LC0020 fix at MyPage.al:11:17 and applies it through the tool. - private static async Task<(string Result, string EquivalenceKey)> ApplyLc0020Async( + private static async Task<(ModelContextProtocol.Protocol.CallToolResult Result, string EquivalenceKey)> ApplyLc0020Async( string projectPath, string filePath, GuardedFileWriter writer) { using var sessionManager = new ProjectSessionManager(new ProjectLoader()); @@ -207,23 +363,23 @@ public async Task ApplyFix_FileNotValidUtf8_ReturnsUnsupportedEncodingAndLeavesF var analyzerSet = await analyzerResolver.ResolveAsync(projectPath, null); const int line = 11, column = 17; - var fixes = await codeFixRunner.GetFixesAsync(session, filePath, "LC0020", line, column, analyzerSet); - Assert.True(fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17."); + var lookup = await codeFixRunner.GetFixesAsync(session, filePath, "LC0020", line, column, analyzerSet); + Assert.True(lookup.Fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17."); var result = await ApplyFixTool.ApplyFix( sessionManager, codeFixRunner, analyzerResolver, writer, projectPath, filePath, "LC0020", line, column, - fixes[0].EquivalenceKey); + lookup.Fixes[0].EquivalenceKey); - return (result, fixes[0].EquivalenceKey); + return (result, lookup.Fixes[0].EquivalenceKey); } [Fact] - public async Task GetFixes_RulesetSuppressesRule_ReturnsNoFixes() + public async Task GetFixes_RulesetSuppressesRule_ReturnsSuppressedByRuleset() { // FixAllRulesetProject ships a custom.ruleset.json setting LC0020 to "None". Even though - // PageA.al still has a redundant ApplicationArea, get_fixes must not offer a fix for it - // (CodeFixRunner.FindDiagnosticAsync must honor ruleset suppression). + // PageA.al still has a redundant ApplicationArea, get_fixes must not offer a fix for it, + // and must say the ruleset is why. var tempProjectPath = TestAnalyzers.CopyFixtureWithAnalyzers("FixAllRulesetProject", "alcops-ruleset-getfixes-test"); try @@ -241,18 +397,19 @@ public async Task GetFixes_RulesetSuppressesRule_ReturnsNoFixes() // (not a location mismatch) is what suppresses the result below. const int line = 11, column = 17; var withoutRuleset = TestAnalyzers.LoadAnalyzersWithoutRuleset(loader, tempProjectPath); - var controlFixes = await codeFixRunner.GetFixesAsync( + var control = await codeFixRunner.GetFixesAsync( session, filePath, "LC0020", line, column, withoutRuleset); - Assert.True(controlFixes.Count > 0, + Assert.True(control.NotFoundReason is null && control.Fixes.Count > 0, "Expected a fixable LC0020 at line 11, column 17 with no ruleset applied — fixture or location may have changed."); // With the resolved AnalyzerSet (loads FixAllRulesetProject's custom.ruleset.json, // which sets LC0020 to "None"), the same location must yield no fixes. var analyzerSet = await analyzerResolver.ResolveAsync(tempProjectPath, null); - var fixes = await codeFixRunner.GetFixesAsync( + var lookup = await codeFixRunner.GetFixesAsync( session, filePath, "LC0020", line, column, analyzerSet); - Assert.Empty(fixes); + Assert.Equal(FixNotFoundReason.SuppressedByRuleset, lookup.NotFoundReason); + Assert.Empty(lookup.Fixes); } finally { diff --git a/tests/ALCops.Mcp.Tests/FixRoundTripTests.cs b/tests/ALCops.Mcp.Tests/FixRoundTripTests.cs new file mode 100644 index 0000000..997bd98 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/FixRoundTripTests.cs @@ -0,0 +1,94 @@ +using ALCops.Mcp.Models; +using ALCops.Mcp.Services; +using ALCops.Mcp.Tools; +using Microsoft.Dynamics.Nav.CodeAnalysis.CodeFixes; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// +/// Every equivalence key the tools advertise (Ambiguous candidates, get_fixes fixes) must be accepted +/// back verbatim by apply_fix and apply_fix_all. AC0012 in FixAllProject has two fix providers. +/// +public sealed class FixRoundTripTests +{ + private const string Rule = "AC0012"; + + [Fact] + public async Task AdvertisedKeys_RoundTrip_ThroughApplyFixAllGetFixesAndApplyFix() + { + var projectPath = TestAnalyzers.CopyFixtureWithAnalyzers("FixAllProject", "alcops-roundtrip-test"); + try + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var runner = new CodeFixRunner(); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + var writer = new GuardedFileWriter(); + + // 1. No key: Ambiguous, with the candidates. + var ambiguous = await ApplyFixAllTool.ApplyFixAll( + sessionManager, runner, analyzerResolver, writer, projectPath, Rule); + var candidateKeys = ToolResultAssert.Error(ambiguous, "Ambiguous") + .GetProperty("candidates").EnumerateArray() + .Select(c => c.GetProperty("equivalenceKey").GetString()!) + .ToHashSet(StringComparer.Ordinal); + Assert.True(candidateKeys.Count > 1); + + // 2. Every candidate key is accepted by apply_fix_all. + foreach (var key in candidateKeys) + { + var dryRun = await ApplyFixAllTool.ApplyFixAll( + sessionManager, runner, analyzerResolver, writer, projectPath, Rule, + equivalenceKey: key, dryRun: true); + var root = ToolResultAssert.Ok(dryRun); + Assert.Equal(key, root.GetProperty("equivalenceKey").GetString()); + } + + // 3. Locate one occurrence, then get_fixes there advertises the same key set. + var session = await sessionManager.GetOrLoadProjectAsync(projectPath); + var analyzerSet = await analyzerResolver.ResolveAsync(projectPath, null); + var probe = await runner.ApplyFixAllAsync( + session, Rule, FixAllScope.Project, null, candidateKeys.First(), analyzerSet); + Assert.Equal(FixAllStatus.Completed, probe.Status); + var occurrence = probe.Changes[0].Diagnostics[0]; + + var getFixes = await GetFixesTool.GetFixes( + sessionManager, runner, analyzerResolver, + projectPath, occurrence.FilePath, Rule, occurrence.Line, occurrence.Column); + var fixKeys = ToolResultAssert.OkAs(getFixes).Fixes + .Select(f => f.EquivalenceKey) + .ToHashSet(StringComparer.Ordinal); + Assert.Equal(candidateKeys.Order(StringComparer.Ordinal), fixKeys.Order(StringComparer.Ordinal)); + + // 4. Every key is accepted by apply_fix, each on a fresh copy. + var relativeFile = Path.GetRelativePath(projectPath, occurrence.FilePath); + foreach (var key in fixKeys) + await ApplyFixOnFreshCopyAsync(relativeFile, occurrence.Line, occurrence.Column, key); + } + finally + { + TestAnalyzers.TryDeleteDirectory(projectPath); + } + } + + private static async Task ApplyFixOnFreshCopyAsync(string relativeFile, int line, int column, string key) + { + var projectPath = TestAnalyzers.CopyFixtureWithAnalyzers("FixAllProject", "alcops-roundtrip-apply-test"); + try + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await ApplyFixTool.ApplyFix( + sessionManager, new CodeFixRunner(), analyzerResolver, new GuardedFileWriter(), + projectPath, Path.Combine(projectPath, relativeFile), Rule, line, column, key); + + Assert.True(ToolResultAssert.Ok(result).GetProperty("applied").GetBoolean(), + $"apply_fix rejected advertised key '{key}': {ToolResultAssert.Text(result)}"); + } + finally + { + TestAnalyzers.TryDeleteDirectory(projectPath); + } + } +} diff --git a/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithPragma.al b/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithPragma.al new file mode 100644 index 0000000..b6fddad --- /dev/null +++ b/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithPragma.al @@ -0,0 +1,20 @@ +#pragma warning disable LC0020 +page 50102 PageWithPragma +{ + ApplicationArea = All; + + layout + { + area(content) + { + field(MyField; MyField) + { + ApplicationArea = All; + } + } + } + + var + MyField: Text; +} +#pragma warning restore LC0020 diff --git a/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithoutPragma.al b/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithoutPragma.al new file mode 100644 index 0000000..d331445 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/Fixtures/PragmaProject/PageWithoutPragma.al @@ -0,0 +1,18 @@ +page 50103 PageWithoutPragma +{ + ApplicationArea = All; + + layout + { + area(content) + { + field(MyField; MyField) + { + ApplicationArea = All; + } + } + } + + var + MyField: Text; +} diff --git a/tests/ALCops.Mcp.Tests/GetFixesToolTests.cs b/tests/ALCops.Mcp.Tests/GetFixesToolTests.cs new file mode 100644 index 0000000..cef1139 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/GetFixesToolTests.cs @@ -0,0 +1,253 @@ +using System.Collections.Immutable; +using ALCops.Mcp.Models; +using ALCops.Mcp.Services; +using ALCops.Mcp.Tools; +using Microsoft.Dynamics.Nav.CodeAnalysis.CodeFixes; +using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics; +using ModelContextProtocol.Protocol; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// +/// get_fixes: the success object and every NotFound reason. The reasons that the real +/// analyzer configuration cannot produce (NoAnalyzerForRule, NoFixForDiagnostic) are driven at the +/// level through an decorator. +/// +public sealed class GetFixesToolTests +{ + private static async Task GetFixesAsync( + string projectPath, string fileName, string diagnosticId, int line, int column) + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + return await GetFixesTool.GetFixes( + sessionManager, new CodeFixRunner(), analyzerResolver, + projectPath, Path.Combine(projectPath, fileName), diagnosticId, line, column); + } + + private static async Task WithFixtureAsync(string fixtureName, Func> body) + { + var projectPath = TestAnalyzers.CopyFixtureWithAnalyzers(fixtureName, "alcops-getfixes-test"); + try + { + return await body(projectPath); + } + finally + { + TestAnalyzers.TryDeleteDirectory(projectPath); + } + } + + [Fact] + public async Task Success_ReturnsObjectWithPositionAndFixes() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + var result = await GetFixesAsync(projectPath, "MyPage.al", "LC0020", 11, 17); + + var root = ToolResultAssert.Ok(result); + Assert.Equal(System.Text.Json.JsonValueKind.Object, root.ValueKind); + + var fixes = ToolResultAssert.OkAs(result); + Assert.Equal("LC0020", fixes.DiagnosticId); + Assert.Equal(Path.GetFullPath(Path.Combine(projectPath, "MyPage.al")), fixes.FilePath); + Assert.Equal(11, fixes.Line); + Assert.Equal(17, fixes.Column); + Assert.NotEmpty(fixes.Fixes); + Assert.All(fixes.Fixes, f => + { + Assert.NotNull(f.EquivalenceKey); + Assert.False(string.IsNullOrEmpty(f.Title)); + Assert.False(string.IsNullOrEmpty(f.ProviderName)); + }); + return 0; + }); + } + + [Fact] + public async Task NoFixProvider_ForUnknownRule() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + var result = await GetFixesAsync(projectPath, "MyPage.al", "ZZ9999", 11, 17); + + var root = ToolResultAssert.Error(result, "NotFound", "NoFixProvider"); + Assert.Equal("ZZ9999", root.GetProperty("diagnosticId").GetString()); + return 0; + }); + } + + [Fact] + public async Task FileNotInProject_ForUnknownFile() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + var result = await GetFixesAsync(projectPath, "Nope.al", "LC0020", 11, 17); + + ToolResultAssert.Error(result, "NotFound", "FileNotInProject"); + return 0; + }); + } + + [Fact] + public async Task NoDiagnosticAtPosition_WhenRuleIsNotReportedThere() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + var result = await GetFixesAsync(projectPath, "MyPage.al", "LC0020", 1, 1); + + var root = ToolResultAssert.Error(result, "NotFound", "NoDiagnosticAtPosition"); + Assert.Contains("re-run analyze", root.GetProperty("message").GetString()); + Assert.False(root.TryGetProperty("candidates", out _)); + return 0; + }); + } + + [Fact] + public async Task SuppressedByRuleset_WhenRulesetSetsRuleToNone() + { + await WithFixtureAsync("FixAllRulesetProject", async projectPath => + { + var result = await GetFixesAsync(projectPath, "PageA.al", "LC0020", 11, 17); + + ToolResultAssert.Error(result, "NotFound", "SuppressedByRuleset"); + return 0; + }); + } + + [Fact] + public async Task SuppressedByPragma_WhenPragmaDisablesRuleAtPosition() + { + await WithFixtureAsync("PragmaProject", async projectPath => + { + // Control: the same page without the pragma has a fixable LC0020. + var control = await GetFixesAsync(projectPath, "PageWithoutPragma.al", "LC0020", 11, 17); + Assert.NotEmpty(ToolResultAssert.OkAs(control).Fixes); + + // The pragma line shifts the field-level ApplicationArea down by one line. + var result = await GetFixesAsync(projectPath, "PageWithPragma.al", "LC0020", 12, 17); + + ToolResultAssert.Error(result, "NotFound", "SuppressedByPragma"); + return 0; + }); + } + + [Fact] + public async Task Invalid_WhenProjectFolderHasNoAppJson() + { + var folder = Path.Combine(Path.GetTempPath(), $"alcops-getfixes-noappjson-{Guid.NewGuid():N}"); + Directory.CreateDirectory(folder); + try + { + var result = await GetFixesAsync(folder, "MyPage.al", "LC0020", 11, 17); + ToolResultAssert.Error(result, "Invalid"); + + var missing = await GetFixesAsync(Path.Combine(folder, "missing"), "MyPage.al", "LC0020", 11, 17); + ToolResultAssert.Error(missing, "Invalid"); + + // A path Path.GetFullPath rejects is an argument error too, not a fault. + var malformed = await GetFixesAsync("C:\\bad\0path", "MyPage.al", "LC0020", 11, 17); + ToolResultAssert.Error(malformed, "Invalid"); + } + finally + { + TestAnalyzers.TryDeleteDirectory(folder); + } + } + + [Theory] + [InlineData("C:\\bad\0file.al")] + [InlineData("")] + public async Task Invalid_WhenFilePathIsMalformed(string filePath) + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + + var result = await GetFixesTool.GetFixes( + sessionManager, new CodeFixRunner(), analyzerResolver, + projectPath, filePath, "LC0020", 11, 17); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("filePath", root.GetProperty("message").GetString()); + return 0; + }); + } + + [Fact] + public async Task NoAnalyzerForRule_WhenFixProviderExistsButNoAnalyzerReportsTheRule() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + var session = await sessionManager.GetOrLoadProjectAsync(projectPath); + var real = await analyzerResolver.ResolveAsync(projectPath, null); + + var provider = new DecoratedProvider(real, hideAnalyzers: true); + var runner = new CodeFixRunner(); + var lookup = await runner.GetFixesAsync( + session, Path.Combine(projectPath, "MyPage.al"), "LC0020", 11, 17, provider); + + Assert.Equal(FixNotFoundReason.NoAnalyzerForRule, lookup.NotFoundReason); + + // apply_fix_all agrees instead of reporting zero occurrences (which would read as "clean"). + var fixAll = await runner.ApplyFixAllAsync( + session, "LC0020", FixAllScope.Project, null, null, provider); + + Assert.Equal(FixAllStatus.NotFound, fixAll.Status); + Assert.Equal(FixNotFoundReason.NoAnalyzerForRule, fixAll.NotFoundReason); + return 0; + }); + } + + [Fact] + public async Task NoFixForDiagnostic_WhenProviderRegistersNothing() + { + await WithFixtureAsync("ApplyFixProject", async projectPath => + { + using var sessionManager = new ProjectSessionManager(new ProjectLoader()); + var (analyzerResolver, _) = TestAnalyzers.CreateAnalyzerResolver(); + var session = await sessionManager.GetOrLoadProjectAsync(projectPath); + var real = await analyzerResolver.ResolveAsync(projectPath, null); + + var provider = new DecoratedProvider(real, fixProviders: [new SilentFixProvider()]); + var runner = new CodeFixRunner(); + var filePath = Path.Combine(projectPath, "MyPage.al"); + + var lookup = await runner.GetFixesAsync(session, filePath, "LC0020", 11, 17, provider); + Assert.Equal(FixNotFoundReason.NoFixForDiagnostic, lookup.NotFoundReason); + + var apply = await runner.ApplyFixAsync(session, filePath, "LC0020", 11, 17, "", provider); + Assert.Equal(FixNotFoundReason.NoFixForDiagnostic, apply.NotFoundReason); + Assert.Null(apply.Fix); + return 0; + }); + } + + /// Wraps a real provider, optionally hiding its analyzers or replacing its fix providers. + private sealed class DecoratedProvider( + IAnalyzerProvider inner, + bool hideAnalyzers = false, + ImmutableArray? fixProviders = null) : IAnalyzerProvider + { + public ImmutableArray GetAllAnalyzers() => hideAnalyzers ? [] : inner.GetAllAnalyzers(); + public ImmutableArray GetAllCodeFixProviders() => fixProviders ?? inner.GetAllCodeFixProviders(); + public ImmutableArray GetCodeFixProvidersForDiagnostic(string diagnosticId) => + fixProviders ?? inner.GetCodeFixProvidersForDiagnostic(diagnosticId); + public ImmutableDictionary GetAllDescriptors() => inner.GetAllDescriptors(); + public string GetCopName(string diagnosticId) => inner.GetCopName(diagnosticId); + public bool HasCodeFix(string diagnosticId) => inner.HasCodeFix(diagnosticId); + } + + /// Claims LC0020 but registers no code action. + private sealed class SilentFixProvider : CodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds => ["LC0020"]; + + public override Task RegisterCodeFixesAsync(CodeFixContext context) => Task.CompletedTask; + } +} diff --git a/tests/ALCops.Mcp.Tests/ListRulesToolTests.cs b/tests/ALCops.Mcp.Tests/ListRulesToolTests.cs new file mode 100644 index 0000000..71fa042 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/ListRulesToolTests.cs @@ -0,0 +1,128 @@ +using ALCops.Mcp.Services; +using ALCops.Mcp.Tools; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// list_rules: success shapes, project validation shared with analyze (issue #33), and the catch-all. +public sealed class ListRulesToolTests : IDisposable +{ + private readonly string _projectPath = TestAnalyzers.CopyFixtureWithAnalyzers("ApplyFixProject", "alcops-listrules-test"); + + public void Dispose() => TestAnalyzers.TryDeleteDirectory(_projectPath); + + private static (ProjectAnalyzerResolver Analyzers, WorkspaceStartupResolver Workspace) Resolvers(params string[] projects) + { + var (analyzerResolver, loader) = TestAnalyzers.CreateAnalyzerResolver(); + var workspace = new WorkspaceStartupResolver( + analyzerResolver, loader, NullLogger.Instance, projects); + return (analyzerResolver, workspace); + } + + /// A resolver whose only --projects entry does not exist, so it has no projects at all. + private static (ProjectAnalyzerResolver Analyzers, WorkspaceStartupResolver Workspace) NoProjects() => + Resolvers(Path.Combine(Path.GetTempPath(), $"alcops-listrules-missing-{Guid.NewGuid():N}")); + + [Fact] + public async Task Ok_Compact_ListsRulesOfTheStartupProject() + { + var (analyzers, workspace) = Resolvers(_projectPath); + + var result = await ListRulesTool.ListRules(analyzers, workspace); + + var root = ToolResultAssert.Ok(result); + var rules = root.GetProperty("rules").EnumerateArray().ToList(); + Assert.NotEmpty(rules); + Assert.Contains(rules, r => r.GetProperty("id").GetString() == "LC0020"); + Assert.All(rules, r => + { + Assert.True(r.TryGetProperty("title", out _)); + Assert.True(r.TryGetProperty("cop", out _)); + Assert.False(r.TryGetProperty("hasCodeFix", out _)); + }); + } + + [Fact] + public async Task Ok_Verbose_IncludesFullMetadata() + { + var (analyzers, workspace) = Resolvers(_projectPath); + + var result = await ListRulesTool.ListRules(analyzers, workspace, projectPath: _projectPath, verbose: true); + + var root = ToolResultAssert.Ok(result); + var lc0020 = root.GetProperty("rules").EnumerateArray().Single(r => r.GetProperty("id").GetString() == "LC0020"); + Assert.True(lc0020.GetProperty("hasCodeFix").GetBoolean()); + Assert.True(lc0020.TryGetProperty("severity", out _)); + Assert.True(lc0020.TryGetProperty("copName", out _)); + } + + [Fact] + public async Task Invalid_WhenExplicitProjectIsNotAStartupProject() + { + var (analyzers, workspace) = Resolvers(_projectPath); + var other = Path.Combine(Path.GetTempPath(), "SomeOtherProject"); + + var result = await ListRulesTool.ListRules(analyzers, workspace, projectPath: other); + + var root = ToolResultAssert.Error(result, "Invalid"); + var message = root.GetProperty("message").GetString()!; + Assert.Contains(_projectPath, message, StringComparison.OrdinalIgnoreCase); + Assert.Contains("SomeOtherProject", message); + } + + [Fact] + public async Task Invalid_WithNoStartupProjects_AndExplicitPath_DoesNotRenderEmptyList() + { + var (analyzers, workspace) = NoProjects(); + Assert.Empty(workspace.Config.ProjectDirectories); + + var result = await ListRulesTool.ListRules(analyzers, workspace, projectPath: _projectPath); + + var root = ToolResultAssert.Error(result, "Invalid"); + var message = root.GetProperty("message").GetString()!; + Assert.DoesNotContain("started with: .", message); + Assert.Contains("No AL project available", message); + } + + [Fact] + public async Task Invalid_WithNoStartupProjects_AndNoPath() + { + var (analyzers, workspace) = NoProjects(); + + var result = await ListRulesTool.ListRules(analyzers, workspace); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("No AL project available", root.GetProperty("message").GetString()); + } + + [Theory] + [InlineData("C:\\bad\0path")] + [InlineData(" ")] + [InlineData("")] + public async Task Invalid_WhenProjectPathIsMalformed(string projectPath) + { + var (analyzers, workspace) = Resolvers(_projectPath); + + // An embedded NUL makes Path.GetFullPath throw ArgumentException; whitespace is no path at all. + // Both are argument errors, not faults. + var result = await ListRulesTool.ListRules(analyzers, workspace, projectPath: projectPath); + + var root = ToolResultAssert.Error(result, "Invalid"); + Assert.Contains("is not a valid path", root.GetProperty("message").GetString()); + } + + [Fact] + public async Task Faulted_WhenAnUnexpectedExceptionEscapes() + { + var (_, workspace) = Resolvers(_projectPath); + + // list_rules folds analyzer load failures into warnings, so the only cheap way to make it throw + // is a missing service; the catch-all must still turn that into a Faulted envelope. + var result = await ListRulesTool.ListRules(null!, workspace); + + var root = ToolResultAssert.Error(result, "Faulted"); + Assert.Equal("System.NullReferenceException", root.GetProperty("detail").GetString()); + Assert.False(root.TryGetProperty("reason", out _)); + } +} diff --git a/tests/ALCops.Mcp.Tests/MatchPositionTests.cs b/tests/ALCops.Mcp.Tests/MatchPositionTests.cs new file mode 100644 index 0000000..c4fcef1 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/MatchPositionTests.cs @@ -0,0 +1,70 @@ +using ALCops.Mcp.Services; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// +/// Position matching behind get_fixes / apply_fix. An AL pragma covers whole lines, so a live and a +/// suppressed diagnostic of one rule on one line cannot be built from a fixture cheaply; the ordering +/// is tested on the helper directly. +/// +public sealed class MatchPositionTests +{ + private sealed record Hit(string Name, int Line, int Column, bool Suppressed); + + private static (Hit? Hit, bool Suppressed) Match(int line, int column, params Hit[] hits) => + CodeFixRunner.MatchPosition(hits, h => (h.Line, h.Column), h => h.Suppressed, line, column); + + [Fact] + public void ExactSuppressed_WinsOverSameLineLive() + { + var (hit, suppressed) = Match(5, 20, + new Hit("live", 5, 3, false), + new Hit("pragma", 5, 20, true)); + + Assert.Equal("pragma", hit?.Name); + Assert.True(suppressed); + } + + [Fact] + public void ExactLive_WinsOverExactSuppressed() + { + var (hit, suppressed) = Match(5, 3, + new Hit("pragma", 5, 3, true), + new Hit("live", 5, 3, false)); + + Assert.Equal("live", hit?.Name); + Assert.False(suppressed); + } + + [Fact] + public void SameLineLive_WinsOverSameLineSuppressed_WhenNoExactMatch() + { + var (hit, suppressed) = Match(5, 99, + new Hit("pragma", 5, 3, true), + new Hit("live", 5, 20, false)); + + Assert.Equal("live", hit?.Name); + Assert.False(suppressed); + } + + [Fact] + public void SameLineSuppressed_WhenOnlySuppressedOnLine() + { + var (hit, suppressed) = Match(5, 99, + new Hit("other line", 6, 99, false), + new Hit("pragma", 5, 3, true)); + + Assert.Equal("pragma", hit?.Name); + Assert.True(suppressed); + } + + [Fact] + public void NoHit_WhenNothingOnLine() + { + var (hit, suppressed) = Match(5, 3, new Hit("other line", 6, 3, false)); + + Assert.Null(hit); + Assert.False(suppressed); + } +} diff --git a/tests/ALCops.Mcp.Tests/ToolErrorsTests.cs b/tests/ALCops.Mcp.Tests/ToolErrorsTests.cs new file mode 100644 index 0000000..686c0f8 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/ToolErrorsTests.cs @@ -0,0 +1,78 @@ +using System.Text.Json; +using ALCops.Mcp.Models; +using ALCops.Mcp.Services; +using ModelContextProtocol.Protocol; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// The error envelope: isError, key order and omitted optional fields. +public sealed class ToolErrorsTests +{ + private static string[] Keys(CallToolResult result) => + [.. JsonDocument.Parse(ToolResultAssert.Text(result)).RootElement.EnumerateObject().Select(p => p.Name)]; + + [Fact] + public void Faulted_FromException_HasErrorMessageAndDetailOnly() + { + var result = ToolErrors.Faulted(new InvalidOperationException("boom")); + + Assert.Equal(["error", "message", "detail"], Keys(result)); + var root = ToolResultAssert.Error(result, "Faulted"); + Assert.Equal("boom", root.GetProperty("message").GetString()); + Assert.Equal("System.InvalidOperationException", root.GetProperty("detail").GetString()); + } + + [Fact] + public void NotFound_WithCandidates_KeepsKeyOrder() + { + var result = ToolErrors.NotFound( + FixNotFoundReason.NoFixForEquivalenceKey, "no such key", "C:\\p\\a.al", "LC0020", + [new CodeFixInfo("k1", "Title 1", "Provider")]); + + Assert.Equal(["error", "message", "reason", "candidates", "filePath", "diagnosticId"], Keys(result)); + var root = ToolResultAssert.Error(result, "NotFound", "NoFixForEquivalenceKey"); + var candidate = Assert.Single(root.GetProperty("candidates").EnumerateArray().ToList()); + Assert.Equal(["equivalenceKey", "title", "providerName"], candidate.EnumerateObject().Select(p => p.Name).ToArray()); + } + + [Fact] + public void Invalid_OmitsEveryOptionalField() + { + Assert.Equal(["error", "message"], Keys(ToolErrors.Invalid("bad"))); + } + + [Fact] + public void Ok_LeavesIsErrorUnset() + { + var result = ToolResults.Ok(new { applied = true }); + + Assert.Null(result.IsError); + Assert.Equal("{\"applied\":true}", ToolResultAssert.Text(result)); + } + + [Fact] + public void EveryError_SetsIsError() + { + CallToolResult[] errors = + [ + ToolErrors.Invalid("m"), + ToolErrors.NotFound(FixNotFoundReason.NoFixProvider, "m"), + ToolErrors.Ambiguous("m", [new CodeFixInfo("k", "t", "p")]), + ToolErrors.Stale("f.al", "LC0020", "m"), + ToolErrors.Unavailable(UnavailableReason.NoProxy, "m"), + ToolErrors.Faulted(new IOException("m")), + ToolErrors.Faulted(FileWriteConflictKind.WriteFailed, "m"), + ]; + + Assert.All(errors, e => Assert.True(e.IsError)); + } + + [Fact] + public void NotFoundMessage_NoDiagnosticAtPosition_PointsToAnalyze() + { + var message = ToolErrors.NotFoundMessage(FixNotFoundReason.NoDiagnosticAtPosition, "LC0020", "C:\\p\\a.al", 3, 5); + + Assert.Equal("No LC0020 at C:\\p\\a.al:3:5; re-run analyze and use its line/column.", message); + } +} diff --git a/tests/ALCops.Mcp.Tests/ToolResultAssert.cs b/tests/ALCops.Mcp.Tests/ToolResultAssert.cs new file mode 100644 index 0000000..4ae3156 --- /dev/null +++ b/tests/ALCops.Mcp.Tests/ToolResultAssert.cs @@ -0,0 +1,51 @@ +using System.Text.Json; +using ModelContextProtocol.Protocol; +using Xunit; + +namespace ALCops.Mcp.Tests; + +/// Assertions over the the native tools return. +internal static class ToolResultAssert +{ + /// The single text block of a native tool result. + public static string Text(CallToolResult result) => + Assert.IsType(Assert.Single(result.Content)).Text; + + /// Asserts a success result (no isError, no error property) and returns its JSON root. + public static JsonElement Ok(CallToolResult result) + { + var text = Text(result); + Assert.True(result.IsError != true, $"Expected success, got an error result: {text}"); + + var root = JsonDocument.Parse(text).RootElement; + if (root.ValueKind == JsonValueKind.Object) + Assert.False(root.TryGetProperty("error", out _), $"Success result carries an error property: {text}"); + return root; + } + + /// Asserts a success result and deserializes it with the server's JSON options. + public static T OkAs(CallToolResult result) + { + Ok(result); + return JsonSerializer.Deserialize(Text(result), JsonDefaults.Options)!; + } + + /// + /// Asserts an error result with isError set, as error and, when given, + /// as reason. Returns the JSON root. + /// + public static JsonElement Error(CallToolResult result, string code, string? reason = null) + { + var text = Text(result); + Assert.True(result.IsError == true, $"Expected isError = true: {text}"); + + var root = JsonDocument.Parse(text).RootElement; + Assert.Equal(code, root.GetProperty("error").GetString()); + Assert.False(string.IsNullOrEmpty(root.GetProperty("message").GetString()), $"Empty message: {text}"); + + if (reason is not null) + Assert.Equal(reason, root.GetProperty("reason").GetString()); + + return root; + } +}