Microsoft/bcquality
1
0
Fork 0
mirror of https://github.com/microsoft/BCQuality.git synced 2026-10-05 06:36:55 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Separate analyzer rules from BCQuality knowledge

Browse source 
Retire deterministic compiler and analyzer duplicates, remove their review routing, and clarify the admission test for contextual analyzer knowledge.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde 2026-09-11 09:22:25 +02:00
parent 2b5550c346
commit 60b43f41a7
60 changed files with 45 additions and 1095 deletions
Download patch file Download diff file Expand all files Collapse all files

2
README.md
View file

@ -78,6 +78,8 @@ the agent's judgment; it is not an exhaustive BC manual or a substitute for
compilation, analyzers, tests, or human review. See
[coverage and limits](docs/using-bcquality.md#coverage-and-limits) for the
available domains and the difference between a folder review and a comparison.
Mechanical issues already enforced by the AL compiler or standard analyzers are
intentionally left to those deterministic tools rather than duplicated here.
Functional areas such as Finance, Supply Chain Management, Manufacturing, Jobs,
Warehousing, and Service, and technologies such as PowerShell, pipelines, and

13
docs/contributing.md
View file

@ -13,7 +13,9 @@ capable LLM **would get something wrong, or miss something, without it**, not
simply because the topic is important. Apply this admission test:
> If this file did not exist, would a modern LLM reviewing or generating BC
> code make a mistake this file would have prevented?
> code make a BC-specific mistake that the configured compiler, analyzers, and
> tests would not reliably catch, or would it misinterpret or incorrectly
> remediate one of their diagnostics?
Good candidates encode a BC-specific mechanic that models get wrong, a
version-dependent behavior, or a misleading interpretation of an analyzer
@ -28,6 +30,15 @@ transactions short" does not earn a separate knowledge file merely by being
sound advice. Negative clarifications that prevent false positives are as
valuable as rules that catch defects.
Do not add knowledge whose anti-pattern is fully and deterministically detected
by the AL compiler or a standard analyzer. This applies to authoring as well as
review: an authoring agent should compile with the consuming app's actual
ruleset and correct the resulting diagnostics instead of carrying prose copies
of analyzer rules in context. Analyzer-related knowledge belongs here only when
it adds a BC-specific exception, version boundary, cross-object implication, or
remediation constraint that the diagnostic itself cannot establish. Merely
explaining why a deterministic rule exists is not sufficient.
**Skills hold discovery and execution mechanics; knowledge files hold BC
facts.** Correct or extend a knowledge article when a BC fact is missing or
wrong. Do not hide that fact in a skill's instructions. A genuine routing,

6
docs/using-bcquality.md
View file

@ -151,6 +151,12 @@ removal or another comparison-only regression requires an actual baseline.
The corpus is technical AL guidance, not exhaustive functional validation or
AppSource certification.
BCQuality intentionally does not duplicate mechanical diagnostics already
enforced by the AL compiler or standard analyzers. Run the consuming app's
normal compiler and analyzer pipeline alongside review and authoring. Knowledge
may still discuss a diagnostic when BC-specific context is needed to avoid a
false positive or choose a correct remediation.
### Knowledge by domain
Each article describes one concern. Where samples exist, use its linked

2
evaluation/README.md
View file

@ -36,7 +36,7 @@ This credential-free check proves every selected leaf maps to a same-named knowl
{
"id": "case-a1b2c3d4",
"findings": [
{ "id": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md" }
{ "id": "microsoft/knowledge/appsource/permission-sets-cover-setup-and-usage-without-super.md" }
]
}
]

3
evaluation/review-fixtures.json
View file

@ -7,9 +7,6 @@
"agents": {
"article": "wire-all-three-agent-interfaces"
},
"appsource": {
"context": "AppSourceCop mandatoryAffixes is configured to ABC."
},
"breaking-changes": {
"article": "do-not-expose-sensitive-data-through-public-api"
},

43
microsoft/knowledge/appsource/object-affixes-prevent-collisions.bad.al
View file

@ -1,43 +0,0 @@
// Anti-pattern: an own object with no affix. Another app that also defines a
// "Loyalty Tier" table cannot be installed alongside this one.
table 50379 "Loyalty Tier"
{
Caption = 'Loyalty Tier';
DataClassification = CustomerContent;
fields
{
field(1; "Code"; Code[20])
{
Caption = 'Code';
}
field(10; Description; Text[100])
{
Caption = 'Description';
}
}
keys
{
key(PK; "Code")
{
Clustered = true;
}
}
}
// Anti-pattern (the common half-measure): the extension object carries the
// affix, but the field it adds to the standard Customer table does not. That
// unaffixed field still collides with any other app that adds "Loyalty Points"
// to Customer, and AS0011 flags it.
tableextension 50378 "ABC Customer Ext" extends Customer
{
fields
{
field(50378; "Loyalty Points"; Integer)
{
Caption = 'Loyalty Points';
DataClassification = CustomerContent;
}
}
}

40
microsoft/knowledge/appsource/object-affixes-prevent-collisions.good.al
View file

@ -1,40 +0,0 @@
// Own object: the affix "ABC" is carried at object-name level.
table 50377 "ABC Loyalty Tier"
{
Caption = 'Loyalty Tier';
DataClassification = CustomerContent;
fields
{
field(1; "Code"; Code[20])
{
Caption = 'Code';
}
field(10; Description; Text[100])
{
Caption = 'Description';
}
}
keys
{
key(PK; "Code")
{
Clustered = true;
}
}
}
// Extension of a standard object: the added field is individually affixed,
// because the object name (Customer) belongs to the base application.
tableextension 50376 "ABC Customer Ext" extends Customer
{
fields
{
field(50376; "Loyalty Points ABC"; Integer)
{
Caption = 'Loyalty Points';
DataClassification = CustomerContent;
}
}
}

30
microsoft/knowledge/appsource/object-affixes-prevent-collisions.md
View file

@ -1,30 +0,0 @@
---
bc-version: [all]
domain: appsource
keywords: [object-affix, prefix, suffix, as0011, appsourcecop, collision, tableextension, first-party, isv]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Apply a reserved affix to objects and to members added to base objects
## Description
An AppSource extension must prevent name collisions through its registered affix or, on BC23 and later for objects it owns, a namespace with at least two levels. The affix still applies to every field, key, control, or action added to a base-application object; see `two-level-namespace-replaces-object-affix-not-extension-member-affix.md`. Without either mechanism, two apps that both define a `Loyalty Tier` table cannot coexist, and two apps that add an unaffixed `Loyalty Points` field to `Customer` still collide regardless of their namespaces.
AppSourceCop enforces this. The primary rule is AS0011 ("An affix is required"); the affixes are configured through `mandatoryAffixes` (and `mandatoryPrefix`) in `AppSourceCop.json`. Two placements matter and are easy to get half-right: an object you define carries the affix at **object-name** level, while a member you add to a **standard** object carries the affix on that **member's** name. Adding an affixed object is not enough — an unaffixed field bolted onto `Customer` still collides and still fails validation.
This rule scopes to Marketplace ISV extensions, which is what AppSourceCop validates. A first-party Microsoft in-box module (publisher `Microsoft`, an object range reserved for first-party use, and no `AppSourceCop.json`/`mandatoryAffixes` in the app) is not built or shipped as an Marketplace extension and is not subject to AS0011, so an unaffixed action or field it adds to a base-application page is not a collision risk to flag. Renaming an existing shipped first-party member to add an affix is itself a breaking change to that module's own history and is not required by this rule.
## Best Practice
Own objects use the registered affix (for example `ABC Loyalty Tier`) or, when targeting BC23 or later, a qualifying namespace. Every field or action added to a standard object remains individually affixed (for example `Loyalty Points ABC` on a `Customer` tableextension).
See sample: [`object-affixes-prevent-collisions.good.al`](object-affixes-prevent-collisions.good.al).
## Anti Pattern
An owned object with neither a qualifying namespace nor an affix, an unaffixed extension member, or the common half-measure where the extension object carries the affix but a field it adds to a standard table does not. AS0011 flags the missing collision protection and the field can still collide with another app.
See sample: [`object-affixes-prevent-collisions.bad.al`](object-affixes-prevent-collisions.bad.al).

22
microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al
View file

@ -1,22 +0,0 @@
namespace Contoso;
table 50462 "Rental Agreement"
{
DataClassification = CustomerContent;
fields
{
field(1; "No."; Code[20]) { }
}
}
tableextension 50463 "Rental Customer Ext" extends Customer
{
fields
{
field(50463; "Loyalty Points"; Integer)
{
DataClassification = CustomerContent;
}
}
}

22
microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al
View file

@ -1,22 +0,0 @@
namespace Contoso.Rentals;
table 50460 "Rental Agreement"
{
DataClassification = CustomerContent;
fields
{
field(1; "No."; Code[20]) { }
}
}
tableextension 50461 "Rental Customer Ext" extends Customer
{
fields
{
field(50461; "Loyalty Points RNT"; Integer)
{
DataClassification = CustomerContent;
}
}
}

28
microsoft/knowledge/appsource/two-level-namespace-replaces-object-affix-not-extension-member-affix.md
View file

@ -1,28 +0,0 @@
---
bc-version: [23..]
domain: appsource
keywords: [namespace, two-level, affix, prefix, suffix, as0011, tableextension, pageextension, false-positive]
technologies: [al]
countries: [w1]
application-area: [all]
---
# A two-level namespace replaces an object affix, not an extension-member affix
## Description
Current AppSource naming guidance accepts a namespace with at least two levels, such as `Contoso.Rentals`, instead of a registered prefix or suffix on the names of objects the app owns. The namespace does not qualify members added to another publisher's object: fields, keys, controls, and actions introduced through table or page extensions still share the target object's flat member namespace and still need the registered affix.
The requirement comes from AppSourceCop rule AS0011, which only runs when the app enables AppSourceCop and configures a mandatory affix — normally an `AppSourceCop.json` next to the app manifest. An app that ships no such configuration is not subject to AS0011, and its extension members are not a compliance gap. This is the usual situation for first-party, in-box apps that ship as part of the product rather than through AppSource: their uniqueness comes from allocated object ID ranges and a controlled source tree, not from a registered affix. Confirm the extending app actually configures a mandatory affix before reporting an unaffixed extension member.
## Best Practice
Choose one collision strategy for owned objects: a registered affix or a globally meaningful namespace with at least two levels. Regardless of that choice, apply the registered affix to every member added to a base or third-party object. Keep the affix configured for AppSourceCop so member validation remains deterministic. Do not raise a missing member affix against an app that does not enable AppSourceCop with a mandatory affix; there AS0011 never fires, and the app's namespace is not the reason — the absent configuration is.
See sample: [`two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al`](two-level-namespace-replaces-object-affix-not-extension-member-affix.good.al).
## Anti Pattern
Using `namespace Contoso;` as though one level satisfied the AppSource alternative, or declaring `namespace Contoso.Rentals;` and then adding an unaffixed `Loyalty Points` field to `Customer` in an app that does configure a mandatory affix. The namespace distinguishes the extension's own objects; it cannot disambiguate members on Customer. The mirror-image mistake is reporting an unaffixed extension member in an app that enables no mandatory affix at all — AS0011 does not apply there, and the finding is a false positive.
See sample: [`two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al`](two-level-namespace-replaces-object-affix-not-extension-member-affix.bad.al).

9
microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.bad.al
View file

@ -1,9 +0,0 @@
// This published object previously used namespace Contoso.Rentals.
namespace Contoso.RentalManagement;
codeunit 50467 "Rental Agreement Mgt."
{
procedure CreateAgreement()
begin
end;
}

8
microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.good.al
View file

@ -1,8 +0,0 @@
namespace Contoso.Rentals;
codeunit 50466 "Rental Agreement Mgt."
{
procedure CreateAgreement()
begin
end;
}

26
microsoft/knowledge/breaking-changes/namespace-is-part-of-published-object-identity.md
View file

@ -1,26 +0,0 @@
---
bc-version: [23..]
domain: breaking-changes
keywords: [namespace, published-object, dependency, breaking-change, as0007, compile-time-identity]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Treat a published namespace as part of object identity
## Description
AL resolves an object by namespace and name. Once an app ships and dependent extensions compile against that identity, changing the namespace breaks their references even when the object name and ID stay unchanged. AppSourceCop AS0007 rejects changing the namespace of published objects; namespaces are therefore not a cosmetic folder-like label that can be reorganized after release.
## Best Practice
Choose a globally meaningful namespace before first publication and keep it stable. Add new functional areas beneath that structure without moving existing published objects. If an identity must move, use the platform's supported move/obsoletion lifecycle rather than a source-only namespace rename.
See sample: [`namespace-is-part-of-published-object-identity.good.al`](namespace-is-part-of-published-object-identity.good.al).
## Anti Pattern
Changing `namespace Contoso.Rentals;` to `namespace Contoso.RentalManagement;` as a cleanup while leaving the object name and ID untouched. Every dependent `using` directive and qualified reference targets the old identity and stops compiling.
See sample: [`namespace-is-part-of-published-object-identity.bad.al`](namespace-is-part-of-published-object-identity.bad.al).

25
microsoft/knowledge/style/applicationarea-required-on-page-controls.bad.al
View file

@ -1,25 +0,0 @@
page 50375 "Sample App Area Bad"
{
PageType = Card;
SourceTable = Customer;
layout
{
area(Content)
{
group(General)
{
// Anti-pattern: no ApplicationArea. AS0062 flags this control,
// and it is silently hidden in the Web client for profiles whose
// enabled areas do not already cover it.
field("No."; Rec."No.")
{
ToolTip = 'Specifies the number that identifies the customer.';
}
field(Name; Rec.Name)
{
ToolTip = 'Specifies the customer''s name.';
}
}
}
}
}

40
microsoft/knowledge/style/applicationarea-required-on-page-controls.good.al
View file

@ -1,40 +0,0 @@
page 50374 "Sample App Area Good"
{
PageType = Card;
SourceTable = Customer;
layout
{
area(Content)
{
group(General)
{
field("No."; Rec."No.")
{
ApplicationArea = All;
ToolTip = 'Specifies the number that identifies the customer.';
}
field(Name; Rec.Name)
{
ApplicationArea = All;
ToolTip = 'Specifies the customer''s name.';
}
}
}
}
actions
{
area(Processing)
{
action(Refresh)
{
ApplicationArea = All;
ToolTip = 'Reloads the current record.';
trigger OnAction()
begin
CurrPage.Update(false);
end;
}
}
}
}

28
microsoft/knowledge/style/applicationarea-required-on-page-controls.md
View file

@ -1,28 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [application-area, page-control, as0062, appsourcecop, hidden-control, web-client]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Every page control needs an `ApplicationArea` (AppSourceCop AS0062)
## Description
A field control on a page or pageextension that has no `ApplicationArea` property is silently hidden in the Web client for every profile whose enabled application areas do not cover it. There is no error and no warning at runtime — the field simply does not appear, which reads as data loss to the user. AppSourceCop AS0062 flags any page control or action that is missing the `ApplicationArea` property, and AppSource technical validation rejects the app until it is set.
Set the property to an area the app actually enables. `All` makes the control visible under every profile and is the common default; if the app declares narrower areas in `app.json`, use one of those. The property applies to field controls and to actions. This is a sibling concern to `caption-required-on-page-fields.md` and `tooltip-required-on-page-fields.md`; note that the ToolTip requirement is the separate CodeCop rule AA0218, not AS0062.
## Best Practice
Every field control and action carries `ApplicationArea = All;` (or a declared area of the app). The value is set once per control and keeps the control visible in the Web client.
See sample: [`applicationarea-required-on-page-controls.good.al`](applicationarea-required-on-page-controls.good.al).
## Anti Pattern
A field control with no `ApplicationArea`. AS0062 flags it, and the control is invisible in the Web client for any profile that does not already enable a matching area.
See sample: [`applicationarea-required-on-page-controls.bad.al`](applicationarea-required-on-page-controls.bad.al).

14
microsoft/knowledge/style/begin-on-same-line-as-then-else-do.bad.al
View file

@ -1,14 +0,0 @@
codeunit 50235 "Sample Begin Own Line Bad"
{
procedure Run(Condition: Boolean)
begin
if Condition then
begin
DoSomething();
DoSomethingElse();
end;
end;
local procedure DoSomething() begin end;
local procedure DoSomethingElse() begin end;
}

24
microsoft/knowledge/style/begin-on-same-line-as-then-else-do.good.al
View file

@ -1,24 +0,0 @@
codeunit 50234 "Sample Begin Same Line Good"
{
procedure Run(Condition: Boolean)
var
i: Integer;
begin
if Condition then begin
DoSomething();
DoSomethingElse();
end else begin
Reset();
Notify();
end;
for i := 1 to 10 do begin
DoSomething();
DoSomethingElse();
end;
end;
local procedure DoSomething() begin end;
local procedure DoSomethingElse() begin end;
local procedure Reset() begin end;
local procedure Notify() begin end;
}

26
microsoft/knowledge/style/begin-on-same-line-as-then-else-do.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [begin, end, compound-statement, aa0005, codecop, formatting]
technologies: [al]
countries: [w1]
application-area: [all]
---
# `begin` goes on the same line as `then`, `else`, or `do` (CodeCop AA0005)
## Description
When a compound block follows `then`, `else`, or `do`, the `begin` keyword must sit on the same line as the preceding keyword, separated by exactly one space. `if Condition then begin` and `for i := 1 to N do begin` are correct. The form that puts `begin` on its own line — common in older AL and in languages like Pascal — is flagged by CodeCop AA0005. The rule does not change indentation of the block body; it only governs the placement of `begin` relative to `then`/`else`/`do`.
## Best Practice
`if Condition then begin … end;`, `else begin … end;`, `for i := 1 to N do begin … end;`. The block body is indented one level below the `if`/`for` line, and `end;` sits at the same indentation as the line that opened the block.
See sample: [`begin-on-same-line-as-then-else-do.good.al`](begin-on-same-line-as-then-else-do.good.al).
## Anti Pattern
A line that ends with `then` (or `else`, or `do`) and is followed by a line whose only content is `begin`. The compiler accepts it but CodeCop AA0005 flags it; the visual cost is a wasted line per block and a layout that looks alien to readers used to current AL style.
See sample: [`begin-on-same-line-as-then-else-do.bad.al`](begin-on-same-line-as-then-else-do.bad.al).

15
microsoft/knowledge/style/block-keywords-start-new-line.bad.al
View file

@ -1,15 +0,0 @@
codeunit 50239 "Sample Block Kw Bad"
{
procedure Dispatch(IsContactName: Boolean; IsSalespersonCode: Boolean)
var
i: Integer;
begin
if IsContactName then ValidateContactName() else if IsSalespersonCode then ValidateSalespersonCode();
for i := 1 to 10 do begin DoSomething(i); DoSomethingElse(i); end;
end;
local procedure ValidateContactName() begin end;
local procedure ValidateSalespersonCode() begin end;
local procedure DoSomething(I: Integer) begin end;
local procedure DoSomethingElse(I: Integer) begin end;
}

23
microsoft/knowledge/style/block-keywords-start-new-line.good.al
View file

@ -1,23 +0,0 @@
codeunit 50238 "Sample Block Kw Good"
{
procedure Dispatch(IsContactName: Boolean; IsSalespersonCode: Boolean)
var
i: Integer;
begin
if IsContactName then
ValidateContactName()
else
if IsSalespersonCode then
ValidateSalespersonCode();
for i := 1 to 10 do begin
DoSomething(i);
DoSomethingElse(i);
end;
end;
local procedure ValidateContactName() begin end;
local procedure ValidateSalespersonCode() begin end;
local procedure DoSomething(I: Integer) begin end;
local procedure DoSomethingElse(I: Integer) begin end;
}

26
microsoft/knowledge/style/block-keywords-start-new-line.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [block-keyword, end, if, repeat, until, for, while, case, aa0018]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Block keywords (`end`, `if`, `repeat`, `until`, `for`, `while`, `case`) start a new line (CodeCop AA0018)
## Description
CodeCop AA0018 requires that the block-introducing keywords `if`, `repeat`, `until`, `for`, `while`, `case`, and the block-terminating keyword `end` always start a new line. Multiple statements packed onto one line — `if A then X() else if B then Y();` written inline, or `for i := 1 to 10 do begin X(i); Y(i); end;` — defeat code review tooling that operates line-by-line and obscure the control flow. The rule does not prohibit short single-statement constructs spread across two lines (`if Cond then X();`); it prohibits packing the entire control structure onto one line.
## Best Practice
Each `if`, `else if`, `repeat`, `for`, `while`, and `case` starts a line. Each `end;` (the closing of a `begin … end` block or a `case`) starts a line. Branch bodies are on their own line, indented.
See sample: [`block-keywords-start-new-line.good.al`](block-keywords-start-new-line.good.al).
## Anti Pattern
`if IsContactName then ValidateContactName() else if IsSalespersonCode then ValidateSalespersonCode();` collapses an `if/else if` chain onto a single line; AA0018 flags both the `else` and the second `if`. The same applies to `for i := 1 to 10 do begin DoX(i); DoY(i); end;` — `end` is not at the start of its line.
See sample: [`block-keywords-start-new-line.bad.al`](block-keywords-start-new-line.bad.al).

26
microsoft/knowledge/style/event-subscriber-param-names-match-publisher.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [event-subscriber, parameter-name, publisher, signature, eventsubscriber, false-positive]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Event subscriber parameter names must match the publisher signature
## Description
In AL, an `[EventSubscriber]` procedure is bound to its publisher by event name and parameter list. For every parameter the subscriber declares, the name is not a style choice — it must match the name the publisher declared. The compiler validates the match at build time and emits an error if the subscriber renames a parameter. This means a reviewer cannot apply a generic "use better names" pass to subscriber parameters: `Sender`, `Rec`, `xRec`, `RunTrigger`, the table-and-field-specific parameter names a publisher emits — all are dictated by the publisher and must be reproduced verbatim.
A subscriber may, however, declare fewer parameters than the publisher. AL binds each subscriber parameter to the publisher parameter of the same name, so the subscriber can omit any parameters its handler does not use, from any position, and can even declare the ones it keeps in a different order than the publisher. This compiles and binds correctly, so a shorter or differently ordered subscriber signature is not a signature mismatch. In shipping BCApps code, `Test Runner - Mgt::OnBeforeTestMethodRun` publishes `CurrentTestMethodLine, CodeunitID, CodeunitName, FunctionName, FunctionTestPermissions, Skip`, and subscribers such as `ALTestRunnerResetEnvironment` bind to it while omitting `Skip` and declaring `CurrentTestMethodLine` last.
## Best Practice
Copy each parameter's name and type from the publisher verbatim for every parameter the subscriber keeps, and omit the ones the handler does not use. When in doubt, navigate to the publisher (`OnAfterValidateEvent`, `OnBeforePostSalesDoc`, etc.) and copy its parameter list. Style rules that apply to other locals — descriptive names, no spaces — do not apply to subscriber parameters. Do not flag a subscriber for declaring fewer parameters than the publisher, for omitting one from the middle of the list, or for declaring them in a different order, as long as every parameter it does declare matches a publisher parameter by name and type: that is valid AL, not a mismatch.
## Anti Pattern
Renaming a publisher parameter to look prettier in the subscriber. The build breaks immediately, because the name is what the runtime binds on. More insidiously, a parameter name that happens to match by coincidence in one event publisher but not in a similar one will compile in some versions of BC and fail in others when the publisher signature evolves.
Detection: a subscriber parameter whose name or type does not correspond to any parameter on the publisher — not a subscriber that merely declares fewer parameters, drops one from the middle, or lists them in a different order.

11
microsoft/knowledge/style/function-call-parentheses-required.bad.al
View file

@ -1,11 +0,0 @@
codeunit 50213 "Sample Parens Bad"
{
procedure Run()
var
Customer: Record Customer;
begin
Customer.Init;
if Customer.FindFirst then
Customer.Modify;
end;
}

11
microsoft/knowledge/style/function-call-parentheses-required.good.al
View file

@ -1,11 +0,0 @@
codeunit 50212 "Sample Parens Good"
{
procedure Run()
var
Customer: Record Customer;
begin
Customer.Init();
if Customer.FindFirst() then
Customer.Modify();
end;
}

26
microsoft/knowledge/style/function-call-parentheses-required.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [parentheses, function-call, method-call, aa0008, codecop]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Always write parentheses on procedure calls (CodeCop AA0008)
## Description
AL allows a parameterless procedure to be called without parentheses — `Customer.Init` instead of `Customer.Init()` — and the result is syntactically identical at runtime. CodeCop AA0008 still flags the parenthesis-less form. The reason is twofold: written without parentheses, a procedure call is visually indistinguishable from a property read, which makes BC code harder to scan; and the same identifier may exist as both a property and a procedure on different objects, so the parentheses are the only local signal that this is a call. The rule applies to every parameterless invocation, including `Init`, `Insert`, `Modify`, `Delete`, `DeleteAll`, `FindFirst`, `FindSet`, `Next`, `Get`, `CalcFields`, and user-defined procedures.
## Best Practice
Always write `()` on a procedure call, even when it takes no arguments: `Customer.Init();`, `TempBuffer.DeleteAll();`, `if Customer.FindFirst() then …`. The same applies inside expressions and as a condition.
See sample: [`function-call-parentheses-required.good.al`](function-call-parentheses-required.good.al).
## Anti Pattern
`Customer.Init;`, `TempBuffer.DeleteAll;`, `if Customer.FindFirst then …`. Every one of those is an AA0008 violation. Reviewers should treat a parameterless procedure name appearing without parentheses as a defect, even though the compiler accepts it.
See sample: [`function-call-parentheses-required.bad.al`](function-call-parentheses-required.bad.al).

14
microsoft/knowledge/style/label-suffix-approved-list.bad.al
View file

@ -1,14 +0,0 @@
codeunit 50201 "Sample Label Suffix Bad"
{
var
CannotDeleteLine: Label 'Cannot delete this line.';
Text000: Label 'Update complete';
UpdateLocation: Label 'Update location?';
WrongSuffixTok: Label 'Customer %1 not found.';
procedure ShowMessages()
begin
Error(WrongSuffixTok, '10000');
Message(Text000);
end;
}

15
microsoft/knowledge/style/label-suffix-approved-list.good.al
View file

@ -1,15 +0,0 @@
codeunit 50200 "Sample Label Suffix Good"
{
var
UpdateCompleteMsg: Label 'Update complete.';
CustomerNotFoundErr: Label 'Customer %1 does not exist.';
DeleteRecordQst: Label 'Delete this record?';
CustomerNameLbl: Label 'Customer Name';
GetMethodTok: Label 'GET', Locked = true;
TelemetryStartedTxt: Label 'Operation started for customer %1.', Locked = true;
procedure ShowMessage()
begin
Message(UpdateCompleteMsg);
end;
}

26
microsoft/knowledge/style/label-suffix-approved-list.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [label, textconst, suffix, aa0074, codecop, msg, err, qst, lbl, tok]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Use approved suffixes on Label and TextConst names (CodeCop AA0074)
## Description
CodeCop AA0074 flags `Label` and `TextConst` identifiers that do not end with an approved usage suffix. The suffix signals at the call site how the text is consumed and what translation behaviour it should get. The approved suffixes and their intended usage are: `Msg` for text shown via `Message()`; `Err` for text passed to `Error()`; `Qst` for text used with `Confirm` or `StrMenu`; `Lbl` for captions and tooltips; `Tok` for short tokens such as `'GET'`, `'PUT'`, `'HTTPS'`, GUIDs, or JSON/XML snippets that are not translated (typically with `Locked = true`); and `Txt` for general text including telemetry messages. A `Label` named `Text000` or `CannotDeleteLine` without a suffix violates the rule, regardless of how readable the prose is.
## Best Practice
Pick the suffix that matches the call where the label is consumed: `UpdateCompleteMsg` for `Message(...)`, `CustomerNotFoundErr` for `Error(...)`, `DeleteRecordQst` for `Confirm(...)`, `CustomerNameLbl` for tooltips and captions, `GetMethodTok` for locked tokens, `TelemetryDataTxt` for telemetry payloads. Suffix choices between `Tok`, `Lbl`, `Txt`, and `Msg` are judgment calls when the suffix is valid for the usage — what matters is that the suffix is on the approved list and matches the actual call.
See sample: [`label-suffix-approved-list.good.al`](label-suffix-approved-list.good.al).
## Anti Pattern
A `Label` declared with no suffix (`CannotDeleteLine: Label '…';`), a generic name (`Text000: Label '…';`), or a suffix that contradicts the usage (`WrongSuffixTok: Label 'Customer %1 not found.'` then passed to `Error()`). All three trip AA0074 or its reviewers and obscure the call-site contract.
See sample: [`label-suffix-approved-list.bad.al`](label-suffix-approved-list.bad.al).

14
microsoft/knowledge/style/lowercase-reserved-keywords.bad.al
View file

@ -1,14 +0,0 @@
codeunit 50245 "Sample Upper Keywords Bad"
{
procedure Walk(VAR Customer: Record Customer)
VAR
Found: Boolean;
BEGIN
IF Customer.FindSet() THEN
REPEAT
Found := TRUE;
UNTIL Customer.Next() = 0;
IF Found THEN
EXIT;
END;
}

14
microsoft/knowledge/style/lowercase-reserved-keywords.good.al
View file

@ -1,14 +0,0 @@
codeunit 50244 "Sample Lower Keywords Good"
{
procedure Walk(var Customer: Record Customer)
var
Found: Boolean;
begin
if Customer.FindSet() then
repeat
Found := true;
until Customer.Next() = 0;
if Found then
exit;
end;
}

28
microsoft/knowledge/style/lowercase-reserved-keywords.md
View file

@ -1,28 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [reserved-keyword, lowercase, aa0241, codecop, if, then, begin]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Reserved keywords are written in lowercase (CodeCop AA0241)
## Description
CodeCop AA0241 requires reserved AL keywords — `if`, `then`, `else`, `begin`, `end`, `var`, `procedure`, `local`, `internal`, `for`, `while`, `repeat`, `until`, `case`, `of`, `do`, `not`, `and`, `or`, `exit`, `break`, `skip`, `quit`, and the rest — to be lowercase. Old Navision and C/AL code used `IF…THEN…BEGIN…END` in uppercase, and that style still lingers in training data and legacy modules. New AL code is lowercase. The rule applies to keywords only — type names (`Record`, `Codeunit`, `Integer`), property names (`Caption`, `ToolTip`), and identifiers are unaffected.
Test codeunits that retain legacy uppercase forms (`OPENEDIT`, `ASSERTERROR`, `VALUE`) are an accepted exception: the test framework historically uses those identifiers and rewriting them brings no benefit. The rule applies to new code in modified lines, not to long-standing test patterns.
## Best Practice
Write keywords lowercase: `if Condition then begin … end;`, `repeat … until Found;`, `for i := 1 to N do …`. The standard AL formatter normalizes casing automatically.
See sample: [`lowercase-reserved-keywords.good.al`](lowercase-reserved-keywords.good.al).
## Anti Pattern
`IF Condition THEN BEGIN DoSomething(); END;`, `REPEAT GetNext(); UNTIL Found;`. Uppercase keywords trip AA0241 and signal C/AL-era code that has not been modernized.
See sample: [`lowercase-reserved-keywords.bad.al`](lowercase-reserved-keywords.bad.al).

11
microsoft/knowledge/style/no-begin-end-around-single-statement.bad.al
View file

@ -1,11 +0,0 @@
codeunit 50237 "Sample Single Stmt Bad"
{
procedure Validate(IsAssemblyOutputLine: Boolean)
var
SalesLine: Record "Sales Line";
begin
if IsAssemblyOutputLine then begin
SalesLine.TestField("Order Line No.", 0);
end;
end;
}

10
microsoft/knowledge/style/no-begin-end-around-single-statement.good.al
View file

@ -1,10 +0,0 @@
codeunit 50236 "Sample Single Stmt Good"
{
procedure Validate(IsAssemblyOutputLine: Boolean)
var
SalesLine: Record "Sales Line";
begin
if IsAssemblyOutputLine then
SalesLine.TestField("Order Line No.", 0);
end;
}

26
microsoft/knowledge/style/no-begin-end-around-single-statement.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [begin, end, single-statement, aa0013, codecop, compound]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Do not wrap a single statement in `begin … end` (CodeCop AA0013)
## Description
CodeCop AA0013 flags `begin … end` blocks that contain exactly one statement. The compound-block syntax exists to group multiple statements as a unit; using it for a single statement adds two lines and a level of nesting without adding meaning. `if IsAssemblyOutputLine then begin TestField("Order Line No.", 0); end;` should be `if IsAssemblyOutputLine then TestField("Order Line No.", 0);` — one statement, no block. The same logic applies after `else`, `for`, `while`, and `repeat`.
## Best Practice
A single statement following `then`, `else`, `do`, or a case label is written on its own line, indented one level, with no `begin … end`. Use `begin … end` only when there are two or more statements to group.
See sample: [`no-begin-end-around-single-statement.good.al`](no-begin-end-around-single-statement.good.al).
## Anti Pattern
`if Cond then begin OneCall(); end;` — single statement wrapped in a block. AA0013 flags it. The reviewer signal is "a `begin` followed by exactly one statement before its `end`."
See sample: [`no-begin-end-around-single-statement.bad.al`](no-begin-end-around-single-statement.bad.al).

11
microsoft/knowledge/style/no-space-before-method-parenthesis.bad.al
View file

@ -1,11 +0,0 @@
codeunit 50231 "Sample No Space Paren Bad"
{
procedure Lookup(CustomerNo: Code[20])
var
Customer: Record Customer;
GreetingMsg: Label 'Hello %1';
begin
if Customer.Get ( CustomerNo ) then
Message ( GreetingMsg, Customer.Name );
end;
}

11
microsoft/knowledge/style/no-space-before-method-parenthesis.good.al
View file

@ -1,11 +0,0 @@
codeunit 50230 "Sample No Space Paren Good"
{
procedure Lookup(CustomerNo: Code[20])
var
Customer: Record Customer;
GreetingMsg: Label 'Hello %1';
begin
if Customer.Get(CustomerNo) then
Message(GreetingMsg, Customer.Name);
end;
}

26
microsoft/knowledge/style/no-space-before-method-parenthesis.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [spacing, parenthesis, method-call, aa0002, codecop]
technologies: [al]
countries: [w1]
application-area: [all]
---
# No space between a method name and its opening parenthesis (CodeCop AA0002)
## Description
CodeCop AA0002 forbids whitespace between a procedure/method name and its `(`. `Customer.Get(CustomerNo)` is correct; `Customer.Get (CustomerNo)` is not. The rule applies to user-defined procedures, system methods (`Insert`, `FindFirst`, `CalcFields`), trigger-style invocations, and the parenthesised cast/conversion forms (`Format(Value)`, `CopyStr(Source, 1, 10)`). The whitespace between `(` and the first argument, and between the last argument and `)`, is also forbidden by the same rule.
## Best Practice
`Customer.Get(CustomerNo)`, `Customer.SetFilter("No.", '%1', '*A*')`, `Message(GreetingMsg, UserName)`. The standard AL formatter enforces this automatically.
See sample: [`no-space-before-method-parenthesis.good.al`](no-space-before-method-parenthesis.good.al).
## Anti Pattern
`Customer.Get ( CustomerNo )`, `Message ( GreetingMsg, UserName )`. Both trip AA0002 and read as if the call had an extra unnamed parameter — a small but persistent friction every reader pays.
See sample: [`no-space-before-method-parenthesis.bad.al`](no-space-before-method-parenthesis.bad.al).

17
microsoft/knowledge/style/optioncaption-required-and-matches-membercount.bad.al
View file

@ -1,17 +0,0 @@
table 50255 "Sample OptionCaption Bad"
{
fields
{
field(1; Status; Option)
{
Caption = 'Status';
OptionMembers = Open,Released,Pending;
}
field(2; Priority; Option)
{
Caption = 'Priority';
OptionMembers = Low,Medium,High,Critical;
OptionCaption = 'Low,Medium,High';
}
}
}

18
microsoft/knowledge/style/optioncaption-required-and-matches-membercount.good.al
View file

@ -1,18 +0,0 @@
table 50254 "Sample OptionCaption Good"
{
fields
{
field(1; Status; Option)
{
Caption = 'Status';
OptionMembers = Open,Released,Pending;
OptionCaption = 'Open,Released,Pending';
}
field(2; Priority; Option)
{
Caption = 'Priority';
OptionMembers = Low,Medium,High,Critical;
OptionCaption = 'Low,Medium,High,Critical';
}
}
}

26
microsoft/knowledge/style/optioncaption-required-and-matches-membercount.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [optioncaption, option, member-count, aa0221, aa0223, aa0224]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Option fields need `OptionCaption`, and its element count must match `OptionMembers` (CodeCop AA0221/AA0223/AA0224)
## Description
CodeCop AA0221 requires an `OptionCaption` on every option-type field that is not sourced from a table column (table-sourced option fields inherit the captions of the underlying field). AA0223 and AA0224 add two integrity checks: the number of comma-separated entries in `OptionCaption` must equal the number of entries in `OptionMembers`, and each caption must align by position with its member. The position alignment is what the platform uses to translate option values — the `OptionMembers` list never changes per locale, the `OptionCaption` list does. A mismatch in count or order produces silent corruption: the option `Released` shows the caption that belongs to `Pending`, and the bug is locale-dependent.
## Best Practice
`OptionMembers = Open,Released,Pending;` and `OptionCaption = 'Open,Released,Pending';` — same count, same order. When adding a new member, update both lines in the same commit.
See sample: [`optioncaption-required-and-matches-membercount.good.al`](optioncaption-required-and-matches-membercount.good.al).
## Anti Pattern
`OptionMembers = Open,Released,Pending;` with no `OptionCaption` at all (the user sees the raw English members and translation is impossible), or `OptionMembers = Low,Medium,High,Critical;` paired with `OptionCaption = 'Low,Medium,High';` — count mismatch, `Critical` displays as blank or carries the wrong caption depending on platform version.
See sample: [`optioncaption-required-and-matches-membercount.bad.al`](optioncaption-required-and-matches-membercount.bad.al).

11
microsoft/knowledge/style/single-space-after-not-operator.bad.al
View file

@ -1,11 +0,0 @@
codeunit 50233 "Sample Not Spacing Bad"
{
procedure Check(): Boolean
var
Customer: Record Customer;
begin
if NOT Customer.IsEmpty() then
exit(true);
exit(false);
end;
}

11
microsoft/knowledge/style/single-space-after-not-operator.good.al
View file

@ -1,11 +0,0 @@
codeunit 50232 "Sample Not Spacing Good"
{
procedure Check(): Boolean
var
Customer: Record Customer;
begin
if not Customer.IsEmpty() then
exit(true);
exit(false);
end;
}

26
microsoft/knowledge/style/single-space-after-not-operator.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [spacing, not, operator, aa0003, codecop]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Exactly one space between `not` and its argument (CodeCop AA0003)
## Description
CodeCop AA0003 requires exactly one space between the `not` operator and the expression it negates. `if not Customer.FindFirst() then …` is correct; `if not Customer.FindFirst() then …` (two spaces) and `if notCustomer.FindFirst() then …` (zero — which fails parsing anyway) are not. The rule is also the place where uppercase `NOT` is flagged in combination with CodeCop AA0241 (reserved keywords must be lowercase): `if NOT Condition then` is doubly wrong.
## Best Practice
`if not Condition then`, `if not Customer.IsEmpty() then`, `exit(not Result)`. One space, lowercase keyword, no parentheses around the bare boolean.
See sample: [`single-space-after-not-operator.good.al`](single-space-after-not-operator.good.al).
## Anti Pattern
`if NOT condition then`, `if not condition then`, `if !condition then` (which is not even AL — `!` is not a negation operator in AL). All three either trip AA0003 / AA0241 or fail to compile.
See sample: [`single-space-after-not-operator.bad.al`](single-space-after-not-operator.bad.al).

12
microsoft/knowledge/style/single-space-around-binary-operators.bad.al
View file

@ -1,12 +0,0 @@
codeunit 50229 "Sample Spaces Op Bad"
{
procedure Compute(Amount: Decimal; Quantity: Decimal): Decimal
var
Price: Decimal;
begin
Price:=Amount*Quantity;
if (Amount>0)and(Quantity>0) then
exit(Price);
exit(0);
end;
}

12
microsoft/knowledge/style/single-space-around-binary-operators.good.al
View file

@ -1,12 +0,0 @@
codeunit 50228 "Sample Spaces Op Good"
{
procedure Compute(Amount: Decimal; Quantity: Decimal): Decimal
var
Price: Decimal;
begin
Price := Amount * Quantity;
if (Amount > 0) and (Quantity > 0) then
exit(Price);
exit(0);
end;
}

26
microsoft/knowledge/style/single-space-around-binary-operators.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [spacing, binary-operator, aa0001, codecop, formatting]
technologies: [al]
countries: [w1]
application-area: [all]
---
# One space on each side of every binary operator (CodeCop AA0001)
## Description
CodeCop AA0001 requires exactly one space on each side of every binary operator: assignment (`:=`), arithmetic (`+`, `-`, `*`, `/`, `mod`, `div`), comparison (`=`, `<>`, `<`, `<=`, `>`, `>=`), logical (`and`, `or`, `xor`), and string concatenation. `x:=1+2`, `Price:=Amount*Quantity`, `if a=b then`, and `if a and b then` all violate the rule. The rule applies to the binary use of `-` (subtraction); the unary minus (`-Profit`) takes no leading space.
## Best Practice
Write `x := 1 + 2`, `Price := Amount * Quantity`, `if a = b then`, `if a and b then`. The standard AL formatter inserts these spaces automatically; running `Alt+Shift+F` (Format Document) in the AL extension is the simplest way to bring an entire file into compliance.
See sample: [`single-space-around-binary-operators.good.al`](single-space-around-binary-operators.good.al).
## Anti Pattern
`x:=1+2;`, `Price:=Amount*Quantity;`, `if a=b then`, `if a and b then`. All trip AA0001.
See sample: [`single-space-around-binary-operators.bad.al`](single-space-around-binary-operators.bad.al).

14
microsoft/knowledge/style/this-keyword-in-codeunits.bad.al
View file

@ -1,14 +0,0 @@
codeunit 50215 "Sample This Bad"
{
procedure ProcessRecord(Customer: Record Customer)
var
Helper: Codeunit "Sample This Helper";
begin
ValidateCustomer(Customer);
Helper.DoWork();
end;
local procedure ValidateCustomer(Customer: Record Customer)
begin
end;
}

14
microsoft/knowledge/style/this-keyword-in-codeunits.good.al
View file

@ -1,14 +0,0 @@
codeunit 50214 "Sample This Good"
{
procedure ProcessRecord(Customer: Record Customer)
var
Helper: Codeunit "Sample This Helper";
begin
this.ValidateCustomer(Customer);
Helper.DoWork(this);
end;
local procedure ValidateCustomer(Customer: Record Customer)
begin
end;
}

26
microsoft/knowledge/style/this-keyword-in-codeunits.md
View file

@ -1,26 +0,0 @@
---
bc-version: [25..]
domain: style
keywords: [this, codeunit, self-reference, aa0248, scope]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Use the `this` keyword for self-reference inside codeunits (CodeCop AA0248)
## Description
CodeCop AA0248 recommends prefixing self-references inside a codeunit with `this`. `this.ValidateCustomer(Customer)` is unambiguous: the call resolves to a procedure on the current codeunit, not to a local variable or a procedure on a passed-in object. Without the prefix, a reader of a 200-line procedure has to scan the whole codeunit to confirm whether `ValidateCustomer` is local. `this` also makes it possible to pass the current codeunit as an argument — `SomeOtherCodeunit.DoWork(this)` — which is the only way to expose the running codeunit instance to a collaborator. The rule applies only to codeunits, not to pages, reports, queries, or tables — those object types do not have a `this` reference in AL.
## Best Practice
Inside a codeunit, prefix calls to procedures and accesses to global variables on the same codeunit with `this.`, and pass `this` when an external codeunit needs a reference to the running instance.
See sample: [`this-keyword-in-codeunits.good.al`](this-keyword-in-codeunits.good.al).
## Anti Pattern
Calling a codeunit-local procedure as a bare identifier (`ValidateCustomer(Customer)`) when other readings are possible. The ambiguity costs reading time on every encounter and grows with codeunit size.
See sample: [`this-keyword-in-codeunits.bad.al`](this-keyword-in-codeunits.bad.al).

13
microsoft/knowledge/style/variable-declaration-order-by-type.bad.al
View file

@ -1,13 +0,0 @@
codeunit 50247 "Sample Var Order Bad"
{
procedure Run()
var
CustomerNo: Code[20];
TempBuffer: Record "Integer" temporary;
Amount: Decimal;
Customer: Record Customer;
IsValid: Boolean;
begin
IsValid := Customer.Get(CustomerNo);
end;
}

13
microsoft/knowledge/style/variable-declaration-order-by-type.good.al
View file

@ -1,13 +0,0 @@
codeunit 50246 "Sample Var Order Good"
{
procedure Run()
var
Customer: Record Customer;
TempBuffer: Record "Integer" temporary;
CustomerNo: Code[20];
Amount: Decimal;
IsValid: Boolean;
begin
IsValid := Customer.Get(CustomerNo);
end;
}

26
microsoft/knowledge/style/variable-declaration-order-by-type.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [variable-declaration, order, var, complex-types, aa0021]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Order variable declarations by type, complex types first (CodeCop AA0021)
## Description
CodeCop AA0021 requires that variable declarations inside a `var` block follow a fixed ordering by type, with complex (composite) types appearing before primitive types. The canonical order is `Record`, then `Report`, `Codeunit`, `XmlPort`, `Page`, `Query`, `Notification`, `BigText`, `DateFormula`, `RecordId`, `RecordRef`, `FieldRef`, `FilterPageBuilder`, then the simple types `Text`, `Code`, `Integer`, `Decimal`, `Boolean`, `Date`, `Time`, `DateTime`, `Char`, `Byte`. Inside each type group the variables can be alphabetical or in usage order. Temporary records still sort under `Record`.
## Best Practice
Declare all `Record` variables first, then other complex types, then primitives. A consistent order makes diffs review-friendly and matches the convention enforced by the AL formatter and CodeCop.
See sample: [`variable-declaration-order-by-type.good.al`](variable-declaration-order-by-type.good.al).
## Anti Pattern
A `var` block where records and primitives are interleaved — `CustomerNo: Code[20];` between two `Record` variables, or `Amount: Decimal;` declared above the `Customer: Record Customer;` it is computed from. AA0021 flags it and the block is harder to scan; readers expect composite types at the top.
See sample: [`variable-declaration-order-by-type.bad.al`](variable-declaration-order-by-type.bad.al).

19
microsoft/knowledge/style/variable-name-must-not-shadow.bad.al
View file

@ -1,19 +0,0 @@
codeunit 50249 "Sample Shadow Bad"
{
var
Customer: Record Customer;
procedure ProcessSales()
var
Customer: Text;
Amount: Decimal;
begin
Customer := 'C-100';
Amount := 0;
end;
procedure Amount(): Decimal
begin
exit(0);
end;
}

19
microsoft/knowledge/style/variable-name-must-not-shadow.good.al
View file

@ -1,19 +0,0 @@
codeunit 50248 "Sample No Shadow Good"
{
var
CustomerRec: Record Customer;
procedure ProcessSales()
var
CustomerName: Text;
SalesAmount: Decimal;
begin
CustomerName := CustomerRec.Name;
SalesAmount := GetAmount();
end;
procedure GetAmount(): Decimal
begin
exit(0);
end;
}

26
microsoft/knowledge/style/variable-name-must-not-shadow.md
View file

@ -1,26 +0,0 @@
---
bc-version: [all]
domain: style
keywords: [variable-name, shadow, conflict, aa0198, aa0202, aa0204, codecop]
technologies: [al]
countries: [w1]
application-area: [all]
---
# Local variable names must not shadow globals, fields, methods, or actions (CodeCop AA0198/AA0202/AA0204)
## Description
Three CodeCop rules — AA0198, AA0202, AA0204 — together forbid a local variable from sharing a name with a global variable on the same object, with a field on the same table or page source, with a procedure on the same object, or with an action on the same page. The compiler resolves the conflict by binding the closer scope, so a local `Customer: Text` will silently override a global `Customer: Record Customer` for the duration of a procedure — every call site reading `Customer.Name` from inside that procedure refers to the text, and the breakage is invisible to a reader who has both declarations on screen.
## Best Practice
Differentiate every local declaration from globals, fields, procedures, and actions on the same object. `Customer` global plus `CustomerName` local; method `GetAmount` plus local `SalesAmount`. The standard pattern is to attach a noun suffix to the local (`CustomerName`, `CustomerRec`, `CustomerNo`) rather than to the global.
See sample: [`variable-name-must-not-shadow.good.al`](variable-name-must-not-shadow.good.al).
## Anti Pattern
A procedure that declares a local `Customer: Text` inside a codeunit that already has a global `Customer: Record Customer`. The local wins and the global becomes unreachable inside the procedure. AA0198/AA0202/AA0204 flag this category of conflict whether the colliding entity is a global, a field, a method, or an action.
See sample: [`variable-name-must-not-shadow.bad.al`](variable-name-must-not-shadow.bad.al).

29
microsoft/skills/review/al-appsource-review.md
View file

@ -16,7 +16,7 @@ application-area: [all]
Reviews AL source and app metadata changes against the `appsource` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. AppSource findings are narrow by design — they apply when the review scope contains AppSourceCop configuration, AL object or extension-member names, or AppSource-facing `app.json` metadata. The skill returns `not-applicable` when none of those apply.
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. AppSource findings are narrow by design — they apply to AppSource-facing metadata and complete permission coverage that requires repository context. Mechanical AppSourceCop diagnostics are intentionally outside this skill. The skill returns `not-applicable` when none of those surfaces apply.
## Source
@ -37,19 +37,14 @@ Discard files that are not applicable. Retain conditionally applicable files (an
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
- The changed files and AL object types — especially `app.json`, `AppSourceCop.json`, namespace declarations, permission-set objects, new objects, and table/page/report extensions that add fields, keys, controls, or actions to base objects.
- The changed object and member names, weighted toward prefix/suffix consistency with `mandatoryAffixes` or `mandatoryPrefix`, plus AppSource-facing help metadata.
- Tokens extracted from the diff that relate to AppSource (`AppSourceCop`, `mandatoryAffixes`, `mandatoryPrefix`, `AS0011`, `prefix`, `suffix`, `namespace`, `using`, `permissionset`, `Assignable`, `Permissions`, `SUPER`, `tableextension`, `pageextension`, `reportextension`, `field`, `key`, `control`, `action`, `app.json`, `help`, `ContextSensitiveHelpPage`, `Copilot`, `https`).
- The changed files and AL object types — especially `app.json`, permission-set objects, setup and usage entry points, and AppSource-facing help metadata.
- Tokens extracted from the diff that relate to AppSource (`permissionset`, `Assignable`, `Permissions`, `SUPER`, `tabledata`, `execute`, `app.json`, `help`, `ContextSensitiveHelpPage`, `Copilot`, `https`).
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone. When the diff contains no AppSource-related source or metadata changes by any of the above signals, return `outcome: "not-applicable"` without evaluating files.
The following targeted checks cover every current `appsource` article across the Microsoft and community layers. Treat each as a candidate-selection cue: when the signal appears in changed code, add the named article to the worklist and evaluate it in Action.
- Select exactly one naming-collision owner. When no namespace declaration is present, a new/renamed object lacks the reserved prefix/suffix, or an extension object adds an unaffixed member to a base object — `object-affixes-prevent-collisions`.
- For BC23 or later, use `two-level-namespace-replaces-object-affix-not-extension-member-affix` instead when the changed source actually declares or changes a namespace and relies on it as the owned-object affix alternative, but has fewer than two levels or incorrectly applies that exception to members on another publisher's object. Never worklist this article for an unaffixed source file with no namespace declaration.
- The app has no assignable permission set covering its setup and usage paths, omits visible object/tabledata grants, or requires `SUPER` for normal operation — `permission-sets-cover-setup-and-usage-without-super`. Require repository-level app context; one isolated permission-set object cannot prove complete coverage.
Before emitting an affix finding, compare every owned object name and every member added to another publisher's object against the configured `mandatoryAffixes`/`mandatoryPrefix`. A matching prefix or suffix is compliant. Do not flag an `ABC`-prefixed object or an `ABC`-suffixed extension member when `ABC` is the configured affix.
- For BC v27 or later, `app.json` adds or changes the `help` URL to a path deeper than two levels, or a changed Copilot/context-sensitive help arrangement would ground the app under an overly broad truncated parent — `keep-copilot-help-url-to-two-path-levels`.
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
@ -66,13 +61,13 @@ For each worklist entry, evaluate the diff against the file's `## Best Practice`
Set `confidence` to:
- `high` when the detection is based on an unambiguous pattern match (affix configuration/name or URL path depth).
- `high` when the detection is based on an unambiguous pattern match such as URL path depth.
- `medium` when detection relies on heuristics or when any frontmatter dimension was `unknown`.
- `low` when the finding is an advisory derived only from applicability.
After evaluating each worklist entry, also consider whether the diff exhibits an AppSource defect the agent recognises from its general AL knowledge that no knowledge file in the worklist covers. Such candidates are agent findings within this skill's domain — emit them with `references: []`, an `id` slug prefixed with `agent:`, `confidence` capped at `medium`, `severity` capped at `minor` (agent findings are advisory and non-gating), and a `message` that is self-contained (describing both the issue and a concrete recommendation, since there is no knowledge-file footer for the consumer to fall back on). Hold every candidate to the precision bar in `skills/do.md` (*Agent findings*): emit only a concrete, material AppSource defect a knowledgeable BC reviewer would agree is wrong — steelman it first and drop anything stylistic, speculative, dependent on code outside the diff, or merely a valid alternative; when in doubt, omit. The scope is strictly AppSource; defects outside this domain belong to other leaves and MUST NOT be emitted here. Before emitting, check the worklist for a knowledge file that matches the candidate — if one exists, upgrade the candidate to a knowledge-backed finding instead. See `skills/do.md` for the full contract.
For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: add the configured affix to one object or extension member, or replace a deep help URL with a known two-level canonical URL). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example, replacing a deep help URL with a known two-level canonical URL). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
Omit `suggested-code` only when the appropriate fix depends on context the skill cannot determine, when multiple defensible replacements exist, or when the fix spans non-contiguous code. If a finding is mechanical-looking but you omit `suggested-code`, set `findings[].suggested-code-omission-reason` to a short explanation. See `skills/do.md` for the full contract.
@ -80,7 +75,7 @@ Outcome selection:
- `completed` — the skill evaluated every worklist item.
- `no-knowledge` — no applicable AppSource knowledge survived filtering.
- `not-applicable` — the diff touches no AppSource source, analyzer configuration, or app-metadata surface.
- `not-applicable` — the diff touches no AppSource permission or app-metadata surface.
- `partial` — a budget was hit before the worklist was exhausted.
- `failed` — an unrecoverable error occurred.
@ -98,19 +93,19 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s
},
"findings": [
{
"id": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md",
"id": "microsoft/knowledge/appsource/keep-copilot-help-url-to-two-path-levels.md",
"severity": "major",
"message": "The tableextension adds an unaffixed Loyalty Points field to Customer, so it violates the configured AppSource affix and can collide with another extension.",
"message": "The app help URL is deeper than two path levels, so Copilot truncates it and may ground answers on unrelated sibling documentation.",
"location": {
"file": "src/CustomerExt.TableExt.al",
"line": 8
"file": "app.json",
"line": 12
},
"references": [
{ "path": "microsoft/knowledge/appsource/object-affixes-prevent-collisions.md" }
{ "path": "microsoft/knowledge/appsource/keep-copilot-help-url-to-two-path-levels.md" }
],
"confidence": "high",
"domain": "AppSource",
"suggested-code": "field(50100; \"Loyalty Points ABC\"; Integer)"
"suggested-code": "\"help\": \"https://contoso.com/docs/myapp\""
}
],
"suppressed": []

