AC0014 ToolTip Punctuation, LC0092 LocalVariable, GlobalVariable, VarParameter

[MODUSCarstenScholling]

AC0014: Make allowed ToolTip ending punctuation configurable

Implements #361

The current implementation of AC0014 hardcodes a trailing dot as the only accepted ToolTip ending. Some teams use different punctuation conventions (e.g. exclamation marks in specific locales or house styles).

Changes:

• The rule is renamed from ToolTip must end with a dot to ToolTip must end with a punctuation.
• A new ToolTipAllowedPunctuations setting in alcops.json accepts an array of { Character, Name } objects that define which punctuation characters are accepted.
• Default behavior is unchanged: when the setting is absent, a trailing dot is required.
• The diagnostic message now includes the configured punctuation names so users receive actionable feedback.

Example configuration:

{
  "ToolTipAllowedPunctuations": [
    { "Character": ".", "Name": "dot" },
    { "Character": "!", "Name": "exclamation mark" }
  ]
}

LC0092: Add variable/parameter sub-target resolution with inheritance fallback

Implements #363
Implements #342

Implement additions to LC0092 for variable/parameter target handling.

Changes:

• Add new naming targets:
  • LocalVariable
  • GlobalVariable
  • VarParameter
• Update symbol dispatch:
  • Local variable symbols -> LocalVariable
  • Global variable symbols -> GlobalVariable
  • Non-var parameters -> Parameter
  • var parameters -> VarParameter
• Add inheritance fallback for new target area:
  • LocalVariable -> Variable
  • GlobalVariable -> Variable
  • Parameter -> LocalVariable -> Variable
  • VarParameter -> Parameter -> LocalVariable -> Variable
  • ReturnValue -> LocalVariable -> Variable
• Keep resolution priority:
  • nearest user override first
  • then nearest built-in default

Tests:

• own override wins for LocalVariable, GlobalVariable, Parameter, VarParameter
• nearest ancestor fallback when own override is missing
• full fallback to Variable when only Variable override exists

[Arthurvdv]

Code Review: AC0014 ToolTip Punctuation + LC0092 Variable/Parameter targets

Scope: 23 files, +360/−85. Two unrelated features bundled. CI is green (build + all test matrix pass).

Overall the design is sound and the LC0092 inheritance-chain refactor is a genuine improvement (cleaner than the old two-level branch logic, well tested). But there are a few correctness/UX bugs in AC0014 and an undocumented behavior change in LC0092 worth resolving before merge.

🔴 Should fix before merge

1. Diagnostic Title contains {0} — renders literally in IDEs
ALCops.ApplicationCopAnalyzers.resx:

ToolTipMustEndWithPunctuationTitle = "ToolTip must end with one of the following punctuations: {0}"

DiagnosticDescriptor.Title is never given message arguments (only MessageFormat is formatted via Diagnostic.Create(..., args)). The title will display the literal {0} in the Error List / tooltips. Keep the title static (e.g. "ToolTip must end with a punctuation") and put the {0} only in MessageFormat.

2. Null/empty Character silently disables the check (ToolTipPunctuation.cs, AnalyzeEndsWithPunctuation)

if (tooltipText.EndsWith(punctuation.Character + "'", ...))

Punctuation.Character is string?. If a config entry omits Character (null), this becomes EndsWith("'"), which matches every tooltip (the label text includes the closing quote). One malformed entry disables AC0014 globally with no feedback. Validate that Character is non-empty (and ideally exactly one char) and skip/ignore invalid entries.

3. Empty array [] flags every tooltip

settings.ToolTipAllowedPunctuations ?? [new Punctuation { Character = ".", Name = "dot" }]

The ?? only catches null. A user who writes "ToolTipAllowedPunctuations": [] gets a non-null empty list → no punctuation ever matches → every tooltip is flagged, and the message reads ...punctuations: ''.. Treat empty as "fall back to default" (is null or { Count: 0 }).

4. The new configurable behavior is not tested at the analyzer level
The ToolTipMustEndWithPunctuation tests only cover the default (dot). ALCopsSettingsProviderTests.PunctuationSettings only tests JSON deserialization. There is no analyzer test proving that a custom punctuation (e.g. !) is accepted, that the message lists names, or the null/empty edge cases above. The core feature of this PR is effectively unverified by behavior tests. Add NoDiagnostic/HasDiagnostic fixtures driven by a custom-punctuation config.

🟠 Behavior changes worth confirming

5. ReturnValue now inherits LocalVariable → Variable (NamingPattern.cs InheritanceMap)
The PR description lists inheritance for Parameter/VarParameter/LocalVariable/GlobalVariable but not ReturnValue. The new entry [ReturnValue] = LocalVariable means a user's Variable override would now leak into ReturnValue resolution (previously ReturnValue had no inheritance). ReturnValue still has its own built-in default so default behavior is unchanged, but user-override resolution changes. Was this intended? If not, drop it; if yes, document it.

6. Parameter now inherits from Variable (via LocalVariable) — intended per description, but it's a breaking change for existing configs: a user who set only a Variable override previously did not affect parameters; now they do. Call this out in release notes.

🟡 Minor / style

• Mixed tabs and spaces. New code in ToolTipPunctuation.cs, NamingPattern.cs, and both test files mixes tab indentation into a spaces-only codebase. Re-indent with spaces.
• Double-quoting in the message. MessageFormat wraps '{0}' while the analyzer passes $"'{p.Name}'" per name, producing ...punctuations: ''dot''.. Pick one quoting layer.
• public enum NamingTarget. You also added InternalsVisibleTo to the csproj, which already grants the test project access (it's used in [TestCase] attributes). Making the enum public widens the package's API surface unnecessarily — keep it internal.
• Character as string? allows multi-char values (".."); consider validating single character, or document that multi-char is allowed.
• Two unrelated features in one PR (AC0014 + LC0092). Against the repo's feat/<one-thing> convention; makes review/revert harder. Not blocking, just noting.
• English nit: "punctuations" is uncountable; "punctuation marks" reads better.

✅ Good

• LC0092 chain-based resolution is cleaner and correctly prioritizes nearest user override, then nearest built-in. Test coverage for the inheritance matrix (own-override-wins, nearest-ancestor fallback, full fallback to Variable) is thorough.
• IsVar / SymbolKind.LocalVariable/GlobalVariable dispatch is correct and matches the registered symbol kinds.
• Settings access via ALCopsSettingsProvider.GetSettings(...) matches the established pattern and is cached.
• AC0014 keeps its ID and default behavior; schema + $defs/Punctuation added correctly.

---

Generated with Claude Opus 4.8

[Arthurvdv]

Look great so far!

I've posted the remarks of the code reviewer. If I can assist on anything just let me know 🙋

[MODUSCarstenScholling]

• Fixed 1-4 (kept option to have more than one character in 'Character')
• Accepted 5-6
• Fixed tabs, double-quote
• Kept public enum NamingTarget, Character, Punctuations

NamingTarget:
Restored NamingTarget as public because it is intentionally consumed by tests and forms part of the analyzer configuration surface. This also lets NamingPatternSettings use direct enum-based TestCase values, avoiding a string parsing workaround.

[Arthurvdv]

Thanks! I'm going to include this in the upcoming v1.0.0 release, so I'll merge it into the release/v1.0.0 branch for the next beta release.

Based on the feedback from the beta release, any changes to these new rules can be submitted as a PR against the release/v1.0.0 branch. Once I'm ready to release the stable v1.0.0 version, I'll merge all changes back into the main branch.