Skip to content

Commit

Permalink
Create DoNotCompareSpanToNullAnalyzer (#6838)
Browse files Browse the repository at this point in the history
* Add DoNotCompareSpanToNullAnalyzer

* Specify FixAllProvider.

* Update analyzer ID.

* Make fixer work for ArgumentSyntax as well.

* Improve message and description

* Also flag span comparisons to 'default'.

* Update messaging and add more tests

* Update RulesMissingDocumentation.md
  • Loading branch information
CollinAlpert authored Mar 5, 2024
1 parent 59b2d9d commit 3e60082
Show file tree
Hide file tree
Showing 24 changed files with 853 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Composition;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.NetCore.Analyzers;
using Microsoft.NetCore.Analyzers.Usage;

namespace Microsoft.NetCore.CSharp.Analyzers.Usage
{
[ExportCodeFixProvider(LanguageNames.CSharp), Shared]
public sealed class CSharpDoNotCompareSpanToNullFixer : DoNotCompareSpanToNullFixer
{
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
var condition = root.FindNode(context.Span, getInnermostNodeForTie: true);
if (condition is not BinaryExpressionSyntax binaryExpression)
{
return;
}

var useIsEmptyCodeAction = CodeAction.Create(
MicrosoftNetCoreAnalyzersResources.DoNotCompareSpanToNullIsEmptyCodeFixTitle,
_ => Task.FromResult(context.Document.WithSyntaxRoot(root.ReplaceNode(binaryExpression, MakeIsEmptyCheck(binaryExpression)))),
MicrosoftNetCoreAnalyzersResources.DoNotCompareSpanToNullIsEmptyCodeFixTitle
);
context.RegisterCodeFix(useIsEmptyCodeAction, context.Diagnostics);
}

private static SyntaxNode MakeIsEmptyCheck(BinaryExpressionSyntax binaryExpression)
{
ExpressionSyntax memberAccess = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, GetComparatorExpression(binaryExpression), SyntaxFactory.IdentifierName(IsEmpty));
if (binaryExpression.IsKind(SyntaxKind.NotEqualsExpression))
{
return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, memberAccess);
}

return memberAccess;
}

private static ExpressionSyntax GetComparatorExpression(BinaryExpressionSyntax binaryExpression)
{
return binaryExpression.Left.IsKind(SyntaxKind.NullLiteralExpression)
|| binaryExpression.Left.IsKind(SyntaxKind.DefaultLiteralExpression)
|| binaryExpression.Left.IsKind(SyntaxKind.DefaultExpression)
? binaryExpression.Right
: binaryExpression.Left;
}
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ CA1871 | Performance | Info | DoNotPassNonNullableValueToArgumentNullExceptionTh
CA2262 | Usage | Info | ProvideHttpClientHandlerMaxResponseHeaderLengthValueCorrectly, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2262)
CA2263 | Usage | Info | PreferGenericOverloadsAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2263)
CA2264 | Usage | Warning | DoNotPassNonNullableValueToArgumentNullExceptionThrowIfNull, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2264)
CA2265 | Usage | Warning | DoNotCompareSpanToNullAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2265)
Original file line number Diff line number Diff line change
Expand Up @@ -2132,4 +2132,19 @@ Widening and user defined conversions are not supported with generic types.</val
<data name="DoNotPassNullableStructToArgumentNullExceptionThrowIfNullCodeFixTitle" xml:space="preserve">
<value>Replace the 'ArgumentNullException.ThrowIfNull' call with a conditional</value>
</data>
<data name="DoNotCompareSpanToNullOrDefaultDescription" xml:space="preserve">
<value>Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</value>
</data>
<data name="DoNotCompareSpanToNullMessage" xml:space="preserve">
<value>Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</value>
</data>
<data name="DoNotCompareSpanToDefaultMessage" xml:space="preserve">
<value>Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</value>
</data>
<data name="DoNotCompareSpanToNullOrDefaultTitle" xml:space="preserve">
<value>Do not compare Span&lt;T&gt; to 'null' or 'default'</value>
</data>
<data name="DoNotCompareSpanToNullIsEmptyCodeFixTitle" xml:space="preserve">
<value>Use 'IsEmpty'</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using Microsoft.CodeAnalysis.CodeFixes;