5
microsoft/skills/review/al-breaking-changes-review.md
View file

@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
- The changed AL object names and types — especially codeunits, tables, and table extensions that expose procedures, fields, or events to other apps, and any member whose access is being widened.
- The changed procedures, fields, and triggers, weighted toward non-`local` procedures, published table fields, event publishers, and any member whose signature, access modifier, or obsolete state is being altered.
- Tokens extracted from the diff that relate to API stability and deprecation (`signature`, `parameter`, `return`, `var`, `Obsolete`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `Pending`, `Removed`, `CLEAN`, `SecretText`, `token`, `internal`, `local`, `public`, `protected`, `Scope`, `namespace`, `using`, `AS0007`).
- Tokens extracted from the diff that relate to API stability and deprecation (`signature`, `parameter`, `return`, `var`, `Obsolete`, `ObsoleteState`, `ObsoleteReason`, `ObsoleteTag`, `Pending`, `Removed`, `CLEAN`, `SecretText`, `token`, `internal`, `local`, `public`, `protected`, `Scope`).
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
@ -50,8 +50,7 @@ The following targeted checks cover every current `breaking-changes` article:
- A published procedure changes parameter count/order/type/name, `var`, return type, or array shape instead of preserving the old signature and adding an overload — `do-not-change-published-procedure-signatures`.
- A public procedure/event/interface exposes a credential or other sensitive value through `Text` or an externally callable contract — `do-not-expose-sensitive-data-through-public-api`.
- Code already marked obsolete is expanded with new behavior instead of routing new callers to its replacement — `do-not-modify-code-already-marked-obsolete`.
- A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`. This owns AS0005 field-name changes; do not substitute the namespace article.
- A published object's namespace changes between the base and changed source while its identity otherwise remains — `namespace-is-part-of-published-object-identity`. Do not apply it to a new, unshipped object or to an ordinary object-name change with no namespace change.
- A shipped table field is deleted, renamed, renumbered, or replaced without retaining the original field as `ObsoleteState = Pending` and migrating its data — `obsolete-table-fields-instead-of-deleting-them`.
For `obsolete-table-fields-instead-of-deleting-them`, compare the baseline ID and name before emitting. When the original field remains under the same ID and name with `ObsoleteState = Pending`, and the replacement uses a new ID, the change follows the rule and must not be flagged.

22
microsoft/skills/review/al-style-review.md
View file

@ -16,7 +16,7 @@ application-area: [all]
Reviews AL source changes against the `style` knowledge domain in BCQuality and emits a findings report. This is a leaf action skill: it invokes no sub-skills. It is one of the skills composed by `al-code-review`.
Style findings cover AL conventions that CodeCop and similar analyzers partially enforce — label suffixes, API page naming, temporary-variable prefixes, label properties, named invocations, `FieldCaption`/`TableCaption` in user messages, `OptionCaption` pairing, Error-parameter passing, `this` keyword, required parentheses, file-naming. Use together with a formal analyzer; this skill adds BCQuality's remedial-knowledge explanations of why each rule exists.
Style findings cover AL conventions that require contextual judgment — API page naming, temporary-variable prefixes, label semantics, named invocations, `FieldCaption`/`TableCaption` in user messages, error-parameter handling, and file naming. Mechanical compiler and analyzer rules are intentionally outside this skill; run the consuming app's configured analyzers separately.
An orchestrator invokes this skill with a `pr-diff`, `file-path`, or `folder-path`. The skill produces a single JSON document conforming to the DO output contract.
@ -40,8 +40,8 @@ Discard files that are not applicable. Retain conditionally applicable files onl
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
- Changed AL objects — especially API pages (`PageType = API`), tables and pages declaring Labels/TextConsts, codeunits issuing `Error`/`Message`/`Confirm`, and any file whose name violates the `<ObjectName>.<ObjectType>.al` convention.
- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, option fields, error-handling call sites, and codeunit-internal method calls.
- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `OptionMembers`, `OptionCaption`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `this.`, `StrSubstNo`).
- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, error-handling call sites, and API declarations.
- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `APIPublisher`, `APIGroup`, `APIVersion`, `EntityName`, `EntitySetName`, `DelayedInsert`, `FieldCaption`, `TableCaption`, `FieldName`, `TableName`, `Page.RunModal`, `Report.Run`, `StrSubstNo`).
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object or declaration. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
@ -50,8 +50,6 @@ Do not worklist `temporary-variable-temp-prefix.md` for an event publisher param
Apply these high-signal mappings before fuzzy topic ranking:
- A `Label` or `TextConst` contains multiple or ambiguous placeholders but has no `Comment`, or its Comment does not explain every placeholder — `label-comment-explains-placeholders.md`. A single placeholder whose meaning is explicit in the text, such as `Customer %1`, is allowed without a Comment and must not be flagged.
- `function-call-parentheses-required.md` applies only to a zero-argument invocation written without `()`. Never worklist it from an invocation that already has parentheses or supplies arguments, including `Error(Label, Arg1, Arg2)`.
Once the candidate worklist is known, resolve layer-precedence conflicts per READ and record suppressions.
When the post-conflict worklist is empty because no applicable style knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable style knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
@ -60,7 +58,7 @@ When the post-conflict worklist is empty because no applicable style knowledge e
For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Style findings rarely reach `blocker` — reserve it for cases where the knowledge file documents a platform-level requirement (for example, API page property constraints the OData runtime rejects). Most style findings are `minor` or `info`; egregious misuse (`Error` with pre-built Text losing translation and telemetry classification) may reach `major`.
Severity calibration — a formal analyzer already flags the mechanical presence/naming conventions (the `this` keyword AA0248, approved label suffixes AA0074, variable-declaration order by type AA0021, a missing `ToolTip`, required parentheses). On those, BCQuality's value is the *explanation* of why the rule exists, not a second gate; emit them at `info` so a consumer that gates on severity does not re-flag what CodeCop/AppSourceCop already reports. Reserve `minor` for style issues with concrete downstream impact the analyzer does not catch — lost translation or telemetry classification from a string-built `Error`, an `OptionCaption` that does not match its `OptionMembers`, or a misleading named invocation. A procedure-local `Label` is valid and is not a correctness or localization finding; an explicit repository preference for object scope is at most low-severity maintainability guidance. This keeps the domain's default output advisory and prevents analyzer-redundant noise from competing with substantive review.
Severity calibration — reserve `minor` for style issues with concrete downstream impact that deterministic tooling does not establish, such as lost translation or telemetry classification from a string-built `Error` or a misleading named invocation. A procedure-local `Label` is valid and is not a correctness or localization finding; an explicit repository preference for object scope is at most low-severity maintainability guidance. Do not rediscover or report mechanical compiler or analyzer diagnostics, even at `info`.
Set `confidence` to:
@ -70,7 +68,7 @@ Set `confidence` to:
After evaluating each worklist entry, also consider whether the diff exhibits a style defect the agent recognises from its general AL knowledge that no knowledge file in the worklist covers. Such candidates are agent findings within this skill's domain — emit them with `references: []`, an `id` slug prefixed with `agent:`, `confidence` capped at `medium`, `severity` capped at `minor` (agent findings are advisory and non-gating), and a `message` that is self-contained (describing both the issue and a concrete recommendation, since there is no knowledge-file footer for the consumer to fall back on). Hold every candidate to the precision bar in `skills/do.md` (*Agent findings*): emit only a clear, widely-accepted AL style violation with a concrete basis a knowledgeable BC reviewer would agree on — steelman it first and drop personal preference, speculation, and any single defensible formatting choice among several; when in doubt, omit. The scope is strictly style — naming, labelling, formatting, and analyzer-adjacent conventions. A correctness, logic, data-integrity, or contract defect is NOT a style finding even when it can be reworded as a convention: a method that mutates a shared `Record`'s filters, an unfiltered `DeleteAll`, a violated interface contract, or a wrong boolean guard are behavioural defects, not conventions — do not emit them here under a style framing. If a specific domain leaf covers the concern (performance, security, error-handling, …) it belongs there; if no knowledge file in any domain covers it, it belongs to the `al-code-review` super-skill's cross-cutting self-review agent channel (`from-sub-skill: "agent"`, `severity` capped at `minor`), not to this leaf. A reliable test: if you cannot cite a style `## Best Practice`/`## Anti Pattern` for the concern, it is very likely not a style finding. Before emitting, check the worklist for a knowledge file that matches the candidate — if one exists, upgrade the candidate to a knowledge-backed finding instead. See `skills/do.md` for the full contract.
For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: delete unreachable lines; replace `Count() > 0` with `not IsEmpty()`; add a missing `ToolTip`, `OptionCaption`, or `DataClassification`; replace a string-concatenated `Error` with a Label-backed call; change an over-broad permission token; or add an obvious `else`/guard branch). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
For every emitted finding, decide whether the fix is mechanical. A fix is mechanical when it is small, local, and unambiguous from the diff context (for example: add a missing contextual `ToolTip`, replace a string-concatenated `Error` with a Label-backed call, or correct an API naming property whose intended value is clear). For mechanical findings, emit `findings[].suggested-code` with the literal replacement for the source lines indicated by `location`. The payload must be a verbatim replacement — no diff markers, no fences, no commentary — that the consumer can render as a one-click suggestion. When a `.good.al` companion exists and the diff context matches the `.bad.al` shape, adapt the `.good.al` replacement into `suggested-code`.
Omit `suggested-code` only when the appropriate fix depends on context the skill cannot determine, when multiple defensible replacements exist, or when the fix spans non-contiguous code. If a finding is mechanical-looking but you omit `suggested-code`, set `findings[].suggested-code-omission-reason` to a short explanation. See `skills/do.md` for the full contract.
@ -91,20 +89,20 @@ Output conforms to the DO output contract. Every finding this skill emits MUST s
"skill": { "id": "al-style-review", "version": 1 },
"outcome": "completed",
"summary": {
"counts": { "blocker": 0, "major": 0, "minor": 0, "info": 1 },
"counts": { "blocker": 0, "major": 0, "minor": 1, "info": 0 },
"coverage": { "worklist-size": 1, "items-evaluated": 1 }
},
"findings": [
{
"id": "microsoft/knowledge/style/label-suffix-approved-list.md",
"severity": "info",
"message": "A Label named Text000 has no approved suffix (Msg/Err/Qst/Tok/Lbl/Txt). Per the referenced CodeCop AA0074 guidance, every Label and TextConst carries a suffix indicating its consuming call.",
"id": "microsoft/knowledge/style/label-comment-explains-placeholders.md",
"severity": "minor",
"message": "The label has two ambiguous placeholders but no Comment explaining what each value represents to translators.",
"location": {
"file": "src/Sales/PostingRoutines.Codeunit.al",
"line": 42
},
"references": [
{ "path": "microsoft/knowledge/style/label-suffix-approved-list.md" }
{ "path": "microsoft/knowledge/style/label-comment-explains-placeholders.md" }
],
"confidence": "high",
"domain": "Style"