diff --git a/.github/instructions/ac0014-tooltip-must-end-with-punctuation.instructions.md b/.github/instructions/ac0014-tooltip-must-end-with-punctuation.instructions.md new file mode 100644 index 00000000..8ea8af5c --- /dev/null +++ b/.github/instructions/ac0014-tooltip-must-end-with-punctuation.instructions.md @@ -0,0 +1,42 @@ +--- +applyTo: 'src/ALCops.ApplicationCop/**/ToolTipPunctuation*' +--- + +# AC0014: ToolTipMustEndWithPunctuation + +## Purpose + +Checks that ToolTip text ends with an allowed punctuation character. The allowed set is configurable through `ToolTipAllowedPunctuations` in `alcops.json`. + +## Diagnostic properties + +**AC0014** · Category: Design · Severity: Info · Enabled: **true** +Message: `ToolTip must end with one of the following punctuations: '{0}'.` +No version gate · Full netstandard2.1 support + +## Design decisions + +| Decision | Choice | Rationale | +|---|---|---| +| Rule scope | Implemented in `ToolTipPunctuation` analyzer | Keeps all ToolTip punctuation and phrasing checks in one analyzer for shared extraction logic. | +| Allowed punctuation source | `ALCopsSettingsProvider.GetSettings(compilation.FileSystem)` | Makes punctuation configurable per workspace/app using existing settings infrastructure. | +| Default behavior | Fallback to dot (`.` / `dot`) when settings are missing | Preserves backward compatibility with prior AC0014 behavior. | +| Match logic | Suffix check against raw tooltip text ending before closing quote | Works with AL syntax representation where value is read from property source text. | +| Message parameterization | Report configured punctuation names in diagnostic argument | Gives actionable guidance to users based on current configuration. | + +## Architecture + +1. Extract ToolTip value from property syntax. +2. Resolve settings via file-system-based settings provider. +3. Build allowed punctuation set from `ToolTipAllowedPunctuations` or fallback default. +4. Return if any configured punctuation matches the tooltip ending. +5. Report AC0014 with configured punctuation names when no match exists. + +## Known issues + +- Empty or fully invalid `ToolTipAllowedPunctuations` configurations are ignored and the analyzer falls back to the default dot punctuation. + +## Test coverage + +**HasDiagnostic (7 cases):** PageAction, PageAnalysisView, PageField, TableField, CustomExclamationMissing, InvalidConfigFallbackToDefault, CustomNamesConfigDiagnostic. +**NoDiagnostic (6 cases):** PageAction, PageAnalysisView, PageField, TableField, CustomExclamationAllowed, EmptyConfigFallbackToDefault. diff --git a/.github/instructions/instruction-maintenance.instructions.md b/.github/instructions/instruction-maintenance.instructions.md index 6dcf1755..74d96d9d 100644 --- a/.github/instructions/instruction-maintenance.instructions.md +++ b/.github/instructions/instruction-maintenance.instructions.md @@ -87,6 +87,7 @@ Include: project structure, templates, step-by-step guides, API reference, commo | `release-strategy` | `'.github/**'` | Release channels, versioning, cleanup | | `get-bc-devtools` | `'.github/actions/get-bc-devtools/**'` | BC DevTools discovery and caching | | `ac0013-field-groups-required` | rule-scoped | AC0013 rule | +| `ac0014-tooltip-must-end-with-punctuation` | rule-scoped | AC0014 rule | | `ac0026-allow-in-customizations-for-omitted-fields` | rule-scoped | AC0026 rule | | `ac0031-table-data-access-requires-permissions` | rule-scoped | AC0031 rule | | `ac0032-table-data-access-unused-permissions` | rule-scoped | AC0032 rule | diff --git a/.github/instructions/lc0092-naming-pattern.instructions.md b/.github/instructions/lc0092-naming-pattern.instructions.md index 406c93dc..e84e3dd5 100644 --- a/.github/instructions/lc0092-naming-pattern.instructions.md +++ b/.github/instructions/lc0092-naming-pattern.instructions.md @@ -25,7 +25,7 @@ No version gate · Full netstandard2.1 support |---|---|---| | Cop | LinterCop (LC0092) | Code quality convention; reuse same ID for migration from BusinessCentral.LinterCop | | Severity | Warning | Convention violation, not a bug | -| Single diagnostic ID | LC0092 for all 13 naming targets | Simpler user experience; message differentiates targets | +| Single diagnostic ID | LC0092 for all 16 naming targets | Simpler user experience; message differentiates targets | | Settings format | `NamingPatterns` dictionary in alcops.json | PascalCase keys, named per target | | Default behavior | Built-in MS convention defaults, user can override | Immediate value without configuration | | Object affixes | Strip AppSourceCop affixes before checking, trim whitespace | Avoids false positives on prefixed/suffixed names; handles common `"PTE MyCodeunit"` pattern where space separates affix from name | @@ -44,11 +44,14 @@ No version gate · Full netstandard2.1 support | Message UX | Four-tier strategy: description → suggestion → regex explainer → raw regex | Progressive enhancement; most users see human-readable messages | | Built-in descriptions | Hardcoded per default pattern | Best UX for out-of-box experience | | AllowDescription/DisallowDescription | Optional user-provided description fields in settings | Users can provide custom descriptions for their custom patterns | +| Single-letter variable/parameter names | Exempt from uppercase-start requirement | Common idiom (`i`, `j`, `k` for loops, `t` for text). Aligned with pylint `good-names`, ESLint `id-length`, Checkstyle `allowOneCharVarInForLoop`. Default pattern changed from `^[A-Z]` to `^(?:[A-Za-z]$\|[A-Z])` for LocalVariable, GlobalVariable, and Parameter targets | +| Underscore prefix for variables/parameters | Allow `_` followed by PascalCase | C# convention used in AL for variable disambiguation when the name collides with a parameter or type. PascalCase enforced after `_` (`_Text` passes, `_text` fails) to stay consistent with AL conventions. Pattern: `_[A-Z]` added to LocalVariable, GlobalVariable, and Parameter defaults | +| xRec prefix for variables/parameters | Allow `x` followed by PascalCase | Idiomatic AL convention for "previous record state" (e.g., `xSalesLine`). The platform uses `Rec`/`xRec`; developers extend this pattern to custom variables. Pattern: `x[A-Z]` added to LocalVariable, GlobalVariable, and Parameter defaults | | Auto-suggestion | Pattern-specific name transformation | `^[A-Z]` capitalizes first char, `^(?:[A-Za-z]$\|[A-Z])` capitalizes first char for multi-char names only, `[%&!?]` removes disallowed chars | | RegexExplainer | Mini parser for common constructs (char classes, anchors) | Translates simple regex to English when no description available | -| Single-letter variable/parameter names | Exempt from uppercase-start requirement | Common idiom (`i`, `j`, `k` for loops, `t` for text). Aligned with pylint `good-names`, ESLint `id-length`, Checkstyle `allowOneCharVarInForLoop`. Default pattern changed from `^[A-Z]` to `^(?:[A-Za-z]$\|[A-Z])` for Variable and Parameter targets | -| Underscore prefix for variables/parameters | Allow `_` followed by PascalCase | C# convention used in AL for variable disambiguation when the name collides with a parameter or type. PascalCase enforced after `_` (`_Text` passes, `_text` fails) to stay consistent with AL conventions. Pattern: `_[A-Z]` added to Variable and Parameter defaults | -| xRec prefix for variables/parameters | Allow `x` followed by PascalCase | Idiomatic AL convention for "previous record state" (e.g., `xSalesLine`). The platform uses `Rec`/`xRec`; developers extend this pattern to custom variables. Pattern: `x[A-Z]` added to Variable and Parameter defaults | +| LocalVariable vs GlobalVariable as distinct targets | Yes | Enables different conventions for local vs global variables (e.g. `_` prefix only for locals). `Variable` remains as pure fallback parent, never directly dispatched to. | +| Parameter inherits from LocalVariable | Yes | Parameter naming is closer to local variable conventions than global. Multi-level chain: Parameter → LocalVariable → Variable. | +| VarParameter as distinct target | Yes | `var` parameters are passed by reference; teams may want different conventions (e.g. no underscore prefix). Inherits from `Parameter` → `LocalVariable` → `Variable`. | ## Architecture @@ -60,17 +63,20 @@ Uses `RegisterCompilationStartAction` to: 3. Build `NamingPatternConfig` (resolves effective patterns per target) 4. Register `RegisterSymbolAction` for all relevant SymbolKinds -### NamingTarget enum (13 values) +### NamingTarget enum (16 values) | Target | SymbolKind registration | Classification logic | |---|---|---| -| Procedure | Method | (parent, never directly used for checking) | +| Procedure | Method | Parent target, never directly dispatched to | | LocalProcedure | Method | `method.IsLocal == true` | | GlobalProcedure | Method | `!IsLocal && !IsEvent` | | EventSubscriber | Method | `AttributeKind.EventSubscriber` | | EventDeclaration | Method | `AttributeKind.IntegrationEvent` or `AttributeKind.BusinessEvent` | -| Variable | LocalVariable, GlobalVariable | Direct registration | -| Parameter | Method | Iterate `method.Parameters` | +| Variable | — | Parent target, never directly dispatched to | +| LocalVariable | LocalVariable | `SymbolKind.LocalVariable` | +| GlobalVariable | GlobalVariable | `SymbolKind.GlobalVariable` | +| Parameter | Method | `parameter.IsVar == false` in `method.Parameters` | +| VarParameter | Method | `parameter.IsVar == true` in `method.Parameters` | | ReturnValue | Method | `method.ReturnValueSymbol` | | Object | Table, Page, Codeunit, Report, Query, XmlPort, Enum, Interface, PermissionSet | Direct registration | | Field | Field | Direct registration | @@ -80,13 +86,18 @@ Uses `RegisterCompilationStartAction` to: ### NamingPatternConfig (inner class) -Resolves effective patterns per target using a three-level cascade: +Resolves effective patterns per target using a two-phase chain walk: -1. **User override** for the specific target (from `alcops.json`) -2. **User override for parent** (sub-targets inherit from Procedure) -3. **Built-in defaults** (hardcoded in `BuiltInDefaults` dictionary) +1. **Phase 1 — User overrides**: Walk the inheritance chain from most specific to most general; return the first match. +2. **Phase 2 — Built-in defaults**: Walk the same chain; return the first match. -Inheritance map: LocalProcedure, GlobalProcedure, EventSubscriber, EventDeclaration all inherit from Procedure. +Inheritance chains: +- `LocalProcedure`, `GlobalProcedure`, `EventSubscriber`, `EventDeclaration` → `Procedure` +- `LocalVariable`, `GlobalVariable` → `Variable` +- `Parameter` → `LocalVariable` → `Variable` +- `VarParameter` → `Parameter` → `LocalVariable` → `Variable` + +`Variable` and `Procedure` are pure parent targets with no SymbolKind registration. Overriding `Variable` in `alcops.json` automatically applies to `LocalVariable`, `GlobalVariable`, and `Parameter` unless those have their own override. Returns `ResolvedPatterns` containing: compiled `Regex`, original pattern string, and description (for both allow and disallow). @@ -124,17 +135,20 @@ Pattern-specific name transformations: ### Default patterns -| Target | AllowPattern | DisallowPattern | -|---|---|---| -| Procedure | `^[A-Z]` | (none) | -| Variable | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | `[%&!?]` | -| Parameter | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | (none) | -| ReturnValue | `^[A-Z]` | (none) | -| Object | `^[A-Z]` | (none) | -| Field | `^[A-Za-z]` | `[%&!?]` | -| Action | `^[A-Z]` | (none) | -| EnumValue | (none, opt-in only) | (none) | -| Control | `^[A-Z]` | (none) | +| Target | AllowPattern | DisallowPattern | Note | +|---|---|---|---| +| Procedure | `^[A-Z]` | (none) | Parent; own built-in default | +| Variable | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | `[%&!?]` | Parent; own built-in default | +| LocalVariable | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | `[%&!?]` | Own built-in default | +| GlobalVariable | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | `[%&!?]` | Own built-in default | +| Parameter | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | (none) | Own built-in default; no disallow | +| VarParameter | `^(?:[A-Za-z]$\|[A-Z]\|_[A-Z]\|x[A-Z])` | (none) | Own built-in default; inherits from Parameter | +| ReturnValue | `^[A-Z]` | (none) | | +| Object | `^[A-Z]` | (none) | | +| Field | `^[A-Za-z]` | `[%&!?]` | | +| Action | `^[A-Z]` | (none) | | +| EnumValue | (none, opt-in only) | (none) | | +| Control | `^[A-Z]` | (none) | | ### Settings schema (alcops.json) @@ -173,5 +187,6 @@ Invalid user-supplied patterns fail gracefully: `CompilePattern` catches `Argume - **Object affix stripping tests**: Test cases with AppSourceCop configuration - **ControlAddIn object type**: Add ControlAddIn to the Object target registration - **Procedure sub-target inheritance tests**: Verify LocalProcedure/GlobalProcedure/EventSubscriber/EventDeclaration inheritance +- **Variable sub-target inheritance tests**: Verify LocalVariable/GlobalVariable/Parameter multi-level fallback chain - **Per-object-type overrides**: Allow different patterns for Table vs Codeunit vs Page names - **CodeFix**: Auto-rename to match the pattern (complex, requires semantic analysis) diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/ToolTipMustEndWithDot.cs b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/ToolTipMustEndWithDot.cs deleted file mode 100644 index d2b799f4..00000000 --- a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/ToolTipMustEndWithDot.cs +++ /dev/null @@ -1,81 +0,0 @@ -using Microsoft.Dynamics.Nav.CodeAnalysis; -using RoslynTestKit; - -namespace ALCops.ApplicationCop.Test -{ - public class ToolTipMustEndWithDot : NavCodeAnalysisBase - { - private AnalyzerTestFixture _fixture; - private AnalyzerTestFixture _analysisViewFixture; - private string _testCasePath; - - private static readonly string[] AnalysisViewTestCases = ["PageAnalysisView"]; - - [SetUp] - public void Setup() - { - _fixture = RoslynFixtureFactory.Create(); - _analysisViewFixture = RoslynFixtureFactory.Create( - TestHelper.CreateAnalysisViewConfig()); - - _testCasePath = Path.Combine( - Directory.GetParent( - Environment.CurrentDirectory)!.Parent!.Parent!.FullName, - Path.Combine("Rules", nameof(ToolTipMustEndWithDot))); - } - - [Test] - [TestCase("PageAction")] - [TestCase("PageAnalysisView")] - [TestCase("PageField")] - [TestCase("TableField")] - public async Task HasDiagnostic(string testCase) - { - SkipTestIfVersionIsTooLow( - ["TableField"], - testCase, - "13.0", - "ToolTips on fields in a table object are not supported in versions lower than 13.0." - ); - SkipTestIfVersionIsTooLow( - AnalysisViewTestCases, - testCase, - "18.0.36", - "PageAnalysisView requires net10.0 SDK." - ); - - var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) - .ConfigureAwait(false); - - var fixture = AnalysisViewTestCases.Contains(testCase) ? _analysisViewFixture : _fixture; - fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithDot); - } - - [Test] - [TestCase("PageAction")] - [TestCase("PageAnalysisView")] - [TestCase("PageField")] - [TestCase("TableField")] - public async Task NoDiagnostic(string testCase) - { - SkipTestIfVersionIsTooLow( - ["TableField"], - testCase, - "13.0", - "ToolTips on fields in a table object are not supported in versions lower than 13.0." - ); - SkipTestIfVersionIsTooLow( - AnalysisViewTestCases, - testCase, - "18.0.36", - "PageAnalysisView requires net10.0 SDK." - ); - - var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) - .ConfigureAwait(false); - - var fixture = AnalysisViewTestCases.Contains(testCase) ? _analysisViewFixture : _fixture; - fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithDot); - } - } -} \ No newline at end of file diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/CustomExclamationMissing.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/CustomExclamationMissing.al new file mode 100644 index 00000000..1b04a525 --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/CustomExclamationMissing.al @@ -0,0 +1,16 @@ +page 50100 MyPage +{ + layout + { + area(Content) + { + field(MyField; MyField) + { + ToolTip = [|'My ToolTip.'|]; + } + } + } + + var + MyField: Text; +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageAction.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageAction.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageAction.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageAction.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageAnalysisView.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageAnalysisView.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageAnalysisView.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageAnalysisView.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageField.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageField.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/PageField.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/PageField.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/TableField.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/TableField.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/HasDiagnostic/TableField.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/HasDiagnostic/TableField.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/CustomExclamationAllowed.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/CustomExclamationAllowed.al new file mode 100644 index 00000000..1a4af2f2 --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/CustomExclamationAllowed.al @@ -0,0 +1,16 @@ +page 50100 MyPage +{ + layout + { + area(Content) + { + field(MyField; MyField) + { + ToolTip = [|'My ToolTip!'|]; + } + } + } + + var + MyField: Text; +} diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageAction.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageAction.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageAction.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageAction.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageAnalysisView.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageAnalysisView.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageAnalysisView.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageAnalysisView.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageField.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageField.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/PageField.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/PageField.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/TableField.al b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/TableField.al similarity index 100% rename from src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithDot/NoDiagnostic/TableField.al rename to src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/NoDiagnostic/TableField.al diff --git a/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/ToolTipMustEndWithPunctuation.cs b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/ToolTipMustEndWithPunctuation.cs new file mode 100644 index 00000000..66fb26d6 --- /dev/null +++ b/src/ALCops.ApplicationCop.Test/Rules/ToolTipMustEndWithPunctuation/ToolTipMustEndWithPunctuation.cs @@ -0,0 +1,180 @@ +using RoslynTestKit; + +namespace ALCops.ApplicationCop.Test +{ + public class ToolTipMustEndWithPunctuation : NavCodeAnalysisBase + { + private AnalyzerTestFixture _fixture; + private AnalyzerTestFixture _analysisViewFixture; + private AnalyzerTestFixture _customExclamationFixture; + private AnalyzerTestFixture _emptyPunctuationListFixture; + private AnalyzerTestFixture _invalidPunctuationFixture; + private AnalyzerTestFixture _customNamesFixture; + private string _testCasePath; + + private static readonly string[] AnalysisViewTestCases = ["PageAnalysisView"]; + + [SetUp] + public void Setup() + { + _fixture = RoslynFixtureFactory.Create(); + _analysisViewFixture = RoslynFixtureFactory.Create( + TestHelper.CreateAnalysisViewConfig()); + + _customExclamationFixture = RoslynFixtureFactory.Create( + TestHelper.CreateConfigWithSettings( + """ + { + "ToolTipAllowedPunctuations": [ + { + "Character": "!", + "Name": "exclamation mark" + } + ] + } + """)); + + _emptyPunctuationListFixture = RoslynFixtureFactory.Create( + TestHelper.CreateConfigWithSettings( + """ + { + "ToolTipAllowedPunctuations": [] + } + """)); + + _invalidPunctuationFixture = RoslynFixtureFactory.Create( + TestHelper.CreateConfigWithSettings( + """ + { + "ToolTipAllowedPunctuations": [ + { + "Character": null, + "Name": "broken" + }, + { + "Character": "", + "Name": "empty" + }, + { + "Character": "..", + "Name": "two chars" + } + ] + } + """)); + + _customNamesFixture = RoslynFixtureFactory.Create( + TestHelper.CreateConfigWithSettings( + """ + { + "ToolTipAllowedPunctuations": [ + { + "Character": "!", + "Name": "exclamation mark" + }, + { + "Character": "?", + "Name": "question mark" + } + ] + } + """)); + + _testCasePath = Path.Combine( + Directory.GetParent( + Environment.CurrentDirectory)!.Parent!.Parent!.FullName, + Path.Combine("Rules", nameof(ToolTipMustEndWithPunctuation))); + } + + [Test] + [TestCase("PageAction")] + [TestCase("PageAnalysisView")] + [TestCase("PageField")] + [TestCase("TableField")] + public async Task HasDiagnostic(string testCase) + { + SkipTestIfVersionIsTooLow( + ["TableField"], + testCase, + "13.0", + "ToolTips on fields in a table object are not supported in versions lower than 13.0." + ); + SkipTestIfVersionIsTooLow( + AnalysisViewTestCases, + testCase, + "18.0.36", + "PageAnalysisView requires net10.0 SDK." + ); + + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = AnalysisViewTestCases.Contains(testCase) ? _analysisViewFixture : _fixture; + fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithPunctuation); + } + + [Test] + [TestCase("PageAction")] + [TestCase("PageAnalysisView")] + [TestCase("PageField")] + [TestCase("TableField")] + public async Task NoDiagnostic(string testCase) + { + SkipTestIfVersionIsTooLow( + ["TableField"], + testCase, + "13.0", + "ToolTips on fields in a table object are not supported in versions lower than 13.0." + ); + SkipTestIfVersionIsTooLow( + AnalysisViewTestCases, + testCase, + "18.0.36", + "PageAnalysisView requires net10.0 SDK." + ); + + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = AnalysisViewTestCases.Contains(testCase) ? _analysisViewFixture : _fixture; + fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithPunctuation); + } + + [Test] + [TestCase("CustomExclamationMissing", "CustomExclamation")] + [TestCase("PageField", "InvalidPunctuationFallback")] + [TestCase("PageField", "CustomNames")] + public async Task HasDiagnosticWithCustomSettings(string testCase, string fixtureName) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = GetFixture(fixtureName); + fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithPunctuation); + } + + [Test] + [TestCase("CustomExclamationAllowed", "CustomExclamation")] + [TestCase("PageField", "EmptyPunctuationListFallback")] + public async Task NoDiagnosticWithCustomSettings(string testCase, string fixtureName) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = GetFixture(fixtureName); + fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.ToolTipMustEndWithPunctuation); + } + + private AnalyzerTestFixture GetFixture(string fixtureName) + { + return fixtureName switch + { + "CustomExclamation" => _customExclamationFixture, + "EmptyPunctuationListFallback" => _emptyPunctuationListFixture, + "InvalidPunctuationFallback" => _invalidPunctuationFixture, + "CustomNames" => _customNamesFixture, + _ => throw new ArgumentOutOfRangeException(nameof(fixtureName), fixtureName, "Unknown fixture name.") + }; + } + } +} \ No newline at end of file diff --git a/src/ALCops.ApplicationCop.Test/TestHelper.cs b/src/ALCops.ApplicationCop.Test/TestHelper.cs index 6ef40347..ed71a0d6 100644 --- a/src/ALCops.ApplicationCop.Test/TestHelper.cs +++ b/src/ALCops.ApplicationCop.Test/TestHelper.cs @@ -31,4 +31,16 @@ internal static AnalyzerTestFixtureConfig CreateAnalysisViewConfig() ThrowsWhenInputDocumentContainsError = false }; } + + internal static AnalyzerTestFixtureConfig CreateConfigWithSettings(string settingsJson) + { + return new AnalyzerTestFixtureConfig + { + FileSystem = new MemoryFileSystem(new Dictionary + { + { "alcops.json", System.Text.Encoding.UTF8.GetBytes(settingsJson) } + }), + ThrowsWhenInputDocumentContainsError = false + }; + } } diff --git a/src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.resx b/src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.resx index 92d1501e..e0325a1e 100644 --- a/src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.resx +++ b/src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.resx @@ -399,14 +399,14 @@ The user assistance model recommends keeping ToolTips short (about 200 characters including spaces) so users can scan them quickly. The UI can render longer ToolTips, but longer text reduces scannability and consistency. - - ToolTip must end with a dot + + ToolTip must end with a punctuation - - ToolTip must end with a dot. + + ToolTip must end with one of the following punctuations: '{0}'. - - End ToolTips with a dot to keep phrasing consistent and clearly separated from surrounding UI text. + + End ToolTips with a punctuation to keep phrasing consistent and clearly separated from surrounding UI text. ToolTip should start with 'Specifies' diff --git a/src/ALCops.ApplicationCop/Analyzers/ToolTipPunctuation.cs b/src/ALCops.ApplicationCop/Analyzers/ToolTipPunctuation.cs index 037f0fdf..fd2d1d6e 100644 --- a/src/ALCops.ApplicationCop/Analyzers/ToolTipPunctuation.cs +++ b/src/ALCops.ApplicationCop/Analyzers/ToolTipPunctuation.cs @@ -1,20 +1,27 @@ using System.Collections.Immutable; using ALCops.Common.Extensions; using ALCops.Common.Reflection; +using ALCops.Common.Settings; using Microsoft.Dynamics.Nav.CodeAnalysis; using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics; using Microsoft.Dynamics.Nav.CodeAnalysis.Syntax; +using Microsoft.Dynamics.Nav.CodeAnalysis.Utilities; namespace ALCops.ApplicationCop.Analyzers; [DiagnosticAnalyzer] public sealed class ToolTipPunctuation : DiagnosticAnalyzer { + private static readonly List DefaultAllowedPunctuations = + [ + new Punctuation { Character = ".", Name = "dot" } + ]; + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create( DiagnosticDescriptors.ToolTipDoNotUseLineBreaks, DiagnosticDescriptors.ToolTipMaximumLength, - DiagnosticDescriptors.ToolTipMustEndWithDot, + DiagnosticDescriptors.ToolTipMustEndWithPunctuation, DiagnosticDescriptors.ToolTipShouldStartWithSpecifies ); @@ -52,8 +59,8 @@ private void AnalyzeToolTipPunctuation(SyntaxNodeAnalysisContext ctx) if (ctx.IsDiagnosticEnabled(DiagnosticDescriptors.ToolTipMaximumLength)) AnalyzeMaximumLength(ctx, tooltipText, tooltipProperty); - if (ctx.IsDiagnosticEnabled(DiagnosticDescriptors.ToolTipMustEndWithDot)) - AnalyzeEndsWithDot(ctx, tooltipText, tooltipProperty); + if (ctx.IsDiagnosticEnabled(DiagnosticDescriptors.ToolTipMustEndWithPunctuation)) + AnalyzeEndsWithPunctuation(ctx, tooltipText, tooltipProperty); if (ctx.IsDiagnosticEnabled(DiagnosticDescriptors.ToolTipShouldStartWithSpecifies)) AnalyzeStartsWithSpecifies(ctx, tooltipText, tooltipProperty); @@ -79,14 +86,44 @@ private static void AnalyzeMaximumLength(SyntaxNodeAnalysisContext ctx, string t } } - private static void AnalyzeEndsWithDot(SyntaxNodeAnalysisContext ctx, string tooltipText, PropertyValueSyntax tooltipProperty) + private static void AnalyzeEndsWithPunctuation(SyntaxNodeAnalysisContext ctx, string tooltipText, PropertyValueSyntax tooltipProperty) { - if (!tooltipText.EndsWith(".'", StringComparison.OrdinalIgnoreCase)) + var fileSystem = ctx.SemanticModel.Compilation.FileSystem; + var settings = ALCopsSettingsProvider.GetSettings(fileSystem); + + var allowedPunctuations = ResolveAllowedPunctuations(settings); + + foreach (Punctuation punctuation in allowedPunctuations) { - ctx.ReportDiagnostic(Diagnostic.Create( - DiagnosticDescriptors.ToolTipMustEndWithDot, - tooltipProperty.GetLocation())); + if (tooltipText.EndsWith(punctuation.Character + "'", StringComparison.Ordinal)) + { + return; + } } + + ctx.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.ToolTipMustEndWithPunctuation, + tooltipProperty.GetLocation(), + string.Join("', '", allowedPunctuations.Select(p => p.Name)))); + } + + private static List ResolveAllowedPunctuations(ALCopsSettings settings) + { + if (settings.ToolTipAllowedPunctuations is null or { Count: 0 }) + { + return DefaultAllowedPunctuations; + } + + var normalizedPunctuations = settings.ToolTipAllowedPunctuations + .Where(p => !string.IsNullOrWhiteSpace(p.Character)) + .Select(p => new Punctuation + { + Character = p.Character, + Name = string.IsNullOrWhiteSpace(p.Name) ? p.Character : p.Name + }) + .ToList(); + + return normalizedPunctuations.Count == 0 ? DefaultAllowedPunctuations : normalizedPunctuations; } private static void AnalyzeStartsWithSpecifies(SyntaxNodeAnalysisContext ctx, string tooltipText, PropertyValueSyntax tooltipProperty) diff --git a/src/ALCops.ApplicationCop/DiagnosticDescriptors.cs b/src/ALCops.ApplicationCop/DiagnosticDescriptors.cs index 0d33b81b..f1edc413 100644 --- a/src/ALCops.ApplicationCop/DiagnosticDescriptors.cs +++ b/src/ALCops.ApplicationCop/DiagnosticDescriptors.cs @@ -275,15 +275,15 @@ public static class DiagnosticDescriptors description: ApplicationCopAnalyzers.ToolTipMaximumLengthDescription, helpLinkUri: GetHelpUri(DiagnosticIds.ToolTipMaximumLength)); - public static readonly DiagnosticDescriptor ToolTipMustEndWithDot = new( - id: DiagnosticIds.ToolTipMustEndWithDot, - title: ApplicationCopAnalyzers.ToolTipMustEndWithDotTitle, - messageFormat: ApplicationCopAnalyzers.ToolTipMustEndWithDotMessageFormat, + public static readonly DiagnosticDescriptor ToolTipMustEndWithPunctuation = new( + id: DiagnosticIds.ToolTipMustEndWithPunctuation, + title: ApplicationCopAnalyzers.ToolTipMustEndWithPunctuationTitle, + messageFormat: ApplicationCopAnalyzers.ToolTipMustEndWithPunctuationMessageFormat, category: Category.Design, defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true, - description: ApplicationCopAnalyzers.ToolTipMustEndWithDotDescription, - helpLinkUri: GetHelpUri(DiagnosticIds.ToolTipMustEndWithDot)); + description: ApplicationCopAnalyzers.ToolTipMustEndWithPunctuationDescription, + helpLinkUri: GetHelpUri(DiagnosticIds.ToolTipMustEndWithPunctuation)); public static readonly DiagnosticDescriptor ToolTipShouldStartWithSpecifies = new( id: DiagnosticIds.ToolTipShouldStartWithSpecifies, diff --git a/src/ALCops.ApplicationCop/DiagnosticIds.cs b/src/ALCops.ApplicationCop/DiagnosticIds.cs index a34938d1..9b6b011d 100644 --- a/src/ALCops.ApplicationCop/DiagnosticIds.cs +++ b/src/ALCops.ApplicationCop/DiagnosticIds.cs @@ -15,7 +15,7 @@ public static class DiagnosticIds public static readonly string CaptionRequired = "AC0011"; public static readonly string IntegrationEventInInternalCodeunit = "AC0012"; public static readonly string FieldGroupsRequired = "AC0013"; - public static readonly string ToolTipMustEndWithDot = "AC0014"; + public static readonly string ToolTipMustEndWithPunctuation = "AC0014"; public static readonly string ToolTipShouldStartWithSpecifies = "AC0015"; public static readonly string ToolTipDoNotUseLineBreaks = "AC0016"; public static readonly string ToolTipMaximumLength = "AC0017"; diff --git a/src/ALCops.Common.Test/Settings/ALCopsSettingsProviderTests.cs b/src/ALCops.Common.Test/Settings/ALCopsSettingsProviderTests.cs index 099f6309..66eaadd9 100644 --- a/src/ALCops.Common.Test/Settings/ALCopsSettingsProviderTests.cs +++ b/src/ALCops.Common.Test/Settings/ALCopsSettingsProviderTests.cs @@ -196,4 +196,47 @@ public void GetSettings_WithMemoryFileSystem_ReturnsDefaultsWhenNoConfig() // Assert: defaults Assert.That(settings.CyclomaticComplexityThreshold, Is.EqualTo(8)); } + + [Test] + public void PunctuationSettings() + { + // Arrange: alcops.json in the app folder itself + string appFolder = Path.Combine(_tempRoot, "App1"); + Directory.CreateDirectory(appFolder); + + File.WriteAllText( + Path.Combine(appFolder, "alcops.json"), + @"{ + ""ToolTipAllowedPunctuations"": [ + { + ""Character"": ""."", + ""Name"": ""dot"" + }, + { + ""Character"": ""!"", + ""Name"": ""exclamation mark"" + } + ] +}"); + + List? expectedPunctuations = [ + new Punctuation { Character = ".", Name = "dot" }, + new Punctuation { Character = "!", Name = "exclamation mark" } + ]; + + var settings = ALCopsSettingsProvider.GetSettings(new RelativeFileSystem(appFolder)); + + Assert.That(settings.ToolTipAllowedPunctuations?.Count, Is.EqualTo(2)); + + if (settings.ToolTipAllowedPunctuations != null) + { + foreach(Punctuation punctuation in settings.ToolTipAllowedPunctuations) + { + var expected = expectedPunctuations.FirstOrDefault(p => p.Character == punctuation.Character); + + Assert.That(punctuation.Character, Is.EqualTo(expected?.Character)); + Assert.That(punctuation.Name, Is.EqualTo(expected?.Name)); + } + } + } } diff --git a/src/ALCops.Common/Settings/ALCopsSettings.cs b/src/ALCops.Common/Settings/ALCopsSettings.cs index ee282ade..aa03dfcc 100644 --- a/src/ALCops.Common/Settings/ALCopsSettings.cs +++ b/src/ALCops.Common/Settings/ALCopsSettings.cs @@ -8,4 +8,5 @@ public sealed class ALCopsSettings public string[]? LanguagesToTranslate { get; set; } public Dictionary? NamingPatterns { get; set; } public string? UseSequentialGuidScope { get; set; } + public List? ToolTipAllowedPunctuations { get; set; } } diff --git a/src/ALCops.Common/Settings/Punctuation.cs b/src/ALCops.Common/Settings/Punctuation.cs new file mode 100644 index 00000000..0177d725 --- /dev/null +++ b/src/ALCops.Common/Settings/Punctuation.cs @@ -0,0 +1,7 @@ +namespace ALCops.Common.Settings; + +public sealed class Punctuation +{ + public string? Character { get; set; } + public string? Name { get; set; } +} diff --git a/src/ALCops.Common/Settings/alcops.schema.json b/src/ALCops.Common/Settings/alcops.schema.json index e5923f39..c2d56c2f 100644 --- a/src/ALCops.Common/Settings/alcops.schema.json +++ b/src/ALCops.Common/Settings/alcops.schema.json @@ -68,6 +68,19 @@ "type": "string", "enum": ["AllGuidFields"], "description": "Controls the scope of the sequential GUID check. When set to 'AllGuidFields', all CreateGuid() calls are flagged, not just those in primary key fields. Used by PC0029." + }, + "ToolTipAllowedPunctuations": { + "type": "array", + "items": { + "$ref": "#/$defs/Punctuation" + }, + "default": [ + { + "Character": ".", + "Name": "dot" + } + ], + "description": "The set of allowed ending punctuations for tooltip text. Used by AC0014. Default: [{\"Character\":\".\",\"Name\":\"dot\"}]" } }, "$defs": { @@ -93,6 +106,21 @@ "description": "A human-readable description shown in the diagnostic message when DisallowPattern is violated. Example: \"should not contain special characters\"" } } + }, + "Punctuation": { + "type": "object", + "description": "Definition of allowed punctuations like '.', '!' for the end of tooltips.", + "additionalProperties": false, + "properties": { + "Character": { + "type": "string", + "description": "A character allowed as punctuation (i.e. '.', '!')." + }, + "Name": { + "type": "string", + "description": "A name of the allowed character (i.e. 'dot', 'exclamation mark')." + } + } } } } diff --git a/src/ALCops.LinterCop.Test/Rules/NamingPattern/NamingPatternSettings.cs b/src/ALCops.LinterCop.Test/Rules/NamingPattern/NamingPatternSettings.cs new file mode 100644 index 00000000..89479c03 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/NamingPattern/NamingPatternSettings.cs @@ -0,0 +1,94 @@ +using RoslynTestKit; +using NamingPatternTarget = ALCops.LinterCop.Analyzers.NamingPattern.NamingTarget; +using NamingPatternConfig = ALCops.LinterCop.Analyzers.NamingPattern.NamingPatternConfig; +using NamingPatternSetting = ALCops.Common.Settings.NamingPattern; + +namespace ALCops.LinterCop.Test +{ + public class NamingPatternSettings : NavCodeAnalysisBase + { + // Verifies that each target resolves to the pattern of the closest ancestor + // that has an override. When all targets have overrides, each resolves its own. + [Test] + [TestCase(NamingPatternTarget.LocalVariable, "LocalVariable")] + [TestCase(NamingPatternTarget.GlobalVariable, "GlobalVariable")] + [TestCase(NamingPatternTarget.Parameter, "Parameter")] + [TestCase(NamingPatternTarget.VarParameter, "VarParameter")] + public async Task InheritanceResolvesOwnOverrideFirst( + NamingPatternTarget target, string expectedSourceTarget) + { + var config = new NamingPatternConfig(GetOverridesForAllTargets()); + + var expected = AllowPatternFor(expectedSourceTarget); + var actual = config.GetPatterns(target).AllowPatternString; + + Assert.That(actual, Is.EqualTo(expected)); + } + + // Verifies fallback: removing a target's own override + // causes it to inherit from the proper ancestor that has one. + [Test] + [TestCase(NamingPatternTarget.LocalVariable, "Variable")] + [TestCase(NamingPatternTarget.GlobalVariable, "Variable")] + [TestCase(NamingPatternTarget.Parameter, "LocalVariable")] + [TestCase(NamingPatternTarget.VarParameter, "Parameter")] + public async Task InheritanceFallsBackToNearestAncestorWhenOwnOverrideMissing( + NamingPatternTarget target, string expectedSourceTarget) + { + // Overrides for all targets except the target under test itself, which is the nearest ancestor to Parameter and VarParameter. + var overrides = GetOverridesForAllTargets(target.ToString()); + var config = new NamingPatternConfig(overrides); + + var expected = AllowPatternFor(expectedSourceTarget); + var actual = config.GetPatterns(target).AllowPatternString; + + Assert.That(actual, Is.EqualTo(expected)); + } + + // Verifies the full chain: only Variable has an override -> all descendants inherit it. + [Test] + [TestCase(NamingPatternTarget.LocalVariable)] + [TestCase(NamingPatternTarget.GlobalVariable)] + [TestCase(NamingPatternTarget.Parameter)] + [TestCase(NamingPatternTarget.VarParameter)] + public async Task InheritanceFallsBackToVariableWhenOnlyVariableHasOverride( + NamingPatternTarget target) + { + var overrides = new Dictionary + { + ["Variable"] = new NamingPatternSetting { AllowPattern = AllowPatternFor("Variable") } + }; + + var config = new NamingPatternConfig(overrides); + + var expected = AllowPatternFor("Variable"); + var actual = config.GetPatterns(target).AllowPatternString; + + Assert.That(actual, Is.EqualTo(expected)); + } + + private static string AllowPatternFor(string targetName) => $"{targetName}_Allow"; + private static string DisallowPatternFor(string targetName) => $"{targetName}_Disallow"; + + private static Dictionary GetOverridesForAllTargets(params string[] toSkip) + { + var overrides = new Dictionary(); + + foreach (NamingPatternTarget t in Enum.GetValues(typeof(NamingPatternTarget))) + { + var name = t.ToString(); + + if (!toSkip.Contains(name)) + { + overrides[name] = new NamingPatternSetting + { + AllowPattern = AllowPatternFor(name), + DisallowPattern = DisallowPatternFor(name) + }; + } + } + + return overrides; + } + } +} diff --git a/src/ALCops.LinterCop/ALCops.LinterCop.csproj b/src/ALCops.LinterCop/ALCops.LinterCop.csproj index a607110d..31628278 100644 --- a/src/ALCops.LinterCop/ALCops.LinterCop.csproj +++ b/src/ALCops.LinterCop/ALCops.LinterCop.csproj @@ -13,6 +13,9 @@ ../../Microsoft.Dynamics.BusinessCentral.Development.Tools + + + ALCops.LinterCop.LinterCopAnalyzers.resources diff --git a/src/ALCops.LinterCop/Analyzers/NamingPattern.cs b/src/ALCops.LinterCop/Analyzers/NamingPattern.cs index 27cae24d..00ca1d05 100644 --- a/src/ALCops.LinterCop/Analyzers/NamingPattern.cs +++ b/src/ALCops.LinterCop/Analyzers/NamingPattern.cs @@ -104,7 +104,9 @@ private static void AnalyzeMethod(SymbolAnalysisContext ctx, NamingPatternConfig if (string.IsNullOrEmpty(parameter.Name)) continue; - CheckNameForSymbol(ctx, parameter, parameter.Name, NamingTarget.Parameter, config, "Parameter"); + CheckNameForSymbol(ctx, parameter, parameter.Name, + parameter.IsVar ? NamingTarget.VarParameter : NamingTarget.Parameter, + config, "Parameter"); } // Check return value @@ -120,7 +122,10 @@ private static void AnalyzeVariable(SymbolAnalysisContext ctx, NamingPatternConf if (ctx.IsObsolete()) return; - CheckName(ctx, ctx.Symbol.Name, NamingTarget.Variable, config, "Variable"); + var target = ctx.Symbol.Kind == EnumProvider.SymbolKind.LocalVariable + ? NamingTarget.LocalVariable + : NamingTarget.GlobalVariable; + CheckName(ctx, ctx.Symbol.Name, target, config, "Variable"); } private static void AnalyzeObject(SymbolAnalysisContext ctx, NamingPatternConfig config, @@ -415,7 +420,7 @@ private static string StripAffixes(string name, List? affixes) return affixes.Count > 0 ? affixes : null; } - internal enum NamingTarget + public enum NamingTarget { Procedure, LocalProcedure, @@ -423,7 +428,10 @@ internal enum NamingTarget EventSubscriber, EventDeclaration, Variable, + LocalVariable, + GlobalVariable, Parameter, + VarParameter, ReturnValue, Object, Field, @@ -434,16 +442,24 @@ internal enum NamingTarget internal sealed class NamingPatternConfig { + private static readonly (string? Allow, string? Disallow, string? AllowDesc, string? DisallowDesc) pascalCase = (@"^[A-Z]", null, "should start with an uppercase letter", null); + private static readonly (string? Allow, string? Disallow, string? AllowDesc, string? DisallowDesc) pascalCaseUnderscoreNoSpecial = (@"^(?:[A-Za-z]$|[A-Z]|_[A-Z]|x[A-Z])", @"[%&!?]", "should start with an uppercase letter, underscore followed by uppercase, or x followed by uppercase for xRec pattern (single-letter names are exempt)", "should not contain special characters (%, &, !, ?)"); + private static readonly (string? Allow, string? Disallow, string? AllowDesc, string? DisallowDesc) pascalCaseUnderscore = (@"^(?:[A-Za-z]$|[A-Z]|_[A-Z]|x[A-Z])", null, "should start with an uppercase letter, underscore followed by uppercase, or x followed by uppercase for xRec pattern (single-letter names are exempt)", null); + private static readonly (string? Allow, string? Disallow, string? AllowDesc, string? DisallowDesc) anyCaseNoSpecial = (@"^[A-Za-z]", @"[%&!?]", "should start with a letter", "should not contain special characters (%, &, !, ?)"); + private static readonly Dictionary BuiltInDefaults = new() { - [NamingTarget.Procedure] = (@"^[A-Z]", null, "should start with an uppercase letter", null), - [NamingTarget.Variable] = (@"^(?:[A-Za-z]$|[A-Z]|_[A-Z]|x[A-Z])", @"[%&!?]", "should start with an uppercase letter, underscore followed by uppercase, or x followed by uppercase for xRec pattern (single-letter names are exempt)", "should not contain special characters (%, &, !, ?)"), - [NamingTarget.Parameter] = (@"^(?:[A-Za-z]$|[A-Z]|_[A-Z]|x[A-Z])", null, "should start with an uppercase letter, underscore followed by uppercase, or x followed by uppercase for xRec pattern (single-letter names are exempt)", null), - [NamingTarget.ReturnValue] = (@"^[A-Z]", null, "should start with an uppercase letter", null), - [NamingTarget.Object] = (@"^[A-Z]", null, "should start with an uppercase letter", null), - [NamingTarget.Field] = (@"^[A-Za-z]", @"[%&!?]", "should start with a letter", "should not contain special characters (%, &, !, ?)"), - [NamingTarget.Action] = (@"^[A-Z]", null, "should start with an uppercase letter", null), - [NamingTarget.Control] = (@"^[A-Z]", null, "should start with an uppercase letter", null), + [NamingTarget.Procedure] = pascalCase, + [NamingTarget.Variable] = pascalCaseUnderscoreNoSpecial, + [NamingTarget.LocalVariable] = pascalCaseUnderscoreNoSpecial, + [NamingTarget.GlobalVariable] = pascalCaseUnderscoreNoSpecial, + [NamingTarget.Parameter] = pascalCaseUnderscore, + [NamingTarget.VarParameter] = pascalCaseUnderscore, + [NamingTarget.ReturnValue] = pascalCase, + [NamingTarget.Object] = pascalCase, + [NamingTarget.Field] = anyCaseNoSpecial, + [NamingTarget.Action] = pascalCase, + [NamingTarget.Control] = pascalCase, }; private static readonly Dictionary InheritanceMap = new() @@ -452,6 +468,11 @@ internal sealed class NamingPatternConfig [NamingTarget.GlobalProcedure] = NamingTarget.Procedure, [NamingTarget.EventSubscriber] = NamingTarget.Procedure, [NamingTarget.EventDeclaration] = NamingTarget.Procedure, + [NamingTarget.LocalVariable] = NamingTarget.Variable, + [NamingTarget.GlobalVariable] = NamingTarget.Variable, + [NamingTarget.Parameter] = NamingTarget.LocalVariable, + [NamingTarget.VarParameter] = NamingTarget.Parameter, + [NamingTarget.ReturnValue] = NamingTarget.LocalVariable, }; private readonly Dictionary _resolvedPatterns; @@ -479,37 +500,45 @@ public ResolvedPatterns GetPatterns(NamingTarget target) => private static (string? Allow, string? Disallow, string? AllowDesc, string? DisallowDesc) ResolvePatternStrings( NamingTarget target, Dictionary? userOverrides) { - // Check if user has explicit override for this target - if (userOverrides is not null && TryGetUserOverride(userOverrides, target, out var userSetting)) + // Build chain: target → parent → grandparent → ... + var chain = new List(); + var current = target; + + while (true) { - return ( - !string.IsNullOrEmpty(userSetting.AllowPattern) ? userSetting.AllowPattern : null, - !string.IsNullOrEmpty(userSetting.DisallowPattern) ? userSetting.DisallowPattern : null, - !string.IsNullOrEmpty(userSetting.AllowDescription) ? userSetting.AllowDescription : null, - !string.IsNullOrEmpty(userSetting.DisallowDescription) ? userSetting.DisallowDescription : null); + chain.Add(current); + + if (!InheritanceMap.TryGetValue(current, out var next)) + { + break; + } + + current = next; } - // Check if this target inherits from a parent - if (InheritanceMap.TryGetValue(target, out var parent)) + // Phase 1: first user override in the chain (specific wins over general). + if (userOverrides is not null) { - // Try user override for the parent - if (userOverrides is not null && TryGetUserOverride(userOverrides, parent, out var parentSetting)) + foreach (var t in chain) { - return ( - !string.IsNullOrEmpty(parentSetting.AllowPattern) ? parentSetting.AllowPattern : null, - !string.IsNullOrEmpty(parentSetting.DisallowPattern) ? parentSetting.DisallowPattern : null, - !string.IsNullOrEmpty(parentSetting.AllowDescription) ? parentSetting.AllowDescription : null, - !string.IsNullOrEmpty(parentSetting.DisallowDescription) ? parentSetting.DisallowDescription : null); + if (TryGetUserOverride(userOverrides, t, out var s)) + return ( + !string.IsNullOrEmpty(s.AllowPattern) ? s.AllowPattern : null, + !string.IsNullOrEmpty(s.DisallowPattern) ? s.DisallowPattern : null, + !string.IsNullOrEmpty(s.AllowDescription) ? s.AllowDescription : null, + !string.IsNullOrEmpty(s.DisallowDescription) ? s.DisallowDescription : null); } - - // Fall through to built-in default for parent - if (BuiltInDefaults.TryGetValue(parent, out var parentDefault)) - return parentDefault; } - // Use built-in default for this target - if (BuiltInDefaults.TryGetValue(target, out var builtIn)) - return builtIn; + // Phase 2: first built-in default in the chain (specific wins over general). + // Parameter has its own entry and wins over LocalVariable → Variable. + foreach (var t in chain) + { + if (BuiltInDefaults.TryGetValue(t, out var builtIn)) + { + return builtIn; + } + } return (null, null, null, null); }