-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Proposal] Use Span-based methods to split strings in analyzers #50581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This PR is targeting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like there's no test project for Analyzer.Utilities projects, so I'm not sure where to add unit tests.
return Split(source, destination, separatorSpan, options); | ||
} | ||
|
||
public static int Split(this ReadOnlySpan<char> source, Span<Range> destination, ReadOnlySpan<char> separator, StringSplitOptions options = StringSplitOptions.None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel uneasy about this method taking a StringSplitOptions parameter and silently ignoring unimplemented options: currently StringSplitOptions.TrimEntries, but perhaps others in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TrimEntries
is not available for netstandard2.0
which is the TF that analyzers target, so I'm not sure it should support this. And I believe there's no reason for that as there's MemoryExtensions
, but since it cannot be used in analyzers my intention was to provide an internal
helper with similar Split
methods to reduce allocations.
Hello @stephentoub! May I ask you to review this please? Thank you! |
In .NET 8+ there are
MemoryExtensions.Split
methods to split strings that areSpan
-based, but they are unavailable for analyzers since they targetnetstandard2.0
. What about providing someSpan
-based helpers that could be used in analyzers?Changes
Added
SpanExtensions
static class with these methods as simplified versions ofMemoryExtensions.Split
:Replaced
string.Split
calls in a few places with one of the methods above. This should not only avoid allocatingstring[]
array in each case, but this also allows to reduce allocations further, for example, if it's needed to callTrim
on string parts or slice those parts. So that parts can be "materialized" into astring
only if it's necessary, e.g. to pass to an API that takes astring
.