diff --git a/CHANGELOG.md b/CHANGELOG.md index 45106a764b..aee9304f6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add analyzer "Unnecessary null coalescing" ([RCS1269](https://josefpihrt.github.io/docs/roslynator/analyzers/RCS1269)) ([#1826](https://github.com/dotnet/roslynator/pull/1826)) - Add `roslyn5.0` NuGet package flavor (`analyzers/dotnet/roslyn5.0/cs`) ([PR](https://github.com/dotnet/roslynator/pull/1787)) ### Breaking diff --git a/src/Analyzers.CodeFixes/CSharp/CodeFixes/UnnecessaryNullCoalescingCodeFixProvider.cs b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UnnecessaryNullCoalescingCodeFixProvider.cs new file mode 100644 index 0000000000..eff67eb38d --- /dev/null +++ b/src/Analyzers.CodeFixes/CSharp/CodeFixes/UnnecessaryNullCoalescingCodeFixProvider.cs @@ -0,0 +1,97 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; +using Roslynator.CodeFixes; +using Roslynator.CSharp.Refactorings; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Roslynator.CSharp.CodeFixes; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(UnnecessaryNullCoalescingCodeFixProvider))] +[Shared] +public sealed class UnnecessaryNullCoalescingCodeFixProvider : BaseCodeFixProvider +{ + public override ImmutableArray FixableDiagnosticIds + { + get { return ImmutableArray.Create(DiagnosticIdentifiers.UnnecessaryNullCoalescing); } + } + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode root = await context.GetSyntaxRootAsync().ConfigureAwait(false); + + if (!TryFindFirstAncestorOrSelf( + root, + context.Span, + out SyntaxNode node, + predicate: f => f.IsKind(SyntaxKind.CoalesceExpression, SyntaxKind.CoalesceAssignmentExpression))) + { + return; + } + + Diagnostic diagnostic = context.Diagnostics[0]; + Document document = context.Document; + + CodeAction codeAction = CodeAction.Create( + "Remove unnecessary null coalescing", + ct => RefactorAsync(document, node, ct), + GetEquivalenceKey(diagnostic)); + + context.RegisterCodeFix(codeAction, diagnostic); + } + + private static Task RefactorAsync(Document document, SyntaxNode node, CancellationToken cancellationToken) + { + if (node is BinaryExpressionSyntax coalesceExpression) + { + return SimplifyCoalesceExpressionRefactoring.RefactorAsync( + document, + coalesceExpression, + coalesceExpression.Right, + cancellationToken); + } + + var assignment = (AssignmentExpressionSyntax)node; + + if (assignment.Parent is ExpressionStatementSyntax expressionStatement) + { + if (expressionStatement.IsEmbedded()) + { + return document.ReplaceNodeAsync( + expressionStatement, + Block().WithTriviaFrom(expressionStatement).WithFormatterAnnotation(), + cancellationToken); + } + + return document.RemoveStatementAsync(expressionStatement, cancellationToken); + } + + return ReplaceAssignmentWithLeftAsync(document, assignment, cancellationToken); + } + + private static Task ReplaceAssignmentWithLeftAsync( + Document document, + AssignmentExpressionSyntax assignment, + CancellationToken cancellationToken) + { + IEnumerable trivia = assignment.DescendantTrivia( + TextSpan.FromBounds(assignment.OperatorToken.FullSpan.Start, assignment.Right.FullSpan.End)); + + ExpressionSyntax newNode = assignment.Left + .WithTrailingTrivia(trivia) + .Parenthesize() + .WithFormatterAnnotation(); + + return document.ReplaceNodeAsync(assignment, newNode, cancellationToken); + } +} diff --git a/src/Analyzers.xml b/src/Analyzers.xml index ec2723ed61..18a9809dbd 100644 --- a/src/Analyzers.xml +++ b/src/Analyzers.xml @@ -7338,6 +7338,65 @@ string s = """ + + RCS1269 + UnnecessaryNullCoalescing + Unnecessary null coalescing + Info + true + true + 8.0 + + + + + + + + + + + This analyzer uses nullable flow analysis. A non-nullable field or property used with `??=` for lazy initialization should be declared as `T?`. + RCS9001 UsePatternMatching diff --git a/src/Analyzers/CSharp/Analysis/UnnecessaryNullCoalescingAnalyzer.cs b/src/Analyzers/CSharp/Analysis/UnnecessaryNullCoalescingAnalyzer.cs new file mode 100644 index 0000000000..02a20bdfdb --- /dev/null +++ b/src/Analyzers/CSharp/Analysis/UnnecessaryNullCoalescingAnalyzer.cs @@ -0,0 +1,166 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; + +namespace Roslynator.CSharp.Analysis; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class UnnecessaryNullCoalescingAnalyzer : BaseDiagnosticAnalyzer +{ + private static ImmutableArray _supportedDiagnostics; + + public override ImmutableArray SupportedDiagnostics + { + get + { + if (_supportedDiagnostics.IsDefault) + Immutable.InterlockedInitialize(ref _supportedDiagnostics, DiagnosticRules.UnnecessaryNullCoalescing); + + return _supportedDiagnostics; + } + } + + public override void Initialize(AnalysisContext context) + { + base.Initialize(context); + + context.RegisterCompilationStartAction(startContext => + { + if (((CSharpCompilation)startContext.Compilation).LanguageVersion < LanguageVersion.CSharp8) + return; + + startContext.RegisterSyntaxNodeAction(f => AnalyzeCoalesceExpression(f), SyntaxKind.CoalesceExpression); + startContext.RegisterSyntaxNodeAction(f => AnalyzeCoalesceAssignmentExpression(f), SyntaxKind.CoalesceAssignmentExpression); + }); + } + + private static void AnalyzeCoalesceExpression(SyntaxNodeAnalysisContext context) + { + var coalesceExpression = (BinaryExpressionSyntax)context.Node; + + if (coalesceExpression.SpanContainsDirectives()) + return; + + ExpressionSyntax left = coalesceExpression.Left; + ExpressionSyntax right = coalesceExpression.Right; + + if (left?.IsMissing != false || right?.IsMissing != false) + return; + + if (!IsNullableAnnotationsEnabled(context)) + return; + + ExpressionSyntax coalesced = coalesceExpression.WalkUpParentheses(); + + if (coalesced.Parent is BinaryExpressionSyntax outerCoalesce + && outerCoalesce.IsKind(SyntaxKind.CoalesceExpression) + && outerCoalesce.Right == coalesced + && HasNotNullReferenceFlow(outerCoalesce.Left, context.SemanticModel, context.CancellationToken)) + { + return; + } + + if (!IsUnnecessaryNullCoalescingLeft(left, context.SemanticModel, context.CancellationToken)) + return; + + ReportDiagnostic(context, coalesceExpression.SyntaxTree, coalesceExpression.OperatorToken, right); + } + + private static void AnalyzeCoalesceAssignmentExpression(SyntaxNodeAnalysisContext context) + { + var assignment = (AssignmentExpressionSyntax)context.Node; + + if (assignment.SpanContainsDirectives()) + return; + + ExpressionSyntax left = assignment.Left; + ExpressionSyntax right = assignment.Right; + + if (left?.IsMissing != false || right?.IsMissing != false) + return; + + if (!IsNullableAnnotationsEnabled(context)) + return; + + if (!IsUnnecessaryNullCoalescingLeft(left, context.SemanticModel, context.CancellationToken)) + return; + + ReportDiagnostic(context, assignment.SyntaxTree, assignment.OperatorToken, right); + } + + private static bool IsNullableAnnotationsEnabled(SyntaxNodeAnalysisContext context) + { + return (context.SemanticModel.GetNullableContext(context.Node.SpanStart) & NullableContext.AnnotationsEnabled) != 0; + } + + private static bool IsUnnecessaryNullCoalescingLeft( + ExpressionSyntax left, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + ExpressionSyntax walkedLeft = left.WalkDownParentheses(); + + if (IsHandledBySimplifyCoalesceExpression(walkedLeft, semanticModel, cancellationToken)) + return false; + + return HasNotNullReferenceFlow(left, semanticModel, cancellationToken); + } + + private static bool HasNotNullReferenceFlow( + ExpressionSyntax expression, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + TypeInfo typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); + + if (typeInfo.Nullability.FlowState != NullableFlowState.NotNull) + return false; + + ITypeSymbol type = typeInfo.Type; + + return type?.IsErrorType() == false + && type.IsReferenceType; + } + + private static bool IsHandledBySimplifyCoalesceExpression( + ExpressionSyntax left, + SemanticModel semanticModel, + CancellationToken cancellationToken) + { + switch (left.Kind()) + { + case SyntaxKind.ObjectCreationExpression: + case SyntaxKind.AnonymousObjectCreationExpression: + case SyntaxKind.ArrayCreationExpression: + case SyntaxKind.ImplicitArrayCreationExpression: + case SyntaxKind.InterpolatedStringExpression: + case SyntaxKind.ThisExpression: + case SyntaxKind.StringLiteralExpression: + case SyntaxKind.TypeOfExpression: + return true; + } + + Optional optional = semanticModel.GetConstantValue(left, cancellationToken); + + return optional.HasValue + && optional.Value is not null; + } + + private static void ReportDiagnostic( + SyntaxNodeAnalysisContext context, + SyntaxTree syntaxTree, + SyntaxToken operatorToken, + ExpressionSyntax right) + { + DiagnosticHelpers.ReportDiagnostic( + context, + DiagnosticRules.UnnecessaryNullCoalescing, + Location.Create(syntaxTree, TextSpan.FromBounds(operatorToken.SpanStart, right.Span.End))); + } +} diff --git a/src/Common/DiagnosticIdentifiers.Generated.cs b/src/Common/DiagnosticIdentifiers.Generated.cs index 7d6e59085e..9550dc2cdb 100644 --- a/src/Common/DiagnosticIdentifiers.Generated.cs +++ b/src/Common/DiagnosticIdentifiers.Generated.cs @@ -280,5 +280,6 @@ public static partial class DiagnosticIdentifiers public const string UseRawStringLiteral = "RCS1266"; public const string UseStringInterpolationInsteadOfStringConcat = "RCS1267"; public const string SimplifyNumericComparison = "RCS1268"; + public const string UnnecessaryNullCoalescing = "RCS1269"; } } \ No newline at end of file diff --git a/src/Common/DiagnosticRules.Generated.cs b/src/Common/DiagnosticRules.Generated.cs index aa33f46444..80a7c9ce3e 100644 --- a/src/Common/DiagnosticRules.Generated.cs +++ b/src/Common/DiagnosticRules.Generated.cs @@ -3323,5 +3323,17 @@ public static partial class DiagnosticRules helpLinkUri: DiagnosticIdentifiers.SimplifyNumericComparison, customTags: []); + /// RCS1269 + public static readonly DiagnosticDescriptor UnnecessaryNullCoalescing = DiagnosticDescriptorFactory.Create( + id: DiagnosticIdentifiers.UnnecessaryNullCoalescing, + title: "Unnecessary null coalescing", + messageFormat: "Unnecessary null coalescing", + category: DiagnosticCategories.Roslynator, + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: null, + helpLinkUri: DiagnosticIdentifiers.UnnecessaryNullCoalescing, + customTags: WellKnownDiagnosticTags.Unnecessary); + } } \ No newline at end of file diff --git a/src/Tests/Analyzers.Tests/RCS1269UnnecessaryNullCoalescingTests.cs b/src/Tests/Analyzers.Tests/RCS1269UnnecessaryNullCoalescingTests.cs new file mode 100644 index 0000000000..45e2b0baee --- /dev/null +++ b/src/Tests/Analyzers.Tests/RCS1269UnnecessaryNullCoalescingTests.cs @@ -0,0 +1,497 @@ +// Copyright (c) .NET Foundation and Contributors. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Roslynator.CSharp.CodeFixes; +using Roslynator.Testing.CSharp; +using Xunit; + +namespace Roslynator.CSharp.Analysis.Tests; + +public class RCS1269UnnecessaryNullCoalescingTests : AbstractCSharpDiagnosticVerifier +{ + public override DiagnosticDescriptor Descriptor { get; } = DiagnosticRules.UnnecessaryNullCoalescing; + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NonNullableParameter() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + C(string[] errors) + { + Errors = errors [|?? []|]; + } + + string[] Errors { get; } +} +", @" +#nullable enable + +class C +{ + C(string[] errors) + { + Errors = errors; + } + + string[] Errors { get; } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NonNullableField_CoalesceAssignment() + { + await VerifyDiagnosticAndFixAsync(""" +#nullable enable + +class C +{ + string _context = ""; + + void M(string completionContext) + { + _context [|??= completionContext|]; + } +} +""", """ +#nullable enable + +class C +{ + string _context = ""; + + void M(string completionContext) + { + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NonNullableProperty_CoalesceAssignment() + { + await VerifyDiagnosticAndFixAsync(""" +#nullable enable + +class C +{ + string Handler { get; set; } = ""; + + void M(C other) + { + Handler [|??= other.Handler|]; + } +} +""", """ +#nullable enable + +class C +{ + string Handler { get; set; } = ""; + + void M(C other) + { + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NonNullableLocal() + { + await VerifyDiagnosticAndFixAsync(""" +#nullable enable + +class C +{ + void M(string x) + { + string y = x [|?? ""|]; + } +} +""", """ +#nullable enable + +class C +{ + void M(string x) + { + string y = x; + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_CoalesceAssignment_Expression() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + string M(string x, string y) + { + return x [|??= y|]; + } +} +", @" +#nullable enable + +class C +{ + string M(string x, string y) + { + return x; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NullableLocal_AfterNullCheck() + { + await VerifyDiagnosticAndFixAsync(""" +#nullable enable + +class C +{ + void M(string? s) + { + if (s is not null) + { + string t = s [|?? ""|]; + } + } +} +""", """ +#nullable enable + +class C +{ + void M(string? s) + { + if (s is not null) + { + string t = s; + } + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NestedCoalesce_OuterOnly() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + void M(string? maybe, string notNull, string extra) + { + string x = (maybe ?? notNull) [|?? extra|]; + } +} +", @" +#nullable enable + +class C +{ + void M(string? maybe, string notNull, string extra) + { + string x = (maybe ?? notNull); + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_NestedCoalesce_EntireChain() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + void M(string a, string b, string c) + { + string x = a [|?? b ?? c|]; + } +} +", @" +#nullable enable + +class C +{ + void M(string a, string b, string c) + { + string x = a; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_CoalesceAssignment_EmbeddedIf() + { + await VerifyDiagnosticAndFixAsync(""" +#nullable enable + +class C +{ + string _context = ""; + + void M(bool flag, string completionContext) + { + if (flag) + _context [|??= completionContext|]; + } +} +""", """ +#nullable enable + +class C +{ + string _context = ""; + + void M(bool flag, string completionContext) + { + if (flag) + { + } + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_CoalesceAssignment_WithCoalesceRight() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + void M(string x, string? a, string b) + { + x [|??= a ?? b|]; + } +} +", @" +#nullable enable + +class C +{ + void M(string x, string? a, string b) + { + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_ClassConstraint() + { + await VerifyDiagnosticAndFixAsync(@" +#nullable enable + +class C +{ + void M(T x, T y) where T : class + { + T z = x [|?? y|]; + } +} +", @" +#nullable enable + +class C +{ + void M(T x, T y) where T : class + { + T z = x; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_NullableParameter() + { + await VerifyNoDiagnosticAsync(""" +#nullable enable + +class C +{ + void M(string? x) + { + string y = x ?? ""; + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_NullableContextOff() + { + await VerifyNoDiagnosticAsync(""" +class C +{ + void M(string x) + { + string y = x ?? ""; + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_NullableDisableRegion() + { + await VerifyNoDiagnosticAsync(""" +#nullable enable + +class C +{ + void M(string x) + { +#nullable disable + string y = x ?? ""; +#nullable restore + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task Test_ProjectLevelNullableEnable() + { + await VerifyDiagnosticAndFixAsync(""" +class C +{ + void M(string x) + { + string y = x [|?? ""|]; + } +} +""", """ +class C +{ + void M(string x) + { + string y = x; + } +} +""", options: WellKnownCSharpTestOptions.Default_NullableReferenceTypes); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_StringLiteral() + { + await VerifyNoDiagnosticAsync(""" +#nullable enable + +class C +{ + void M() + { + string y = "" ?? "a"; + } +} +"""); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_This() + { + await VerifyNoDiagnosticAsync(@" +#nullable enable + +class C +{ + void M() + { + C y = this ?? this; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_ObjectCreation() + { + await VerifyNoDiagnosticAsync(@" +#nullable enable + +class C +{ + void M() + { + C y = new C() ?? this; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_NullableValueType() + { + await VerifyNoDiagnosticAsync(@" +#nullable enable + +class C +{ + void M(int? x) + { + if (x is not null) + { + int y = x ?? 0; + } + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_NullableClassConstraint() + { + await VerifyNoDiagnosticAsync(@" +#nullable enable + +class C +{ + void M(T x, T y) where T : class? + { + T z = x ?? y; + } +} +"); + } + + [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.UnnecessaryNullCoalescing)] + public async Task TestNoDiagnostic_Directives() + { + await VerifyNoDiagnosticAsync(@" +#nullable enable + +class C +{ + void M(string x, string y, string z) + { + string a = x ?? +#if DEBUG + y +#else + z +#endif + ; + } +} +"); + } +} diff --git a/src/VisualStudioCode/package/src/configurationFiles.generated.ts b/src/VisualStudioCode/package/src/configurationFiles.generated.ts index abe06c678e..85cb83734c 100644 --- a/src/VisualStudioCode/package/src/configurationFiles.generated.ts +++ b/src/VisualStudioCode/package/src/configurationFiles.generated.ts @@ -928,6 +928,9 @@ roslynator_analyzers.enabled_by_default = true|false # Simplify numeric comparison #dotnet_diagnostic.rcs1268.severity = suggestion +# Unnecessary null coalescing +#dotnet_diagnostic.rcs1269.severity = suggestion + # Use pattern matching #dotnet_diagnostic.rcs9001.severity = silent