Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/instructions/common-library.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ Higher-level utilities that wrap SDK functionality.

| File | Purpose |
|------|---------|
| `AppSourceCopConfigurationProvider.cs` | Adapter wrapping `Microsoft.Dynamics.Nav.Analyzers.Common.AppSourceCopConfiguration`. Exposes `MandatoryAffixes`, `MandatorySuffix`, `MandatoryPrefix`. Uses init-only setters on net8.0, regular setters on netstandard2.1. |
| `AppSourceCopConfigurationProvider.cs` | Adapter wrapping `Microsoft.Dynamics.Nav.Analyzers.Common.AppSourceCopConfiguration`. Exposes `MandatoryAffixes`, `MandatorySuffix`, `MandatoryPrefix` via `GetAppSourceCopConfiguration(Compilation)` (SDK-cached per module spec) and the merged affix list via `GetMandatoryNameAffixes(Compilation)` (delegates to the SDK merge; NOT cached — re-reads AppSourceCop.json every call, so cache per compilation at the call site). Uses init-only setters on net8.0, regular setters on netstandard2.1. |
| `MandatoryAffixes.cs` | Shared AppSourceCop mandatory-affix logic (loose SDK semantics: every configured value from `mandatoryPrefix`/`mandatorySuffix`/`mandatoryAffixes` is a candidate at either end of a name). `GetAffixes(Compilation)` (delegates to `AppSourceCopConfigurationProvider.GetMandatoryNameAffixes`; cache per compilation at the call site), `GetIndexAfterLeadingAffix(name, affixes)` (nullable index after a leading affix; requires a non-empty remainder), `StripAffixes(name, affixes)` (removes at most one affix per end, trims residual whitespace, never returns empty). Used by LC0054 (InterfaceObjectNameGuide) and PC0021 (TransferFieldsNameMismatch, issue #436). |
| `ManifestHelper.cs` | `GetManifest(Compilation)` returning `NavAppManifest?`. On net8.0 delegates directly; on netstandard2.1 uses reflection to create a typed delegate, trying two different type paths for AL version compatibility. **Throws `FileNotFoundException` in test contexts** because `Microsoft.Dynamics.Nav.Analyzers.Common` assembly isn't available. Analyzers must catch this and treat as null manifest. |
| `ODataNameHelper.cs` | `MangleIntoValidXmlIdentifier(string name)` returning `string?`. Accesses `NameTransformations.MangleIntoValidXmlIdentifier` in `Microsoft.Dynamics.Nav.AL.Common` via `Type.GetType()` + `GetMethod()` + `CreateDelegate()`. Returns null if the SDK method is unavailable (older SDK versions). Callers should check `IsAvailable` property to exit early. Used by PC0033 (DuplicateODataEntityName). |
| `AcronymRegistry.cs` | Case-insensitive registry of canonical acronym casings (`LCY`, `OData`, `UoM`, `VAT`, ...). Exposes `DefaultAcronyms` (curated BC/web/data list as a flat array, excludes 2-letter abbreviations and `ID`), `Default` singleton, `Create(IEnumerable<string>?)` merge factory, `TryGetCanonical(word, out canonical)` (returns the preferred first-added variant), and `TryGetVariants(word, out variants)` (returns all registered variants for the case-insensitive key, ordered; first entry is canonical). Supports **multiple variants per key** — e.g. defaults list both `BoM`/`Bom` and `UoM`/`Uom`. User entries for a key **displace** built-in variants for that key (user list is authoritative per key; multiple user entries under the same key accumulate). Used by LC0098 (EventSubscriberNamingPattern); designed as shared infrastructure for future rules that render identifiers from natural-language input. |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ Include: project structure, templates, step-by-step guides, API reference, commo
| `sdk-analyzer-infrastructure` | `'src/ALCops.*/Analyzers/**'` | NAV SDK internals: callback ordering, incremental compilation, GetOperation perf |
| `analyzer-exception-harness` | `'src/ALCops.Common/Diagnostics/**'` | XX0000 harness: base class + context decorators that convert analyzer exceptions into located diagnostics |
| `fc0002-casing-mismatch` | rule-scoped | FC0002 rule (both analyzers, XmlPort casing matrix) |
| `lc0054-interface-object-name-guide` | rule-scoped | LC0054 rule (shared MandatoryAffixes helper) |
| `fc0004-permission-declaration-order` | rule-scoped | FC0004 rule |
| `fc0005-use-parenthesis-for-method-assignment` | rule-scoped | FC0005 rule |
| `fc0006-permission-values-should-be-lowercase` | rule-scoped | FC0006 rule |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
applyTo: 'src/ALCops.LinterCop/**/InterfaceObjectNameGuide*'
---

# LC0054: Interface Object Name Guide

## Purpose

Interface object names should follow the naming guide: start with a capital `I`, no whitespace
directly after the `I`. When AppSource mandatory affixes are configured, the name may start with
an affix (e.g. `"ABC ICustomer"`); the `I` rule then applies to the remainder after the affix.

## Diagnostic properties

| Property | Value |
|---|---|
| ID | `LC0054` |
| Category | Design |
| Severity | Warning |
| Enabled by default | **No** (tests enable it via a ruleset) |
| Location | Interface object name |

## Design decisions

| Decision | Rationale |
|---|---|
| Affix logic lives in `ALCops.Common.Helpers.MandatoryAffixes` | Shared with PC0021 (issue #436); loose SDK semantics via `AppSourceCopConfigurationProvider.GetMandatoryNameAffixes` |
| `mandatorySuffix` is a candidate leading affix | Behavior alignment with the SDK's own affix validation (`RuleIdentifiersMustHaveValidAffixes`): every configured value is valid at either end. Previously LC0054 ignored `mandatorySuffix` |
| Affixes resolved per compilation via `CompilationStartAction` closure + `Lazy<string[]>` | Replaced a static mutable `Affixes` field populated by a compilation-start action (cross-compilation data race). The SDK's `GetMandatoryNameAffixes(Compilation)` re-reads AppSourceCop.json, so the `Lazy` caches it per compilation |
| Fast path: name starts with `I` and no whitespace at index 1 (or name is exactly `"I"`) | No affix lookup needed for compliant names. Length guard fixes a pre-existing `IndexOutOfRangeException` for a name of exactly `"I"` |
| Empty-remainder guard after affix strip | `RemoveSpecialCharacters(remainder)` can be empty (e.g. `"ABC -"`); report instead of indexing `[0]` |

## Analyzer flow

1. Skip obsolete symbols; require `IInterfaceTypeSymbol`.
2. Name starts with `I` and is single-character or `name[1]` is not whitespace → compliant, return.
3. `MandatoryAffixes.GetIndexAfterLeadingAffix(name, affixes)` → null ⇒ report.
4. Remainder after the affix has no letters/digits, or its first letter/digit is not `I` ⇒ report.
5. Whitespace directly after the first `I` in the remainder ⇒ report.

## Test coverage

Located in `src/ALCops.LinterCop.Test/Rules/InterfaceObjectNameGuide/`. Because the rule is
disabled by default, `AnalyzerTestFixtureConfig.RuleSetPath` points to
`InterfaceObjectNameGuide.ruleset.json` (action `Warning`). Affix cases inject an
`AppSourceCop.json` (`mandatoryPrefix: "ABC "`, `mandatorySuffix: "XYZ "`,
`mandatoryAffixes: ["FOO "]`) via `MemoryFileSystem` in dedicated
`HasDiagnosticWithAffixes`/`NoDiagnosticWithAffixes` methods.

**HasDiagnostic (5 cases):** NoLeadingI, WhitespaceAfterI, AffixWithoutI, AffixThenIWithWhitespace, AffixThenNoLettersOrDigits.
**NoDiagnostic (5 cases):** LeadingI, SingleCharacterI, PrefixThenI, AffixThenI, SuffixAsLeadingAffixThenI.
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ PK fields; a constant-`true` third argument (`SkipFieldsNotMatchingType`) suppre
| Use `IsRemoved()` (Removed/Moved), NOT `IsObsolete()` | `ObsoleteState = Pending` tables/fields still participate at runtime and must keep firing |
| Field-level `#pragma warning disable` honored on either side | Checked via `IsEitherFieldSuppressed` against field syntax directives |
| Enum→Integer, Code→Text, Integer→BigInteger/Decimal treated as compatible | Safe implicit conversions performed by the platform |
| Mandatory affixes stripped before name comparison (PC0021 only) | Issue #436: AppSource `mandatoryPrefix`/`mandatorySuffix`/`mandatoryAffixes` force extension field names to differ from the paired field. `AreFieldNamesEquivalent` compares raw names first, then affix-stripped effective names via `MandatoryAffixes.StripAffixes` (loose SDK semantics: any affix, either end; whitespace trimmed after strip) |
| Affix stripping only for TableExtension fields declared in the current module | Fields on own (non-extension) tables carry the affix on the table object, not the fields; dependency extensions have their own unknown affixes. Checked via `field.ContainingSymbol is ITableExtensionTypeSymbol` + `field.Location` in the compilation's syntax-tree paths |
| Affix list cached per `Compilation` via `ConditionalWeakTable` (`AffixesCache`) | The SDK's `GetMandatoryNameAffixes(Compilation)` re-reads AppSourceCop.json on every call (it bypasses the SDK's module-spec config cache) |

## SDK behavior notes

Expand All @@ -58,8 +61,14 @@ Rules folders: `Rules/TransferFieldsTypeMismatch/` and `Rules/TransferFieldsName

**TransferFieldsTypeMismatch HasDiagnostic (17 cases):** InvocationRecWithCodeunit, InvocationRecWithPage, InvocationRecWithTable, InvocationRecWithTablexRec, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsTrue, InvocationWithReturnValue, InvocationWithVarGlobals, InvocationWithVarLocalAndGlobal, InvocationWithVarLocals, InvocationWithVarParam, InvocationWithTableExtension, Invocation_SourceTableObsoleteStatePending, TableExt_Multiple_SameBase, TableExtension, TableExtensionTypeWithType, TableExtensionTypeWithTypeLength.
**TransferFieldsTypeMismatch NoDiagnostic (18 cases):** BuiltInInvocation, Invocation_ObsoleteStateRemoved, Invocation_Pragma, Invocation_SourceTableObsoleteStateRemoved, Invocation_TargetTableObsoleteStateRemoved, InvocationCodeToText, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsFalse, InvocationWithTableExtension, InvocationWithType, InvocationWithTypeLength, TableExt_BothObsoleteStateRemoved, TableExt_ObsoleteStateRemoved, TableExt_Paired_Extension_Pragma, TableExt_Paired_SingleTableExt, TableExt_SourceBaseTableObsoleteStateRemoved, TableExt_TargetBaseTableObsoleteStateRemoved, TableExt_Unpaired.
**TransferFieldsNameMismatch HasDiagnostic (16 cases):** InvocationRecWithCodeunit, InvocationRecWithPage, InvocationRecWithTable, InvocationRecWithTablexRec, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsTrue, InvocationWithReturnValue, InvocationWithVarGlobals, InvocationWithVarLocalAndGlobal, InvocationWithVarLocals, InvocationWithVarParam, InvocationWithTableExtension, Invocation_SourceTableObsoleteStatePending, TableExt_Multiple_SameBase, TableExtension, TableExt_NamespaceCasingMismatch.
**TransferFieldsNameMismatch NoDiagnostic (14 cases):** BuiltInInvocation, Invocation_ObsoleteStateRemoved, Invocation_Pragma, Invocation_SourceTableObsoleteStateRemoved, Invocation_TargetTableObsoleteStateRemoved, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsFalse, InvocationWithTableExtension, TableExt_ObsoleteStateRemoved, TableExt_Paired_Extension_Pragma, TableExt_Paired_SingleTableExt, TableExt_SourceBaseTableObsoleteStateRemoved, TableExt_TargetBaseTableObsoleteStateRemoved, TableExt_Unpaired.
**TransferFieldsNameMismatch HasDiagnostic (18 cases):** InvocationRecWithCodeunit, InvocationRecWithPage, InvocationRecWithTable, InvocationRecWithTablexRec, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsTrue, InvocationWithReturnValue, InvocationWithVarGlobals, InvocationWithVarLocalAndGlobal, InvocationWithVarLocals, InvocationWithVarParam, InvocationWithTableExtension, Invocation_SourceTableObsoleteStatePending, TableExt_Multiple_SameBase, TableExtension, TableExt_NamespaceCasingMismatch, Affix_Invocation_CoreNameDiffers, Affix_Invocation_OwnTableFieldsNotStripped.
**TransferFieldsNameMismatch NoDiagnostic (18 cases):** BuiltInInvocation, Invocation_ObsoleteStateRemoved, Invocation_Pragma, Invocation_SourceTableObsoleteStateRemoved, Invocation_TargetTableObsoleteStateRemoved, InvocationSkipFieldsNotMatchingType, InvocationWithInitPrimaryKeyFieldsIsFalse, InvocationWithTableExtension, TableExt_ObsoleteStateRemoved, TableExt_Paired_Extension_Pragma, TableExt_Paired_SingleTableExt, TableExt_SourceBaseTableObsoleteStateRemoved, TableExt_TargetBaseTableObsoleteStateRemoved, TableExt_Unpaired, Affix_Invocation_PrefixStripped, Affix_Invocation_SuffixStripped, Affix_Invocation_AffixTrimmed, Affix_TableExt_BothSidesStripped.

The `Affix_*` cases run in dedicated `HasDiagnosticWithAffixes`/`NoDiagnosticWithAffixes` methods that
inject an `AppSourceCop.json` (`mandatoryPrefix: "ABC "`, `mandatorySuffix: " XYZ"`,
`mandatoryAffixes: ["FOO"]`) via `MemoryFileSystem`. This requires
`Microsoft.Dynamics.Nav.Analyzers.Common.dll` as a `Private=True` reference in the test csproj
(ALCops.Common references it with `Private=False`).

## Known issues

Expand Down
12 changes: 12 additions & 0 deletions src/ALCops.Common/Helpers/AppSourceCopConfigurationProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ public static class AppSourceCopConfigurationProvider

return AppSourceCopConfiguration.From(appSourceCopConf);
}

/// <summary>
/// Returns the merged, distinct, non-empty list of mandatory affixes (mandatoryPrefix,
/// mandatorySuffix and mandatoryAffixes) for the compilation's AppSourceCop.json, or an
/// empty array when none is present. Delegates to the SDK so the merge semantics stay
/// identical to AppSourceCop's own affix validation. Note: unlike
/// <see cref="GetAppSourceCopConfiguration"/>, the underlying SDK overload re-reads the
/// configuration on every call; cache the result per compilation at the call site.
/// </summary>
public static string[] GetMandatoryNameAffixes(Compilation compilation)
=> Microsoft.Dynamics.Nav.Analyzers.Common.AppSourceCopConfiguration.AppSourceCopConfigurationProvider
.GetMandatoryNameAffixes(compilation);
}

public sealed class AppSourceCopConfiguration
Expand Down
76 changes: 76 additions & 0 deletions src/ALCops.Common/Helpers/MandatoryAffixes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
using ALCops.Common.Extensions;
using Microsoft.Dynamics.Nav.CodeAnalysis;

namespace ALCops.Common.Helpers;

/// <summary>
/// Shared logic for AppSourceCop mandatory affixes (mandatoryPrefix, mandatorySuffix and
/// mandatoryAffixes). Mirrors the SDK's loose semantics where every configured value is a
/// candidate at either end of a name (see AppSourceCop's RuleIdentifiersMustHaveValidAffixes
/// and AppSourceCopConfigurationProvider.GetMandatoryNameAffixes).
/// </summary>
public static class MandatoryAffixes
{
/// <summary>
/// Returns the merged, distinct, non-empty list of mandatory affixes for the compilation.
/// Returns an empty array when no AppSourceCop.json is present or no affixes are configured.
/// Delegates to the SDK's own merge (AppSourceCopConfigurationProvider.GetMandatoryNameAffixes),
/// which re-reads the configuration on every call; cache the result per compilation at the
/// call site (e.g. a CompilationStartAction closure or a ConditionalWeakTable).
/// </summary>
public static string[] GetAffixes(Compilation compilation)
=> AppSourceCopConfigurationProvider.GetMandatoryNameAffixes(compilation);

/// <summary>
/// Returns the index of the first character after a leading affix, or <c>null</c> when the
/// name does not start with any affix or no character follows the affix.
/// </summary>
public static int? GetIndexAfterLeadingAffix(string name, string[] affixes)
{
foreach (string affix in affixes)
{
if (name.Length > affix.Length && name.StartsWith(affix, SemanticFacts.NameEqualityComparison))
return affix.Length;
}

return null;
}

/// <summary>
/// Removes at most one affix from the start and at most one affix from the end of the name,
/// trimming residual whitespace after each removal. Never returns an empty string: a removal
/// that would consume the entire name is skipped.
/// </summary>
public static string StripAffixes(string name, string[] affixes)
=> StripTrailingAffix(StripLeadingAffix(name, affixes), affixes);

private static string StripLeadingAffix(string name, string[] affixes)
{
foreach (string affix in affixes)
{
if (name.Length <= affix.Length || !name.StartsWith(affix, SemanticFacts.NameEqualityComparison))
continue;

string stripped = name.Substring(affix.Length).TrimStart();
if (stripped.Length > 0)
return stripped;
}

return name;
}

private static string StripTrailingAffix(string name, string[] affixes)
{
foreach (string affix in affixes)
{
if (name.Length <= affix.Length || !name.EndsWith(affix, SemanticFacts.NameEqualityComparison))
continue;

string stripped = name.Substring(0, name.Length - affix.Length).TrimEnd();
if (stripped.Length > 0)
return stripped;
}

return name;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
interface [|"ABC I Customer"|]
{
// After stripping the mandatory prefix "ABC ", a whitespace follows the 'I'
procedure DoSomething();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
interface [|"ABC -"|]
{
// After stripping the mandatory prefix "ABC ", no letters or digits remain
procedure DoSomething();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
interface [|"ABC Customer"|]
{
// Mandatory prefix "ABC " is stripped, but the remainder does not start with 'I'
procedure DoSomething();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
interface [|Customer|]
{
procedure DoSomething();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
interface [|"I Customer"|]
{
procedure DoSomething();
}
Loading
Loading