diff --git a/src/EditorFeatures/CSharpTest/AddBraces/AddBracesTests.cs b/src/EditorFeatures/CSharpTest/AddBraces/AddBracesTests.cs index 56be0c1cc06c4a57f420900503ea55e5f87e0db1..6cf4e9573c06ec3d34f6d086b3a6dd2d6045dcdd 100644 --- a/src/EditorFeatures/CSharpTest/AddBraces/AddBracesTests.cs +++ b/src/EditorFeatures/CSharpTest/AddBraces/AddBracesTests.cs @@ -1369,6 +1369,297 @@ static void Main() new TestParameters(options: Option(CSharpCodeStyleOptions.PreferBraces, (PreferBracesPreference)bracesPreference, NotificationOption.Silent))); } + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None)] + [InlineData((int)PreferBracesPreference.WhenMultiline)] + [InlineData((int)PreferBracesPreference.Always)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task DoNotFireForIfElseWithIntercedingDirectiveInBoth(int bracesPreference) + { + await TestMissingInRegularAndScriptAsync( + @" +#define test +class Program +{ + static void Main() + { + [|if (true) +#if test + return; + else|] +#endif + return; + } +}", + new TestParameters(options: Option(CSharpCodeStyleOptions.PreferBraces, (PreferBracesPreference)bracesPreference, NotificationOption.Silent))); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, false)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task OnlyFireForIfWithIntercedingDirectiveInElseAroundIf(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { +#if test + [|if (true) + return; + else|] +#endif + return; + } +}", + + @" +#define test +class Program +{ + static void Main() + { +#if test + if (true) + { + return; + } + else +#endif + return; + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, false)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task OnlyFireForElseWithIntercedingDirectiveInIfAroundElse(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { + [|if (true) +#if test + return; + else|] + return; +#endif + } +}", + + @" +#define test +class Program +{ + static void Main() + { + if (true) +#if test + return; + else + { + return; + } +#endif + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, false)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task OnlyFireForElseWithIntercedingDirectiveInIf(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { +#if test + [|if (true) +#endif + return; + else|] + return; + } +}", + + @" +#define test +class Program +{ + static void Main() + { +#if test + if (true) +#endif + return; + else + { + return; + } + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, false)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task OnlyFireForIfWithIntercedingDirectiveInElse(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { + [|if (true) + return; + else|] +#if test + return; +#endif + } +}", + + @" +#define test +class Program +{ + static void Main() + { + if (true) + { + return; + } + else +#if test + return; +#endif + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, true)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task FireForIfElseWithDirectiveAroundIf(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { +#if test + [|if (true) + return; +#endif + else|] + { + return; + } + } +}", + + @" +#define test +class Program +{ + static void Main() + { +#if test + if (true) + { + return; + } +#endif + else + { + return; + } + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, true)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task FireForIfElseWithDirectiveAroundElse(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { + [|if (true) + { + return; + } +#if test + else|] + return; +#endif + } +}", + + @" +#define test +class Program +{ + static void Main() + { + if (true) + { + return; + } +#if test + else + { + return; + } +#endif + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] [InlineData((int)PreferBracesPreference.None, false)] [InlineData((int)PreferBracesPreference.WhenMultiline, false)] @@ -1408,6 +1699,45 @@ static void Main() expectDiagnostic); } + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] + [InlineData((int)PreferBracesPreference.None, false)] + [InlineData((int)PreferBracesPreference.WhenMultiline, false)] + [InlineData((int)PreferBracesPreference.Always, true)] + [WorkItem(32480, "https://github.com/dotnet/roslyn/issues/32480")] + public async Task FireForIfWithDirectiveAfterEmbeddedStatement(int bracesPreference, bool expectDiagnostic) + { + await TestAsync( + @" +#define test +class Program +{ + static void Main() + { + [|if (true)|] + return; +#if test +#endif + } +}", + + @" +#define test +class Program +{ + static void Main() + { + if (true) + { + return; + } +#if test +#endif + } +}", + (PreferBracesPreference)bracesPreference, + expectDiagnostic); + } + [Theory, Trait(Traits.Feature, Traits.Features.CodeActionsAddBraces)] [InlineData((int)PreferBracesPreference.None, false)] [InlineData((int)PreferBracesPreference.WhenMultiline, false)] diff --git a/src/Features/CSharp/Portable/AddBraces/CSharpAddBracesDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/AddBraces/CSharpAddBracesDiagnosticAnalyzer.cs index 2dcc45b9e64aea1e4ae232cf5c925bb6bf192229..37d84033e23fb14c21bbcddc5d72c122ea7ee591 100644 --- a/src/Features/CSharp/Portable/AddBraces/CSharpAddBracesDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/AddBraces/CSharpAddBracesDiagnosticAnalyzer.cs @@ -1,11 +1,13 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Diagnostics; +using System.Threading; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.CSharp.CodeStyle; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; using FormattingRangeHelper = Microsoft.CodeAnalysis.CSharp.Utilities.FormattingRangeHelper; namespace Microsoft.CodeAnalysis.CSharp.Diagnostics.AddBraces @@ -44,6 +46,7 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context) var cancellationToken = context.CancellationToken; var optionSet = context.Options.GetDocumentOptionSetAsync(statement.SyntaxTree, cancellationToken).GetAwaiter().GetResult(); + if (optionSet == null) { return; @@ -99,7 +102,7 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context) return; } - if (statement.ContainsInterleavedDirective(cancellationToken)) + if (ContainsInterleavedDirective(statement, embeddedStatement, cancellationToken)) { return; } @@ -114,6 +117,31 @@ public void AnalyzeNode(SyntaxNodeAnalysisContext context) SyntaxFacts.GetText(firstToken.Kind()))); } + /// + /// Check if there are interleaved directives on the statement. + /// Handles special case with if/else. + /// + /// + /// + /// + /// + private static bool ContainsInterleavedDirective(SyntaxNode statement, StatementSyntax embeddedStatement, CancellationToken cancellationToken) + { + if (statement.Kind() == SyntaxKind.IfStatement) + { + var ifStatementNode = (IfStatementSyntax)statement; + var elseNode = ifStatementNode.Else; + if (elseNode != null && !embeddedStatement.IsMissing) + { + // For IF/ELSE statements, only the IF part should be checked for interleaved directives when the diagnostic is triggered on the IF. + // A separate diagnostic will be triggered to handle the ELSE part. + var ifStatementSpanWithoutElse = TextSpan.FromBounds(statement.Span.Start, embeddedStatement.Span.End); + return statement.ContainsInterleavedDirective(ifStatementSpanWithoutElse, cancellationToken); + } + } + return statement.ContainsInterleavedDirective(cancellationToken); + } + /// /// In general, statements are considered multiline if any of the following span more than one line: /// diff --git a/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs index 8202c370af6c451b74b2471a59dcfcd7433e2a1e..d5ba56196c4a7772567cc531ff85ae249ab12602 100644 --- a/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs +++ b/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs @@ -254,6 +254,17 @@ public static bool IsAnyLambdaOrAnonymousMethod(this SyntaxNode node) public static bool ContainsInterleavedDirective(this SyntaxNode syntaxNode, CancellationToken cancellationToken) => CSharpSyntaxFactsService.Instance.ContainsInterleavedDirective(syntaxNode, cancellationToken); + /// + /// Similar to except that the span to check + /// for interleaved directives can be specified separately to the node passed in. + /// + /// + /// + /// + /// + public static bool ContainsInterleavedDirective(this SyntaxNode syntaxNode, TextSpan span, CancellationToken cancellationToken) + => CSharpSyntaxFactsService.Instance.ContainsInterleavedDirective(span, syntaxNode, cancellationToken); + public static bool ContainsInterleavedDirective( this SyntaxToken token, TextSpan textSpan, diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs index ec827264f083f5b7328f0cf1ce8aed91f6be0c39..47fee3f19bc7cd2e3a4e4d78bc5138aa7ec988bd 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs @@ -411,7 +411,7 @@ public ImmutableArray GetFileBanner(SyntaxToken firstToken) public bool ContainsInterleavedDirective(SyntaxNode node, CancellationToken cancellationToken) => ContainsInterleavedDirective(node.Span, node, cancellationToken); - private bool ContainsInterleavedDirective( + public bool ContainsInterleavedDirective( TextSpan span, SyntaxNode node, CancellationToken cancellationToken) { foreach (var token in node.DescendantTokens())