diff --git a/.claude/rules/codefix-development.md b/.claude/rules/codefix-development.md index 273d75a2..9f139e95 100644 --- a/.claude/rules/codefix-development.md +++ b/.claude/rules/codefix-development.md @@ -115,7 +115,7 @@ Key details: - **Delegate signature.** `FixAllProvider.Create(...)` requires `Task` (nullable). Returning `Task` compiles but binds to a different overload path and can misbehave. - **`Optional>` quirk.** In the AL SDK, `fixAllSpans.HasValue` can be `true` while `fixAllSpans.Value.IsDefaultOrEmpty` is also `true` (observed with RoslynTestKit's default document-scope FixAll). Always guard with `!IsDefaultOrEmpty` and fall back to `fixAllContext.GetDocumentDiagnosticsAsync(document)` so FixAll works both in VS Code and in tests. - **One-pass rewrite for `SeparatedSyntaxList`.** Use `root.RemoveNodes(collection, SyntaxRemoveOptions.KeepNoTrivia)` (plural). It handles separator removal correctly across siblings in the same list, whereas per-diagnostic `ReplaceNode` calls conflict. -- **Trivia handling.** For node removals, prefer `SyntaxRemoveOptions.KeepNoTrivia` so multi-line signatures do not leave dangling comments or blank continuation lines. This also removes directives attached to the node; if a directive can be paired outside the removed node, explicitly preserve, transfer, or remove its matching directive before the node rewrite. See `ParameterNotReferencedCodeFixProvider` for a parameter-list implementation. +- **Trivia handling.** For node removals, prefer `SyntaxRemoveOptions.KeepNoTrivia` so multi-line signatures do not leave dangling comments or blank continuation lines. This also removes directives attached to the node; if a directive can be paired outside the removed node, explicitly preserve, transfer, or remove its matching directive before the node rewrite. See `ParameterNotReferencedCodeFixProvider` for a parameter-list implementation and "Compiler directives and disabled text" below for the policy. - **Scope filter via `CodeActionEquivalenceKey`.** Register multiple `CodeAction`s with distinct `EquivalenceKey`s if a rule needs "Fix all of kind X" variants. Read `fixAllContext.CodeActionEquivalenceKey` inside `FixAllAsync` to know which action the user invoked. - **Single-fix path stays consistent.** Use `root.RemoveNode(node, SyntaxRemoveOptions.KeepNoTrivia)` (singular) for the individual quick-fix so single and Fix-All behave identically. @@ -133,9 +133,29 @@ Copy the nearest existing provider rather than inventing a transformation: | Add `Locked = true` to a label | `EmptyCaptionLocked`, `LabelWithTokSuffixMustBeLocked` | | Remove siblings from a `SeparatedSyntaxList` with FixAll | `ParameterNotReferencedCodeFixProvider` | | Insert a statement before another, using the semantic model | `UsePartialRecordsOnRead` | +| Move or remove nodes that may carry `#if` / `#pragma` / `#region` trivia | `PermissionDeclarationOrderCodeFixProvider`, `ParameterNotReferencedCodeFixProvider` | Always build the replacement from the existing nodes and tokens (`WithTriviaFrom`, keep the receiver expression) and guard every navigation step: a missing optional node returns the unchanged document instead of throwing. +## Compiler directives and disabled text + +**Policy: preserve, else bail out.** A fix keeps every directive (`#if`/`#elif`/`#else`/`#endif`, `#pragma`, `#region`/`#endregion`, `#define`/`#undef`) and any disabled text inside the span it rewrites, removes or moves. When it cannot, it does not register: check in `RegisterCodeFixesAsync` so no lightbulb appears (LC0095's `HasConditionalDirective`), or return the unchanged document. Deleting a directive is a correctness bug. `#if` decides what compiles, `#pragma warning` decides which warnings are suppressed, and AppSource apps use both to guard obsoleted objects (`#if not CLEAN25` plus `#pragma warning disable AL0432`). Record a per-fix deviation in the rule doc's CodeFix table. + +**Where directives live.** A directive takes its own line and is *leading trivia of the next token*. So a directive after the last element of a list sits on the closing token (`;`, `)`, `}`) or on the next sibling, outside the node being edited, and one before the first element belongs to that element. An inactive `#if` branch is a single `DisabledTextTrivia`: the "nodes" in it do not exist in the tree, so counts, `SeparatedSyntaxList` indexes and "is this the last entry" checks see fewer elements than the source shows. Tests define no preprocessor symbols, which is why `#if CLEAN25` yields disabled text and `#if not CLEAN25` yields real nodes. + +**What silently drops them:** + +- `WithoutTrivia()`, `WithLeadingTrivia(SyntaxFactory.TriviaList())`, or `WithTriviaFrom(other)` on a node that carried directives. +- Rebuilding a list or node from `SyntaxFactory` (FC0004's `BuildMultiLinePermissionValue` strips every entry's trivia; it is safe there only because the analyzer never reports lists with directives). +- `RemoveNode(s)` with `KeepNoTrivia`, which drops every directive in the removed node's trivia. `KeepUnbalancedDirectives` keeps `#define`/`#undef` and those `#if`/`#region` directives whose partners (`GetRelatedDirectives`) are not all inside the removed span, and still drops `#pragma`. `KeepDirectives` keeps all of them. Source: `SyntaxNodeRemover.AddDirectives` in `Microsoft.Dynamics.Nav.CodeAnalysis.Syntax/SyntaxNodeRemover.cs` (same logic at 12.0 and 18.x). +- A `SourceText.WithChanges` edit, or `SyntaxFactory.Parse*` output, over a span that contains directive lines. + +**Detection.** `node.ContainsDirectives` / `token.ContainsDirectives` is the cheap gate. `trivia.IsDirective`, `trivia.GetStructure() is ConditionalDirectiveTriviaSyntax` (`#if`/`#elif`) or `is PragmaWarningDirectiveTriviaSyntax`, `node.GetDirectives(filter)` / `GetFirstDirective` and `DirectiveTriviaSyntax.IsActive` identify them. All of these, and `SyntaxRemoveOptions.KeepDirectives` / `KeepUnbalancedDirectives`, are `yes` at every column of the nav-sdk-docs reference tables, including `ns2.0 12.0`, so they need no guard. The `SyntaxKind` values (`DisabledTextTrivia`, `IfDirectiveTrivia`, ...) exist at every version but their ordinals move, so compare them through `EnumProvider.SyntaxKind`. `EnumProvider` exposes `PragmaWarningDirectiveTrivia`, `RegionDirectiveTrivia` and `EndRegionDirectiveTrivia`; add any other kind you need there. + +**Preserving.** Move trivia explicitly and keep the directives in order. FC0004 lifts `#region` runs into a group tree and re-emits them where each group now starts and ends. LC0095 pairs `#pragma warning disable`/`restore`, deletes a balanced pair that only wrapped removed parameters, and transfers the others to the next remaining parameter or to the `)`. The nav-sdk-docs page `docs/60-code-fixes/syntax-editor-and-editing.md` covers `SyntaxRemoveOptions` and the `KeepDirectives` pitfall. + +**Fixtures.** A fix that removes, moves or rebuilds nodes ships the directive fixture set in `testing.md` (Testing Code Fixes). + ## Passing data from analyzer to CodeFix via diagnostic properties When the CodeFix needs information computed by the analyzer (e.g. a replacement name), the analyzer passes it through `ImmutableDictionary` properties on the diagnostic. **Always use the `CodeFixProperties` record pattern** below. Do not use raw dictionary lookups, `out` parameters, or magic strings. diff --git a/.claude/rules/diagnostics/fc0004-permission-declaration-order.md b/.claude/rules/diagnostics/fc0004-permission-declaration-order.md index aab0d77f..8499c6b8 100644 --- a/.claude/rules/diagnostics/fc0004-permission-declaration-order.md +++ b/.claude/rules/diagnostics/fc0004-permission-declaration-order.md @@ -32,7 +32,7 @@ Registers `RegisterCompilationAction`, one diagnostic per object on the `Propert ## Deliberate non-reports -- Lists containing any non-region directive (`#if`, `#pragma`, ...) or unbalanced regions: AZ refuses to sort them, so no diagnostic and no fix. +- Lists containing any non-region directive (`#if`, `#pragma`, ...) or unbalanced regions: AZ refuses to sort them, so no diagnostic and no fix. This covers both `#if` branches. In an active branch (`#if not X`) the entries are real nodes and the `#if`/`#pragma` lines are leading trivia of the next entry or of the `;`. In an inactive branch the entries are one disabled-text trivia, so the list has fewer entries than the source shows. The region tree refuses both, so the directive deletion the pre-AZ fix did when it rebuilt the whole list ([#454](https://github.com/ALCops/Analyzers/issues/454)) cannot come back. - An `#endregion` on the line after the `;` (a common hand-written layout) is outside the property, so the list counts as unbalanced and is never checked; AZ behaves the same because it does not pass the closing token for separated lists. - Entries with equal sort keys in any relative order: the sort is stable, so they are never reported. @@ -57,3 +57,4 @@ Registers `RegisterCompilationAction`, one diagnostic per object on the `Propert | Empty `#region ... #endregion` pairs stay with the entry that carried them | An empty group has nothing to sort; as a tree node it would be flattened after the entries and drift to the end of the list. | | Single-line lists (no newline separators, no directives) are rewritten as multi-line via `BuildMultiLinePermissionValue` | Unchanged behaviour from the original rule. | | The fix bails out defensively on non-region directives or unbalanced regions | Mirrors the analyzer, which never reports such lists. | +| Directives outside the list (on the line before `Permissions`, or after the `;`) are left alone | They are trivia of the property's own tokens or of the next sibling, and `PropertySyntax.WithValue` keeps both on the reorder path and on the single-line rebuild path. The `HasFix` directive fixtures pin this. | diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 79d4f5ea..a46333d8 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -123,6 +123,17 @@ HasFix files use current/expected pairs: `HasFix` / `HasFixAll` test methods and their `current.al` / `expected.al` layout: `.claude/skills/new-codefix/references/hasfix-tests.md` (used by `/new-codefix`). Key rule: `TestCodeFix` takes the `DiagnosticDescriptor` object, `TestFixAll` takes the ID string. +### Directive fixtures (mandatory for fixes that remove, move or rebuild nodes) + +Such a fix ships a fixture for each of these around the edited node (or, where the node cannot carry one, around its closest sibling): + +1. an active `#if not CLEAN25 ... #endif`; +2. an inactive `#if CLEAN25 ... #endif` (disabled text); +3. a `#pragma warning disable ... restore` pair; +4. a `#region ... #endregion` pair. + +Each one is a `HasFix` case whose `expected.al` keeps every directive line, a `NoFix` case (`fixture.NoCodeFix(code, descriptor)`, LC0095 layout) when the fix bails out, or a `NoDiagnostic` case when the analyzer itself declines (FC0004). Tests define no preprocessor symbols, so `#if CLEAN25` is disabled text and `#if not CLEAN25` keeps its contents as real nodes; the two exercise different trees. Confirm a directive `HasFix` case fails when `expected.al` omits one directive line, or it may pass for the wrong reason. The policy behind this is in `codefix-development.md` (Compiler directives and disabled text). + ## Version-Conditional Test Skipping ### Skipping an entire test method (net8.0-only rules) diff --git a/.claude/skills/new-codefix/SKILL.md b/.claude/skills/new-codefix/SKILL.md index 36f6d310..7a79d420 100644 --- a/.claude/skills/new-codefix/SKILL.md +++ b/.claude/skills/new-codefix/SKILL.md @@ -21,13 +21,14 @@ Work out the fix, then **stop and confirm before editing any file**: | FixAll: `BatchFixer` or custom `FixAllProvider`? | Custom when several diagnostics edit a shared ancestor node (`ParameterListSyntax`, property lists). | | Does the fix need analyzer data via `Diagnostic.Properties` (`CodeFixProperties`)? | If yes, the analyzer and its tests change in the same PR. | | Does the cop already reference `…CodeAnalysis.Workspaces.dll` and `System.Composition.AttributedModel.dll`? | `DocumentationCop` and `TestAutomationCop` currently have no `CodeFixes/`; adding one means adding these references to the `.csproj`. | +| Which directives can occur inside the span the fix rewrites, removes or moves (`#region`, active `#if`, inactive `#if` = disabled text, `#pragma`), and what happens to each? | Preserve or bail out, never delete; name the fixtures that prove it. See `codefix-development.md` (Compiler directives and disabled text) and `testing.md` (Directive fixtures). | | Which nav-sdk-docs pages back the fix shape? | `/nav-sdk-docs:sdk-lookup` on the `SyntaxEditor`, `CodeAction` or `FixAllProvider` member you intend to use; the `docs/60-code-fixes/` pages name the pitfalls (`FindNode` ties, trivia, `BatchFixer` merge semantics). | ## Steps 1. **Resx:** add `{RuleName}CodeAction` (the fix title) to `ALCops.{Cop}Analyzers.resx`. 2. **Provider:** create `src/ALCops.{Cop}/CodeFixes/{RuleName}CodeFixProvider.cs` from `references/codefix-template.md`; `FixableDiagnosticIds` from `DiagnosticDescriptors.{RuleName}.Id`; preserve trivia; compare AL names via `SemanticFacts`; use `SyntaxFactory` per the reference section in `codefix-development.md`. -3. **Tests:** `HasFix/{Case}/current.al` + `expected.al` and the `HasFix` method from `references/hasfix-tests.md`; add `HasFixAll` with ≥2 markers on sibling nodes when the answer to the FixAll question was "custom". +3. **Tests:** `HasFix/{Case}/current.al` + `expected.al` and the `HasFix` method from `references/hasfix-tests.md`; add `HasFixAll` with ≥2 markers on sibling nodes when the answer to the FixAll question was "custom". A fix that removes, moves or rebuilds nodes also gets the directive fixture set (`testing.md`, Directive fixtures). 4. **Run:** `dotnet build ALCops.sln`; `dotnet test src/ALCops.{Cop}.Test/ --filter "FullyQualifiedName~{RuleName}"`. Report real output. Then `/code-review` on the branch (`REVIEW.md` carries the code-fix checklist); fix or justify every correctness finding. 5. **Document:** append `## CodeFix: {RuleName}CodeFixProvider` with a Decision | Rationale table (fix shape, FixAll choice, trivia handling, intentionally unfixed cases) to the rule doc. 6. Commit `feat({ID}): add CodeFix …` on a `feat/` branch. @@ -45,5 +46,6 @@ Work out the fix, then **stop and confirm before editing any file**: | Dropping a qualified receiver (`Rec.`, `Customer.`) when rewriting an invocation (#441 PC0035) | Rewrite only the member/arguments; keep the receiver expression and its trivia. | | A fix that assumes a `MemberAccessExpressionSyntax` receiver and returns the document unchanged on bare self or `this` | Handle the receiver-less `IdentifierNameSyntax` / `InvocationExpressionSyntax` shape too, and add `*BareSelf*` and `*ThisSelf*` `HasFix` cases whenever the diagnostic can fire on those forms (`receiver-forms.md`). | | Rewriting a node that may be missing (unblocked `then` branch, #398 PC0035) | Guard every navigation step; return the unchanged document instead of throwing. | +| Stripping or rebuilding trivia (`WithoutTrivia`, `WithLeadingTrivia(TriviaList())`, `RemoveNode(…, KeepNoTrivia)`, a `SyntaxFactory`-built list) on a node that may carry directives (#454 FC0004) | Gate on `ContainsDirectives`, preserve or bail out, and add the directive fixtures. | | Comparing AL identifiers with `StringComparison.OrdinalIgnoreCase` | `SemanticFacts` name comparison. | | Forgetting the rule doc's CodeFix section | Step 5 is part of "done". | diff --git a/REVIEW.md b/REVIEW.md index 24108a08..af3fadb5 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -4,7 +4,7 @@ Two parts. The first is specific to this repository and names the `.claude/rules ## ALCops house rules -Severity: a finding is a **correctness** finding when it changes what a rule reports, whether the analyzer loads in `alc` or the editor, or whether the netstandard2.1 build compiles. Findings about the `.claude/` documentation contract are nits unless the change adds a rule without its rule doc. +Severity: a finding is a **correctness** finding when it changes what a rule reports, what a CodeFix produces, whether the analyzer loads in `alc` or the editor, or whether the netstandard2.1 build compiles. Findings about the `.claude/` documentation contract are nits unless the change adds a rule without its rule doc. | Check | Why | Reasoning lives in | |---|---|---| @@ -17,6 +17,7 @@ Severity: a finding is a **correctness** finding when it changes what a rule rep | Mutable instance fields; accumulate-here report-there patterns; per-invocation registrations where a per-body registration would do | Instances are shared across passes and projects; per-file passes see one file; cost is paid per callback. | `.claude/rules/sdk-analysis-scope.md`, `.claude/rules/analyzer-performance.md` | | A C# feature or SDK member that does not exist on `netstandard2.1` used without `#if NETSTANDARD2_1` or a version gate; a change that was not built for all three target frameworks | Local builds target `net10.0` only; the CI matrix builds `netstandard2.1;net8.0;net10.0` and analyzers evaluate one compilation at a time. | `.claude/rules/netstandard21-compatibility.md` | | Tests: `[TestCase("X")]` without `X.al`; `NoDiagnostic` fixtures without `[|...|]` markers; `expected.al` that still contains markers; `TestCodeFix` given an id string or `TestFixAll` given a descriptor; a `this` fixture without `SkipTestIfVersionIsTooLow`; a fixture that does not compile without the `InDocumentWithErrors` arrangement | Each of these passes for the wrong reason or fails with an unrelated message. | `.claude/rules/testing.md` | +| A CodeFix that strips or rebuilds trivia (`WithoutTrivia`, `WithLeadingTrivia(SyntaxFactory.TriviaList())`, `RemoveNode(…, KeepNoTrivia)`, a `SyntaxFactory`-built replacement list) or applies a text edit on a span that can contain `#if`, `#pragma` or `#region` lines, without a `ContainsDirectives` gate or explicit re-attachment; a fix that removes, moves or rebuilds nodes without the directive fixtures | Directives are leading trivia of the token that follows them and an inactive `#if` branch is disabled-text trivia; dropping either silently changes what compiles or which warnings are suppressed. | `.claude/rules/codefix-development.md` (Compiler directives and disabled text), `.claude/rules/testing.md` | | A new rule without `DiagnosticIds`, the three resx entries, a `DiagnosticDescriptors` entry with the `alcops.dev` help URI, and `.claude/rules/diagnostics/{id}-{slug}.md`; a new CodeFix without the rule doc's `## CodeFix` section; a changed design decision without its rule-doc row | The rule doc is the contract false-positive triage reads first; the help URI is what users click. | `CLAUDE.md` (Keeping `.claude/` in sync), `.claude/skills/new-analyzer/references/rule-doc.md` | | `ALCopsSettings.cs` changed without `alcops.schema.json` (or the reverse) | A parity test enforces it; the finding saves a CI round-trip. | `.claude/rules/settings-schema.md` | | Issue or PR numbers in code comments, XML docs or `.claude/` general guides; measurements or "not yet implemented" phrasing in general guides | The repository keeps deep context in rule docs and GitHub, not in code; `Validate-Rules.ps1` rejects the stale-fact patterns. | `CLAUDE.md` (Workflow), `.claude/scripts/Validate-Rules.ps1` | diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/current.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/current.al new file mode 100644 index 00000000..e49b9e6a --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/current.al @@ -0,0 +1,39 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + ObsoleteState = Pending; + ObsoleteReason = 'Replaced by table Alpha.'; + ObsoleteTag = '25.0'; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; +#pragma warning disable AL0432 + [|Permissions = tabledata Charlie = R, + tabledata Bravo = R, + tabledata Alpha = R|]; +#pragma warning restore AL0432 +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/expected.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/expected.al new file mode 100644 index 00000000..50c35d97 --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/PreserveDirectivesAroundProperty/expected.al @@ -0,0 +1,39 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + ObsoleteState = Pending; + ObsoleteReason = 'Replaced by table Alpha.'; + ObsoleteTag = '25.0'; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; +#pragma warning disable AL0432 + Permissions = tabledata Alpha = R, + tabledata Bravo = R, + tabledata Charlie = R; +#pragma warning restore AL0432 +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/current.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/current.al new file mode 100644 index 00000000..7a0a2804 --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/current.al @@ -0,0 +1,37 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + ObsoleteState = Pending; + ObsoleteReason = 'Replaced by table Alpha.'; + ObsoleteTag = '25.0'; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; +#if not CLEAN25 + [|Permissions = tabledata Bravo = R, tabledata Alpha = R|]; +#endif +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/expected.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/expected.al new file mode 100644 index 00000000..0894e668 --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/HasFix/SingleLineWithDirectivesAroundProperty/expected.al @@ -0,0 +1,38 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + ObsoleteState = Pending; + ObsoleteReason = 'Replaced by table Alpha.'; + ObsoleteTag = '25.0'; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; +#if not CLEAN25 + Permissions = tabledata Alpha = R, + tabledata Bravo = R; +#endif +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/ActiveIfWithPragmaAroundEntry.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/ActiveIfWithPragmaAroundEntry.al new file mode 100644 index 00000000..315dbfc3 --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/ActiveIfWithPragmaAroundEntry.al @@ -0,0 +1,41 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + ObsoleteState = Pending; + ObsoleteReason = 'Replaced by table Alpha.'; + ObsoleteTag = '25.0'; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; + [|Permissions = tabledata Charlie = R, +#if not CLEAN25 +#pragma warning disable AL0432 + tabledata Bravo = R, +#pragma warning restore AL0432 +#endif + tabledata Alpha = R|]; +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/IfAroundLastEntry.al b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/IfAroundLastEntry.al new file mode 100644 index 00000000..b1ba8534 --- /dev/null +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/NoDiagnostic/IfAroundLastEntry.al @@ -0,0 +1,37 @@ +table 50100 Alpha +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50101 Bravo +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +table 50102 Charlie +{ + Caption = '', Locked = true; + fields + { + field(1; MyField; Integer) { } + } +} + +permissionset 50100 "My Permission Set" +{ + Assignable = true; + [|Permissions = tabledata Charlie = R, + tabledata Bravo = R, +#if not CLEAN25 + tabledata Alpha = R +#endif + |]; +} diff --git a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/PermissionDeclarationOrder.cs b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/PermissionDeclarationOrder.cs index 96ecb30f..dd254060 100644 --- a/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/PermissionDeclarationOrder.cs +++ b/src/ALCops.FormattingCop.Test/Rules/PermissionDeclarationOrder/PermissionDeclarationOrder.cs @@ -62,6 +62,8 @@ public async Task HasDiagnostic(string testCase) [TestCase("QualifiedNamesSorted")] [TestCase("SpacesIgnoredInCompare")] [TestCase("QuotedNamespaceSegment")] + [TestCase("ActiveIfWithPragmaAroundEntry")] + [TestCase("IfAroundLastEntry")] public async Task NoDiagnostic(string testCase) { var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) @@ -84,6 +86,8 @@ public async Task NoDiagnostic(string testCase) [TestCase("PreserveCommentSlots")] [TestCase("SingleLineInsideRegion")] [TestCase("EmptyRegionStaysInPlace")] + [TestCase("PreserveDirectivesAroundProperty")] + [TestCase("SingleLineWithDirectivesAroundProperty")] public async Task HasFix(string testCase) { var currentCode = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasFix), testCase, "current.al"))