fix(AC0032): treat Next as a read operation in MethodOperationMap - #490
Merged
Conversation
Cover `Next` as a database read in both permission cops: AC0032 (unused permissions) - NoDiagnostic/NextOnVarParameterFilledElsewhere.al - the #466 repro: a helper codeunit positions the set with FindSet on a var parameter and the caller only iterates with repeat .. until Next() = 0. - NoDiagnostic/NextWithoutParentheses.al, NoDiagnostic/NextStandAlone.al - NoDiagnostic/RecordRefNext.al - RecordRef whole-object bailout - HasDiagnostic/NextPartialUnused.al - only `r` of `rimd` is used AC0031 (requires permissions) - HasDiagnostic/NextWithoutPermissions.al - NoDiagnostic/NextWithPermissions.al, PageSourceTableNext.al, TemporaryRecordNext.al Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`Next()` advances the server-side cursor and fetches the next row, so it reads the database in the object that calls it. Because permissions do not flow through the call stack, a record set positioned by another object's `FindSet` still requires `tabledata <T> = r` in the iterating object. Missing that mapping made AC0032 report such a permission as unused (issue #466). Both permission cops consult the single map, so AC0031 now symmetrically demands `r` for `Next()` on records without a covering permission - a true positive that was previously missed. `RecordRef.Next()` needs no extra code: it is picked up by the existing whole-object RecordRef bailout. `Next` is deliberately NOT added to `RecordMethodClassification.ReadMethods`; its consumers (AC0030's return-value check, the partial-record analysis) reason about the buffer a read fills, which does not apply to `Next`. The doc comment now states that divergence. Fixes #466 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <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
Fixes #466.
Nextwas missing fromMethodOperationMap, so a codeunit that only iterates a record set (repeat … until Rec.Next() = 0) positioned by another object'sFindSetlooked like it never read the table, and AC0032 reported itstabledata X = rentry as unused.Next()advances the server-side cursor and reads rows in the calling object, and permissions do not flow through the call stack, so thatris genuinely required.MethodOperationMap: addNext→Read. Every consumer (AC0031RequiredPermissionDetector, AC0032 pre-filter / resolution / RecordRef bailout) picks it up automatically.RecordMethodClassification.ReadMethodsdeliberately does not gainNext(AC0030 and partial-record analysis reason about the buffer a read fills); doc comment updated to explain the intentional divergence..claude/rulesupdated.AC0031 now reports
Next()on a non-temporary record that is not covered by any permission source — e.g. a procedure that iterates avarrecord parameter withoutPermissions = tabledata X = r. These are true positives at runtime (the object needsr), but existing projects may see new AC0031 diagnostics after upgrading. Please call this out in the release notes.Verification
dotnet test ALCops.sln— all green (AC0030 / PC partial-record suites unaffected)dotnet format ALCops.sln --verify-no-changes— no findingsALCops.CommonandALCops.ApplicationCopbuild onnetstandard2.1;net8.0;net10.0withContinuousIntegrationBuild=true, 0 warningsNextWithoutPermissionsfail; post-fix: passDocs companion: ALCops/alcops.dev#156
🤖 Generated with Claude Code