fix(PC0037): suppress same-field assignment in own validate trigger#371
Merged
Conversation
PC0037 raised a false positive when a field is assigned to the current record (Rec/self) inside that field's own OnValidate / OnBeforeValidate / OnAfterValidate trigger. Such by-design value transformation (default values, rounding) cannot use Validate() without recursion, so it must not be flagged (issue #357). Suppression is narrow: only the same field on the current record is exempted. Cross-field cascades, xRec, and other same-table record variables still fire. Covers tables, table extensions, pages and page extensions (including modify(...) before/after validate via the internal Target symbol) and the Rec., bare implicit-with, and this. reference forms. The this. form relies on IInstanceReferenceOperation, which is absent in the netstandard2.1 SDK, so it is guarded with #if !NETSTANDARD2_1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nd.Change The modified base field/control of a modify(...) extension is only reachable via the internal 'Target' property (the public IChangeSymbol interface exposes only ChangeKind and Type), so reflection stays. Tighten it: only reflect when the owner's public Kind is SymbolKind.Change, and name the reflected property via a const instead of a bare literal. Adds EnumProvider.SymbolKind.Change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the IInstanceReferenceOperation type check (guarded with #if !NETSTANDARD2_1) for detecting a this/self instance with EnumProvider.OperationKind.ThisReference (guarded != default). IInstanceReferenceOperation is absent from the netstandard2.1 compile floor (AL 12.0.13), so the #if guard silently dropped this. suppression on the netstandard2.1 binary that serves AL 14.0-15.2. The OperationKind enum is reachable via EnumProvider on every TFM and resolves to default on SDKs without the member (where no this code can exist), so the guard makes it a no-op there and exact elsewhere. Mirrors the AC0032 / PR #353 approach of resolving this via the operation tree instead of typed nodes. Rec/xRec are reserved AL keywords sharing one record type, so the name remains the only public discriminator between the current record and the xRec before-image (IsThis/HasImplicitWith live on the internal SynthesizedGlobalVariableSymbol). Adds EnumProvider.OperationKind.ThisReference; sorts the OperationKind and SymbolKind members alphabetically. Documents the operation-level this/self pattern in the analyzer-development and netstandard2.1 instruction files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The OnValidateSameFieldThisReference fixture uses the AL 'this' keyword (runtime 14.0, BC 2024 wave 2) and the four extension fixtures declare the extension target in the same module (table/page extension support for a same-module target requires runtime 13.0). Skip these on older runtimes via SkipTestIfVersionIsTooLow, matching the existing guards in AC0032 and DuplicateODataEntityName, to avoid AL0118/AL0334 compile errors on the netstandard2.1 / older-SDK test matrix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The case-list parameter is an array, so the four table/page extension fixtures share a single SkipTestIfVersionIsTooLow call with a combined reason instead of two near-identical calls. 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
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.
Closes #357
Problem
PC0037 (
UseValidateForFieldAssignment) raised a false positive when a field is assigned to the current record (Rec/self) inside that field's ownOnValidate/OnBeforeValidate/OnAfterValidatetrigger. That by-design value transformation (default values, rounding a unit price) cannot useValidate()without recursion, so it should not be flagged.Change
Adds a narrow suppression guard in
UseValidateForFieldAssignment:IsCurrentRecordInstance): athis/self reference, detected viainstance.Kind == EnumProvider.OperationKind.ThisReference(guarded!= default), or an instance symbol namedRec(covers explicitRec.and a page's implicit-with bare reference).Rec/xRecare reserved AL keywords sharing one record type, so the name is the only public discriminator from thexRecbefore-image.ResolveTriggerOwnerField): from the trigger's containing symbol —IFieldSymbol(table field),IControlSymbol.RelatedFieldSymbol(page control), or, formodify(...)before/after validate in extensions, the change-modify symbol's internalTarget(read via reflection, guarded byowner.Kind == SymbolKind.Change, resolved recursively).Still fires (proves narrowness)
Recinside another field'sOnValidate.xRec."Same Field"and other same-table record variables.netstandard2.1 / SDK-version compatibility
this/self is detected via theOperationKind.ThisReferenceenum throughEnumProvider, not theIInstanceReferenceOperationtype. That type is absent from the netstandard2.1 compile floor (AL 12.0.13), and referencing it would force an#if !NETSTANDARD2_1guard that silently dropsthis.suppression on the netstandard2.1 binary serving AL 14.0-15.2. The enum resolves todefaulton SDKs without the member (where nothiscode can exist), so the!= defaultguard makes it a no-op there and exact elsewhere — no#if, works on every TFM.EnumProvider.OperationKind.ThisReferenceandEnumProvider.SymbolKind.Change(both sorted into their alphabetical slots).netstandard2.1andnet10.0.Tests
src/ALCops.PlatformCop.Test— all 468 pass (PC0037: 20/20).13.0(extension target declared in the same module) andOnValidateSameFieldThisReferencebelow14.0(thethiskeyword), viaSkipTestIfVersionIsTooLow, matching AC0032 / DuplicateODataEntityName.Docs
pc0037-use-validate-for-field-assignment.instructions.mdupdated (design decisions, architecture, known issues).analyzer-development.instructions.mdandnetstandard21-compatibility.instructions.mdgained reusable guidance on detectingthis/self viaOperationKind.ThisReferenceat the operation level.