Skip to content

Ruleset hardening: promote four legacy warning rules to Error - #11260

Open
Jesper Schulz-Wedde (JesperSchulz) wants to merge 6 commits into
mainfrom
jesperschulz-four-rule-warning-wave
Open

Ruleset hardening: promote four legacy warning rules to Error#11260
Jesper Schulz-Wedde (JesperSchulz) wants to merge 6 commits into
mainfrom
jesperschulz-four-rule-warning-wave

Conversation

@JesperSchulz

@JesperSchulz Jesper Schulz-Wedde (JesperSchulz) commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Promotes AS0103, PTE0004, AS0011, and AA0136 from Warning to the repository default Error posture. This follows #11160 and tracks accepted legacy exceptions under AB#640773.

  • AS0103 / PTE0004: identical old compiler sets of 580 records each (115 suffix paths), reconciled through current-compiler CI to 176 physical tables. Uses 142 shared ID-qualified scopes and fixes 34 production tables through owning permission sets.
  • AS0011: scopes 47 established identifiers whose renaming would break references: 17 legacy test objects and 30 published app objects.
  • AA0136: removes unreachable statements at 37 reconciled line sites in 29 physical files across 22 suffix paths. No AA0136 suppression was added.

Permission-table classification

Category Tables Resolution
Test-only 64 Shared AS0103, PTE0004 object scope
Upgrade/internal/transient/demo 76 Shared AS0103, PTE0004 object scope
Obsolete compatibility 2 Clean-only shared object scope; intentionally excluded from Clean permission sets
Production/business 34 Added to owning permission sets

Production fixes cover 29 Russian statutory/tax tables, two Dynamics SL historical tables, W1 Exchange Sync, and HybridSL SL PurchOrd / SL PurOrdDet. Exchange Sync permissions remain available in Clean across W1 and propagated GB/RU full-access sets. HybridSL permissions for the two purchase-order tables now sit outside not CLEAN28 in all seven owning sets. GB tables Fin. Charge Interest Rate and Postcode Notification Memory cannot be referenced from the Clean permission baseline because their obsolete tag is below the required 99.9; their invalid Clean references were removed and their declarations carry Clean-only compatibility scopes.

CI reconciliation

First wave

Run 34336508154 on 9450a20d00 had 30 failed Compile Apps legs. All 31 failed-job logs were downloaded and every ##[error] normalized.

  • 218 deduped rule diagnostics: Clean 98, Default 120.
  • Removed all 117 CI-reported AA0137 declaration/object pairs plus 76 additional declarations proven newly unused by a full source comparison; post-fix rescan was zero.
  • Removed two AA0228 local methods, fixed one AA0005 single-statement block, and corrected three current-compiler permission sites.
  • Separated three Cleanup failures, one altool infrastructure error, and the aggregate gate without unrelated source changes.

Second wave

Run 34347870936 on 49830b2bdd settled with 8 failed Compile Apps jobs and one aggregate gate. All 9 logs were downloaded: 66 ##[error] lines, 44 mode-split rule diagnostics, and 24 unique roots. There were no Cleanup-only or infrastructure failures.

  • AS0103/PTE0004: fixed HybridSL tables 47039 SL PurchOrd and 47031 SL PurOrdDet in CA/US Clean.
  • AS0105: removed invalid GB Clean permission references and replaced them with two Clean-only declaration scopes.
  • AA0228/AA0137: removed RU UpdateEvaluationDate and Currency fallout.
  • AA0205: removed CH StartDate and its orphaned GetPeriodNoOneDate procedure.
  • AA0137: removed all 15 CZ scope-local declarations from disabled tests.

A later wave may still surface because projects can fail fast; each replacement Clean + Default matrix remains the hard gate.

Contributor posture

New production tables must be added to owning permission sets, new identifiers must use configured affixes, and unreachable code must be removed rather than suppressed. Pragmas here cover established legacy, intentional non-production, or obsolete compatibility objects only; every new restore is ID-qualified.

Validation

  • Ruleset entries: 69 -> 65; None=8 and Hidden=1 unchanged
  • AL0424 remains Warning
  • Added bare restores: 0; AA0136 suppressions: 0
  • git diff --check, preprocessor balance, encoding/newline audits, and complete diff reviews: clean
  • Invoke-MiSnapApp against current main: SUCCESS: No missing files

The starting census is compiler-version-sensitive due to AB#649216. Local targeted compilation remains unavailable because the project package cache is absent; remote Clean + Default CI is the final diagnostic proof.

Jesper Schulz-Wedde and others added 3 commits September 9, 2026 11:39
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration labels Sep 9, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Sep 9, 2026
Jesper Schulz-Wedde and others added 2 commits September 9, 2026 13:42
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟡\ Medium\ Severity\ —\ Testing}$

In src/Layers/RU/Tests/ERM-Finance/ERMSourceCurrency.Codeunit.al, the [Test] methods PurchaseInvoiceNormalVATFCYPaymentLoss and PurchaseInvoiceNormalVATFCYPaymentGain previously contained an early exit; followed by ~100 lines of unreachable setup/assertion code each (an AA0136 'unreachable code' violation). This PR deletes the unreachable code but leaves the early exit; in place, so both [Test] methods now consist of only local variable declarations and exit; — they still carry the [Test] attribute and will report as 'passed' in test runs while exercising no setup, no batch job execution, and no assertions. This silently overstates test coverage for the FX unrealised/realised gain and loss scenarios these methods were meant to verify. Recommend either restoring the scenario (removing the early exit and fixing whatever caused it to be skipped) or removing/disabling the two methods explicitly with a tracked follow-up item, rather than leaving misleadingly-named empty passing tests.

Agent judgement — not directly backed by a BCQuality knowledge article.

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Breaking\ Changes}$

The published procedure GetPeriodNoOneDate was deleted from codeunit 117507 "Create Service Base Calendar" with no Obsolete warning window. Because AL procedures are public unless marked local/internal, dependent extensions can bind to this method today and will stop compiling after this change. Keep the procedure in place, mark it [Obsolete(...)] with the replacement/removal guidance, and remove it only in a later cleanup release.

Knowledge:

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.41.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant