From 8bfe17ac260b3a253f812f0574a521e7401126aa Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 7 Sep 2023 13:37:07 -0700 Subject: [PATCH] Fix 'use compound coalesce' analyzer in the presence of pp directives --- ...undCoalesceAssignmentDiagnosticAnalyzer.cs | 23 ++++------- ...mpoundCoalesceAssignmentCodeFixProvider.cs | 2 - .../UseCompoundCoalesceAssignmentTests.cs | 41 ++++++++++++++++++- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentDiagnosticAnalyzer.cs index b2eb81cbe4a35..7c276f8caa364 100644 --- a/src/Analyzers/CSharp/Analyzers/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentDiagnosticAnalyzer.cs @@ -132,7 +132,7 @@ private void AnalyzeIfStatement(SyntaxNodeAnalysisContext context) if (ifStatement.Else != null) return; - if (!GetWhenTrueAssignment(ifStatement, out _, out var assignment)) + if (!GetWhenTrueAssignment(ifStatement, out var whenTrue, out var assignment)) return; if (!IsReferenceEqualsNullCheck(semanticModel, ifStatement.Condition, cancellationToken, out var testedExpression)) @@ -153,24 +153,15 @@ private void AnalyzeIfStatement(SyntaxNodeAnalysisContext context) return; } - if (ifStatement.Statement is BlockSyntax block) - { - // Single is safe here as GetWhenTrueAssignment will return null if we have a block without a single - // statement in it. - var firstStatement = block.Statements.Single(); - - // Don't want to offer anything if our if-statement body has any conditional directives in it. That - // means there's some other code that may run under some other conditions, that we do not want to now - // run conditionally outside of the 'if' statement itself. - if (firstStatement.GetLeadingTrivia().Any(t => t.HasStructure && t.GetStructure() is ConditionalDirectiveTriviaSyntax)) - return; - } + // Don't want to offer anything if our if-statement body has any conditional directives in it. That + // means there's some other code that may run under some other conditions, that we do not want to now + // run conditionally outside of the 'if' statement itself. + if (whenTrue.GetLeadingTrivia().Any(static t => t.GetStructure() is ConditionalDirectiveTriviaSyntax)) + return; + // pointers cannot use ??= if (semanticModel.GetTypeInfo(testedExpression, cancellationToken).Type is IPointerTypeSymbol or IFunctionPointerTypeSymbol) - { - // pointers cannot use ??= return; - } // Good match. context.ReportDiagnostic(DiagnosticHelper.Create( diff --git a/src/Analyzers/CSharp/CodeFixes/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentCodeFixProvider.cs index 50fe8a45e3cf1..eaf13b91bee28 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseCompoundAssignment/CSharpUseCompoundCoalesceAssignmentCodeFixProvider.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Collections.Immutable; using System.Composition; using System.Diagnostics.CodeAnalysis; @@ -13,7 +12,6 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.CSharp.UseCoalesceExpression; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; diff --git a/src/Analyzers/CSharp/Tests/UseCompoundAssignment/UseCompoundCoalesceAssignmentTests.cs b/src/Analyzers/CSharp/Tests/UseCompoundAssignment/UseCompoundCoalesceAssignmentTests.cs index 0feba99d17c85..bf479f677e23c 100644 --- a/src/Analyzers/CSharp/Tests/UseCompoundAssignment/UseCompoundCoalesceAssignmentTests.cs +++ b/src/Analyzers/CSharp/Tests/UseCompoundAssignment/UseCompoundCoalesceAssignmentTests.cs @@ -4,7 +4,6 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.CSharp.Shared.Extensions; using Microsoft.CodeAnalysis.CSharp.UseCompoundAssignment; using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; using Microsoft.CodeAnalysis.Test.Utilities; @@ -897,6 +896,46 @@ static void Main(object o) """); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/63552")] + public async Task TestIfStatementWithPreprocessorBlock7() + { + await TestMissingAsync( + """ + using System; + class C + { + static void Main(object o) + { + if (o is null) + #if true + o = ""; + #endif + } + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/63552")] + public async Task TestIfStatementWithPreprocessorBlock8() + { + await TestMissingAsync( + """ + using System; + class C + { + static void Main(object o) + { + if (o is null) + #if true + o = ""; + #else + o = ""; + #endif + } + } + """); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62473")] public async Task TestPointerCannotUseCoalesceAssignment() {