feat: Expand analyzer rules to cover template strings #30
Conversation
Updates the Roslyn Analyzer with new diagnostics to report more errors in near real-time in the IDE. This commit adds support for detecting and reporting the following diagnostics; - COMPOSITE0005 - Empty or invalid template string - COMPOSITE0006 - Primary key separator missing from template string
WalkthroughAdds a shared template-validation pipeline and tokens under CompositeKey.Analyzers.Common, updates tokeniser control flow to forbid multiple primary delimiters, integrates validation into the source generator, introduces a Roslyn analyzer for template strings, and adds comprehensive unit tests for validation and analyzer behaviours. Minor test infra cleanup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant SG as SourceGenerator.Parser
participant TV as TemplateValidation
participant TT as TemplateStringTokenizer
note over Dev,SG: Build/generate
Dev->>SG: Provide attribute (template, primaryKeySeparator)
SG->>TV: ValidateTemplateFormat(template, sep, availableProperties)
TV->>TV: ValidateTemplateString
TV->>TT: TokenizeTemplateString(template, sep)
TT-->>TV: TokenizeResult (tokens or failure)
TV->>TV: ValidatePrimaryKeySeparator(tokens, sep)
TV->>TV: HasValidTemplateStructure(tokens)
alt Primary delimiter present
TV->>TV: ValidatePartitionAndSortKeyStructure(tokens)
end
TV->>TV: ValidatePropertyReferences(tokens, properties)
TV-->>SG: TemplateValidationResult (success/failure with descriptor/args)
alt Failure
SG-->>Dev: Report diagnostic (descriptor, args, location)
else Success
SG-->>Dev: Proceed with key spec construction
end
sequenceDiagram
autonumber
actor Dev as Developer
participant A as TemplateStringAnalyzer
participant TV as TemplateValidation
participant TT as TemplateStringTokenizer
Dev->>A: Compile code with [CompositeKey(...)]
A->>A: Extract template & primaryKeySeparator from attribute
A->>TV: ValidateTemplateString(template)
alt Non-empty
A->>TV: TokenizeTemplateString(template, sep)
TV->>TT: Tokenize
TT-->>TV: Tokens or failure
TV-->>A: TokenizeResult / failure
A->>TV: ValidatePrimaryKeySeparator / Structure / Partition-Sort / PropertyReferences
TV-->>A: ValidationResult
end
alt Any failure
A-->>Dev: Report diagnostic at precise argument location
else All checks pass
A-->>Dev: No diagnostics
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #30 +/- ##
==========================================
- Coverage 93.29% 92.19% -1.11%
==========================================
Files 30 32 +2
Lines 1163 1319 +156
Branches 173 211 +38
==========================================
+ Hits 1085 1216 +131
- Misses 36 48 +12
- Partials 42 55 +13 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs (1)
3-20: Add XML docs for public API.These public tokens and factories are part of the shared surface. Please add brief XML summaries to aid consumers and keep API docs coherent.
src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs (1)
163-173: Avoid recomputing available properties per property token.availableProperties is rebuilt for every property token inside ToPropertyKeyPart. Build it once before the loop and reuse to reduce allocations.
List<KeyPart> keyParts = []; - foreach (var templateToken in templateTokens) + // Build once and reuse in ToPropertyKeyPart + var availableProperties = properties + .Select(p => new TemplateValidation.PropertyInfo(p.Spec.Name, p.Spec.HasGetter, p.Spec.HasSetter)) + .ToList(); + + foreach (var templateToken in templateTokens) { @@ - PropertyKeyPart? ToPropertyKeyPart(PropertyTemplateToken templateToken) + PropertyKeyPart? ToPropertyKeyPart(PropertyTemplateToken templateToken) { - var availableProperties = properties - .Select(p => new TemplateValidation.PropertyInfo(p.Spec.Name, p.Spec.HasGetter, p.Spec.HasSetter)) - .ToList(); - var propertyValidation = TemplateValidation.ValidatePropertyReferences([templateToken], availableProperties); if (!propertyValidation.IsSuccess) { ReportDiagnostic(propertyValidation.Descriptor, _location, propertyValidation.MessageArgs); return null; }Also applies to: 188-201
src/CompositeKey.Analyzers.UnitTests/Analyzers/TemplateStringAnalyzerTests.cs (2)
489-531: Add test: template named argument not first.Covers the case where template: appears after other named args to validate precise location selection.
public async Task AttributeWithNamedArguments_TargetsCorrectly() { @@ } + + [Fact] + public async Task TemplateNamedArgumentAfterOthers_TargetsTemplateArgument() + { + var test = new TemplateStringAnalyzerTest + { + TestCode = """ + using CompositeKey; + + [CompositeKey( + InvariantCulture = true, + {|COMPOSITE0005:template: ""|} + )] + public partial record TestRecord(string Id); + """ + }; + + await test.RunAsync(); + }
314-441: Add test: multiple primary separators should be invalid.Ensures the tokenizer’s “single primary delimiter” rule surfaces as COMPOSITE0005 from the analyser.
public async Task ConsecutiveSeparators_ReportsInvalidTemplate() { @@ } + + [Fact] + public async Task MultiplePrimarySeparators_ReportsInvalidTemplate() + { + var test = new TemplateStringAnalyzerTest + { + TestCode = """ + using CompositeKey; + + [CompositeKey({|COMPOSITE0005:"{A}|{B}|{C}"|}, PrimaryKeySeparator = '|')] + public partial record TestKey(string A, string B, string C); + """ + }; + + await test.RunAsync(); + }src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (4)
10-29: Consider a value-type result to cut allocations in hot paths.TemplateValidationResult is created frequently by analyzers/generators. Making it a readonly record struct reduces heap pressure without changing call sites.
Apply:
- public record TemplateValidationResult + public readonly record struct TemplateValidationResult { [MemberNotNullWhen(false, nameof(Descriptor), nameof(MessageArgs))] public required bool IsSuccess { get; init; }Before merging, please confirm your LangVersion/TFM supports record struct + required. I can generate a script to verify project settings if helpful.
45-58: Minor: prefer !Any(...) over All(...) for readability.Equivalent logic, clearer intent.
- if (primaryKeySeparator.HasValue && tokens.All(t => t.Type != TemplateToken.TemplateTokenType.PrimaryDelimiter)) + if (primaryKeySeparator.HasValue && !tokens.Any(t => t.Type == TemplateToken.TemplateTokenType.PrimaryDelimiter)) {
79-83: Confirm: constants-only templates are considered valid.HasValidTemplateStructure returns true for templates with only Constant tokens. If you intended to always require at least one Property, tighten the predicate.
- return tokens.Any() && tokens.Any(t => t.Type is TemplateToken.TemplateTokenType.Property or TemplateToken.TemplateTokenType.Constant); + return tokens.Any() && tokens.Any(t => t.Type == TemplateToken.TemplateTokenType.Property);If constants-only keys are valid by design, ignore this.
84-96: Avoid double scanning for the primary delimiter.Compute the index directly in one pass.
- var primaryDelimiterToken = tokens.FirstOrDefault(t => t.Type == TemplateToken.TemplateTokenType.PrimaryDelimiter); - if (primaryDelimiterToken == null) + var idx = tokens.FindIndex(t => t.Type == TemplateToken.TemplateTokenType.PrimaryDelimiter); + if (idx == -1) return true; // No composite key, just simple key - - primaryDelimiterIndex = tokens.IndexOf(primaryDelimiterToken); + primaryDelimiterIndex = idx;src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs (1)
55-62: Make the test tokens realistic w.r.t. tokenizer rules.When '#' is the primaryKeySeparator, additional '#' characters cannot be Delimiter tokens (the tokenizer would fail). Use a different delimiter char (e.g., '_') for the non-primary delimiters to better reflect invariants.
var tokens = new List<TemplateToken> { TemplateToken.Constant("USER"), TemplateToken.PrimaryDelimiter('#'), TemplateToken.Property("UserId"), - TemplateToken.Delimiter('#'), + TemplateToken.Delimiter('_'), TemplateToken.Constant("POST"), - TemplateToken.Delimiter('#'), + TemplateToken.Delimiter('_'), TemplateToken.Property("PostId") };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs(1 hunks)src/CompositeKey.Analyzers.Common/Tokenization/TemplateStringTokenizer.cs(2 hunks)src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs(1 hunks)src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs(1 hunks)src/CompositeKey.Analyzers.UnitTests/Analyzers/TemplateStringAnalyzerTests.cs(1 hunks)src/CompositeKey.Analyzers.UnitTests/Infrastructure/CompositeKeyAnalyzerTestBase.cs(0 hunks)src/CompositeKey.Analyzers/Analyzers/TemplateStringAnalyzer.cs(1 hunks)src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs(3 hunks)
💤 Files with no reviewable changes (1)
- src/CompositeKey.Analyzers.UnitTests/Infrastructure/CompositeKeyAnalyzerTestBase.cs
🧰 Additional context used
🧬 Code graph analysis (6)
src/CompositeKey.Analyzers.UnitTests/Analyzers/TemplateStringAnalyzerTests.cs (2)
src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs (16)
Fact(11-22)Fact(47-69)Fact(71-89)Fact(91-114)Fact(119-144)Fact(146-170)Fact(172-192)Fact(194-214)Fact(216-234)Fact(239-257)Fact(259-276)Fact(278-294)Fact(296-308)Fact(310-322)Fact(324-336)Fact(338-350)src/CompositeKey.Analyzers.UnitTests/Infrastructure/CompositeKeyAnalyzerTestBase.cs (2)
CompositeKeyAnalyzerTestBase(12-45)CompositeKeyAnalyzerTestBase(18-28)
src/CompositeKey.Analyzers/Analyzers/TemplateStringAnalyzer.cs (3)
src/CompositeKey.Analyzers/Infrastructure/CompositeKeyAnalyzerBase.cs (2)
CompositeKeyAnalyzerBase(14-153)AttributeData(70-76)src/CompositeKey.Analyzers.Common/Diagnostics/DiagnosticDescriptors.cs (1)
DiagnosticDescriptors(10-101)src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (9)
TemplateValidation(8-165)HasValidTemplateStructure(79-82)ValidatePartitionAndSortKeyStructure(84-107)TemplateValidationResult(18-21)TemplateValidationResult(23-28)TemplateValidationResult(33-43)TemplateValidationResult(45-58)TemplateValidationResult(60-77)TemplateValidationResult(115-164)
src/CompositeKey.Analyzers.Common/Tokenization/TemplateStringTokenizer.cs (2)
src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs (4)
TemplateToken(5-5)TemplateToken(7-7)TemplateToken(9-9)TemplateToken(11-11)src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (1)
TokenizeResult(109-113)
src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs (2)
src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (3)
TemplateValidation(8-165)HasValidTemplateStructure(79-82)ValidatePartitionAndSortKeyStructure(84-107)src/CompositeKey.Analyzers.Common/Diagnostics/DiagnosticDescriptors.cs (1)
DiagnosticDescriptors(10-101)
src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs (3)
src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (3)
TemplateValidation(8-165)HasValidTemplateStructure(79-82)ValidatePartitionAndSortKeyStructure(84-107)src/CompositeKey.Analyzers.Common/Diagnostics/DiagnosticDescriptors.cs (1)
DiagnosticDescriptors(10-101)src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs (4)
TemplateToken(5-5)TemplateToken(7-7)TemplateToken(9-9)TemplateToken(11-11)
src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (4)
src/CompositeKey.Analyzers.Common/Diagnostics/DiagnosticDescriptors.cs (1)
DiagnosticDescriptors(10-101)src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs (4)
List(120-128)List(130-273)List(275-313)List(315-377)src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs (4)
TemplateToken(5-5)TemplateToken(7-7)TemplateToken(9-9)TemplateToken(11-11)src/CompositeKey.Analyzers.Common/Tokenization/TemplateStringTokenizer.cs (4)
TokenizeResult(7-54)TokenizeResult(97-97)TokenizeResult(98-98)TemplateStringTokenizer(3-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (8)
src/CompositeKey.Analyzers.Common/Tokenization/TemplateToken.cs (1)
1-29: LGTM: namespace move and token records are clean.No functional concerns found.
src/CompositeKey.Analyzers.Common/Tokenization/TemplateStringTokenizer.cs (2)
37-47: Good: single primary delimiter enforcement.Explicitly failing on a second occurrence of the primary separator keeps tokenisation deterministic and simplifies downstream validation.
3-6: Add or verify LangVersion ≥ 12 in project settingsAll projects currently inherit the default C# version (no explicit LangVersion), and CompositeKey.Analyzers.Common targets netstandard2.0, which on older SDKs defaults to C# 7.3/8.0. Without a global LangVersion override, C# 12 primary constructors will break the build. Add to Directory.Build.props:
<PropertyGroup> <LangVersion>12</LangVersion> </PropertyGroup>—or ensure global.json points to an SDK that defaults to C# 12 or higher.
src/CompositeKey.SourceGeneration/SourceGenerator.Parser.cs (1)
137-151: Tokenisation and separator validation routing looks correct.Deconstructing TokenizeResult and delegating 0006 via TemplateValidation keeps the parser lean.
src/CompositeKey.Analyzers.Common/Validation/TemplateValidation.cs (2)
109-113: LGTM: tokenisation entry-point is clean and efficient.AsSpan() avoids intermediate strings; constructor injection of the separator is clear.
115-164: Validation pipeline order and diagnostics mapping look solid.Early exits, specific 0006 when separator is missing, and 0007 for property issues are correct.
src/CompositeKey.Analyzers.Common.UnitTests/Validation/TemplateValidationTests.cs (2)
296-308: Good: multiple primary separators correctly fail tokenisation.This guards against ambiguous composite key structures.
562-579: End-to-end happy-path coverage is comprehensive.Valid composite key with formats and separator is covered; nice.
Updates the Roslyn Analyzer with new diagnostics to report more errors in near real-time in the IDE.
This commit adds support for detecting and reporting the following diagnostics;
Summary by CodeRabbit