fix(AC0032): detect this. self-access via the operation tree (#343)#349
Merged
Merged
Conversation
…self AC0032 (unused permissions) resolves table data access at the syntax level. It recognised variable receivers (`MyTable.Modify()`, `Rec.Modify()`) but missed two self-access forms: - `this.Modify()` — the AL `this` self-reference keyword (BC 2024 wave 2) - `Modify()` — bare implicit self Both made a table's self-permission appear unused, producing a false positive (issue #343). Root cause: the `this` receiver is a `ThisExpressionSyntax` (neither the identifier fast path nor the `GetSymbolInfo` fallback resolved it), and the implicit-self branch checked `containingObject is IRecordTypeSymbol` — always false, since a table object's symbol is an `ITableTypeSymbol`, not a record. Changes: - Resolve `this` via `SemanticModel.GetOperation(thisNode)?.Type` (binds to the record type); guarded with `#if !NETSTANDARD2_1` (type absent in that SDK). - Resolve bare implicit self from the containing `ITableTypeSymbol`. - New `TryGetSelfPermission` accepts either a record type or a table type. - Tests: AC0032 `ThisKeywordSelfAccess`, `ImplicitSelfBareCall` (NoDiagnostic), `ThisKeywordPartialUnused` (HasDiagnostic); AC0031 future-proof `ThisKeywordSelfCallInTable` (HasDiagnostic). `this` cases guarded at runtime 14.0. - Docs: AC0031/AC0032 instruction files, analyzer-development guide (four receiver forms), netstandard21-compatibility guide (`ThisExpressionSyntax`). - Follow-up audit tracked in #348. Fixes #343 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'this' keyword parses from runtime 14.0, but the SDK only binds a table's `this` to its backing record type (via GetRecord) from runtime 16.0 (BC 2025 wave 2). On 14.0-15.2, GetOperation(this).Type does not resolve to a record, so `this.Modify()` self-access is not detected and the NoDiagnostic test fails. Bump the ThisKeywordSelfAccess guard from 14.0 to 16.0 and document the limitation in the AC0032 instruction file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… 16.0+ The previous note attributed the 16.0 boundary to an SDK runtime binding difference. That is wrong: the binder resolves a table's `this` to its record identically on AL 14.0/15.2/16.0 (verified against the tagged NAV SDK source). The real cause is a netstandard2.1 compile-time limitation. The public `ThisExpressionSyntax` type (and `SyntaxKind.ThisExpression` / `IInstanceReferenceOperation`) is absent from the netstandard2.1 build floor (AL 12.0.13); these were added in AL 14.0. The `this` branch is therefore `#if !NETSTANDARD2_1`-excluded with no reflection/operation-tree fallback. AL 14.0-15.2 ship a netstandard2.0 SDK and run ALCops's netstandard2.1 binary, so `this.` self-access is undetected there and the #343 false positive persists on those versions; it is fixed only on the net8.0/net10.0 binaries (AL 16.0+). - Correct the analyzer comment, AC0032 known-limitation, analyzer-development guide, and netstandard21-compatibility guide. - Keep the `ThisKeywordSelfAccess` guard at 16.0 with an accurate reason. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve non-identifier method receivers (the AL `this` self-reference and expression receivers) through the receiver's IOperation.Type instead of pattern-matching ThisExpressionSyntax. This mirrors AC0031 (RequiredPermissionDetector) and never references the ThisExpressionSyntax type, which is absent at the netstandard2.1 compile floor (AL 12.0.13). Because IOperation, SemanticModel.GetOperation and IOperation.Type all exist at that floor, the fix works on every TFM and AL version (14.0+), not just the net8.0/net10.0 builds (AL 16.0+). Removes the prior #if !NETSTANDARD2_1 guard and the associated known-limitation, and reverts the ThisKeywordSelfAccess test guard from 16.0 back to 14.0. Renames TryGetSelfPermission to TryGetPermissionForType to reflect that it now serves any resolved receiver/self type. Updates the AC0032, analyzer-development and netstandard21-compatibility instruction files accordingly. Fixes #343 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Arthurvdv
changed the title
this keyword and bare implicit self (#343)
Arthurvdv
added a commit
that referenced
this pull request
Jun 18, 2026
fix(AC0032): detect this. self-access via the operation tree (#349)
MODUSCarstenScholling
pushed a commit
to MODUSCarstenScholling/ALCops-Analyzers
that referenced
this pull request
Jun 19, 2026
) (ALCops#349) fix(AC0032): detect self-access via `this` keyword and bare implicit self AC0032 (unused permissions) resolves table data access at the syntax level. It recognised variable receivers (`MyTable.Modify()`, `Rec.Modify()`) but missed two self-access forms, both of which made a table's self-permission appear unused and produced a false positive (issue ALCops#343): - `this.Modify()` — the AL `this` self-reference keyword (BC 2024 wave 2) - `Modify()` — bare implicit self Root cause: the `this` receiver matched neither the identifier fast path nor the `GetSymbolInfo` fallback, and the implicit-self branch checked `containingObject is IRecordTypeSymbol` — always false, since a table object's symbol is an `ITableTypeSymbol`, not a record. Changes: - Resolve any non-identifier receiver (the `this` self-reference and expression receivers) via the operation tree: `SemanticModel.GetOperation(receiver)?.Type`. In a table, `this` binds to the record; non-record objects (e.g. a codeunit, where `this` is the instance) yield a non-record type and are ignored. This mirrors AC0031 (`RequiredPermissionDetector`) and never references `ThisExpressionSyntax`, which is absent at the netstandard2.1 compile floor (AL 12.0.13). Because `IOperation`, `GetOperation` and `IOperation.Type` all exist at that floor, the fix needs no `#if` guard and works on AL 14.0+ across every TFM — not just the net8.0/net10.0 builds (AL 16.0+). - Resolve bare implicit self from the containing `ITableTypeSymbol`. - `TryGetPermissionForType` accepts either a record type (via `OriginalDefinition`) or a table type directly. Fixes ALCops#343 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the AC0032 (unused permissions) false positive from #343, where a table that grants a
tabledatapermission on itself and accesses itself via thethiskeyword (this.Get/Insert/Modify) had its self-permission incorrectly flagged as unused.Root cause
AC0032 resolves table data access at the syntax level and handled only variable receivers. Two self-access forms were missed:
this.Modify()— the ALthisself-reference keyword; it matched neither the identifier fast path nor theGetSymbolInfofallback.Modify()— bare implicit self; the implicit-self branch checkedcontainingObject is IRecordTypeSymbol, which is always false because a table object's declared symbol is anITableTypeSymbol, not anIRecordTypeSymbol.Changes
thisself-reference and expression receivers) via the operation tree:SemanticModel.GetOperation(receiver)?.Type. In a table,thisbinds to the record; non-record objects (e.g. a codeunit, wherethisis the codeunit instance) yield a non-record type and are ignored.ITableTypeSymbol.TryGetPermissionForTypehelper accepts a record type (viaOriginalDefinition) or a table type directly.Why the operation tree (not
ThisExpressionSyntax)The earlier iteration pattern-matched
ThisExpressionSyntaxbehind#if !NETSTANDARD2_1. That type — plusSyntaxKind.ThisExpressionandIInstanceReferenceOperation— is absent at the netstandard2.1 compile floor (AL 12.0.13, predating the Fall 2024thisfeature), so the fix only reached AL 16.0+ (net8.0/net10.0 binaries). AL 14.0–15.2 ship a netstandard2.0 SDK and run ALCops's netstandard2.1 binary, where the guarded branch was excluded → the false positive persisted there.IOperation,SemanticModel.GetOperationandIOperation.Typeall exist at the floor. Resolving the receiver type off the baseIOperationtherefore needs no type reference and no#ifguard, so the fix now works on AL 14.0+ (every TFM). This mirrors AC0031 (RequiredPermissionDetector), which already resolvesthisthis way and passes on 14.0–15.2.All four receiver forms now handled
MyTable.Modify(),Rec.Modify(),Modify(),this.Modify().Tests
ThisKeywordSelfAccess,ImplicitSelfBareCall(NoDiagnostic);ThisKeywordPartialUnused(HasDiagnostic).ThisKeywordSelfCallInTable(HasDiagnostic) — confirmsthis.access is detected as requiring a permission.thiscases guarded at runtime 14.0 (SkipTestIfVersionIsTooLow).Docs
analyzer-developmentguide: "Record method receiver forms" section (resolvethis/expression receivers viaIOperation.Type, do not referenceThisExpressionSyntax).netstandard21-compatibilityguide:ThisExpressionSyntaxrow rewritten to recommend the operation-tree approach.Follow-up
Created #348 to audit all other syntax-level analyzers for the same four receiver forms (triage only).
Fixes #343