From 4e4e7ab9a1933916b7b850b01ffd7b6c8be5e1a7 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Fri, 26 Jun 2026 14:29:45 +0200 Subject: [PATCH 1/5] fix(PC0037): suppress same-field assignment in own validate trigger 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> --- ...idate-for-field-assignment.instructions.md | 13 ++- .../OnValidateDifferentFieldOnRec.al | 15 +++ .../OnValidateOtherRecordSameField.al | 15 +++ .../HasDiagnostic/OnValidateXRecSameField.al | 13 +++ .../OnValidateSameFieldThisReference.al | 13 +++ .../PageControlOnValidateSameField.al | 27 ++++++ ...ControlOnValidateSameFieldBareReference.al | 27 ++++++ ...xtensionControlOnAfterValidateSameField.al | 35 +++++++ ...tensionControlOnBeforeValidateSameField.al | 35 +++++++ ...eExtensionFieldOnAfterValidateSameField.al | 21 ++++ ...ExtensionFieldOnBeforeValidateSameField.al | 21 ++++ .../TableFieldOnValidateSameField.al | 14 +++ .../UseValidateForFieldAssignment.cs | 11 +++ .../UseValidateForFieldAssignment.cs | 97 +++++++++++++++++++ 14 files changed, 354 insertions(+), 3 deletions(-) create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateDifferentFieldOnRec.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateOtherRecordSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateXRecSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/OnValidateSameFieldThisReference.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameFieldBareReference.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnAfterValidateSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnBeforeValidateSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnAfterValidateSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnBeforeValidateSameField.al create mode 100644 src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableFieldOnValidateSameField.al diff --git a/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md b/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md index 0838a92..eae0e20 100644 --- a/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md +++ b/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md @@ -26,7 +26,8 @@ Detects direct field assignments on non-temporary record variables and recommend | Flag all fields (including PK, system fields) | Community consensus: no exceptions, use pragma for justified cases | | Exclude only `temporary` record variables | `IRecordTypeSymbol.Temporary == true` — temporary records don't persist and don't need subscriber execution | | Do NOT exclude non-Normal table types | Christian Hovenbitzer: inherently temporary tables should still validate | -| Do NOT exclude assignments inside OnValidate triggers | Even cascading field assignments should use Validate to trigger subscribers on the target field | +| Suppress same-field assignment to the current record inside its own validate trigger | Assigning a field to `Rec`/self inside that field's own `OnValidate`/`OnBeforeValidate`/`OnAfterValidate` is by design (default values, value transformation such as rounding); calling `Validate()` there is impossible/recursive. Issue #357 | +| Suppression is narrow: same field only, current record only | Cross-field cascades, `xRec`, and other same-table variables still fire (they are different records / different fields, so subscribers must run) | | Do NOT exclude assignments after Init() | The Init+assign+Insert pattern should still use Validate where possible | | No ChangeCompany handling | Use pragma for legacy ChangeCompany+write patterns | | Register for CompoundAssignmentStatement | Guard with `!= default` for netstandard2.1 where the OperationKind doesn't exist | @@ -36,19 +37,25 @@ Detects direct field assignments on non-temporary record variables and recommend - **Analyzer**: Registers for `OperationKind.AssignmentStatement` and `OperationKind.CompoundAssignmentStatement` - **Detection**: Checks if `IAssignmentStatement.Target` is `IFieldAccess` with a non-temporary `IRecordTypeSymbol` instance +- **Own-validate suppression** (`IsAssignmentToOwnValidateField`): walks to the nearest `TriggerDeclarationSyntax`, requires its name to be `OnValidate`/`OnBeforeValidate`/`OnAfterValidate`, requires the assigned instance to be the current record (`IInstanceReferenceOperation` for `this`/self, or instance symbol named `Rec` covering explicit `Rec.` and a page's implicit-with bare reference), then compares the assigned field name (`IsSameName`) to the trigger's owner field +- **Owner field resolution** (`ResolveTriggerOwnerField`): the trigger symbol's `ContainingSymbol` is the owner — an `IFieldSymbol` (table field), an `IControlSymbol` (page control, resolved via `RelatedFieldSymbol`), or a change-modify symbol for `modify(...)` extensions whose modified base field/control is read via the internal `Target` property (`PropertyAccessor.GetPropertyIfExists`), then resolved recursively - **Location**: Reports on `fieldAccess.Syntax.GetIdentifierNameSyntax()` (the field identifier token) - **CodeFix**: Navigates from diagnostic span to parent `AssignmentStatementSyntax`, rewrites to `ExpressionStatement(InvocationExpression(MemberAccess(Rec, "Validate"), ArgumentList(FieldName, Value)))` ## Test coverage -**HasDiagnostic (5 cases):** SimpleAssignment, CompoundAssignment, InsideOnValidateTrigger, AfterInit, PrimaryKeyField. -**NoDiagnostic (3 cases):** TemporaryVariable, ValidateCall, NonRecordVariable. +**HasDiagnostic (7 cases):** SimpleAssignment, CompoundAssignment, AfterInit, PrimaryKeyField, OnValidateDifferentFieldOnRec, OnValidateXRecSameField, OnValidateOtherRecordSameField. +**NoDiagnostic (12 cases):** TemporaryVariable, ValidateCall, NonRecordVariable, InsideOnValidateTrigger, TableFieldOnValidateSameField, TableExtensionFieldOnBeforeValidateSameField, TableExtensionFieldOnAfterValidateSameField, PageControlOnValidateSameField, PageExtensionControlOnBeforeValidateSameField, PageExtensionControlOnAfterValidateSameField, OnValidateSameFieldThisReference, PageControlOnValidateSameFieldBareReference. **HasFix (1 case):** SimpleAssignment. ## Known issues - CompoundAssignmentStatement OperationKind does not exist in netstandard2.1 SDK. Guarded with `!= default` check. - The CodeFix does not handle compound assignments (`+=`, `-=`) — only simple `:=` is auto-fixable. +- `IInstanceReferenceOperation` (used to detect `this`/self) is absent in the netstandard2.1 SDK; the `this.` form is guarded with `#if !NETSTANDARD2_1`, so on netstandard2.1 only the `Rec`-named instance is suppressed. The explicit `Rec.` / bare implicit-with forms still work on all targets. +- `OnBeforeValidate`/`OnAfterValidate` are only valid on **modified** fields/controls (`modify(...)`), not on newly-added extension fields (AL0162). The table/page extension fixtures therefore use `modify("Unit Price")` on a base field. +- A bare table self field reference (`"Field" := ...`, no `Rec.`) binds to the table object type (not `IRecordTypeSymbol`), so it never fires today and needs no suppression. Page bare references use implicit-with → `Rec`, so they fire and are suppressed. +- `xRec` and other same-table record variables keep firing — they are different records. ## Related diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateDifferentFieldOnRec.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateDifferentFieldOnRec.al new file mode 100644 index 0000000..f2deeae --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateDifferentFieldOnRec.al @@ -0,0 +1,15 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) + { + trigger OnValidate() + begin + Rec.[|"Line Amount"|] := Rec."Unit Price" * Rec.Quantity; + end; + } + field(2; Quantity; Decimal) { } + field(3; "Line Amount"; Decimal) { } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateOtherRecordSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateOtherRecordSameField.al new file mode 100644 index 0000000..1a3da23 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateOtherRecordSameField.al @@ -0,0 +1,15 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) + { + trigger OnValidate() + var + OtherSalesLine: Record "Sales Line"; + begin + OtherSalesLine.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateXRecSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateXRecSameField.al new file mode 100644 index 0000000..e318b7c --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/HasDiagnostic/OnValidateXRecSameField.al @@ -0,0 +1,13 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) + { + trigger OnValidate() + begin + xRec.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/OnValidateSameFieldThisReference.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/OnValidateSameFieldThisReference.al new file mode 100644 index 0000000..f6d29e8 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/OnValidateSameFieldThisReference.al @@ -0,0 +1,13 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) + { + trigger OnValidate() + begin + this.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameField.al new file mode 100644 index 0000000..71daf1a --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameField.al @@ -0,0 +1,27 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +page 50100 "Sales Line Card" +{ + PageType = Card; + SourceTable = "Sales Line"; + + layout + { + area(Content) + { + field(UnitPrice; Rec."Unit Price") + { + trigger OnValidate() + begin + Rec.[|"Unit Price"|] := 100; + end; + } + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameFieldBareReference.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameFieldBareReference.al new file mode 100644 index 0000000..1827dd4 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageControlOnValidateSameFieldBareReference.al @@ -0,0 +1,27 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +page 50100 "Sales Line Card" +{ + PageType = Card; + SourceTable = "Sales Line"; + + layout + { + area(Content) + { + field(UnitPrice; Rec."Unit Price") + { + trigger OnValidate() + begin + [|"Unit Price"|] := 100; + end; + } + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnAfterValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnAfterValidateSameField.al new file mode 100644 index 0000000..44c58b2 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnAfterValidateSameField.al @@ -0,0 +1,35 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +page 50100 "Sales Line Card" +{ + PageType = Card; + SourceTable = "Sales Line"; + + layout + { + area(Content) + { + field(UnitPrice; Rec."Unit Price") { } + } + } +} + +pageextension 50101 "Sales Line Card Ext" extends "Sales Line Card" +{ + layout + { + modify(UnitPrice) + { + trigger OnAfterValidate() + begin + Rec.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnBeforeValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnBeforeValidateSameField.al new file mode 100644 index 0000000..fc623ba --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/PageExtensionControlOnBeforeValidateSameField.al @@ -0,0 +1,35 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +page 50100 "Sales Line Card" +{ + PageType = Card; + SourceTable = "Sales Line"; + + layout + { + area(Content) + { + field(UnitPrice; Rec."Unit Price") { } + } + } +} + +pageextension 50101 "Sales Line Card Ext" extends "Sales Line Card" +{ + layout + { + modify(UnitPrice) + { + trigger OnBeforeValidate() + begin + Rec.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnAfterValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnAfterValidateSameField.al new file mode 100644 index 0000000..bd2f457 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnAfterValidateSameField.al @@ -0,0 +1,21 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +tableextension 50101 "Sales Line Ext" extends "Sales Line" +{ + fields + { + modify("Unit Price") + { + trigger OnAfterValidate() + begin + Rec.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnBeforeValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnBeforeValidateSameField.al new file mode 100644 index 0000000..ae73d53 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableExtensionFieldOnBeforeValidateSameField.al @@ -0,0 +1,21 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) { } + } +} + +tableextension 50101 "Sales Line Ext" extends "Sales Line" +{ + fields + { + modify("Unit Price") + { + trigger OnBeforeValidate() + begin + Rec.[|"Unit Price"|] := 100; + end; + } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableFieldOnValidateSameField.al b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableFieldOnValidateSameField.al new file mode 100644 index 0000000..cf80255 --- /dev/null +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/NoDiagnostic/TableFieldOnValidateSameField.al @@ -0,0 +1,14 @@ +table 50100 "Sales Line" +{ + fields + { + field(1; "Unit Price"; Decimal) + { + trigger OnValidate() + begin + Rec.[|"Unit Price"|] := Round(Rec."Unit Price", 0.01); + end; + } + field(2; Quantity; Decimal) { } + } +} diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs index 3930a40..04133f0 100644 --- a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs @@ -25,6 +25,9 @@ public void Setup() [TestCase("CompoundAssignment")] [TestCase("AfterInit")] [TestCase("PrimaryKeyField")] + [TestCase("OnValidateDifferentFieldOnRec")] + [TestCase("OnValidateXRecSameField")] + [TestCase("OnValidateOtherRecordSameField")] public async Task HasDiagnostic(string testCase) { var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) @@ -38,6 +41,14 @@ public async Task HasDiagnostic(string testCase) [TestCase("ValidateCall")] [TestCase("NonRecordVariable")] [TestCase("InsideOnValidateTrigger")] + [TestCase("TableFieldOnValidateSameField")] + [TestCase("TableExtensionFieldOnBeforeValidateSameField")] + [TestCase("TableExtensionFieldOnAfterValidateSameField")] + [TestCase("PageControlOnValidateSameField")] + [TestCase("PageExtensionControlOnBeforeValidateSameField")] + [TestCase("PageExtensionControlOnAfterValidateSameField")] + [TestCase("OnValidateSameFieldThisReference")] + [TestCase("PageControlOnValidateSameFieldBareReference")] public async Task NoDiagnostic(string testCase) { var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) diff --git a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs index 534c745..f2caeb6 100644 --- a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Immutable; using ALCops.Common.Extensions; using ALCops.Common.Reflection; @@ -49,6 +50,9 @@ private static void AnalyzeAssignmentOperation(OperationAnalysisContext ctx) if (recordType.Temporary) return; + if (IsAssignmentToOwnValidateField(ctx, fieldAccess)) + return; + var location = fieldAccess.Syntax?.GetIdentifierNameSyntax()?.GetLocation() ?? fieldAccess.Syntax?.GetLocation(); @@ -61,4 +65,97 @@ private static void AnalyzeAssignmentOperation(OperationAnalysisContext ctx) location, fieldAccess.FieldSymbol.Name)); } + + private static readonly string[] ValidateTriggerNames = + { + "OnValidate", + "OnBeforeValidate", + "OnAfterValidate" + }; + + // Assigning a field to the current record (Rec/self) inside that same field's own + // OnValidate / OnBeforeValidate / OnAfterValidate trigger is by design (default + // values, value transformation such as rounding) and cannot use Validate(). + private static bool IsAssignmentToOwnValidateField(OperationAnalysisContext ctx, IFieldAccess fieldAccess) + { + var syntax = fieldAccess.Syntax; + if (syntax is null) + return false; + + var trigger = syntax.FirstAncestorOrSelf(); + if (trigger is null) + return false; + + if (!IsValidateTrigger(trigger)) + return false; + + if (!IsCurrentRecordInstance(fieldAccess.Instance)) + return false; + + var ownerField = ResolveTriggerOwnerField(ctx); + if (ownerField is null) + return false; + + return fieldAccess.FieldSymbol.Name.IsSameName(ownerField.Name); + } + + private static bool IsValidateTrigger(TriggerDeclarationSyntax trigger) + { + var name = trigger.Name?.Identifier.ValueText; + if (name is null) + return false; + + foreach (var validateName in ValidateTriggerNames) + { + if (string.Equals(name, validateName, StringComparison.OrdinalIgnoreCase)) + return true; + } + + return false; + } + + // The current record is referenced either as 'this'/self (an instance reference) or + // through the synthesized 'Rec' global variable (including a page's implicit-with). + // 'xRec' and any user-declared record variable are different records and must keep firing. + private static bool IsCurrentRecordInstance(IOperation? instance) + { + if (instance is null) + return false; + +#if !NETSTANDARD2_1 + if (instance is IInstanceReferenceOperation) + return true; +#endif + + var symbol = instance.GetSymbolSafe(); + return symbol is not null + && string.Equals(symbol.Name, "Rec", StringComparison.OrdinalIgnoreCase); + } + + // The trigger symbol's containing symbol is the field (table/table extension) or the + // control (page/page extension) that owns the trigger. For a control, the owner field + // is the table field bound to its source expression. For OnBeforeValidate/OnAfterValidate + // the owner is a change-modify symbol whose modified base field/control is exposed via + // an internal 'Target' property. + private static IFieldSymbol? ResolveTriggerOwnerField(OperationAnalysisContext ctx) + => ResolveOwnerField(ctx.ContainingSymbol?.ContainingSymbol); + + private static IFieldSymbol? ResolveOwnerField(ISymbol? owner) + { + switch (owner) + { + case null: + return null; + case IFieldSymbol field: + return field; + case IControlSymbol control: + return control.RelatedFieldSymbol; + } + + var target = owner.GetPropertyIfExists("Target"); + if (target is not null && !ReferenceEquals(target, owner)) + return ResolveOwnerField(target); + + return null; + } } From e47c08fc1fc96fd423af83e0a81a362998fd183d Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Fri, 26 Jun 2026 14:51:44 +0200 Subject: [PATCH 2/5] refactor(PC0037): guard change-modify Target reflection with SymbolKind.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> --- src/ALCops.Common/Reflection/EnumProvider.cs | 3 +++ .../Analyzers/UseValidateForFieldAssignment.cs | 12 +++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/ALCops.Common/Reflection/EnumProvider.cs b/src/ALCops.Common/Reflection/EnumProvider.cs index 6f27aca..f1dd2ae 100644 --- a/src/ALCops.Common/Reflection/EnumProvider.cs +++ b/src/ALCops.Common/Reflection/EnumProvider.cs @@ -792,6 +792,8 @@ public static class SymbolKind new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Action))); private static readonly Lazy _class = new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Class))); + private static readonly Lazy _change = + new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Change))); private static readonly Lazy _codeunit = new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Codeunit))); private static readonly Lazy _control = @@ -864,6 +866,7 @@ public static class SymbolKind #endif public static NavCodeAnalysis.SymbolKind Action => _action.Value; + public static NavCodeAnalysis.SymbolKind Change => _change.Value; public static NavCodeAnalysis.SymbolKind Class => _class.Value; public static NavCodeAnalysis.SymbolKind Codeunit => _codeunit.Value; public static NavCodeAnalysis.SymbolKind ControlAddIn => _controlAddIn.Value; diff --git a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs index f2caeb6..39b5230 100644 --- a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs @@ -140,6 +140,11 @@ private static bool IsCurrentRecordInstance(IOperation? instance) private static IFieldSymbol? ResolveTriggerOwnerField(OperationAnalysisContext ctx) => ResolveOwnerField(ctx.ContainingSymbol?.ContainingSymbol); + // Name of the internal 'SourceChangeModifySymbol.Target' property. It is not exposed on + // the public IChangeSymbol interface (which only surfaces ChangeKind and Type), so the + // modified base field/control can only be reached via reflection. + private const string ChangeModifyTargetPropertyName = "Target"; + private static IFieldSymbol? ResolveOwnerField(ISymbol? owner) { switch (owner) @@ -152,7 +157,12 @@ private static bool IsCurrentRecordInstance(IOperation? instance) return control.RelatedFieldSymbol; } - var target = owner.GetPropertyIfExists("Target"); + // modify(field)/modify(control) in extensions: the owner is a change-modify symbol + // (SymbolKind.Change) whose modified base symbol is exposed via the internal 'Target'. + if (owner.Kind != EnumProvider.SymbolKind.Change) + return null; + + var target = owner.GetPropertyIfExists(ChangeModifyTargetPropertyName); if (target is not null && !ReferenceEquals(target, owner)) return ResolveOwnerField(target); From cce8b64ba37b2f72c2289d0d8e743be4f608873a Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Fri, 26 Jun 2026 15:08:04 +0200 Subject: [PATCH 3/5] refactor(PC0037): detect this/self via OperationKind.ThisReference enum 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> --- .../analyzer-development.instructions.md | 29 +++++++++++++++++++ ...etstandard21-compatibility.instructions.md | 1 + ...idate-for-field-assignment.instructions.md | 5 ++-- src/ALCops.Common/Reflection/EnumProvider.cs | 15 ++++++---- .../UseValidateForFieldAssignment.cs | 22 ++++++++++---- 5 files changed, 58 insertions(+), 14 deletions(-) diff --git a/.github/instructions/analyzer-development.instructions.md b/.github/instructions/analyzer-development.instructions.md index 56941c6..b0e6aed 100644 --- a/.github/instructions/analyzer-development.instructions.md +++ b/.github/instructions/analyzer-development.instructions.md @@ -380,6 +380,35 @@ This is the same mechanism AC0031 (`RequiredPermissionDetector.TryGetFromInvocat `invocation.Instance.Type`. Keep the variable-map fast path first so `GetOperation` (the ~0.3ms call) only runs for the rare non-identifier receivers. +### Detecting `this`/self at the operation level (`OperationKind.ThisReference`) + +When you already hold the bound `IOperation` (e.g. `IFieldAccess.Instance` inside a +`RegisterOperationAction`) rather than syntax, detect a `this`/self reference via the +**`OperationKind` enum**, not the `IInstanceReferenceOperation` type: + +```csharp +// Works on every TFM. Never names IInstanceReferenceOperation. +var thisReferenceKind = EnumProvider.OperationKind.ThisReference; +if (thisReferenceKind != default && instance.Kind == thisReferenceKind) + return true; // `this`/self +``` + +`IInstanceReferenceOperation`, `ThisExpressionSyntax`, and `SyntaxKind.ThisExpression` are all +absent from the netstandard2.1 compile floor (AL 12.0.13). Referencing the interface forces an +`#if !NETSTANDARD2_1` guard that silently drops `this.` detection on the netstandard2.1 binary that +serves AL 14.0–15.2. `EnumProvider.OperationKind.ThisReference` resolves to `default` (the enum's +`None`) on SDKs without the member — where no `this` code can exist anyway — so the `!= default` +guard makes it a no-op there and exact elsewhere. Add the member to `EnumProvider` with the +string-literal `ParseEnum<…>("ThisReference")` form (like `CompoundAssignmentStatement`), since +`nameof(OperationKind.ThisReference)` will not compile at the floor. + +Note that `this` yields a record-typed symbol (`BoundThisReference.ExpressionSymbol => Type`), so +`GetSymbolSafe().Name` is the table name, **not** `"Rec"`. To distinguish the current record from +the `xRec` before-image (both share the same record type), compare the global's name to the +reserved keyword `"Rec"` — the `IsThis`/`HasImplicitWith` flags that the compiler uses live on the +internal `SynthesizedGlobalVariableSymbol` and are not publicly reachable. See PC0037 +`UseValidateForFieldAssignment.IsCurrentRecordInstance`. + ## 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. diff --git a/.github/instructions/netstandard21-compatibility.instructions.md b/.github/instructions/netstandard21-compatibility.instructions.md index 15fd88a..79c28c8 100644 --- a/.github/instructions/netstandard21-compatibility.instructions.md +++ b/.github/instructions/netstandard21-compatibility.instructions.md @@ -22,6 +22,7 @@ Some `Microsoft.Dynamics.Nav.CodeAnalysis` APIs only exist in newer SDK versions |---|---|---| | `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`. | +| `IInstanceReferenceOperation` (`this`/self when you hold the `IOperation`) | net8.0+ only (do not reference) | When you already have the bound instance operation, detect self via `instance.Kind == EnumProvider.OperationKind.ThisReference` (guarded `!= default`) instead of `instance is IInstanceReferenceOperation`. The `OperationKind` enum member is reachable through `EnumProvider` (string-literal `ParseEnum<…>("ThisReference")`) and resolves to `default` on SDKs without it, so no `#if` is needed and it works on every TFM. See PC0037 `UseValidateForFieldAssignment.IsCurrentRecordInstance` and the this/self note in `analyzer-development.instructions.md`. | ### Pattern for `IFieldSymbol.Type` diff --git a/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md b/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md index eae0e20..673ddad 100644 --- a/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md +++ b/.github/instructions/pc0037-use-validate-for-field-assignment.instructions.md @@ -37,7 +37,8 @@ Detects direct field assignments on non-temporary record variables and recommend - **Analyzer**: Registers for `OperationKind.AssignmentStatement` and `OperationKind.CompoundAssignmentStatement` - **Detection**: Checks if `IAssignmentStatement.Target` is `IFieldAccess` with a non-temporary `IRecordTypeSymbol` instance -- **Own-validate suppression** (`IsAssignmentToOwnValidateField`): walks to the nearest `TriggerDeclarationSyntax`, requires its name to be `OnValidate`/`OnBeforeValidate`/`OnAfterValidate`, requires the assigned instance to be the current record (`IInstanceReferenceOperation` for `this`/self, or instance symbol named `Rec` covering explicit `Rec.` and a page's implicit-with bare reference), then compares the assigned field name (`IsSameName`) to the trigger's owner field +- **Own-validate suppression** (`IsAssignmentToOwnValidateField`): walks to the nearest `TriggerDeclarationSyntax`, requires its name to be `OnValidate`/`OnBeforeValidate`/`OnAfterValidate`, requires the assigned instance to be the current record, then compares the assigned field name (`IsSameName`) to the trigger's owner field +- **Current-record detection** (`IsCurrentRecordInstance`): true when the instance is a `this`/self reference — detected via `instance.Kind == EnumProvider.OperationKind.ThisReference` (guarded `!= default`), **not** the `IInstanceReferenceOperation` type — or when its symbol is named `Rec` (covers explicit `Rec.` and a page's implicit-with bare reference). `Rec`/`xRec` are reserved AL keywords, so the name is the only public discriminator between the current record and the `xRec` before-image. See the this/self note in `analyzer-development.instructions.md`. - **Owner field resolution** (`ResolveTriggerOwnerField`): the trigger symbol's `ContainingSymbol` is the owner — an `IFieldSymbol` (table field), an `IControlSymbol` (page control, resolved via `RelatedFieldSymbol`), or a change-modify symbol for `modify(...)` extensions whose modified base field/control is read via the internal `Target` property (`PropertyAccessor.GetPropertyIfExists`), then resolved recursively - **Location**: Reports on `fieldAccess.Syntax.GetIdentifierNameSyntax()` (the field identifier token) - **CodeFix**: Navigates from diagnostic span to parent `AssignmentStatementSyntax`, rewrites to `ExpressionStatement(InvocationExpression(MemberAccess(Rec, "Validate"), ArgumentList(FieldName, Value)))` @@ -52,7 +53,7 @@ Detects direct field assignments on non-temporary record variables and recommend - CompoundAssignmentStatement OperationKind does not exist in netstandard2.1 SDK. Guarded with `!= default` check. - The CodeFix does not handle compound assignments (`+=`, `-=`) — only simple `:=` is auto-fixable. -- `IInstanceReferenceOperation` (used to detect `this`/self) is absent in the netstandard2.1 SDK; the `this.` form is guarded with `#if !NETSTANDARD2_1`, so on netstandard2.1 only the `Rec`-named instance is suppressed. The explicit `Rec.` / bare implicit-with forms still work on all targets. +- `this`/self detection uses the `OperationKind.ThisReference` enum (via `EnumProvider`, guarded `!= default`), **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 approach works on every TFM with no guard. See AC0032 / PR #353. - `OnBeforeValidate`/`OnAfterValidate` are only valid on **modified** fields/controls (`modify(...)`), not on newly-added extension fields (AL0162). The table/page extension fixtures therefore use `modify("Unit Price")` on a base field. - A bare table self field reference (`"Field" := ...`, no `Rec.`) binds to the table object type (not `IRecordTypeSymbol`), so it never fires today and needs no suppression. Page bare references use implicit-with → `Rec`, so they fire and are suppressed. - `xRec` and other same-table record variables keep firing — they are different records. diff --git a/src/ALCops.Common/Reflection/EnumProvider.cs b/src/ALCops.Common/Reflection/EnumProvider.cs index f1dd2ae..eda786d 100644 --- a/src/ALCops.Common/Reflection/EnumProvider.cs +++ b/src/ALCops.Common/Reflection/EnumProvider.cs @@ -484,6 +484,8 @@ public static class OperationKind new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.AssignmentStatement))); private static readonly Lazy _binaryOperatorExpression = new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.BinaryOperatorExpression))); + private static readonly Lazy _compoundAssignmentStatement = + new(() => ParseEnum("CompoundAssignmentStatement")); private static readonly Lazy _conversionExpression = new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.ConversionExpression))); private static readonly Lazy _emptyStatement = @@ -506,17 +508,18 @@ public static class OperationKind new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.ParameterReferenceExpression))); private static readonly Lazy _returnValueReferenceExpression = new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.ReturnValueReferenceExpression))); + private static readonly Lazy _thisReference = + new(() => ParseEnum("ThisReference")); private static readonly Lazy _xmlPortDataItemAccess = new(() => ParseEnum(nameof(NavCodeAnalysis.OperationKind.XmlPortDataItemAccess))); - private static readonly Lazy _compoundAssignmentStatement = - new(() => ParseEnum("CompoundAssignmentStatement")); public static NavCodeAnalysis.OperationKind AssignmentStatement => _assignmentStatement.Value; public static NavCodeAnalysis.OperationKind BinaryOperatorExpression => _binaryOperatorExpression.Value; + public static NavCodeAnalysis.OperationKind CompoundAssignmentStatement => _compoundAssignmentStatement.Value; public static NavCodeAnalysis.OperationKind ConversionExpression => _conversionExpression.Value; public static NavCodeAnalysis.OperationKind EmptyStatement => _emptyStatement.Value; - public static NavCodeAnalysis.OperationKind ExpressionStatement => _expressionStatement.Value; public static NavCodeAnalysis.OperationKind ExitStatement => _exitStatement.Value; + public static NavCodeAnalysis.OperationKind ExpressionStatement => _expressionStatement.Value; public static NavCodeAnalysis.OperationKind FieldAccess => _fieldAccess.Value; public static NavCodeAnalysis.OperationKind GlobalReferenceExpression => _globalReferenceExpression.Value; public static NavCodeAnalysis.OperationKind InvocationExpression => _invocationExpression.Value; @@ -524,8 +527,8 @@ public static class OperationKind public static NavCodeAnalysis.OperationKind LocalReferenceExpression => _localReferenceExpression.Value; public static NavCodeAnalysis.OperationKind ParameterReferenceExpression => _parameterReferenceExpression.Value; public static NavCodeAnalysis.OperationKind ReturnValueReferenceExpression => _returnValueReferenceExpression.Value; + public static NavCodeAnalysis.OperationKind ThisReference => _thisReference.Value; public static NavCodeAnalysis.OperationKind XmlPortDataItemAccess => _xmlPortDataItemAccess.Value; - public static NavCodeAnalysis.OperationKind CompoundAssignmentStatement => _compoundAssignmentStatement.Value; } /// @@ -790,10 +793,10 @@ public static class SymbolKind { private static readonly Lazy _action = new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Action))); - private static readonly Lazy _class = - new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Class))); private static readonly Lazy _change = new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Change))); + private static readonly Lazy _class = + new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Class))); private static readonly Lazy _codeunit = new(() => ParseEnum(nameof(NavCodeAnalysis.SymbolKind.Codeunit))); private static readonly Lazy _control = diff --git a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs index 39b5230..d3b99f6 100644 --- a/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop/Analyzers/UseValidateForFieldAssignment.cs @@ -114,19 +114,29 @@ private static bool IsValidateTrigger(TriggerDeclarationSyntax trigger) return false; } - // The current record is referenced either as 'this'/self (an instance reference) or - // through the synthesized 'Rec' global variable (including a page's implicit-with). - // 'xRec' and any user-declared record variable are different records and must keep firing. + // The current record is referenced either as 'this'/self or through the synthesized 'Rec' + // global variable (including a page's implicit-with bare reference). 'xRec' (the before-image) + // and any user-declared record variable are different records and must keep firing. private static bool IsCurrentRecordInstance(IOperation? instance) { if (instance is null) return false; -#if !NETSTANDARD2_1 - if (instance is IInstanceReferenceOperation) + // 'this'/self reference (AL 14.0+). Detected via the OperationKind enum, NOT the + // IInstanceReferenceOperation type: that type (and SyntaxKind.ThisExpression) is absent + // from the netstandard2.1 compile floor (AL 12.0.13, predating the Fall 2024 'this' + // feature). Referencing it would force an #if guard that silently drops 'this' detection + // on the netstandard2.1 binary that serves AL 14.0-15.2. The enum resolves to default on + // SDKs without the member (where no 'this' code can exist anyway). See AC0032 / PR #353. + var thisReferenceKind = EnumProvider.OperationKind.ThisReference; + if (thisReferenceKind != default && instance.Kind == thisReferenceKind) return true; -#endif + // Current record global 'Rec'. 'Rec' and 'xRec' are reserved AL keywords the compiler + // synthesizes by these exact names (TableObjectMembers/PageObjectMembers), sharing the + // same record type, so the name is the only public, stable discriminator between the + // current record and the 'xRec' before-image (the distinguishing IsThis/HasImplicitWith + // flags live on the internal SynthesizedGlobalVariableSymbol and are not publicly reachable). var symbol = instance.GetSymbolSafe(); return symbol is not null && string.Equals(symbol.Name, "Rec", StringComparison.OrdinalIgnoreCase); From d57e2374d81b6a9a8d9cb1b57048a72c199657dc Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Fri, 26 Jun 2026 15:38:51 +0200 Subject: [PATCH 4/5] test(PC0037): guard this-keyword and extension-target-self fixtures 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> --- .../UseValidateForFieldAssignment.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs index 04133f0..0f95a73 100644 --- a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs @@ -51,6 +51,24 @@ public async Task HasDiagnostic(string testCase) [TestCase("PageControlOnValidateSameFieldBareReference")] public async Task NoDiagnostic(string testCase) { + SkipTestIfVersionIsTooLow( + ["TableExtensionFieldOnBeforeValidateSameField", "TableExtensionFieldOnAfterValidateSameField"], + testCase, + "13.0", + "No support for tableextensions when target itself is already declared in the same module"); + + SkipTestIfVersionIsTooLow( + ["PageExtensionControlOnBeforeValidateSameField", "PageExtensionControlOnAfterValidateSameField"], + testCase, + "13.0", + "No support for pageextensions when target itself is already declared in the same module"); + + SkipTestIfVersionIsTooLow( + ["OnValidateSameFieldThisReference"], + 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); From c1e73782d1738c542cac6566238e3d385b87ae16 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Fri, 26 Jun 2026 15:40:01 +0200 Subject: [PATCH 5/5] test(PC0037): merge the 13.0 extension-target guards into one call 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> --- .../UseValidateForFieldAssignment.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs index 0f95a73..d561e07 100644 --- a/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs +++ b/src/ALCops.PlatformCop.Test/Rules/UseValidateForFieldAssignment/UseValidateForFieldAssignment.cs @@ -52,16 +52,15 @@ public async Task HasDiagnostic(string testCase) public async Task NoDiagnostic(string testCase) { SkipTestIfVersionIsTooLow( - ["TableExtensionFieldOnBeforeValidateSameField", "TableExtensionFieldOnAfterValidateSameField"], + [ + "TableExtensionFieldOnBeforeValidateSameField", + "TableExtensionFieldOnAfterValidateSameField", + "PageExtensionControlOnBeforeValidateSameField", + "PageExtensionControlOnAfterValidateSameField" + ], testCase, "13.0", - "No support for tableextensions when target itself is already declared in the same module"); - - SkipTestIfVersionIsTooLow( - ["PageExtensionControlOnBeforeValidateSameField", "PageExtensionControlOnAfterValidateSameField"], - testCase, - "13.0", - "No support for pageextensions when target itself is already declared in the same module"); + "No support for table/page extensions when target itself is already declared in the same module"); SkipTestIfVersionIsTooLow( ["OnValidateSameFieldThisReference"],