namespace Microsoft.NetCore.Analyzers.Usage
{
public abstract class DoNotCompareSpanToNullFixer : CodeFixProvider
{
protected const string IsEmpty = nameof(IsEmpty);

public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

public override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(DoNotCompareSpanToNullAnalyzer.RuleId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Usage
{
using static MicrosoftNetCoreAnalyzersResources;

[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class DoNotCompareSpanToNullAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA2265";

internal static readonly DiagnosticDescriptor DoNotCompareSpanToNullRule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(DoNotCompareSpanToNullOrDefaultTitle)),
CreateLocalizableResourceString(nameof(DoNotCompareSpanToNullMessage)),
DiagnosticCategory.Usage,
RuleLevel.BuildWarning,
description: CreateLocalizableResourceString(nameof(DoNotCompareSpanToNullOrDefaultDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor DoNotCompareSpanToDefaultRule = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(DoNotCompareSpanToNullOrDefaultTitle)),
CreateLocalizableResourceString(nameof(DoNotCompareSpanToDefaultMessage)),
DiagnosticCategory.Usage,
RuleLevel.BuildWarning,
description: CreateLocalizableResourceString(nameof(DoNotCompareSpanToNullOrDefaultDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(static context =>
{
var typeProvider = WellKnownTypeProvider.GetOrCreate(context.Compilation);
if (!typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemSpan1, out var spanType)
|| !typeProvider.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemReadOnlySpan1, out var readOnlySpanType))
{
return;
}
context.RegisterOperationAction(ctx => AnalyzeComparison(ctx, spanType, readOnlySpanType), OperationKind.Binary);
});

}

private static void AnalyzeComparison(OperationAnalysisContext context, INamedTypeSymbol spanType, INamedTypeSymbol readOnlySpanType)
{
var binaryOperation = (IBinaryOperation)context.Operation;
var leftOperand = binaryOperation.LeftOperand.WalkDownConversion();
var rightOperand = binaryOperation.RightOperand.WalkDownConversion();
if (rightOperand.HasNullConstantValue() && binaryOperation.LeftOperand.Type is not null && IsSpan(binaryOperation.LeftOperand.Type)
|| leftOperand.HasNullConstantValue() && binaryOperation.RightOperand.Type is not null && IsSpan(binaryOperation.RightOperand.Type))
{
context.ReportDiagnostic(binaryOperation.CreateDiagnostic(DoNotCompareSpanToNullRule));
}

if (rightOperand is IDefaultValueOperation && binaryOperation.LeftOperand.Type is not null && IsSpan(binaryOperation.LeftOperand.Type)
|| leftOperand is IDefaultValueOperation && binaryOperation.RightOperand.Type is not null && IsSpan(binaryOperation.RightOperand.Type))
{
context.ReportDiagnostic(binaryOperation.CreateDiagnostic(DoNotCompareSpanToDefaultRule));
}

bool IsSpan(ITypeSymbol typeSymbol)
{
var originalType = typeSymbol.OriginalDefinition;

return originalType.Equals(spanType, SymbolEqualityComparer.Default)
|| originalType.Equals(readOnlySpanType, SymbolEqualityComparer.Default);
}
}

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DoNotCompareSpanToNullRule, DoNotCompareSpanToDefaultRule);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,31 @@ Obecné přetypování (IL unbox.any) používané sekvencí vrácenou metodou E
<target state="translated">Nevolejte ToImmutableCollection pro hodnotu ImmutableCollection</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToDefaultMessage">
<source>Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</source>
<target state="new">Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullIsEmptyCodeFixTitle">
<source>Use 'IsEmpty'</source>
<target state="new">Use 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullMessage">
<source>Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</source>
<target state="new">Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultDescription">
<source>Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</source>
<target state="new">Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultTitle">
<source>Do not compare Span&lt;T&gt; to 'null' or 'default'</source>
<target state="new">Do not compare Span&lt;T&gt; to 'null' or 'default'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCreateTaskCompletionSourceWithWrongArgumentsDescription">
<source>TaskCompletionSource has constructors that take TaskCreationOptions that control the underlying Task, and constructors that take object state that's stored in the task. Accidentally passing a TaskContinuationOptions instead of a TaskCreationOptions will result in the call treating the options as state.</source>
<target state="translated">TaskCompletionSource má konstruktory, které přijímají možnosti TaskCreationOptions určující příslušnou úlohu, a konstruktory, které přijímají stav objektu uložený v úloze. Když se místo TaskCreationOptions omylem předá TaskContinuationOptions, způsobí to, že volání bude možnosti považovat za stav.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,31 @@ Erweiterungen und benutzerdefinierte Konvertierungen werden bei generischen Type
<target state="translated">"ToImmutableCollection" nicht für einen ImmutableCollection-Wert aufrufen</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToDefaultMessage">
<source>Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</source>
<target state="new">Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullIsEmptyCodeFixTitle">
<source>Use 'IsEmpty'</source>
<target state="new">Use 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullMessage">
<source>Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</source>
<target state="new">Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultDescription">
<source>Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</source>
<target state="new">Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultTitle">
<source>Do not compare Span&lt;T&gt; to 'null' or 'default'</source>
<target state="new">Do not compare Span&lt;T&gt; to 'null' or 'default'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCreateTaskCompletionSourceWithWrongArgumentsDescription">
<source>TaskCompletionSource has constructors that take TaskCreationOptions that control the underlying Task, and constructors that take object state that's stored in the task. Accidentally passing a TaskContinuationOptions instead of a TaskCreationOptions will result in the call treating the options as state.</source>
<target state="translated">TaskCompletionSource verfügt über Konstruktoren, die TaskCreationOptions zum Steuern der zugrunde liegenden Aufgabe akzeptieren, und Konstruktoren, die den in der Aufgabe gespeicherten Objektzustand übernehmen. Das versehentliche Übergeben von TaskContinuationOptions anstelle von TaskCreationOptions führt dazu, dass der Aufruf die Optionen als Zustand behandelt.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,31 @@ La ampliación y las conversiones definidas por el usuario no se admiten con tip
<target state="translated">No llame a ToImmutableCollection en un valor ImmutableCollection</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToDefaultMessage">
<source>Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</source>
<target state="new">Comparing a span to 'default' might not do what you intended, make the code more explicit by checking 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullIsEmptyCodeFixTitle">
<source>Use 'IsEmpty'</source>
<target state="new">Use 'IsEmpty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullMessage">
<source>Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</source>
<target state="new">Comparing a span to 'null' might be redundant, the 'null' literal will be implicitly converted to a 'Span&lt;T&gt;.Empty'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultDescription">
<source>Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</source>
<target state="new">Comparing a span to 'null' or 'default' might not do what you intended. 'default' and the 'null' literal are implicitly converted to 'Span&lt;T&gt;.Empty'. Remove the redundant comparison or make the code more explicit by using 'IsEmpty'.</target>
<note />
</trans-unit>
<trans-unit id="DoNotCompareSpanToNullOrDefaultTitle">
<source>Do not compare Span&lt;T&gt; to 'null' or 'default'</source>
<target state="new">Do not compare Span&lt;T&gt; to 'null' or 'default'</target>
<note />
</trans-unit>
<trans-unit id="DoNotCreateTaskCompletionSourceWithWrongArgumentsDescription">
<source>TaskCompletionSource has constructors that take TaskCreationOptions that control the underlying Task, and constructors that take object state that's stored in the task. Accidentally passing a TaskContinuationOptions instead of a TaskCreationOptions will result in the call treating the options as state.</source>
<target state="translated">TaskCompletionSource tiene constructores que toman elementos TaskCreationOption para controlar la tarea subyacente y constructores que toman el estado del objeto que se almacena en la tarea. Si se pasa accidentalmente un elemento TaskContinuationOption en lugar de un objeto TaskCreationOption, la llamada trata las opciones como estado.</target>
Expand Down
Loading

0 comments on commit 3e60082

Please sign in to comment.