dotnet/runtime

Analyzer proposal: treat calls to ROS<char> op_Equality as warning

Open

#54 794 ouverte le 27 juin 2021

Voir sur GitHub
 (21 commentaires) (14 réactions) (0 assignés)C# (5 445 forks)batch import
api-approvedarea-System.Runtimecode-analyzercode-fixerhelp wanted

Métriques du dépôt

Stars
 (17 886 stars)
Métriques de merge PR
 (Merge moyen 12j 11h) (661 PRs mergées en 30 j)

Description

Description

There have been a few issues filed (https://github.com/dotnet/runtime/issues/10151, https://github.com/dotnet/runtime/issues/54793) where developers are confused as to why two ROS<char> sequences can't be compared for content equality using operator ==, like normal string instances can be.

We should consider introducing an analyzer that detects operator == and operator != calls on Span<char> and ReadOnlySpan<char> specifically and introduces a warning, also offering to fix the call sites by rewriting this in terms of SequenceEqual.

Examples

ReadOnlySpan<char> span1;
ReadOnlySpan<char> span2;

// produces warnings
if (span1 == span2) { /* ... */ }
if (span1 != span2) { /* ... */ }

// suggested fixer replacements
if (span1.SequenceEqual(span2)) { /* ... */ }
if (!span1.SequenceEqual(span2)) { /* ... */ }

ReadOnlySpan<T> span3;
ReadOnlySpan<T> span4;

// DOES NOT produce warning
if (span3 == span4) { /* ... */ }

Discussion

This analyzer assumes that the majority use case of ROS<char> is as a substitute for string segments, so these callers would want API behaviors that match the behaviors they're used to on string. However, we shouldn't assume that all callers are treating Span<char> in this fashion; as some may really want to treat these as slices into an arbitrary data buffer and maintain the existing pointer / length equality comparisons. This may affect whether a fixer is enabled by default.

Additionally, this analyzer & fixer would only capture ROS<char> and Span<char>, not any other T. If the caller is using a generic T, they're almost certainly intending this to be a data buffer slice, not stringy data.

Open questions

Should the analyzer also trap calls to MemoryExtensions.IndexOf<char>(ReadOnlySpan<char>, ReadOnlySpan<char>) and related APIs? These have different behaviors than their corresponding string methods, as shown in the below examples.

string str = GetString();
ReadOnlySpan<char> span = str;

int idx1 = str.IndexOf("hello"); // linguistic, current-culture
int idx2 = span.IndexOf("hello"); // ordinal

Related to this last point: https://github.com/dotnet/runtime/issues/43956

Guide contributeur