Skip to content

knowledge(style): a new procedure that changes the page's current record should take it as var Record - #216

Merged
Jesper Schulz-Wedde (JesperSchulz) merged 1 commit into
microsoft:mainfrom
Curabis:community-contribution/r5-var-record-for-mutating-procedures
Oct 5, 2026
Merged

Jesper Schulz-Wedde (JesperSchulz) merged 1 commit into
microsoft:mainfrom
Curabis:community-contribution/r5-var-record-for-mutating-procedures

Conversation

@MichaelDieringer

Copy link
Copy Markdown
Contributor

Summary

AL passes a Record parameter by value (a copy) unless it is declared var. When a page/pageextension trigger hands a codeunit only a key (Rec."No.") or a by-value Rec, and the procedure Gets or Modifys its own copy, the trigger's Rec keeps the old field values. Code later in the same trigger that reads Rec (a message, a TestField) or writes from it then works on stale data. Neither the compiler nor the analyzers flag this. The page display itself refreshes after the action (OnAfterGetRecord/OnFindRecord, Learn "Actions at runtime"), so the article is limited to stale use of Rec inside the same trigger, or by a non-page caller.

What this adds

  • microsoft/knowledge/style/mutating-procedure-for-a-page-caller-takes-var-record.md, with good/bad samples. It is design guidance (minor) for new procedures that change the record a page/pageextension trigger already holds.
  • The cost of var. The callee must not change the page's filters or key on the parameter. If it needs its own filtering, it copies the record or calls SetRecFilter on the copy first.
  • Shipped procedures are out of scope. Adding or removing var on them is AS0078 and is covered by breaking-changes/do-not-change-published-procedure-signatures. Add a new procedure instead: an overload that differs only by var is rejected (AL0440, verified with AL compiler 30.0).
  • Exclusions, each grounded in BCApps:
    • Job queue, TaskScheduler and page background task entry points (Sales Post via Job Queue).
    • API actions over buffer tables that locate the record by SystemId (APIV2 Sales Orders).
    • Generic, table-agnostic RecordId APIs (Approvals Mgmt.ApproveRecordApprovalRequest).
    • Key-based procedures whose change happens inside a System API (Sales Order Agent RetrySending).
    • Other tables, read-only procedures and temporary records.
    • Loops, which performance/avoid-cloning-records-before-modify-delete-in-loops covers.
  • Rec.Get after the call is context only, not evidence, because re-reading is a common Base Application idiom.
  • Wiring: a high-signal worklist cue in al-style-review, and registration in the style review-fixtures override.

Evidence

  • Learn:
    • "Working with AL methods": a parameter passed by value is a copy.
    • "Actions overview > Actions at runtime": the page refreshes after an action. The page also shows the CurrPage.SetSelectionFilter(Rec); codeunit.Run(50000, Rec); example.
    • Codeunit.Run(Number [, var Record]).
    • AppSourceCop AS0078.
    • AL0440.
  • BCApps: the Sales Order Reopen action calls ReleaseSalesDoc.PerformManualReopen(Rec), and Reopen(var SalesHeader) calls Modify(true) on the caller's record. Links are pinned to 837ef80.

Applies to: all BC versions.

Why style: this is guidance on parameter shape. It sits next to pages-must-not-contain-business-logic and var-parameters-require-an-addressable-variable.

Test plan

  • Samples checked with AL compiler 30.0 against Base Application 28.4 symbols
  • validate_frontmatter.py: 0 errors (2 warnings, both in files this PR doesn't touch)
  • Test-KnowledgeIndex.ps1, Test-SkillIndex.ps1, Test-ReviewContract.ps1, Test-KnowledgeRetrieval.ps1
  • Test-ReviewFixtures.ps1: 230 cases, including the -PrepareDirectory deterministic ranking check

🤖 Generated with Claude Code

…ord should take it as var Record

Adds mutating-procedure-for-a-page-caller-takes-var-record with good/bad
samples, a worklist cue in al-style-review, and registration in the style
review-fixtures override.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 49ddf8ae46e925ee589c2ea013ff6607723713e3. The stale-buffer guidance is accurate and narrowly scoped to changed fields read/written later in the same page trigger, not display refresh or key-based APIs in general. The var-parameter remedy follows Base Application prior art, preserves page filters/key, and correctly avoids changing already-published signatures or proposing a var-only overload. The explicit minor ceiling and background/API/temporary/other-table carve-outs prevent broader false positives. Exact-head local frontmatter, fixture/contract, skill/schema and knowledge-index/retrieval checks pass. No merge-critical issue found. GitHub validation workflows await approval and should run before merge.

@JesperSchulz
Jesper Schulz-Wedde (JesperSchulz) merged commit b503249 into microsoft:main Oct 5, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants