Use SDK 8.0 recommended analyzers #680
Conversation
📝 WalkthroughWalkthroughProject-wide analyzer settings updated; small API-surface-preserving code changes: char-based hex checks, culture-invariant formatting for numbers/dates, minor LINQ-to-length rewrites, TryAdd dictionary optimization under newer TFMs, and some classes sealed. Tests and project NoWarn entries updated. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as GetNotUniqueExponents
participant Rows as Polynom rows (enumerable)
participant Dict as Dictionary<int,bool>
Note over Caller,Rows: iterate rows
Caller->>Rows: foreach row
alt Newer TFMs
Caller->>Dict: TryAdd(row.Exponent, false)
alt TryAdd returned false
Caller->>Dict: (no-op) // exponent already exists -> tracked as duplicate via prior update
else TryAdd true
Note right of Dict: first occurrence stored as false
end
else Older TFMs
Caller->>Dict: if !ContainsKey(exp) Add(exp,false) else Dict[exp]=true
end
Note over Dict: collect keys marked true -> not-unique exponents
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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. Comment |
Shane32
added
the
refactoring
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
Directory.Build.props(1 hunks)QRCoder/Extensions/StringExtensions.cs(2 hunks)QRCoder/PayloadGenerator.cs(1 hunks)QRCoder/PayloadGenerator/BezahlCode.cs(1 hunks)QRCoder/PayloadGenerator/ContactData.cs(2 hunks)QRCoder/PayloadGenerator/Mail.cs(1 hunks)QRCoder/PayloadGenerator/RussiaPaymentOrder.cs(3 hunks)QRCoder/QRCodeGenerator.cs(1 hunks)QRCoder/SvgQRCode.cs(1 hunks)QRCoderConsole/Program.cs(1 hunks)QRCoderTests/Helpers/ShouldMatchApprovedBinaryExtensions.cs(1 hunks)QRCoderTests/QRCoderTests.csproj(1 hunks)QRCoderTests/QRGeneratorTests.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
QRCoder/SvgQRCode.cs (1)
QRCoder/Extensions/StringExtensions.cs (1)
StartsWith(40-41)
QRCoderTests/QRGeneratorTests.cs (1)
QRCoder/PayloadGenerator/Payload.cs (1)
Payload(11-33)
QRCoder/PayloadGenerator.cs (1)
QRCoder/Extensions/StringExtensions.cs (1)
ToString(66-67)
QRCoder/PayloadGenerator/RussiaPaymentOrder.cs (3)
QRCoder/PayloadGenerator/BezahlCode.cs (1)
ToString(247-328)QRCoder/PayloadGenerator/ContactData.cs (1)
ToString(115-250)QRCoder/PayloadGenerator/BitcoinLikeCryptoCurrencyAddress.cs (1)
ToString(46-65)
⏰ 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). (6)
- GitHub Check: additional-tests
- GitHub Check: Test .NET 5.0 Windows
- GitHub Check: Test .NET 6.0 Windows
- GitHub Check: Test .NET Framework 4.6.2
- GitHub Check: Test .NET Core 2.1
- GitHub Check: Test .NET Core 3.1
🔇 Additional comments (13)
QRCoderTests/QRGeneratorTests.cs (1)
605-605: LGTM! Good practice to seal the test helper class.Adding the
sealedmodifier to this private nested test class is appropriate. The class is not intended for inheritance, and sealing it follows analyzer recommendations (likely CA1852) and improves code clarity.QRCoderConsole/Program.cs (1)
13-13: LGTM! Good analyzer-driven improvement.Sealing
MainClassis appropriate since it's internal, contains only static methods, and isn't designed for inheritance. This aligns with .NET SDK 8.0 analyzer recommendations and makes the design intent explicit.QRCoderTests/QRCoderTests.csproj (1)
10-10: LGTM! Analyzer warning suppression added.The addition of CA1850 to the NoWarn list aligns with the broader analyzer configuration updates in this PR.
QRCoder/PayloadGenerator/ContactData.cs (1)
10-10: LGTM! Performance optimization by eliminating array allocations.Extracting the trim characters to a static readonly field avoids allocating a new char array on every Trim call, which is a good micro-optimization.
Also applies to: 156-156
QRCoderTests/Helpers/ShouldMatchApprovedBinaryExtensions.cs (1)
48-48: LGTM! Efficient LINQ removal.Replacing
Any()with a directLength > 0check eliminates LINQ overhead while maintaining the same logic.QRCoder/PayloadGenerator/Mail.cs (1)
43-43: LGTM! LINQ usage reduced.Replacing
Any()withCount > 0removes LINQ overhead for a simple emptiness check.QRCoder/PayloadGenerator/BezahlCode.cs (1)
228-228: LGTM! Culture-invariant comparison improved.Using
EqualswithStringComparison.OrdinalIgnoreCaseis more explicit and efficient than callingToUpperInvariant()for case-insensitive comparison.QRCoder/PayloadGenerator.cs (1)
36-36: LGTM! Culture-invariant formatting ensured.Adding
CultureInfo.InvariantCultureto theToString()call ensures consistent numeric formatting regardless of the current culture, aligning with the broader culture-invariant changes in this PR.QRCoder/PayloadGenerator/RussiaPaymentOrder.cs (2)
149-149: LGTM! Culture-invariant date formatting applied.Applying
CultureInfo.InvariantCultureto DateTime formatting ensures consistent date string output across different cultures, which is essential for payment order data.Also applies to: 159-159, 179-179, 189-189
227-227: LGTM! Minor type refinement.Marking the private nested class as
sealedis a good practice that can enable minor compiler optimizations.QRCoder/Extensions/StringExtensions.cs (1)
52-52: LGTM! Char-based StartsWith is more efficient.Replacing
StartsWith("#", StringComparison.Ordinal)withStartsWith('#')improves performance by avoiding string allocation and culture-sensitive comparison overhead. The semantic equivalence is maintained for single-character checks.QRCoder/SvgQRCode.cs (1)
327-327: LGTM! Consistent char-based prefix check.The switch from
StartsWith("#", StringComparison.Ordinal)toStartsWith('#')aligns with the new extension method and provides better performance. The semantic equivalence is preserved for this single-character check.Directory.Build.props (1)
38-44: LGTM! Modernized analyzer configuration.The changes align with the PR objectives:
CA1510 suppression (line 42): Reasonable for a multi-targeting library that can't use
ArgumentNullException.ThrowIfNullon older frameworks.AnalysisLevel update (line 44): Switching from
AnalysisModetoAnalysisLevel=8-recommendedenables version-specific analyzer rules from .NET SDK 8.0, providing more precise control over code analysis.Comment accuracy (line 38): Correctly updated to reflect the addition of throw helper warning suppression.
|
Always nice when one AI bot fixes another AI bot's mistakes :-P |
(Actually I think I wrote that line of code incorrectly...) |
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores