mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
Merge analyzer-policy updates from main
Accept main's removal of deterministic compiler and analyzer duplicates, including the ApplicationArea rule, while retaining the read-only development guidance contract and non-mechanical Learn knowledge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 638b66d2-9f06-4f60-8781-808709e1485c
This commit is contained in:
Jesper Schulz-Wedde
commit
d608d89cf0
60 changed files with 45 additions and 1126 deletions
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -1,9 +0,0 @@
|
|||
// This published object previously used namespace Contoso.Rentals.
|
||||
namespace Contoso.RentalManagement;
|
||||
|
||||
codeunit 50467 "Rental Agreement Mgt."
|
||||
{
|
||||
procedure CreateAgreement()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -1,8 +0,0 @@
|
|||
namespace Contoso.Rentals;
|
||||
|
||||
codeunit 50466 "Rental Agreement Mgt."
|
||||
{
|
||||
procedure CreateAgreement()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -1,37 +0,0 @@
|
|||
page 50375 "Sample App Area Bad"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(General)
|
||||
{
|
||||
field("No."; Rec."No.")
|
||||
{
|
||||
ToolTip = 'Specifies the number that identifies the customer.';
|
||||
}
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
ToolTip = 'Specifies the customer''s name.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pageextension 50377 "Customer App Area Bad" extends "Customer Card"
|
||||
{
|
||||
layout
|
||||
{
|
||||
addlast(General)
|
||||
{
|
||||
// Extension controls do not inherit ApplicationArea from the base page.
|
||||
field("Language Code Sample"; Rec."Language Code")
|
||||
{
|
||||
ToolTip = 'Specifies the language used for the customer.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,54 +0,0 @@
|
|||
page 50374 "Sample App Area Good"
|
||||
{
|
||||
PageType = Card;
|
||||
SourceTable = Customer;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(Content)
|
||||
{
|
||||
group(General)
|
||||
{
|
||||
field("No."; Rec."No.")
|
||||
{
|
||||
ToolTip = 'Specifies the number that identifies the customer.';
|
||||
}
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
ToolTip = 'Specifies the customer''s name.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
actions
|
||||
{
|
||||
area(Processing)
|
||||
{
|
||||
action(Refresh)
|
||||
{
|
||||
ToolTip = 'Reloads the current record.';
|
||||
|
||||
trigger OnAction()
|
||||
begin
|
||||
CurrPage.Update(false);
|
||||
end;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pageextension 50376 "Customer App Area Good" extends "Customer Card"
|
||||
{
|
||||
layout
|
||||
{
|
||||
addlast(General)
|
||||
{
|
||||
field("Language Code Sample"; Rec."Language Code")
|
||||
{
|
||||
ApplicationArea = All;
|
||||
ToolTip = 'Specifies the language used for the customer.';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,32 +0,0 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: style
|
||||
keywords: [application-area, page-control, inheritance, as0062, appsourcecop, web-client]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Page-level `ApplicationArea` inheritance does not apply to extensions
|
||||
|
||||
## Description
|
||||
|
||||
A page control or action needs an effective `ApplicationArea` to appear in cloud experiences. From runtime 10.0, controls on a page object inherit the page-level value, so repeating it on every child is unnecessary when the parent defines a suitable default. This inheritance does not apply to controls added or modified by page and report extensions: extension controls must still set the property explicitly.
|
||||
|
||||
For targets before runtime 10.0, child controls do not inherit and must also set the property. AppSourceCop AS0062 and PTE0008 account for page-level inheritance on runtime 10.0 and later but continue to require explicit values in extensions.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On runtime 10.0 or later, set a suitable page-level default and override only controls that belong to a narrower area. Set `ApplicationArea` explicitly on every control or action introduced by a page or report extension.
|
||||
|
||||
See sample: [`applicationarea-required-on-page-controls.good.al`](applicationarea-required-on-page-controls.good.al).
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A page object that defines neither a parent nor child value, or an extension control that assumes it inherits from the base page. The control has no effective application area and can be hidden or rejected by analyzer validation.
|
||||
|
||||
See sample: [`applicationarea-required-on-page-controls.bad.al`](applicationarea-required-on-page-controls.bad.al).
|
||||
|
||||
## Reference
|
||||
|
||||
[Set different control properties](https://learn.microsoft.com/en-us/training/modules/work-with-pages/8-controls)
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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.
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
|
@ -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).
|
||||
Loading…
Add table
Add a link
Reference in a new issue