Skip to content

fix(PC0037): suppress same-field assignment in own validate trigger#371

Merged
Arthurvdv merged 5 commits into
release/v1.0.0from
fix/pc0037-onvalidate-same-field
Jun 26, 2026
Merged

fix(PC0037): suppress same-field assignment in own validate trigger#371
Arthurvdv merged 5 commits into
release/v1.0.0from
fix/pc0037-onvalidate-same-field

Conversation

@Arthurvdv

@Arthurvdv Arthurvdv commented Jun 26, 2026

Copy link
Copy Markdown
Member

Closes #357

Problem

PC0037 (UseValidateForFieldAssignment) raised a false positive when a field is assigned to the current record (Rec/self) inside that field's own OnValidate / OnBeforeValidate / OnAfterValidate trigger. That by-design value transformation (default values, rounding a unit price) cannot use Validate() without recursion, so it should not be flagged.

Change

Adds a narrow suppression guard in UseValidateForFieldAssignment:

  • Fires only when the assignment is inside a validate trigger, targets the same field the trigger owns, and the instance is the current record.
  • "Current record" detection (IsCurrentRecordInstance): a this/self reference, detected via instance.Kind == EnumProvider.OperationKind.ThisReference (guarded != default), or an instance symbol named Rec (covers explicit Rec. and a page's implicit-with bare reference). Rec/xRec are reserved AL keywords sharing one record type, so the name is the only public discriminator from the xRec before-image.
  • Owner field resolution (ResolveTriggerOwnerField): from the trigger's containing symbol — IFieldSymbol (table field), IControlSymbol.RelatedFieldSymbol (page control), or, for modify(...) before/after validate in extensions, the change-modify symbol's internal Target (read via reflection, guarded by owner.Kind == SymbolKind.Change, resolved recursively).
  • Covers tables, table extensions, pages and page extensions.

Still fires (proves narrowness)

  • Different field on Rec inside another field's OnValidate.
  • xRec."Same Field" and other same-table record variables.
  • Another record variable's same-named field.

netstandard2.1 / SDK-version compatibility

  • this/self is detected via the OperationKind.ThisReference enum through EnumProvider, not the IInstanceReferenceOperation type. That type is absent from the netstandard2.1 compile floor (AL 12.0.13), and referencing it would force an #if !NETSTANDARD2_1 guard that silently drops this. suppression on the netstandard2.1 binary serving AL 14.0-15.2. The enum resolves to default on SDKs without the member (where no this code can exist), so the != default guard makes it a no-op there and exact elsewhere — no #if, works on every TFM.
  • Adds EnumProvider.OperationKind.ThisReference and EnumProvider.SymbolKind.Change (both sorted into their alphabetical slots).
  • Built clean on netstandard2.1 and net10.0.

Tests

src/ALCops.PlatformCop.Test — all 468 pass (PC0037: 20/20).

  • NoDiagnostic (new): TableFieldOnValidateSameField, TableExtensionFieldOnBeforeValidateSameField, TableExtensionFieldOnAfterValidateSameField, PageControlOnValidateSameField, PageExtensionControlOnBeforeValidateSameField, PageExtensionControlOnAfterValidateSameField, OnValidateSameFieldThisReference, PageControlOnValidateSameFieldBareReference.
  • HasDiagnostic (new): OnValidateDifferentFieldOnRec, OnValidateXRecSameField, OnValidateOtherRecordSameField.
  • Runtime guards: the four extension fixtures are skipped below runtime 13.0 (extension target declared in the same module) and OnValidateSameFieldThisReference below 14.0 (the this keyword), via SkipTestIfVersionIsTooLow, matching AC0032 / DuplicateODataEntityName.

Docs

  • pc0037-use-validate-for-field-assignment.instructions.md updated (design decisions, architecture, known issues).
  • analyzer-development.instructions.md and netstandard21-compatibility.instructions.md gained reusable guidance on detecting this/self via OperationKind.ThisReference at the operation level.

Note: base is release/v1.0.0 per request (deviates from the usual main target).

Arthurvdv and others added 5 commits June 26, 2026 14:29
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>
@Arthurvdv Arthurvdv merged commit 13d6021 into release/v1.0.0 Jun 26, 2026
48 checks passed
@Arthurvdv Arthurvdv deleted the fix/pc0037-onvalidate-same-field branch June 26, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant