Clarify locale-safe DateFormula Evaluate inputs (#193)

* Clarify locale-safe DateFormula Evaluate inputs

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Normalize DateFormula article sections

Keep the analyzer-gap explanation in Description and its scoped probe evidence in References, without a novel Validation section. Normative guidance and fixtures are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

evaluation/review-fixtures.json

@@ -52,7 +52,10 @@
       ]
     },
     "style": {
-      "article": "label-comment-explains-placeholders"
+      "articles": [
+        "label-comment-explains-placeholders",
+        "dateformula-evaluate-needs-language-independent-literals"
+      ]
     },
     "telemetry": {
       "article": "telemetry-event-id-stable-unique"

microsoft/knowledge/style/dateformula-evaluate-needs-language-independent-literals.bad.al

@@ -0,0 +1,18 @@
+codeunit 50102 "Bad Review Scheduling"
+{
+    procedure NextReviewDate(Interval: DateFormula; ReferenceDate: Date): Date
+    begin
+        if Format(Interval) = '' then
+            Evaluate(Interval, '1W');
+
+        exit(CalcDate(Interval, ReferenceDate));
+    end;
+
+    procedure NextReviewFromUserInput(UserFormulaText: Text; ReferenceDate: Date): Date
+    var
+        Interval: DateFormula;
+    begin
+        Evaluate(Interval, UserFormulaText);
+        exit(NextReviewDate(Interval, ReferenceDate));
+    end;
+}

microsoft/knowledge/style/dateformula-evaluate-needs-language-independent-literals.good.al

@@ -0,0 +1,18 @@
+codeunit 50101 "Good Review Scheduling"
+{
+    procedure NextReviewDate(Interval: DateFormula; ReferenceDate: Date): Date
+    begin
+        if Format(Interval) = '' then
+            Evaluate(Interval, '<1W>');
+
+        exit(CalcDate(Interval, ReferenceDate));
+    end;
+
+    procedure NextReviewFromUserInput(UserFormulaText: Text; ReferenceDate: Date): Date
+    var
+        Interval: DateFormula;
+    begin
+        Evaluate(Interval, UserFormulaText);
+        exit(NextReviewDate(Interval, ReferenceDate));
+    end;
+}

microsoft/knowledge/style/dateformula-evaluate-needs-language-independent-literals.md

@@ -0,0 +1,38 @@
+---
+bc-version: [all]
+domain: style
+keywords: [dateformula, evaluate, calcdate, date-expression, language-independent, multilanguage, global-language]
+technologies: [al]
+countries: [w1]
+application-area: [all]
+---
+
+# Parse DateFormula constants with language-independent input
+
+## Description
+
+A `DateFormula` stores a formula in a language-independent representation, but `Evaluate` must first interpret its text input. Declaring the destination as `DateFormula` does not make an English literal such as `1W` independent of the session language: French uses `S` for weeks. Passing the resulting typed variable to `CalcDate` satisfies that call's CodeCop AA0462 argument requirement, but cannot repair a parsing failure that already happened in `Evaluate`.
+
+## Best Practice
+
+For an application-defined formula in a normal two-argument `Evaluate` call, use the generic units inside angle brackets, such as `<1W>`. Apply this at the text-to-`DateFormula` boundary, including a visible constant passed through a helper. A label's `Locked = true` prevents translation of its text; it does not make unbracketed English units language independent.
+
+Preserve genuinely localized input: text entered by the user, or already formatted for the same session language, should be parsed in that language. Do not blindly wrap that text in angle brackets. Already invariant `<...>` literals, explicit import-format conversions, a typed formula passed to `CalcDate`, and `Format(Interval) = ''` checks are not findings without an unsafe constant at the parsing boundary.
+
+See sample: [`dateformula-evaluate-needs-language-independent-literals.good.al`](dateformula-evaluate-needs-language-independent-literals.good.al).
+
+## Anti Pattern
+
+A hard-coded, language-fixed formula such as `1W` flows into a normal two-argument `Evaluate` whose destination is known to be `DateFormula`, and the application expects that default to work across session languages. Require the destination type and constant provenance; an arbitrary `Evaluate` call or dynamic text parameter is not enough. The resulting code can compile and work in English while failing when the same default is first needed in another language.
+
+Do not report direct `CalcDate` text arguments under this article: CodeCop AA0462 already owns the requirement for a typed formula or angle-bracketed text there. Its typed-argument check does not establish that an earlier `Evaluate` parsed language-independent input.
+
+See sample: [`dateformula-evaluate-needs-language-independent-literals.bad.al`](dateformula-evaluate-needs-language-independent-literals.bad.al).
+
+## References
+
+[DateFormula data type](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/dateformula/dateformula-data-type) and [CalcDate language behavior](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/methods-auto/system/system-calcdate-dateformula-date-method).
+
+[CodeCop AA0462](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/analyzers/codecop-aa0462) defines the separate direct-`CalcDate` check. In a CodeCop compilation probe against BC28.5 symbols, the direct text control produced AA0462; `Evaluate(Interval, '1W')` followed by typed `CalcDate` did not.
+
+[BaseApp retention scheduling](https://github.com/microsoft/BCApps/blob/8f7a04cb0db8aa96cb97e055c45c61aead49e280/src/Layers/W1/BaseApp/System/RetentionPolicy/RetentionPolicyScheduler.Codeunit.al#L73-L97) initializes a typed formula with an invariant literal.

microsoft/skills/review/al-style-review.md

@@ -3,7 +3,7 @@ kind: action-skill
 id: al-style-review
 version: 1
 title: AL style review
-description: Reviews AL source changes against naming, labelling, and code-convention guidance from BCQuality.
+description: Reviews AL source changes against naming, labelling, localization, and code-convention guidance from BCQuality.
 inputs: [pr-diff, file-path, folder-path]
 outputs: [findings-report]
 bc-version: [all]
@@ -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 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.
+Style findings cover AL conventions that require contextual judgment — API page naming, temporary-variable prefixes, label semantics, date-formula localization, 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, error-handling call sites, and API declarations.
+- Changed declarations, weighted toward `: Label '...'`, `: TextConst '...'`, temporary record variables, `DateFormula` declarations and their `Evaluate` call sites, 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`).
+- Tokens extracted from the diff (`Label`, `TextConst`, `Locked`, `Comment`, `MaxLength`, `temporary`, `DateFormula`, `Evaluate`, `CalcDate`, `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,6 +50,8 @@ 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.
+- A normal two-argument `Evaluate` has a resolved `DateFormula` destination and a hard-coded non-angle-bracket date-formula literal, directly or through a visible constant — `dateformula-evaluate-needs-language-independent-literals.md`. Do not use this cue for dynamic/localized external input, already invariant `<...>` input, or direct `CalcDate(Text, ...)` calls.
+
 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.