feat(pc0038,fc0007,lc0089,lc0090) Improve flow-terminating built-in analysis

.claude/rules/diagnostics/fc0007-statement-blocks-separated-by-blank-line.md

@@ -8,9 +8,9 @@ paths:
 
 ## Purpose
 
-Reports missing blank lines around statement blocks: before/after control-flow constructs (`if`, `case`, `repeat`, `while`, `for`, `foreach`) and before scope-leaving statements (`exit`, built-in `Error(...)`). It is highly opinionated and therefore disabled by default; enable it explicitly and configure it via `alcops.json`.
+Reports missing blank lines around statement blocks: before/after control-flow constructs (`if`, `case`, `repeat`, `while`, `for`, `foreach`) and before scope-leaving statements (`exit`, built-in `Error(...)` and `FieldError(...)`). It is highly opinionated and therefore disabled by default; enable it explicitly and configure it via `alcops.json`.
 
-Registers `RegisterSyntaxNodeAction` on the control-flow statement kinds and `ExitStatement`, plus `RegisterOperationAction` on `InvocationExpression` for built-in `Error`; main type `StatementBlocksSeparatedByBlankLine`.
+Registers `RegisterSyntaxNodeAction` on the control-flow statement kinds and `ExitStatement`, plus `RegisterOperationAction` on `InvocationExpression` for the built-in terminators classified by `ALCops.Common.FlowTerminatingBuiltIns`; main type `StatementBlocksSeparatedByBlankLine`.
 
 ## Design decisions
 
@@ -21,22 +21,24 @@ Registers `RegisterSyntaxNodeAction` on the control-flow statement kinds and `Ex
 | Blank-line check inspects `SourceText.Lines` strictly between the two token positions and requires at least one whitespace-only line, rather than comparing token line numbers | A line-number diff would count a comment-only or directive line as a separator, contradicting the intended semantics. |
 | Sibling `StatementSyntax` nodes are taken from the parent, not from a `BlockSyntax` | Statement sequencing also occurs in contexts without a `BlockSyntax`, for example `repeat`. |
 | Each statement gap has exactly one configuration-aware diagnostic owner: an adjacent block owns its "before" gap only when that check actually runs, otherwise the previous block's "after" check or the scope-leaver's "before" check does | Avoids duplicate diagnostics (e.g. a block followed by `exit`) while still reporting next to one-liners or when `ControlFlowBefore` is disabled. |
-| `Error(...)` detected via `MethodKind.BuiltInMethod` + `IsSameName` | Distinguishes the built-in from user-defined `Error` procedures. |
+| Scope-leaving calls are the shared `FlowTerminatingBuiltIns` set (`Dialog.Error`, `Table.FieldError`, `FieldRef.FieldError`), including incomplete calls whose receiver binds to those types | One semantic definition shared with PC0038 and LC0089; the exact class-and-method match keeps user-defined `Error`/`FieldError` procedures out, and accepting the invalid binding avoids flicker while a call is being typed. |
+| The diagnostic names the terminating call that was found (`Error()`, `FieldError()`) | The message must match the statement the developer is looking at, not always say `Error()`. |
 
 ## Deliberate non-reports
 
 - The first statement in a block and the first statement directly owned by a control-flow construct.
 - One-liner statements (the whole statement on a single line) unless `OneLinerMode = All`; `if X then Y` rarely benefits from surrounding blank lines.
 - `else` that shares its line with the previous token (`if X then Y else Z`), even with `ElseChainBeforeMode = RequireBlank`.
-- An `exit` or `Error(...)` used directly as an `if` branch: branch statements are not siblings in a statement list, so only the containing `if` is governed (by `ControlFlowBefore`/`ControlFlowAfter` and, for one-line guards, `OneLinerMode`).
-- Loop-control statements (`break`, `continue`, `Skip`): only `exit` and built-in `Error(...)` are scope-leavers.
-- User-defined procedures named `Error`.
+- An `exit`, `Error(...)` or `FieldError(...)` used directly as an `if` branch: branch statements are not siblings in a statement list, so only the containing `if` is governed (by `ControlFlowBefore`/`ControlFlowAfter` and, for one-line guards, `OneLinerMode`).
+- Loop-control statements (`break`, `continue`, `Skip`): only `exit` and the built-in `Error`/`FieldError` terminators are scope-leavers.
+- User-defined procedures named `Error` or `FieldError`.
 - Blank lines between the branches of a `case` statement; only the spacing around the whole `case` block is enforced.
 
 ## Known issues
 
 - Comment-only lines between statements are not separators; only whitespace-only lines count, so a `//---- divider` line still yields a diagnostic.
 - Compiler-directive lines (`#region`/`#endregion`, `#pragma`) count as non-blank interior lines and do not satisfy the blank-line requirement.
+- A collectible `Error(ErrorInfo)` inside an `ErrorBehavior::Collect` scope is treated as scope-leaving although execution continues; inherited from the shared classifier (see PC0038's Known issues).
 
 ## Settings
 
@@ -46,7 +48,7 @@ All properties sit under the nested `StatementBlockSpacing` object in `alcops.js
 |---|---|---|
 | `ControlFlowBefore` | `true` | Require a blank line before control-flow blocks. |
 | `ControlFlowAfter` | `true` | Require a blank line after control-flow blocks; skipped for an adjacent control-flow sibling only when that sibling's active "before" check owns the gap. |
-| `ScopeLeavingMode` | `ExitAndError` | Which scope-leaving statements (`exit`, built-in `Error`) require a blank line before them; `Off` disables. |
+| `ScopeLeavingMode` | `ExitAndError` | Which scope-leaving statements (`exit`, built-in `Error`/`FieldError`) require a blank line before them; `ErrorOnly` limits it to the built-in terminators, `Off` disables. |
 | `ElseChainBeforeMode` | `Off` | `RequireBlank` requires a blank line before `else` / `else if` (one-line `else` exempt). |
 | `OneLinerMode` | `None` | `All` includes one-liner statements in the spacing checks. |
 
@@ -55,3 +57,4 @@ All properties sit under the nested `StatementBlockSpacing` object in `alcops.js
 - The rule is `isEnabledByDefault: false`, so the test class injects `StatementBlocksSeparatedByBlankLine.ruleset.json`.
 - Settings variants are named `alcops.json` snippets injected via `MemoryFileSystem` and selected per `TestCase` (`null` = defaults).
 - Two regression fixtures exercise the settings provider: a malformed enum value must fall back to defaults silently, and `"StatementBlockSpacing": null` must be normalized to defaults so the analyzer does not NRE ([#328](https://github.com/ALCops/Analyzers/issues/328)).
+- An incomplete `FieldError` call is tested through a fixture created with `ThrowsWhenInputDocumentContainsError = false` (`HasDiagnosticInDocumentWithErrors`), and a message assertion checks that the diagnostic names `FieldError()`.

.claude/rules/diagnostics/lc0089-cognitive-complexity.md

@@ -22,15 +22,18 @@ Registers `CompilationStartAction` (captures threshold, LC0089i enablement and t
 | Recursion detection in a separate compilation-scoped `CognitiveComplexityRecursionGraphService` | The call graph is built once per compilation while the complexity walk is per method; mixing the two scopes in one class would be wrong. |
 | LC0089 and LC0089i are `Info` and `isEnabledByDefault: false`; LC0089i is gated behind `IsDiagnosticEnabled` | A metric is not a violation and per-increment detail is noise unless a specific method is being investigated; the gate makes the walk-and-report cost zero when disabled. |
 | Guard-clause discount | Standard Cognitive Complexity: an early exit `if <condition> then exit/error/break/continue/skip/quit` simplifies flow and does not increment. |
+| Built-in error guards are recognized through the shared `FlowTerminatingBuiltIns` classifier (`Dialog.Error`, `Table.FieldError`, `FieldRef.FieldError`); user-defined procedures named `Error` or `FieldError` stay ordinary calls | One semantic definition of "terminates the procedure" shared with PC0038 and FC0007; a name match alone would discount a user procedure that merely happens to be called `Error`. |
+| `CurrReport`/`CurrXMLport` `Break`, `Skip` and `Quit` keep their own syntax check instead of joining `FlowTerminatingBuiltIns` | They are valid guard exits for complexity but not general procedure terminators, so they must not leak into PC0038's flow analysis. |
 
 ## Deliberate non-reports
 
 - LC0089 and LC0089i are never emitted unless explicitly enabled via `.editorconfig` or a ruleset.
-- Guard clauses (`if cond then exit/error/break/continue/skip/quit`) add no complexity, so methods made of early exits stay below the threshold.
+- Guard clauses (`if cond then exit/error/fielderror/break/continue/skip/quit`) add no complexity, so methods made of early exits stay below the threshold.
 
 ## Known issues
 
 - `ALCopsSettingsProvider` caches the threshold statically by directory with no invalidation; changing `alcops.json` requires restarting the language server. Cross-cop limitation, not specific to this analyzer.
+- A collectible `Error(ErrorInfo)` inside an `ErrorBehavior::Collect` scope is discounted as a guard although execution continues; inherited from the shared classifier (see PC0038's Known issues).
 
 ## Test notes
 

.claude/rules/diagnostics/pc0038-not-all-code-paths-return-value.md

@@ -2,6 +2,7 @@
 paths:
   - "src/ALCops.PlatformCop/**/NotAllCodePathsReturnValue*"
   - "src/ALCops.PlatformCop.Test/Rules/NotAllCodePathsReturnValue/**"
+  - "src/ALCops.Common/FlowTerminatingBuiltIns.cs"
 ---
 
 # PC0038: NotAllCodePathsReturnValue
@@ -11,7 +12,9 @@ paths:
 Detects procedure declarations with an explicit return type where at least one reachable path does not return a value.
 The rule excludes TryFunction methods.
 
-Registers `RegisterSyntaxNodeAction` on `SyntaxKind.MethodDeclaration`; main type `NotAllCodePathsReturnValue`.
+Registers `RegisterSyntaxNodeAction` on `SyntaxKind.MethodDeclaration`; main type `NotAllCodePathsReturnValue`, with flow-terminating calls classified by `ALCops.Common.FlowTerminatingBuiltIns`.
+
+**References:** [#463](https://github.com/ALCops/Analyzers/issues/463) (`FieldError` false positive), [#468](https://github.com/ALCops/Analyzers/issues/468) (built-in name matching), [#471](https://github.com/ALCops/Analyzers/issues/471) (flow edge cases).
 
 ## Design decisions
 
@@ -21,25 +24,40 @@ Registers `RegisterSyntaxNodeAction` on `SyntaxKind.MethodDeclaration`; main typ
 | TryFunction methods excluded | TryFunction has implicit platform semantics |
 | Path-state analysis over the `IOperation` tree | Works consistently for nested blocks and AL control-flow constructs |
 | A named return variable counts as returned when definitely assigned on every fallthrough path | Matches the AL named-return pattern without forcing `exit()` |
-| Bare `exit` is a missing value unless the named return was already assigned on that path | Prevents silent default-value returns on early exits |
-| Built-in `Error(...)` and `ThrowError` terminate the path | Guard clauses like `if Cond then exit(x) else Error('...')` are pervasive in AL; without this the rule would fire on every such branch |
-| Passing the named return to a `var` parameter or using it as an invocation receiver (`Rec.Get(No)`) counts as assignment | Covers out-parameter initialization and `Get`/`FindFirst` into the return record; intentionally conservative to avoid noise |
+| Bare `exit` is a missing value unless the named return was already assigned on that path; `exit(<value>)`, bare `exit` and assigned fallthrough stay distinct | Prevents silent default-value returns on early exits |
+| Only `Dialog.Error`, `Table.FieldError` and `FieldRef.FieldError` terminate a path, matched on the exact built-in class and method through the shared `FlowTerminatingBuiltIns` classifier; the former `ThrowError` special case is gone | Guard clauses like `if Cond then exit(x) else Error('...')` are pervasive; matching `MethodKind.BuiltInMethod` plus a name would also accept a future unrelated built-in or a crossed pair, and `ThrowError` is not an AL built-in |
+| Incomplete (invalid) calls terminate only when the binder's synthesized receiver is the matching `Dialog`, `Record` or `FieldRef` type | Stops the diagnostic from flickering while an argument is being typed, without treating user-defined `Error`/`FieldError` procedures as terminators |
+| Passing the named return to a `var` parameter or using it as an invocation receiver (`Rec.Get(No)`) counts as assignment | These calls can write the value; the rule stops at the call boundary rather than proving the write interprocedurally, trading a possible false negative for bounded analysis and no noise |
+| `if` conditions, `case` selectors, `while` and `repeat until` conditions, `for` bounds and `foreach` collection expressions contribute the `var` side effects of the invocations they contain | Those expressions execute at least once regardless of the body, so `if not JsonObject.Get(Key, Result) then Error(...)` initializes `Result` |
+| Under `and`/`or` only the left operand is guaranteed; right-operand states are unioned with the short-circuit path, and conditional expressions union both branches. `xor` is not treated as branching | AL short-circuits `and`/`or` and evaluates only one branch of a conditional expression, while `xor` always evaluates both operands |
+| A `case` over an enum or option without `else` is exhaustive when every value visible in the current compilation is covered | Matches how the compiler sees the enum, including enum-extension values; a missing `else` on an exhaustive `case` is not a missing return |
 | Named-return target matching falls back to symbol kind `ReturnValue`, never to name alone | Name comparison misclassified member accesses sharing the return variable's name (`Buf.Result := 5;` with a field named `Result`) |
-| Loops conservatively include the non-executed path for optional loops | A loop body that may not run cannot guarantee a return |
+| Loops conservatively include the non-executed path for optional loops; `break` is a loop exit, not body fallthrough | A loop body that may not run cannot guarantee a return, and a `repeat until` left through `break` never evaluates its condition |
 | Reported at the method name | User requirement |
 
 ## Deliberate non-reports
 
 - Triggers, even with a return type.
 - TryFunction methods.
-- Paths ending in `Error(...)` or `ThrowError`.
-- Named returns passed by `var` or used as a receiver: assumed assigned.
+- Paths ending in a clean `Dialog.Error`, `Table.FieldError` or `FieldRef.FieldError` call, or in an incomplete call whose receiver binds to one of those types.
+- Named returns passed by `var` or used as a receiver: assumed assigned, even when the callee never writes.
+- `case` statements without `else` that cover every enum or option value visible in the compilation.
 
 ## Known issues
 
+- `Error(ErrorInfo)` is treated as terminating even when `ErrorInfo.Collectible = true` inside an `ErrorBehavior::Collect` scope, where execution continues. Telling the two apart needs data flow on the `ErrorInfo` value and the enclosing call context, which the invocation-level classifier does not have.
 - `LoopKind.Repeat` handling depends on SDK loop metadata availability across versions; behaviour is conservative for optional-loop execution.
 - `case` line body extraction uses a reflective fallback to remain compatible across SDK versions.
 
 ## SDK facts
 
 - The AL SDK wraps a case's else clause in `IStatementList` (`BoundStatementList`), not `IBlockStatement`, so both shapes must be traversed.
+- `CompilationUtilities.GetEnumValues` (internal) is the only complete source of enum values: the public enum value lists omit values added by enum extensions. System options expose their values through the public `IContainerSymbol.GetMembers()`.
+- The operation interfaces for conditional expressions differ per target framework, so the analyzer switches on `OperationKind` and reads the operands reflectively instead of naming the interface.
+- Parenthesized expressions wrap their operand in a separate operation and must be unwrapped before the short-circuit rules apply.
+- Bad-call and built-in identity facts (single-candidate bad calls keep the real symbol, built-in classes `Table`/`FieldRef` have `NavTypeKind.None`, bare `Error` is a static built-in on `Dialog`) are in `.claude/rules/symbol-resolution.md`.
+
+## Test notes
+
+- Fixtures are gated with `SkipTestIfVersionIsTooLow`: an enum extension declared in the same module needs runtime `13.0`; the ternary conditional expression and `this` need `14.0`.
+- Incomplete `Error`/`FieldError` calls are tested with a second fixture created with `ThrowsWhenInputDocumentContainsError = false`; those test methods are named `*InDocumentWithErrors`.

.claude/skills/fix-false-positive/references/regression-catalog.md

@@ -17,5 +17,7 @@ Recurring causes of false positives/negatives, mined from `fix(...)` commits. Wh
 | **Message placeholders vs. arguments** | A false "wrong message" report is often `{n}` placeholders that do not match `messageArgs`. | multiple rules #415 |
 | **Statement/blank-line interactions with control flow** | Formatting rules must reason about `begin`/`end` nesting, `else if`, and `case` branches together; a fix for one interaction easily regresses another. | FC0007 #457 |
 | **Non-record DB access types (`DataTransfer`)** | The table is an argument, not the receiver: `SetTables(Database::X, Database::Y)` names the tables and `CopyFields`/`CopyRows` executes. Receiver-keyed maps (`MethodOperationMap`, record variable maps) see nothing, so the access is invisible to both permission rules. Resolve from the `SetTables` that reaches the executor in flow order (strict reset, branch union); bail out when unresolvable or none reaches it. | AC0031/AC0032 #465 |
+| **Built-in method names are not identities** | `MethodKind.BuiltInMethod` plus a method name can match a future built-in on the wrong class. Anchor semantic classification to the exact containing built-in class and method pair; use receiver `NavTypeKind` only for invalid editor-time bindings. | PC0038 #468 |
+| **Flow-analysis operation wrappers and bypasses** | Parenthesized expressions must be unwrapped before applying short-circuit rules; `break` is a loop exit rather than body fallthrough; and enum exhaustiveness must use the compiler's complete enum-value helper because public enum value lists omit enum-extension values. Use `OperationKind` plus reflective operands for operation interfaces that differ by target framework. | PC0038 #471 |
 | **Record methods reach their receiver in four forms** | The receiver may be a named variable (`MyTable.M()`), the implicit `Rec` (`Rec.M()`), bare implicit self (`M()`), or `this` (`this.M()`). Instance-null gates skip bare self; name-keyed maps mis-key `this` (table name instead of "Rec") and bare (null). In tableextensions all self forms redirect to the target table. Resolve with `GetReceiverTableType`. | AC0032 #343, batch #348 |
 | **Version-scoped syntax** (`this`, newer keywords) | Fixtures need `SkipTestIfVersionIsTooLow("14.0")` or `RequireMinimumVersion(...)`; a rule may need a `VersionProvider` gate rather than a code change. | PC0035 fixtures, PC0029 |