diff --git a/src/EditorFeatures/CSharpTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs b/src/EditorFeatures/CSharpTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs index d434972507e5a..359d20c44b402 100644 --- a/src/EditorFeatures/CSharpTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs +++ b/src/EditorFeatures/CSharpTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.cs @@ -821,6 +821,26 @@ IEnumerable M(int a) { yield return a != 0; } +}"); + } + + [WorkItem(36117, "https://github.com/dotnet/roslyn/issues/36117")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseConditionalExpression)] + public async Task TestMissingWhenCrossingPreprocessorDirective() + { + await TestMissingInRegularAndScriptAsync( +@" +class C +{ + int M() + { + bool check = true; +#if true + [||]if (check) + return 3; +#endif + return 2; + } }"); } } diff --git a/src/EditorFeatures/VisualBasicTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.vb b/src/EditorFeatures/VisualBasicTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.vb index 95539cc500d6b..39b37407743b1 100644 --- a/src/EditorFeatures/VisualBasicTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.vb +++ b/src/EditorFeatures/VisualBasicTest/UseConditionalExpression/UseConditionalExpressionForReturnTests.vb @@ -18,7 +18,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.UseConditionalExpr Public Async Function TestOnSimpleReturn() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -28,7 +28,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, 0, 1) end function @@ -39,7 +39,7 @@ end class") Public Async Function TestOnSimpleReturnNoBlocks() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -49,7 +49,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, 0, 1) end function @@ -60,7 +60,7 @@ end class") Public Async Function TestOnSimpleReturnNoBlocks_NotInBlock() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer if true [||]if true @@ -72,7 +72,7 @@ class end function end class", " -class +class C function M() as integer if true Return If(true, 0, 1) @@ -85,7 +85,7 @@ end class") Public Async Function TestMissingReturnValue1() As Task Await TestMissingInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -100,7 +100,7 @@ end class") Public Async Function TestMissingReturnValue2() As Task Await TestMissingInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return @@ -115,7 +115,7 @@ end class") Public Async Function TestMissingReturnValue3() As Task Await TestMissingInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return @@ -130,7 +130,7 @@ end class") Public Async Function TestWithNoElseBlockButFollowingReturn() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -140,7 +140,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, 0, 1) end function @@ -151,7 +151,7 @@ end class") Public Async Function TestMissingWithoutElse() As Task Await TestMissingInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -164,7 +164,7 @@ end class") Public Async Function TestConversion1() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as object [||]if true return ""a"" @@ -174,7 +174,7 @@ class end function end class", " -class +class C function M() as object Return If(true, ""a"", ""b"") end function @@ -185,7 +185,7 @@ end class") Public Async Function TestConversion2() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as string [||]if true return ""a"" @@ -195,7 +195,7 @@ class end function end class", " -class +class C function M() as string Return If(true, ""a"", nothing) end function @@ -206,7 +206,7 @@ end class") Public Async Function TestConversion3() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as string [||]if true return nothing @@ -216,7 +216,7 @@ class end function end class", " -class +class C function M() as string Return If(true, nothing, DirectCast(nothing, String)) end function @@ -227,7 +227,7 @@ end class") Public Async Function TestKeepTriviaAroundIf() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer ' leading [||]if true @@ -238,7 +238,7 @@ class end function end class", " -class +class C function M() as integer ' leading Return If(true, 0, 1) ' trailing @@ -250,7 +250,7 @@ end class") Public Async Function TestFixAll1() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer {|FixAllInDocument:if|} true return 0 @@ -266,7 +266,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, 0, 1) @@ -279,7 +279,7 @@ end class") Public Async Function TestMultiLine1() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return Foo( @@ -290,7 +290,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, Foo( @@ -304,7 +304,7 @@ end class") Public Async Function TestMultiLine2() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return 0 @@ -315,7 +315,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, 0, @@ -329,7 +329,7 @@ end class") Public Async Function TestMultiLine3() As Task Await TestInRegularAndScriptAsync( " -class +class C function M() as integer [||]if true return Foo( @@ -341,7 +341,7 @@ class end function end class", " -class +class C function M() as integer Return If(true, Foo( @@ -357,7 +357,7 @@ end class") Public Async Function TestOnYield() As Task Await TestInRegularAndScriptAsync( " -class +class C iterator function M() as integer [||]if true yield 0 @@ -367,7 +367,7 @@ class end function end class", " -class +class C iterator function M() as integer Yield If(true, 0, 1) end function @@ -381,7 +381,7 @@ end class") " imports system.collections.generic -class +class C iterator function M() as IEnumerable(of integer) [||]if true yield 0 @@ -393,10 +393,45 @@ end class", " imports system.collections.generic -class +class C iterator function M() as IEnumerable(of integer) Yield If(true, 0, 1) end function +end class") + End Function + + + + Public Async Function TestMissingWhenCrossingPreprocessorDirective1() As Task + Await TestMissingInRegularAndScriptAsync( +" +class C + function M() as integer + dim check as boolean = true +#if true + [||]if check + return 3 +#end if + return 2 + end function +end class") + End Function + + + + Public Async Function TestMissingWhenCrossingPreprocessorDirective2() As Task + Await TestMissingInRegularAndScriptAsync( +" +class C + function M() as integer + dim check as boolean = true +#if true + [||]if check + return 3 + end if +#end if + return 2 + end function end class") End Function End Class diff --git a/src/Features/Core/Portable/UseConditionalExpression/AbstractUseConditionalExpressionDiagnosticAnalyzer.cs b/src/Features/Core/Portable/UseConditionalExpression/AbstractUseConditionalExpressionDiagnosticAnalyzer.cs index 9fd6d1c1671e0..caefac9e5f12d 100644 --- a/src/Features/Core/Portable/UseConditionalExpression/AbstractUseConditionalExpressionDiagnosticAnalyzer.cs +++ b/src/Features/Core/Portable/UseConditionalExpression/AbstractUseConditionalExpressionDiagnosticAnalyzer.cs @@ -39,8 +39,7 @@ protected sealed override void InitializeWorker(AnalysisContext context) private void AnalyzeOperation(OperationAnalysisContext context) { var ifOperation = (IConditionalOperation)context.Operation; - var ifStatement = ifOperation.Syntax as TIfStatementSyntax; - if (ifStatement == null) + if (!(ifOperation.Syntax is TIfStatementSyntax ifStatement)) { return; } diff --git a/src/Features/Core/Portable/UseConditionalExpression/ForReturn/UseConditionalExpressionForReturnHelpers.cs b/src/Features/Core/Portable/UseConditionalExpression/ForReturn/UseConditionalExpressionForReturnHelpers.cs index ab21baded5866..1beab83831606 100644 --- a/src/Features/Core/Portable/UseConditionalExpression/ForReturn/UseConditionalExpressionForReturnHelpers.cs +++ b/src/Features/Core/Portable/UseConditionalExpression/ForReturn/UseConditionalExpressionForReturnHelpers.cs @@ -35,8 +35,7 @@ public static bool TryMatchPattern( if (falseStatement == null) { - var parentBlock = ifOperation.Parent as IBlockOperation; - if (parentBlock == null) + if (!(ifOperation.Parent is IBlockOperation parentBlock)) { return false; } diff --git a/src/Features/Core/Portable/UseConditionalExpression/UseConditionalExpressionHelpers.cs b/src/Features/Core/Portable/UseConditionalExpression/UseConditionalExpressionHelpers.cs index 46c82c121ebb5..d3b10ac98020e 100644 --- a/src/Features/Core/Portable/UseConditionalExpression/UseConditionalExpressionHelpers.cs +++ b/src/Features/Core/Portable/UseConditionalExpression/UseConditionalExpressionHelpers.cs @@ -1,17 +1,8 @@ // 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; -using System.Collections.Generic; -using System.Collections.Immutable; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.Formatting; -using Microsoft.CodeAnalysis.Formatting.Rules; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.UseConditionalExpression { @@ -23,9 +14,21 @@ public static bool CanConvert( ISyntaxFactsService syntaxFacts, IConditionalOperation ifOperation, IOperation whenTrue, IOperation whenFalse) { - // Will likely screw things up if the if directive spans any preprocessor directives. - // So do not offer for now. - if (syntaxFacts.SpansPreprocessorDirective(ifOperation.Syntax)) + // Will likely screw things up if the if directive spans any preprocessor directives. So + // do not offer for now. Note: we pass in both the node for the ifOperation and the + // whenFalse portion. The whenFalse portion isn't necessary under the ifOperation. For + // example in: + // + // ```c# + // #if DEBUG + // if (check) + // return 3; + // #endif + // return 2; + // ``` + // + // In this case, we want to see that this cross the `#endif` + if (syntaxFacts.SpansPreprocessorDirective(ifOperation.Syntax, whenFalse.Syntax)) { return false; } diff --git a/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs index b73df14ebe2d1..1341879ab09d4 100644 --- a/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs +++ b/src/Workspaces/CSharp/Portable/Extensions/SyntaxNodeExtensions.cs @@ -170,20 +170,8 @@ public static bool IsReturnableConstruct(this SyntaxNode node) return false; } - public static bool SpansPreprocessorDirective( - this IEnumerable list) - where TSyntaxNode : SyntaxNode - { - if (list == null || list.IsEmpty()) - { - return false; - } - - var tokens = list.SelectMany(n => n.DescendantTokens()); - - // todo: we need to dive into trivia here. - return tokens.SpansPreprocessorDirective(); - } + public static bool SpansPreprocessorDirective(this IEnumerable list) where TSyntaxNode : SyntaxNode + => CSharpSyntaxFactsService.Instance.SpansPreprocessorDirective(list); public static TNode ConvertToSingleLine(this TNode node, bool useElasticTrivia = false) where TNode : SyntaxNode diff --git a/src/Workspaces/CSharp/Portable/Extensions/SyntaxTokenExtensions.cs b/src/Workspaces/CSharp/Portable/Extensions/SyntaxTokenExtensions.cs index df01a82df20d5..5d138464e879b 100644 --- a/src/Workspaces/CSharp/Portable/Extensions/SyntaxTokenExtensions.cs +++ b/src/Workspaces/CSharp/Portable/Extensions/SyntaxTokenExtensions.cs @@ -71,41 +71,7 @@ public static bool IsFirstTokenOnLine(this SyntaxToken token, SourceText text) } public static bool SpansPreprocessorDirective(this IEnumerable tokens) - { - // we want to check all leading trivia of all tokens (except the - // first one), and all trailing trivia of all tokens (except the - // last one). - - var first = true; - var previousToken = default(SyntaxToken); - - foreach (var token in tokens) - { - if (first) - { - first = false; - } - else - { - // check the leading trivia of this token, and the trailing trivia - // of the previous token. - if (SpansPreprocessorDirective(token.LeadingTrivia) || - SpansPreprocessorDirective(previousToken.TrailingTrivia)) - { - return true; - } - } - - previousToken = token; - } - - return false; - } - - private static bool SpansPreprocessorDirective(SyntaxTriviaList list) - { - return list.Any(t => t.GetStructure() is DirectiveTriviaSyntax); - } + => CSharpSyntaxFactsService.Instance.SpansPreprocessorDirective(tokens); /// /// Retrieves all trivia after this token, including it's trailing trivia and diff --git a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs index ba5fa9f4e2ded..f78c029747d5e 100644 --- a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs +++ b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs @@ -126,9 +126,7 @@ public bool IsEntirelyWithinStringOrCharOrNumericLiteral(SyntaxTree syntaxTree, } public bool IsDirective(SyntaxNode node) - { - return node is DirectiveTriviaSyntax; - } + => node is DirectiveTriviaSyntax; public bool TryGetExternalSourceInfo(SyntaxNode node, out ExternalSourceInfo info) { @@ -1949,8 +1947,5 @@ public void GetPartsOfCastExpression(SyntaxNode node, out SyntaxNode type, out S return null; } - - public bool SpansPreprocessorDirective(IEnumerable nodes) - => nodes.SpansPreprocessorDirective(); } } diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs index 47fee3f19bc7c..65b738ed53cf0 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/AbstractSyntaxFactsService.cs @@ -431,5 +431,61 @@ public string GetBannerText(SyntaxNode documentationCommentTriviaSyntax, int ban => DocumentationCommentService.GetBannerText(documentationCommentTriviaSyntax, bannerLength, cancellationToken); protected abstract IDocumentationCommentService DocumentationCommentService { get; } + + public bool SpansPreprocessorDirective(IEnumerable nodes) + { + if (nodes == null || nodes.IsEmpty()) + { + return false; + } + + return SpansPreprocessorDirective(nodes.SelectMany(n => n.DescendantTokens())); + } + + /// + /// Determines if there is preprocessor trivia *between* any of the + /// provided. The will be deduped and then ordered by position. + /// Specifically, the first token will not have it's leading trivia checked, and the last + /// token will not have it's trailing trivia checked. All other trivia will be checked to + /// see if it contains a preprocessor directive. + /// + public bool SpansPreprocessorDirective(IEnumerable tokens) + { + // we want to check all leading trivia of all tokens (except the + // first one), and all trailing trivia of all tokens (except the + // last one). + + var first = true; + var previousToken = default(SyntaxToken); + + // Allow duplicate nodes/tokens to be passed in. Also, allow the nodes/tokens + // to not be in any particular order when passed in. + var orderedTokens = tokens.Distinct().OrderBy(t => t.SpanStart); + + foreach (var token in orderedTokens) + { + if (first) + { + first = false; + } + else + { + // check the leading trivia of this token, and the trailing trivia + // of the previous token. + if (SpansPreprocessorDirective(token.LeadingTrivia) || + SpansPreprocessorDirective(previousToken.TrailingTrivia)) + { + return true; + } + } + + previousToken = token; + } + + return false; + } + + private bool SpansPreprocessorDirective(SyntaxTriviaList list) + => list.Any(t => IsPreprocessorDirective(t)); } } diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsServiceExtensions.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsServiceExtensions.cs index 88ead39455687..f502a7a17dbbe 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsServiceExtensions.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsServiceExtensions.cs @@ -1,5 +1,6 @@ // 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.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -119,6 +120,9 @@ private static bool IsWordOrNumber(this ISyntaxFactsService syntaxFacts, SyntaxT public static bool SpansPreprocessorDirective(this ISyntaxFactsService service, SyntaxNode node) => service.SpansPreprocessorDirective(SpecializedCollections.SingletonEnumerable(node)); + public static bool SpansPreprocessorDirective(this ISyntaxFactsService service, params SyntaxNode[] nodes) + => service.SpansPreprocessorDirective(nodes); + public static bool IsWhitespaceOrEndOfLineTrivia(this ISyntaxFactsService syntaxFacts, SyntaxTrivia trivia) => syntaxFacts.IsWhitespaceTrivia(trivia) || syntaxFacts.IsEndOfLineTrivia(trivia); diff --git a/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxNodeExtensions.vb b/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxNodeExtensions.vb index 25f428298f4d0..3b3cd36486625 100644 --- a/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxNodeExtensions.vb +++ b/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxNodeExtensions.vb @@ -235,13 +235,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions Public Function SpansPreprocessorDirective(Of TSyntaxNode As SyntaxNode)(list As IEnumerable(Of TSyntaxNode)) As Boolean - If list Is Nothing OrElse Not list.Any() Then - Return False - End If - - Dim tokens = list.SelectMany(Function(n) n.DescendantTokens()) - - Return tokens.SpansPreprocessorDirective() + Return VisualBasicSyntaxFactsService.Instance.SpansPreprocessorDirective(list) End Function diff --git a/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxTokenExtensions.vb b/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxTokenExtensions.vb index e36c6cc0ca699..5d99a5018acec 100644 --- a/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxTokenExtensions.vb +++ b/src/Workspaces/VisualBasic/Portable/Extensions/SyntaxTokenExtensions.vb @@ -195,29 +195,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions Public Function SpansPreprocessorDirective(tokens As IEnumerable(Of SyntaxToken)) As Boolean - ' we want to check all leading trivia of all tokens (except the - ' first one), and all trailing trivia of all tokens (except the - ' last one). - - Dim first As Boolean = True - Dim previousToken As SyntaxToken = Nothing - - For Each token In tokens - If first Then - first = False - Else - ' check the leading trivia of this token, and the trailing trivia - ' of the previous token. - If token.LeadingTrivia.ContainsPreprocessorDirective() OrElse - previousToken.TrailingTrivia.ContainsPreprocessorDirective() Then - Return True - End If - End If - - previousToken = token - Next token - - Return False + Return VisualBasicSyntaxFactsService.Instance.SpansPreprocessorDirective(tokens) End Function diff --git a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb index f08520f17219c..d0d92eee86b0d 100644 --- a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb +++ b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb @@ -1913,8 +1913,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Return Nothing End Function - Public Function SpansPreprocessorDirective(nodes As IEnumerable(Of SyntaxNode)) As Boolean Implements ISyntaxFactsService.SpansPreprocessorDirective - Return nodes.SpansPreprocessorDirective() + Public Shadows Function SpansPreprocessorDirective(nodes As IEnumerable(Of SyntaxNode)) As Boolean Implements ISyntaxFactsService.SpansPreprocessorDirective + Return MyBase.SpansPreprocessorDirective(nodes) + End Function + + Public Shadows Function SpansPreprocessorDirective(tokens As IEnumerable(Of SyntaxToken)) As Boolean + Return MyBase.SpansPreprocessorDirective(tokens) End Function End Class End Namespace