fix(LC0031): convert bare LockTable() to bare ReadIsolation in the code fix

.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md

@@ -0,0 +1,72 @@
+---
+paths:
+  - "src/ALCops.LinterCop/**/RecordInstanceIsolationLevel*"
+  - "src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/**"
+---
+
+# LC0031: RecordInstanceIsolationLevel
+
+## Purpose
+
+Flags `LockTable()` on `Record` and `RecordRef` instances and suggests `ReadIsolation(IsolationLevel::UpdLock)`. `LockTable()` sets transaction-wide table state: every subsequent read of that table, on any variable, uses UPDLOCK until commit, and tri-state optimistic reads are disabled for it. `ReadIsolation` is local to one record instance.
+
+Registers `RegisterOperationAction` on `InvocationExpression`; matches built-in methods named `LockTable`.
+
+**References:**
+- [Record instance isolation level](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-read-isolation), [Record.LockTable](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-locktable-method), [Tri-state locking](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-tri-state-locking), [Performance for developers](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/performance/performance-developer) (Microsoft Learn)
+- microsoft/BCQuality [prefer-readisolation-over-locktable-for-reads.md](https://github.com/microsoft/BCQuality/blob/main/microsoft/knowledge/performance/prefer-readisolation-over-locktable-for-reads.md) and [do-not-locktable-in-read-only-procedure.md](https://github.com/microsoft/BCQuality/blob/main/microsoft/knowledge/performance/do-not-locktable-in-read-only-procedure.md)
+- [#530](https://github.com/ALCops/Analyzers/issues/530) (LockTable in table triggers), [#545](https://github.com/ALCops/Analyzers/issues/545) (receiver-form audit)
+
+## Design decisions
+
+| Decision | Rationale |
+|---|---|
+| Severity Info, category Design | The call is legal and not deprecated; the replacement narrows lock scope, so it is a suggestion, not a defect |
+| Version gate `Spring2023OrGreater` (runtime 11.0) | `ReadIsolation` is a runtime 11.0 built-in (BC22) |
+| Self `LockTable()` in table and tableextension triggers is reported on purpose (#530) | The compiler binds bare, `Rec.`, `this.` and named-variable receivers to one `TableClassTypeSymbol` built-in with no self-receiver special case, and the transaction-wide effect is identical inside a trigger. Microsoft's own new code does not write it: W1 app corpus trigger `LockTable()` calls went 80 to 78 from BC 23.5 to 28.4 (two removals, zero additions); Business Foundation, E-Document Core, Subscription Billing and Excise Taxes contain no `LockTable` at all; no Microsoft guidance or community source treats triggers as a special case. The Base Application is legacy here, not the oracle |
+| Built-in matched by name only, so `RecordRef.LockTable()` is reported too | `RecordRef` has both `LockTable` and `ReadIsolation` with the same scope difference |
+| No dead-call detection | Whether a following read depends on the lock is a judgement the developer makes; the alcops.dev page explains convert versus delete |
+
+### Receiver-form verdicts (#545)
+
+| Form | Analyzer | CodeFix |
+|---|---|---|
+| Named variable | ok, pinned | ok, pinned |
+| `Rec.` (table, tableextension, page, `TableNo` OnRun) | ok, pinned | ok, pinned (table); same path elsewhere, not pinned |
+| Bare self in table and tableextension | ok, pinned | fixed (#530), pinned |
+| Bare self on page and in `TableNo` OnRun | ok, pinned | `IdentifierNameSyntax` path; page pinned, OnRun not pinned |
+| `this.` (table, tableextension) | ok, pinned | ok, pinned (table); tableextension same path, not pinned |
+| Namespaced fully qualified variable | ok, pinned | same path as named variable, not pinned |
+| `RecordRef` variable | ok, pinned | ok, pinned |
+| `LockTable(true)` (arguments) | ok, pinned | arguments dropped, see Known issues |
+
+## Deliberate non-reports
+
+- `ReadIsolation` itself, in method and property form: the binder rewrites `ReadIsolation := X` into the same `BoundCall` as `ReadIsolation(X)`, and the rule matches the method name only.
+- Obsolete code (`IsObsolete()`).
+
+## Known issues
+
+- The fix drops `LockTable(Wait, VersionCheck)` arguments because `ReadIsolation` has no equivalent. Accepted at Info severity; the developer reviews the edit.
+- The fix always converts, even when the call is dead. A Remove action was considered and rejected: deleting is a second semantics-changing edit the developer has to judge anyway, and the docs page carries the choice.
+
+## SDK facts
+
+- `TableClassTypeSymbol` declares `LockTable(Wait?, VersionCheck?)` with no version gate and no deprecation, while the same class deprecates `FindSet(ForUpdate, UpdateKey)` with a message; the absence is deliberate. `ReadIsolation` is a property-style built-in (`isProperty: true`) gated on runtime 11.0. `RecordRefClassTypeSymbol` mirrors both (verified against SDK 18.0.41 in `../nav-sdk-source`).
+- `Rec` inside a table is synthesized as a plain record of the table's own type (`TableObjectMembers`); all receiver forms bind to one singleton built-in symbol. `Binder.BindAssignmentStatement` rewrites the property form of `ReadIsolation` into the one-argument call.
+
+## Test notes
+
+- `this` fixtures are gated on runtime 14.0 with `SkipTestIfVersionIsTooLow` in both `HasDiagnostic` and `HasFix`.
+- Tableextension fixtures are gated on SDK 13.0: older compilers reject a tableextension whose target is declared in the same module (AL0334).
+- `HasFix/BareSelfWithLeadingComment` pins trivia preservation; it fails when `WithTriviaFrom` is removed from the fix.
+
+## CodeFix: RecordInstanceIsolationLevelCodeFixProvider
+
+| Decision | Rationale |
+|---|---|
+| Single Replace action | Converting never widens locking; deletion is documented, not automated (Known issues) |
+| Keep the author's receiver form: `memberAccess.Expression` reused verbatim, bare stays bare | A `Rec.` prefix on the bare form was rejected; `this.` needs no `ThisExpressionSyntax` reference this way (`netstandard21-compatibility.md`) |
+| `WithTriviaFrom(invocationExpression)` on the replacement | A fresh `SyntaxFactory` identifier carries only elastic trivia, so the bare form would lose the indentation and comments attached to the `LockTable` token; the member-access form kept them only because the receiver node was reused |
+| Any other expression shape returns the unchanged document | Never throw from a fix |
+| `WellKnownFixAllProviders.BatchFixer` | Each diagnostic replaces its own invocation node; no shared ancestor |

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

@@ -19,7 +19,7 @@ Recurring causes of false positives/negatives, mined from `fix(...)` commits. Wh
 | **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 fields, record methods and user procedures reach their receiver in four forms** | The receiver may be a named variable (`MyTable.M()`, `MyTable.F`), the implicit `Rec`, bare implicit self (`M()`, `F`), or `this`. Instance-null gates skip bare self inside tables and tableextensions; 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`; fixture set in `testing.md` rule 6. | AC0032 #343, batch #348, PC0029 #544 |
+| **Record fields, record methods and user procedures reach their receiver in four forms** | The receiver may be a named variable (`MyTable.M()`, `MyTable.F`), the implicit `Rec`, bare implicit self (`M()`, `F`), or `this`. Instance-null gates skip bare self inside tables and tableextensions; name-keyed maps mis-key `this` (table name instead of "Rec") and bare (null). In tableextensions all self forms redirect to the target table. CodeFixes share the gap: a fix that requires `MemberAccessExpressionSyntax` silently returns the unchanged document on the bare form. Resolve with `GetReceiverTableType`; fixture set in `testing.md` rule 6. | AC0032 #343, batch #348, PC0029 #544, LC0031 #530 |
 | **`Rec` has several origins** | Tables and tableextensions: a synthesized global marked as the object's own instance, bare access binds with a null instance. Pages, page extensions, request pages, reports and xmlports: a synthesized global for the (request page's) `SourceTable`, bare access is rewritten to an explicit `Rec` receiver. A codeunit with `TableNo`: `Rec` is a synthesized **local** of `trigger OnRun` only, no `xRec`, `this` is the codeunit. Report and query dataitems: no `Rec`, the instance is a dataitem access. A `SymbolKind.GlobalVariable` test or a non-null instance gate misses some of these. | PC0029 #544 |
 | **Table shape: implicit primary key** | A table without a `keys` section has a synthesized primary key over its first valid field. `ITableTypeSymbol.Keys` never lists it (also for referenced `.app` tables); only `PrimaryKey` does. Key-membership logic must read `PrimaryKey`, and every rule reading keys needs a fixture without a `keys` section. | PC0029 #544 |
 | **One `SymbolKind` covers several AL constructs** | `SymbolKind.Action` spans `area`, `group`, `action`, `separator`, `actionref`, `customaction`, `systemaction` and `fileuploadaction`; `SymbolKind.Control` spans `area`, `group`, `field`, `part` and the rest. Read `IActionSymbol.ActionKind` / `IControlSymbol.ControlKind` before treating a name, caption or property as developer-chosen. | LC0092 #537, AC0011 |

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInProcedure.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    procedure MyProcedure()
+    begin
+        [|LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTableExtension.al

@@ -0,0 +1,15 @@
+tableextension 50001 MyTableExtension extends MyTable
+{
+    procedure MyProcedure()
+    begin
+        [|LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTrigger.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnInsert()
+    begin
+        [|LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/LockTableWithArguments.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    procedure MyProcedure()
+    begin
+        [|Rec.LockTable(true);|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamedVariable.al

@@ -0,0 +1,17 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyTable: Record MyTable;
+    begin
+        [|MyTable.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamespacedQualifiedRecordVariable.al

@@ -0,0 +1,19 @@
+namespace MyPublisher.MyExtension.MyAppDomain;
+
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyTable: Record MyPublisher.MyExtension.MyAppDomain.MyTable;
+    begin
+        [|MyTable.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunBareSelf.al

@@ -0,0 +1,17 @@
+codeunit 50100 MyCodeunit
+{
+    TableNo = MyTable;
+
+    trigger OnRun()
+    begin
+        [|LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunRecSelf.al

@@ -0,0 +1,17 @@
+codeunit 50100 MyCodeunit
+{
+    TableNo = MyTable;
+
+    trigger OnRun()
+    begin
+        [|Rec.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageBareSelf.al

@@ -0,0 +1,17 @@
+page 50100 MyPage
+{
+    SourceTable = MyTable;
+
+    trigger OnOpenPage()
+    begin
+        [|LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageRecSelf.al

@@ -0,0 +1,17 @@
+page 50100 MyPage
+{
+    SourceTable = MyTable;
+
+    trigger OnOpenPage()
+    begin
+        [|Rec.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTableExtension.al

@@ -0,0 +1,15 @@
+tableextension 50001 MyTableExtension extends MyTable
+{
+    procedure MyProcedure()
+    begin
+        [|Rec.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTrigger.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnDelete()
+    begin
+        [|Rec.LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecordRefVariable.al

@@ -0,0 +1,9 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyRecordRef: RecordRef;
+    begin
+        [|MyRecordRef.LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInProcedure.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    procedure MyProcedure()
+    begin
+        [|this.LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTableExtension.al

@@ -0,0 +1,15 @@
+tableextension 50001 MyTableExtension extends MyTable
+{
+    procedure MyProcedure()
+    begin
+        [|this.LockTable();|]
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTrigger.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnInsert()
+    begin
+        [|this.LockTable();|]
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/current.al

@@ -0,0 +1,15 @@
+tableextension 50001 MyTableExtension extends MyTable
+{
+    procedure MyProcedure()
+    begin
+        [|LockTable()|];
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/expected.al

@@ -0,0 +1,15 @@
+tableextension 50001 MyTableExtension extends MyTable
+{
+    procedure MyProcedure()
+    begin
+        ReadIsolation(IsolationLevel::UpdLock);
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/current.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnInsert()
+    begin
+        [|LockTable()|];
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/expected.al

@@ -0,0 +1,12 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnInsert()
+    begin
+        ReadIsolation(IsolationLevel::UpdLock);
+    end;
+}

src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/current.al

@@ -0,0 +1,13 @@
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; MyField; Integer) { }
+    }
+
+    trigger OnInsert()
+    begin
+        // Serialize inserts on this table
+        [|LockTable()|];
+    end;
+}