Repository navigation
fix(AC0032): count CalcSums as a read operation - #580
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CalcSums is a SQL aggregate over the receiver table, executed in the calling object, so it needs read permission there. Without it a codeunit whose only access is SetRange + CalcSums looked like it never read the table, and AC0032 reported its tabledata entry as unused. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ture An empty marker spans nothing, so the NoDiagnostic assertion passed even if AC0032 reported the entry. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Oct 11, 2026
Open
Arthurvdv
added a commit
that referenced
this pull request
Oct 17, 2026
This reverts commit 535330b. Runtime probes on BC 29 (Docker) and BC 28.5 (SaaS) showed CalcSums and CalcFields succeed with only the user's indirect permission, whether or not the calling object declares tabledata X = r, so the change would make AC0031 ask for permissions that have no effect. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.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
1 participant
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.
Summary
CalcSumsis a SQL aggregate over the receiver table, run in the calling object, so it needs read permission there. It was missing fromMethodOperationMap, so a codeunit whose only access isSetRange+CalcSumshad itstabledata X = rreported as unused by AC0032, and AC0031 did not flag a missingr.CalcSumsas a Read inMethodOperationMap(AC0031 and AC0032 both pick it up; temporary records stay exempt).RecordMethodClassification.ReadMethodsintentionally still excludes it (same reasoning asNext: it does not fill a record buffer).CalcFieldsis deliberately not included: the read lands on the FlowField's CalcFormula source table, not the receiver, so mapping it would charge the wrong table. This needs FlowField source resolution and is left for a separate change. The table-procedure claim in the issue is also still open..clauderule docs updated.Behaviour change: AC0031 now reports
CalcSumson a non-temporary record not covered by any permission source, so existing projects may see new AC0031 diagnostics.Refs #578 (findings for CalcFields and the table-procedure case: #578 (comment))
Tests
TDD: new fixtures failed before the map change (AC0031
HasDiagnostic/CalcSumsWithoutPermissions, AC0032NoDiagnostic/CalcSumsOnly) and pass after.HasDiagnostic/CalcSumsWithoutPermissions,HasDiagnostic/CalcSumsWithoutReadPermission(= mwithoutr),HasDiagnostic/CalcSumsBareSelf,NoDiagnostic/CalcSumsWithPermissions,NoDiagnostic/CalcSumsTemporaryRecordNoDiagnostic/CalcSumsOnly,NoDiagnostic/CalcSumsBareSelf,HasDiagnostic/CalcSumsPartialUnused,HasDiagnostic/CalcSumsTemporaryRecordOnlydotnet test ALCops.slngreen,dotnet format --verify-no-changesclean, Release CI-TFM builds of ApplicationCop and Common clean,Validate-Rules.ps1passes.Review
/code-review(Sonnet, high) — round 1 reviewed the wrong checkout and was discarded; round 2 ran in the PR worktree: no correctness findings. Test-coverage findings addressed: AC0032 temporary-only CalcSums, AC0031= mwithoutr, bare-self CalcSums for both rules. Not changed, with reason: full receiver-form fixture set (receiver resolution is method-agnostic and already covered by the existing receiver-form fixtures); name-only matching inMethodOperationMap(same as every other entry; AL forbids user methods shadowing built-ins); a test pinningReadMethodsexclusion (record-method-classification.mdalready states membership is not symmetric).Round 3 (final): one real finding fixed, an empty
[||]marker in AC0032NoDiagnostic/CalcSumsBareSelfthat could not catch a regression. Not changed: the reported "unrelated AC0002 commit" is already onmain(stale local base);TableType = Temporary, RecordRef/same-named-procedure and receiver-form fixtures are generic to everyMethodOperationMapentry and covered by existing fixtures; the fixtures-before-fix commit order is the intended TDD sequence (squash merge keepsmainbisectable).Docs PR: ALCops/alcops.dev#201
🤖 Generated with Claude Code