From 7a379a2629e2b1a58f8dcabd90ceac32b7904edb Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 11:03:43 +0200 Subject: [PATCH 1/6] test(LC0031): pin all receiver forms and the bare-self code fix Add the full receiver-form fixture set (named variable, Rec, bare, this, tableextension, page, TableNo OnRun, namespaced, RecordRef, LockTable with arguments) plus NoDiagnostic fixtures for the obsolete gate and both ReadIsolation forms. HasFix/BareSelfInTrigger and HasFix/BareSelfInTableExtension are expected to fail at this commit: the code fix returns the unchanged document when the invocation expression is not a MemberAccessExpression. Co-Authored-By: Claude Fable 5.1 --- .../HasDiagnostic/BareSelfInProcedure.al | 12 +++++ .../HasDiagnostic/BareSelfInTableExtension.al | 15 ++++++ .../HasDiagnostic/BareSelfInTrigger.al | 12 +++++ .../HasDiagnostic/LockTableWithArguments.al | 12 +++++ .../HasDiagnostic/NamedVariable.al | 17 +++++++ .../NamespacedQualifiedRecordVariable.al | 19 +++++++ .../HasDiagnostic/OnRunBareSelf.al | 17 +++++++ .../HasDiagnostic/OnRunRecSelf.al | 17 +++++++ .../HasDiagnostic/PageBareSelf.al | 17 +++++++ .../HasDiagnostic/PageRecSelf.al | 17 +++++++ .../HasDiagnostic/RecSelfInTableExtension.al | 15 ++++++ .../HasDiagnostic/RecSelfInTrigger.al | 12 +++++ .../HasDiagnostic/RecordRefVariable.al | 9 ++++ .../HasDiagnostic/ThisSelfInProcedure.al | 12 +++++ .../HasDiagnostic/ThisSelfInTableExtension.al | 15 ++++++ .../HasDiagnostic/ThisSelfInTrigger.al | 12 +++++ .../BareSelfInTableExtension/current.al | 15 ++++++ .../BareSelfInTableExtension/expected.al | 15 ++++++ .../HasFix/BareSelfInTrigger/current.al | 12 +++++ .../HasFix/BareSelfInTrigger/expected.al | 12 +++++ .../HasFix/RecordRefVariable/current.al | 9 ++++ .../HasFix/RecordRefVariable/expected.al | 9 ++++ .../HasFix/ThisSelf/current.al | 12 +++++ .../HasFix/ThisSelf/expected.al | 12 +++++ .../NoDiagnostic/ObsoleteProcedure.al | 13 +++++ .../NoDiagnostic/ReadIsolationMethodForm.al | 12 +++++ .../NoDiagnostic/ReadIsolationPropertyForm.al | 12 +++++ .../RecordInstanceIsolationLevel.cs | 51 ++++++++++++++++--- 28 files changed, 406 insertions(+), 8 deletions(-) create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInProcedure.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTableExtension.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTrigger.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/LockTableWithArguments.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamedVariable.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamespacedQualifiedRecordVariable.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunBareSelf.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunRecSelf.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageBareSelf.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageRecSelf.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTableExtension.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTrigger.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecordRefVariable.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInProcedure.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTableExtension.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTrigger.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/expected.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/expected.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/expected.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/expected.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ObsoleteProcedure.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationMethodForm.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationPropertyForm.al diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInProcedure.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInProcedure.al new file mode 100644 index 00000000..373dff4f --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTableExtension.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTableExtension.al new file mode 100644 index 00000000..f74f14cc --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTrigger.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/BareSelfInTrigger.al new file mode 100644 index 00000000..e26db066 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/LockTableWithArguments.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/LockTableWithArguments.al new file mode 100644 index 00000000..4e0f009d --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamedVariable.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamedVariable.al new file mode 100644 index 00000000..98f62461 --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamespacedQualifiedRecordVariable.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/NamespacedQualifiedRecordVariable.al new file mode 100644 index 00000000..ee960177 --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunBareSelf.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunBareSelf.al new file mode 100644 index 00000000..166718df --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunRecSelf.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/OnRunRecSelf.al new file mode 100644 index 00000000..0a34015f --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageBareSelf.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageBareSelf.al new file mode 100644 index 00000000..0e71e369 --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageRecSelf.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/PageRecSelf.al new file mode 100644 index 00000000..0723cccd --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTableExtension.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTableExtension.al new file mode 100644 index 00000000..9ff85f16 --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTrigger.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecSelfInTrigger.al new file mode 100644 index 00000000..9df7ad59 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecordRefVariable.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/RecordRefVariable.al new file mode 100644 index 00000000..322a8796 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInProcedure.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInProcedure.al new file mode 100644 index 00000000..b841bd30 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTableExtension.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTableExtension.al new file mode 100644 index 00000000..ab13a4a4 --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTrigger.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasDiagnostic/ThisSelfInTrigger.al new file mode 100644 index 00000000..c3088069 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/current.al new file mode 100644 index 00000000..cfca9dab --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTableExtension/expected.al new file mode 100644 index 00000000..d333b11c --- /dev/null +++ b/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) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/current.al new file mode 100644 index 00000000..9915843a --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfInTrigger/expected.al new file mode 100644 index 00000000..5388c430 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/current.al new file mode 100644 index 00000000..f618c3b0 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/current.al @@ -0,0 +1,9 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyRecordRef: RecordRef; + begin + [|MyRecordRef.LockTable()|]; + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/expected.al new file mode 100644 index 00000000..10144cf6 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/RecordRefVariable/expected.al @@ -0,0 +1,9 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyRecordRef: RecordRef; + begin + MyRecordRef.ReadIsolation(IsolationLevel::UpdLock); + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/current.al new file mode 100644 index 00000000..a5ec3bc7 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/current.al @@ -0,0 +1,12 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + procedure MyProcedure() + begin + [|this.LockTable()|]; + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/expected.al new file mode 100644 index 00000000..335337db --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/ThisSelf/expected.al @@ -0,0 +1,12 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + procedure MyProcedure() + begin + this.ReadIsolation(IsolationLevel::UpdLock); + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ObsoleteProcedure.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ObsoleteProcedure.al new file mode 100644 index 00000000..7de2f808 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ObsoleteProcedure.al @@ -0,0 +1,13 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + [Obsolete('Replaced by ReadIsolation.', '25.0')] + procedure MyProcedure() + begin + [|Rec.LockTable();|] + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationMethodForm.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationMethodForm.al new file mode 100644 index 00000000..86bc9422 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationMethodForm.al @@ -0,0 +1,12 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + procedure MyProcedure() + begin + [|Rec.ReadIsolation(IsolationLevel::UpdLock);|] + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationPropertyForm.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationPropertyForm.al new file mode 100644 index 00000000..33a6ae88 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/NoDiagnostic/ReadIsolationPropertyForm.al @@ -0,0 +1,12 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + procedure MyProcedure() + begin + [|ReadIsolation := IsolationLevel::UpdLock;|] + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs index 41090241..5b91060c 100644 --- a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs @@ -22,28 +22,63 @@ public void Setup() [Test] [TestCase("LockTable")] + [TestCase("NamedVariable")] + [TestCase("BareSelfInProcedure")] + [TestCase("BareSelfInTrigger")] + [TestCase("RecSelfInTrigger")] + [TestCase("ThisSelfInProcedure")] + [TestCase("ThisSelfInTrigger")] + [TestCase("BareSelfInTableExtension")] + [TestCase("RecSelfInTableExtension")] + [TestCase("ThisSelfInTableExtension")] + [TestCase("PageRecSelf")] + [TestCase("PageBareSelf")] + [TestCase("OnRunRecSelf")] + [TestCase("OnRunBareSelf")] + [TestCase("NamespacedQualifiedRecordVariable")] + [TestCase("RecordRefVariable")] + [TestCase("LockTableWithArguments")] public async Task HasDiagnostic(string testCase) { + SkipTestIfVersionIsTooLow( + ["ThisSelfInProcedure", "ThisSelfInTrigger", "ThisSelfInTableExtension"], + testCase, + "14.0", + "The 'this' self-reference keyword requires runtime version 14.0 (BC 2024 wave 2)."); + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) .ConfigureAwait(false); _fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.RecordInstanceIsolationLevel); } - // [Test] - // public async Task NoDiagnostic(string testCase) - // { - // var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) - // .ConfigureAwait(false); + [Test] + [TestCase("ObsoleteProcedure")] + [TestCase("ReadIsolationMethodForm")] + [TestCase("ReadIsolationPropertyForm")] + public async Task NoDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); - // _fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.RecordInstanceIsolationLevel); - // } + _fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.RecordInstanceIsolationLevel); + } [Test] [TestCase("ReplaceLockTableWithReadIsolation")] [TestCase("ReplaceLockTableWithReadIsolationUsingRec")] + [TestCase("BareSelfInTrigger")] + [TestCase("BareSelfInTableExtension")] + [TestCase("ThisSelf")] + [TestCase("RecordRefVariable")] public async Task HasFix(string testCase) { + SkipTestIfVersionIsTooLow( + ["ThisSelf"], + testCase, + "14.0", + "The 'this' self-reference keyword requires runtime version 14.0 (BC 2024 wave 2)."); + var currentCode = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasFix), testCase, "current.al")) .ConfigureAwait(false); @@ -59,4 +94,4 @@ public async Task HasFix(string testCase) fixture.TestCodeFix(currentCode, expectedCode, DiagnosticDescriptors.RecordInstanceIsolationLevel); } } -} \ No newline at end of file +} From 81b48e8676633e85f79474cd0aace4759ab55f26 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 11:06:20 +0200 Subject: [PATCH 2/6] fix(LC0031): convert bare LockTable() to bare ReadIsolation in the code fix The code fix returned the unchanged document when the invocation expression was not a MemberAccessExpressionSyntax, which is the case for bare LockTable() inside tables and tableextensions. Add the IdentifierNameSyntax path so bare LockTable() becomes bare ReadIsolation(IsolationLevel::UpdLock). Co-Authored-By: Claude Fable 5.1 --- .../CodeFixes/RecordInstanceIsolationLevel.cs | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs b/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs index 7be3284f..8e1321ee 100644 --- a/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs +++ b/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs @@ -69,9 +69,6 @@ private static async Task ReplaceLockTableWithReadIsolation(Document d if (node is not InvocationExpressionSyntax invocationExpression) return document; - if (invocationExpression.Expression is not MemberAccessExpressionSyntax memberAccess) - return document; - var enumMemberAccess = SyntaxFactory.OptionAccessExpression( SyntaxFactory.IdentifierName(IsolationLevelEnumName), SyntaxFactory.Token(EnumProvider.SyntaxKind.ColonColonToken), @@ -80,12 +77,26 @@ private static async Task ReplaceLockTableWithReadIsolation(Document d var argumentList = SyntaxFactory.ArgumentList( new SeparatedSyntaxList().Add(enumMemberAccess)); - var newMemberAccess = SyntaxFactory.MemberAccessExpression( - memberAccess.Expression, - SyntaxFactory.Token(EnumProvider.SyntaxKind.DotToken), - SyntaxFactory.IdentifierName(ReadIsolationMethodName)); + InvocationExpressionSyntax newInvocation; - var newInvocation = SyntaxFactory.InvocationExpression(newMemberAccess, argumentList); + if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccess) + { + var newMemberAccess = SyntaxFactory.MemberAccessExpression( + memberAccess.Expression, + SyntaxFactory.Token(EnumProvider.SyntaxKind.DotToken), + SyntaxFactory.IdentifierName(ReadIsolationMethodName)); + + newInvocation = SyntaxFactory.InvocationExpression(newMemberAccess, argumentList); + } + else if (invocationExpression.Expression is IdentifierNameSyntax) + { + newInvocation = SyntaxFactory.InvocationExpression( + SyntaxFactory.IdentifierName(ReadIsolationMethodName), argumentList); + } + else + { + return document; + } var root = await syntaxRootTask.ConfigureAwait(false); if (root is null) From 20c20a098820349c2b76f22f47b28e91db50f49a Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 11:35:26 +0200 Subject: [PATCH 3/6] test(LC0031): pin the bare page receiver in the code fix A bare LockTable() on a page has an IdentifierNameSyntax expression like the table form, so the fix must produce a bare ReadIsolation there too. Co-Authored-By: Claude Fable 5.1 --- .../HasFix/PageBareSelf/current.al | 17 +++++++++++++++++ .../HasFix/PageBareSelf/expected.al | 17 +++++++++++++++++ .../RecordInstanceIsolationLevel.cs | 1 + 3 files changed, 35 insertions(+) create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/expected.al diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/current.al new file mode 100644 index 00000000..77f19984 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/current.al @@ -0,0 +1,17 @@ +page 50100 MyPage +{ + SourceTable = MyTable; + + trigger OnOpenPage() + begin + [|LockTable()|]; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/expected.al new file mode 100644 index 00000000..4d4b0954 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/PageBareSelf/expected.al @@ -0,0 +1,17 @@ +page 50100 MyPage +{ + SourceTable = MyTable; + + trigger OnOpenPage() + begin + ReadIsolation(IsolationLevel::UpdLock); + end; +} + +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs index 5b91060c..ad3f661f 100644 --- a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs @@ -71,6 +71,7 @@ public async Task NoDiagnostic(string testCase) [TestCase("BareSelfInTableExtension")] [TestCase("ThisSelf")] [TestCase("RecordRefVariable")] + [TestCase("PageBareSelf")] public async Task HasFix(string testCase) { SkipTestIfVersionIsTooLow( From 8a5ef897b5e5aca1ccc4413bb672d74604f185b2 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 11:35:26 +0200 Subject: [PATCH 4/6] docs(LC0031): record the trigger decision and receiver-form verdicts Add the rule doc: why self LockTable() in table triggers stays reported, the per-form verdict table for analyzer and code fix, the SDK facts the decision rests on, and the code fix decisions. Extend the receiver-forms regression row with the code fix flavour of the bare-self gap. Co-Authored-By: Claude Fable 5.1 --- .../lc0031-record-instance-isolation-level.md | 69 +++++++++++++++++++ .../references/regression-catalog.md | 2 +- 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 .claude/rules/diagnostics/lc0031-record-instance-isolation-level.md diff --git a/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md b/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md new file mode 100644 index 00000000..db0a278a --- /dev/null +++ b/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md @@ -0,0 +1,69 @@ +--- +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 | +| 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`. + +## 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`) | +| 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 | diff --git a/.claude/skills/fix-false-positive/references/regression-catalog.md b/.claude/skills/fix-false-positive/references/regression-catalog.md index 64393e52..8d323f14 100644 --- a/.claude/skills/fix-false-positive/references/regression-catalog.md +++ b/.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 | From f35d9aadec9b9c92e13cca3f7754e45b13a9ec00 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 12:45:07 +0200 Subject: [PATCH 5/6] fix(LC0031): keep the trivia of a bare LockTable() and gate tableextension fixtures A fresh SyntaxFactory identifier carries no trivia, so the bare form lost the indentation and comments attached to the LockTable token; the replacement now copies the trivia of the original invocation. The receiver switch is typed on CodeExpressionSyntax so both forms share one construction path. Tableextension fixtures are gated on SDK 13.0: older compilers report AL0334 for a tableextension whose target is declared in the same module. Co-Authored-By: Claude Fable 5.1 --- .../BareSelfWithLeadingComment/current.al | 13 ++++++ .../BareSelfWithLeadingComment/expected.al | 13 ++++++ .../RecordInstanceIsolationLevel.cs | 13 ++++++ .../CodeFixes/RecordInstanceIsolationLevel.cs | 40 +++++++++---------- 4 files changed, 59 insertions(+), 20 deletions(-) create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/current.al create mode 100644 src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/expected.al diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/current.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/current.al new file mode 100644 index 00000000..704065a6 --- /dev/null +++ b/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; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/expected.al b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/expected.al new file mode 100644 index 00000000..40712b8a --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/HasFix/BareSelfWithLeadingComment/expected.al @@ -0,0 +1,13 @@ +table 50100 MyTable +{ + fields + { + field(1; MyField; Integer) { } + } + + trigger OnInsert() + begin + // Serialize inserts on this table + ReadIsolation(IsolationLevel::UpdLock); + end; +} diff --git a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs index ad3f661f..c2cf32dc 100644 --- a/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs +++ b/src/ALCops.LinterCop.Test/Rules/RecordInstanceIsolationLevel/RecordInstanceIsolationLevel.cs @@ -46,6 +46,12 @@ public async Task HasDiagnostic(string testCase) "14.0", "The 'this' self-reference keyword requires runtime version 14.0 (BC 2024 wave 2)."); + SkipTestIfVersionIsTooLow( + ["BareSelfInTableExtension", "RecSelfInTableExtension"], + testCase, + "13.0", + "No support for tableextensions when target itself is already declared in the same module"); + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) .ConfigureAwait(false); @@ -72,6 +78,7 @@ public async Task NoDiagnostic(string testCase) [TestCase("ThisSelf")] [TestCase("RecordRefVariable")] [TestCase("PageBareSelf")] + [TestCase("BareSelfWithLeadingComment")] public async Task HasFix(string testCase) { SkipTestIfVersionIsTooLow( @@ -80,6 +87,12 @@ public async Task HasFix(string testCase) "14.0", "The 'this' self-reference keyword requires runtime version 14.0 (BC 2024 wave 2)."); + SkipTestIfVersionIsTooLow( + ["BareSelfInTableExtension"], + testCase, + "13.0", + "No support for tableextensions when target itself is already declared in the same module"); + var currentCode = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasFix), testCase, "current.al")) .ConfigureAwait(false); diff --git a/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs b/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs index 8e1321ee..db97c290 100644 --- a/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs +++ b/src/ALCops.LinterCop/CodeFixes/RecordInstanceIsolationLevel.cs @@ -69,6 +69,23 @@ private static async Task ReplaceLockTableWithReadIsolation(Document d if (node is not InvocationExpressionSyntax invocationExpression) return document; + CodeExpressionSyntax target; + switch (invocationExpression.Expression) + { + case MemberAccessExpressionSyntax memberAccess: + target = SyntaxFactory.MemberAccessExpression( + memberAccess.Expression, + SyntaxFactory.Token(EnumProvider.SyntaxKind.DotToken), + SyntaxFactory.IdentifierName(ReadIsolationMethodName)); + break; + case IdentifierNameSyntax: + // A bare LockTable() inside a table, tableextension or page keeps its bare form. + target = SyntaxFactory.IdentifierName(ReadIsolationMethodName); + break; + default: + return document; + } + var enumMemberAccess = SyntaxFactory.OptionAccessExpression( SyntaxFactory.IdentifierName(IsolationLevelEnumName), SyntaxFactory.Token(EnumProvider.SyntaxKind.ColonColonToken), @@ -77,26 +94,9 @@ private static async Task ReplaceLockTableWithReadIsolation(Document d var argumentList = SyntaxFactory.ArgumentList( new SeparatedSyntaxList().Add(enumMemberAccess)); - InvocationExpressionSyntax newInvocation; - - if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccess) - { - var newMemberAccess = SyntaxFactory.MemberAccessExpression( - memberAccess.Expression, - SyntaxFactory.Token(EnumProvider.SyntaxKind.DotToken), - SyntaxFactory.IdentifierName(ReadIsolationMethodName)); - - newInvocation = SyntaxFactory.InvocationExpression(newMemberAccess, argumentList); - } - else if (invocationExpression.Expression is IdentifierNameSyntax) - { - newInvocation = SyntaxFactory.InvocationExpression( - SyntaxFactory.IdentifierName(ReadIsolationMethodName), argumentList); - } - else - { - return document; - } + // A fresh identifier carries no trivia, so copy the indentation and comments of the original call. + var newInvocation = SyntaxFactory.InvocationExpression(target, argumentList) + .WithTriviaFrom(invocationExpression); var root = await syntaxRootTask.ConfigureAwait(false); if (root is null) From 94fd56fd8cfaee9af43136f65c92bef88ce65d87 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Sun, 13 Sep 2026 12:45:07 +0200 Subject: [PATCH 6/6] docs(LC0031): note trivia preservation and the fixture version gates Co-Authored-By: Claude Fable 5.1 --- .../diagnostics/lc0031-record-instance-isolation-level.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md b/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md index db0a278a..d17aaaeb 100644 --- a/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md +++ b/.claude/rules/diagnostics/lc0031-record-instance-isolation-level.md @@ -35,7 +35,7 @@ Registers `RegisterOperationAction` on `InvocationExpression`; matches built-in | `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 | +| `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 | @@ -58,6 +58,8 @@ Registers `RegisterOperationAction` on `InvocationExpression`; matches built-in ## 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 @@ -65,5 +67,6 @@ Registers `RegisterOperationAction` on `InvocationExpression`; matches built-in |---|---| | 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 |