fix(AC0032): detect this. self-access via the operation tree (#343)

.github/instructions/ac0031-table-data-access-requires-permissions.instructions.md

@@ -117,7 +117,7 @@ Uses C#-like namespace resolution (`PermissionTableNameResolver`):
 
 ## Test coverage
 
-**HasDiagnostic (8 cases):** ProcedureCalls, ProcedureCallsExtended, GetBySystemId, Count, ImplicitSelfCallInTable, XmlPorts, Queries, Reports.
+**HasDiagnostic (10 cases):** ProcedureCalls, ProcedureCallsExtended, GetBySystemId, Count, ImplicitSelfCallInTable, ThisKeywordSelfCallInTable, XmlPorts, Queries, Reports, DottedTableName.
 **NoDiagnostic (21 cases):** ProcedureCallsPermissionsProperty, ProcedureCallsPermissionsPropertyFullyQualified, ProcedureCallsInherentPermissionsProperty, ProcedureCallsInherentPermissionsAttribute, PageSourceTable, PageExtensionSourceTable, XmlPortPermissionsProperty, XmlPortInherentPermissions, QueryPermissionsProperty, QueryInherentPermissions, ReportPermissionsProperty, ReportInherentPermissions, XMLPortWithTableElementProps, PermissionsAsObjectId, PermissionPropertyWithPragma, PermissionPropertyWithComment, MultiplePermissionsDifferentType, TestPermissionsDisabled, GetBySystemIdWithPermissions, CountWithPermissions, ImplicitSelfCallWithInherentPermissions.
 **HasFix (9 cases):** AddNewPermissionsProperty, AddNewTableEntry, MergePermissionChar, MergeCanonicalOrder, AddEntryMultiLine, AddEntrySingleLine, AddEntryAlphabetical, AddEntryAppend, AddEntryAlphabeticalFirst.
 

.github/instructions/ac0032-table-data-access-unused-permissions.instructions.md

@@ -53,7 +53,7 @@ src/ALCops.Common/
    - For each method body: syntax pre-filter (`HasPossibleDbInvocation`)
    - Build per-method record variable map from `IMethodSymbol.LocalVariables` + `.Parameters`
    - Walk method body for both `InvocationExpressionSyntax` and standalone `MemberAccessExpressionSyntax` (method calls without parentheses)
-   - Unified `TryGetPermissionFromDbAccess` extracts method name + receiver from either form, resolves via variable map (fast path), falls back to `GetSymbolInfo` for complex receivers
+   - Unified `TryGetPermissionFromDbAccess` extracts method name + receiver from either form, resolves via variable map (fast path), then via the receiver's `IOperation.Type` for non-identifier receivers (`this.`, expression receivers), and falls back to `GetSymbolInfo` for anything unresolved
 5. **Collect data items** (`CollectFromDataItems`):
    - Iterate `containingObject.GetMembers()` for ReportDataItem, QueryDataItem, XmlPortNode symbols
    - For XmlPortNode: also iterate `FlattenedNodes` to reach nested table elements
@@ -66,7 +66,8 @@ src/ALCops.Common/
 |---|---|
 | `AnalyzeApplicationObject` | Entry point; checks Permissions, orchestrates collection and reporting |
 | `CollectFromInvocations` | Builds object-scope record map (vars + data items), walks method bodies, resolves DB calls via unified handler |
-| `TryGetPermissionFromDbAccess` | Unified resolution for both syntax forms (with/without parentheses): pattern-matches to extract method name + receiver, uses variable-map fast path, falls back to `TryGetPermissionViaSymbolInfo` |
+| `TryGetPermissionFromDbAccess` | Unified resolution for both syntax forms (with/without parentheses). Handles all four receiver forms: `MyTable.Modify()` / `Rec.Modify()` (variable-map fast path), `Modify()` (bare implicit self), `this.Modify()` (AL `this` self-reference, resolved via `IOperation.Type`). Falls back to `TryGetPermissionViaSymbolInfo` for unresolved receivers |
+| `TryGetPermissionForType` | Builds a required permission from a resolved receiver/self type (record resolved to its backing table, or a table type directly); returns null for non-record/table types (e.g. `this` inside a codeunit) |
 | `TryGetPermissionViaSymbolInfo` | Fallback for complex receivers: uses GetSymbolInfo on the node and receiver expression to resolve method and receiver type |
 | `CollectFromDataItems` | Iterates report/query FlattenedDataItems and xmlport FlattenedXmlPortNodes (all via reflection) for implicit read permissions |
 | `AddXmlPortNodeToVarMap` | Adds an xmlport table element to the object-scope record map if it references a non-temporary table |
@@ -88,7 +89,9 @@ Each `SyntaxNodeAction` callback is self-contained with no shared mutable state.
 | Data items in object-scope record map | Report data items and xmlport table elements act as implicit record variables in trigger code; added to the same map for fast-path resolution via `GetTypeSymbol()` |
 | XmlPort nested nodes via `GetFlattenedXmlPortNodes` | `GetMembers()` returns only top-level schema nodes; `IXmlPortNodeSymbol.FlattenedNodes` only returns immediate children (not recursive). Uses reflection on the internal `SourceXmlPortTypeSymbol.FlattenedNodes` property which truly flattens all depths |
 | `GetSymbolInfo` fallback for complex receivers | Handles function return values, array indexing, property access; only ~1% of invocations need this path |
-| No `GetOperation` / `OperationWalker` | `GetOperation` in `SyntaxNodeAction` costs ~0.3ms/call (no pre-computed cache); variable map approach is 4.5x faster |
+| Hot path avoids `GetOperation` / `OperationWalker` | `GetOperation` in `SyntaxNodeAction` costs ~0.3ms/call (no pre-computed cache); the variable-map fast path is 4.5x faster and handles the common identifier receivers. `GetOperation` is used only for the rare non-identifier receivers (`this.`, expression receivers), reading the receiver type off the base `IOperation.Type` |
+| Four receiver forms handled explicitly | Database access can target a record via four syntactic forms: `MyTable.Modify()`, `Rec.Modify()`, bare `Modify()` (implicit self), and `this.Modify()` (AL `this` keyword). Variable receivers use the fast path; bare self resolves the containing `ITableTypeSymbol`; `this` (and other expression receivers) resolve via `IOperation.Type` -- the same operation-tree mechanism AC0031 uses, which works on all TFMs (it never references the `ThisExpressionSyntax` type, absent at the netstandard2.1 floor). Missing the `this` form caused the AC0032 false positive in issue #343 |
+| Self-table symbol shapes | The table object's declared symbol is an `ITableTypeSymbol` (not an `IRecordTypeSymbol`); `this`/`Rec` resolve to a separate `IRecordTypeSymbol` wrapper. `TryGetPermissionForType` accepts both: record types via `OriginalDefinition`, table types directly |
 | No cross-callback shared state | Eliminates the fragile two-phase accumulator pattern that caused false positives during incremental compilation |
 | Iterate `DescendantNodes()` for methods | Finds all MethodOrTriggerDeclarationSyntax in the object, skip obsolete ones |
 | Skip obsolete methods via symbol | `GetDeclaredSymbol` + `IsObsolete()` on the method symbol |
@@ -139,8 +142,8 @@ When removing the first entry from a multi-entry list, `SeparatedSyntaxList.Remo
 
 ## Test coverage
 
-**HasDiagnostic (10 cases):** EntireEntryUnused, PartialCharsUnused, MultipleUnusedEntries, NoCodeInCodeunit, UnusedOnReport, UnusedOnQuery, UnusedOnXmlPort, TemporaryRecord, ParameterPartialUnused, ReportDataItemPartialUnused.
-**NoDiagnostic (25 cases):** AllPermissionsUsed, PageSourceTable, TestCodeunitDisabled, ReadUsed, ReportDataItemRead, QueryDataItemRead, PermissionSet, PermissionSetExtension, SystemTable, ParameterOperations, UppercasePermissions, ParameterAllOperations, LocalVarSpacedTable, GlobalVarSpacedTable, ReportDataItemModify, ReportDataItemAliasModify, XmlPortTableElementModify, XmlPortNestedTableElementModify, ReturnParameterRead, ReportNestedDataItemRead, QueryNestedDataItemRead, MethodWithoutParenthesesCount, MethodWithoutParenthesesFindFirst, MethodWithoutParenthesesIsEmpty, MethodWithoutParenthesesChained.
+**HasDiagnostic (11 cases):** EntireEntryUnused, PartialCharsUnused, MultipleUnusedEntries, NoCodeInCodeunit, UnusedOnReport, UnusedOnQuery, UnusedOnXmlPort, TemporaryRecord, ParameterPartialUnused, ReportDataItemPartialUnused, ThisKeywordPartialUnused.
+**NoDiagnostic (27 cases):** AllPermissionsUsed, PageSourceTable, TestCodeunitDisabled, ReadUsed, ReportDataItemRead, QueryDataItemRead, PermissionSet, PermissionSetExtension, SystemTable, ParameterOperations, UppercasePermissions, ParameterAllOperations, LocalVarSpacedTable, GlobalVarSpacedTable, ReportDataItemModify, ReportDataItemAliasModify, XmlPortTableElementModify, XmlPortNestedTableElementModify, ReturnParameterRead, ReportNestedDataItemRead, QueryNestedDataItemRead, MethodWithoutParenthesesCount, MethodWithoutParenthesesFindFirst, MethodWithoutParenthesesIsEmpty, MethodWithoutParenthesesChained, ThisKeywordSelfAccess, ImplicitSelfBareCall.
 **HasFix (4 cases):** RemoveEntireEntry, ReduceChars, RemoveEntireProperty, ReplaceChars.
 
 ## Known limitations

.github/instructions/analyzer-development.instructions.md

@@ -328,8 +328,49 @@ private static void AnalyzeStringLiteral(SyntaxNodeAnalysisContext ctx)
 }
 ```
 
-## EnumProvider (Critical Pattern)
+### Record method receiver forms (syntax level)
+
+When detecting AL record (table data) method calls at the **syntax level**, a call like
+`Modify` can reach a record through **four** receiver forms. Analyzers that pattern-match
+only some of them produce false positives/negatives (see issue #343, AC0032):
+
+| Form | Example | Syntax shape | How to resolve the record/table |
+|---|---|---|---|
+| Named variable | `MyTable.Modify()` | `MemberAccessExpressionSyntax` with `IdentifierNameSyntax` receiver | Variable map (locals/params/globals) or `GetSymbolInfo` on the receiver |
+| Implicit `Rec` | `Rec.Modify()` | same as above (`Rec` is a normal identifier) | same as named variable |
+| Bare implicit self | `Modify()` | `InvocationExpressionSyntax` whose `Expression` is `IdentifierNameSyntax` (no receiver) | The containing object: a table object's declared symbol is an `ITableTypeSymbol` |
+| `this` self-reference | `this.Modify()` | `MemberAccessExpressionSyntax` whose receiver is the AL `this` keyword (runtime 14.0+) | `SemanticModel.GetOperation(receiverNode)?.Type` (in a table, `this` binds to the record). Detect it as "any non-`IdentifierNameSyntax` receiver" -- do **not** pattern-match `ThisExpressionSyntax` (absent at the netstandard2.1 floor) |
+
+Key symbol-shape gotcha: a **table object's declared symbol is `ITableTypeSymbol`, which is
+NOT an `IRecordTypeSymbol`**. The record (`Rec`/`this`) is a separate `IRecordTypeSymbol`
+wrapper whose `OriginalDefinition` is the `ITableTypeSymbol`. Guard with `is ITableTypeSymbol`
+for the object/bare-self and `is IRecordTypeSymbol` for variable/`this` receivers.
+
+Tests that exercise the `this` form must guard on runtime version 14.0 (e.g.
+`SkipTestIfVersionIsTooLow([...], testCase, "14.0", ...)`), since `this` is a Fall 2024 feature.
+
+**Do not** reference `ThisExpressionSyntax` directly (and therefore do not `#if !NETSTANDARD2_1`-guard
+`this` handling). The public `ThisExpressionSyntax` type, the `SyntaxKind.ThisExpression` enum member,
+and `IInstanceReferenceOperation` are all **absent** from the netstandard2.1 compile floor (AL 12.0.13,
+predating the Fall 2024 `this` feature), so a `ThisExpressionSyntax` pattern-match forces a guard that
+silently drops `this` detection on the netstandard2.1 binary -- which is exactly what serves AL 14.0-15.2
+(they ship a netstandard2.0 SDK). Instead, resolve the receiver via the **operation tree**, which is
+fully available at the floor (`GetOperation`, `IOperation`, `IOperation.Type`, `IInvocationExpression.Instance`):
 
+```csharp
+// Works on every TFM and AL version; never names ThisExpressionSyntax.
+if (receiverExpression is not null && receiverExpression is not IdentifierNameSyntax)
+{
+    var receiverType = ctx.SemanticModel.GetOperation(receiverExpression, ct)?.Type;  // `this` binds to the record
+    // ... build the permission from receiverType
+}
+```
+
+This is the same mechanism AC0031 (`RequiredPermissionDetector.TryGetFromInvocation`) uses via
+`invocation.Instance.Type`. Keep the variable-map fast path first so `GetOperation` (the ~0.3ms call)
+only runs for the rare non-identifier receivers.
+
+## EnumProvider (Critical Pattern)
 Never reference `Microsoft.Dynamics.Nav.CodeAnalysis` enum values directly. Always use `EnumProvider` from `ALCops.Common.Reflection`. This provides backward compatibility across SDK versions via reflection-based caching.
 
 ```csharp

.github/instructions/netstandard21-compatibility.instructions.md

@@ -21,6 +21,7 @@ Some `Microsoft.Dynamics.Nav.CodeAnalysis` APIs only exist in newer SDK versions
 | API | Available in | netstandard2.1 workaround |
 |---|---|---|
 | `IFieldSymbol.Type` | net8.0 only | `fieldSymbol.OriginalDefinition.GetTypeSymbol()` (requires `using Microsoft.Dynamics.Nav.CodeAnalysis.Symbols`) |
+| `ThisExpressionSyntax` (AL `this` keyword) | net8.0+ only (do not reference) | The public type, `SyntaxKind.ThisExpression`, and `IInstanceReferenceOperation` are absent from the netstandard2.1 compile floor (AL 12.0.13, predating the Fall 2024 `this` feature), so there is no type-name or `EnumProvider` workaround for the syntax type. Instead, **do not reference it at all**: resolve the receiver via the operation tree (`GetOperation(receiver)?.Type`, or `IInvocationExpression.Instance.Type`), which *is* available at the floor and binds a table's `this` to its record. This avoids the `#if` entirely and works on AL 14.0-15.2 (which run the netstandard2.1 binary). See AC0032 `TableDataAccessUnusedPermissions` and AC0031 `RequiredPermissionDetector`. |
 
 ### Pattern for `IFieldSymbol.Type`
 

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/HasDiagnostic/ThisKeywordSelfCallInTable.al

@@ -0,0 +1,18 @@
+table 50000 MyTable
+{
+    Caption = '', Locked = true;
+
+    fields
+    {
+        field(1; MyField; Integer)
+        {
+            Caption = '', Locked = true;
+            DataClassification = ToBeClassified;
+        }
+    }
+
+    procedure DoSomething()
+    begin
+        [|this.Modify();|]
+    end;
+}

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessRequiresPermissions/TableDataAccessRequiresPermissions.cs

@@ -26,12 +26,19 @@ public void Setup()
         [TestCase("GetBySystemId")]
         [TestCase("Count")]
         [TestCase("ImplicitSelfCallInTable")]
+        [TestCase("ThisKeywordSelfCallInTable")]
         [TestCase("XmlPorts")]
         [TestCase("Queries")]
         [TestCase("Reports")]
         [TestCase("DottedTableName")]
         public async Task HasDiagnostic(string testCase)
         {
+            SkipTestIfVersionIsTooLow(
+                ["ThisKeywordSelfCallInTable"],
+                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);
 

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessUnusedPermissions/HasDiagnostic/ThisKeywordPartialUnused.al

@@ -0,0 +1,19 @@
+table 50000 MyTable
+{
+    Permissions = [|tabledata MyTable = rimd|];
+
+    fields
+    {
+        field(1; MyField; Integer)
+        {
+            Caption = '', Locked = true;
+            DataClassification = ToBeClassified;
+        }
+    }
+
+    procedure SelfAccess()
+    begin
+        this.FindFirst();
+        this.Modify();
+    end;
+}

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessUnusedPermissions/NoDiagnostic/ImplicitSelfBareCall.al

@@ -0,0 +1,20 @@
+table 50000 MyTable
+{
+    Permissions = [|tabledata MyTable = rim|];
+
+    fields
+    {
+        field(1; MyField; Integer)
+        {
+            Caption = '', Locked = true;
+            DataClassification = ToBeClassified;
+        }
+    }
+
+    procedure SelfAccess()
+    begin
+        FindFirst();
+        Insert();
+        Modify();
+    end;
+}

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessUnusedPermissions/NoDiagnostic/ThisKeywordSelfAccess.al

@@ -0,0 +1,20 @@
+table 50000 MyTable
+{
+    Permissions = [|tabledata MyTable = rim|];
+
+    fields
+    {
+        field(1; MyField; Integer)
+        {
+            Caption = '', Locked = true;
+            DataClassification = ToBeClassified;
+        }
+    }
+
+    procedure SelfAccess()
+    begin
+        this.FindFirst();
+        this.Insert();
+        this.Modify();
+    end;
+}

src/ALCops.ApplicationCop.Test/Rules/TableDataAccessUnusedPermissions/TableDataAccessUnusedPermissions.cs

@@ -31,8 +31,15 @@ public void Setup()
         [TestCase("TemporaryRecord")]
         [TestCase("ParameterPartialUnused")]
         [TestCase("ReportDataItemPartialUnused")]
+        [TestCase("ThisKeywordPartialUnused")]
         public async Task HasDiagnostic(string testCase)
         {
+            SkipTestIfVersionIsTooLow(
+                ["ThisKeywordPartialUnused"],
+                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);
 
@@ -65,6 +72,8 @@ public async Task HasDiagnostic(string testCase)
         [TestCase("MethodWithoutParenthesesFindFirst")]
         [TestCase("MethodWithoutParenthesesIsEmpty")]
         [TestCase("MethodWithoutParenthesesChained")]
+        [TestCase("ThisKeywordSelfAccess")]
+        [TestCase("ImplicitSelfBareCall")]
         public async Task NoDiagnostic(string testCase)
         {
             SkipTestIfVersionIsTooLow(
@@ -73,6 +82,12 @@ public async Task NoDiagnostic(string testCase)
                 "13.0",
                 "No support for tableextensions when target itself is already declared in the same module");
 
+            SkipTestIfVersionIsTooLow(
+                ["ThisKeywordSelfAccess"],
+                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(NoDiagnostic), $"{testCase}.al"))
                 .ConfigureAwait(false);