Merged
Conversation
- Implemented 8 diagnostic rules (OPENTXT001-008) for detecting inefficient string patterns - Created analyzers for: Substring, Split, string concatenation, IndexOf+Substring, Trim+Equals - Added code fix provider for Split patterns - Comprehensive test suite with smart detection tests to prevent false positives - Extensive documentation: README, EXAMPLES, CONTRIBUTING, TESTING_STRATEGY, QUICK_REFERENCE - Integrated into solution with proper NuGet package configuration - Analyzers use semantic analysis for type safety and context-aware suggestions
- Implemented 5 analyzers: Substring, Split, StringConcatenation, IndexOfSubstring, TrimEquals - Created 8 diagnostic rules (OPENTXT001-008) with proper categorization - Added SplitCodeFixProvider with automatic fixes - Built comprehensive test suite (37/41 tests passing - 90%) - Added project reference and assembly references for extension methods - Configured release tracking files for RS2008/RS1032 compliance - Added documentation and examples for analyzer usage
Modernized code to use C# 12 features (collection expressions, raw string literals, `field` keyword), refactored extension methods for `ReadOnlySpan<char>`, and improved enum utilities. Updated all project and test files to target net10.0 and upgraded NuGet dependencies. Enhanced analyzers and code fixes with modern idioms, improved test consistency, and performed general code cleanup. Bumped package version to 10.0.0 for major changes.
Replaced FluentAssertions with custom extension methods in AssertionExtensions.cs that provide a similar API using xUnit's Assert. Removed the FluentAssertions package and related global using. This reduces dependencies and keeps assertion syntax familiar.
Refactored assertion helper structs to use primary constructors, removing redundant fields and constructors. Updated analyzer and code fix provider classes to use collection expressions for ImmutableArray properties. Suppressed RS1038 warning in the project file. These changes improve code clarity and leverage modern C# syntax.
Refactored AnalyzeExpressionBody to take no parameters and updated all usages accordingly. Removed the unused context parameter from FindSubstringUsage and updated all call sites. These changes simplify method signatures and improve code clarity.
Added two Equals extension method overloads to TextExtensions, enabling Span<char> and ReadOnlySpan<char> comparisons with nullable strings using StringComparison. These handle null strings safely and delegate to existing span-based logic.
Correct TrimmedEquals null/empty handling for StringSegment overloads. Update tests to use correct overloads and add negative cases for both char and span trimming.
- Introduce StringComparablePatternMatchingSuppressor to hide IDE pattern matching suggestions (IDE0078, IDE0083, IDE0260, RCS1246) for StringComparable/SpanComparable types, preventing false positives. - Add manual verification guide and automated config test for the suppressor. - Update XML docs for StringComparable/SpanComparable to warn about pattern matching limitations and recommend the analyzer. - Add PatternMatchingAssumptionTests to document .NET/IDE behavior and limitations. - Reference analyzer in test project to ensure suppressor is active during tests. - Clarify OPENTXT002/OPENTXT008 diagnostics and update EXAMPLES.md/README.md to better explain allocation behavior and alternatives for SplitAsSegments and SplitToEnumerable. - Add full test suites for EnumValueCaseIgnored<T> and StringBuilderHelper. - Clean up usings, comments, and performance notes; ensure analyzer suppressions are respected in tests.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds Roslyn analyzer support to the Open.Text library to detect inefficient string manipulation patterns and suggest modern alternatives using spans and string segments. The PR also includes significant refactoring of the FirstSplit API, updates to test infrastructure, modernization with C# 13 features, and comprehensive documentation for the new analyzer functionality.
Changes:
- Adds new Open.Text.Analyzers project with 8 analyzers detecting common string performance anti-patterns (Substring, Split, string concatenation in loops, etc.)
- Refactors FirstSplit API with breaking changes to method signatures and improved overload consistency
- Removes FluentAssertions dependency in favor of custom lightweight assertion extensions
- Updates target frameworks and package versions (including some that don't exist yet)
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| Source/Open.Text.csproj | Updates target frameworks to include net10.0 and bumps version to 10.0.0; updates package dependencies |
| Source/Extensions.Split.cs | Major refactoring of FirstSplit API with signature changes and improved overload structure |
| Source/StringComparable.cs | Adds comprehensive documentation about pattern matching limitations |
| Source/EnumValue.cs | Modernizes to use C# 13 field keyword for implicit property backing fields |
| Tests/Open.Text.Tests.csproj | Removes FluentAssertions, adds analyzer project reference, updates packages, removes net6.0 target |
| Tests/AssertionExtensions.cs | New lightweight replacement for FluentAssertions functionality |
| Tests/StringBuilderHelperTests.cs | New comprehensive test suite with 217 lines of tests |
| Analyzers/Open.Text.Analyzers.csproj | New analyzer project with NuGet packaging configuration |
| Analyzers/*.cs | 8 new analyzer implementations with code fix providers |
| Analyzers.Tests/*.cs | Comprehensive test coverage for all analyzers |
| README.md | Documents new Roslyn analyzer features |
| Benchmarks/Open.Text.Benchmarks.csproj | Updates to net10.0 target and modernizes with collection expressions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Simplify logic by moving IndexOf call checks into if-conditions and reducing nested blocks. Streamline diagnostic reporting and invocation iteration for improved readability and maintainability without changing analyzer behavior.
Refactor several Roslyn analyzer classes to improve readability and robustness. Changes include restructuring conditional logic, adopting early returns, and reducing nesting. These updates clarify checks for method names and types, streamline pattern detection in SplitAnalyzer, and simplify variable reference logic in StringConcatenationAnalyzer. No functional behavior is changed; code is now easier to read and maintain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.