From c1caaefcc66d9ed93c60d23e8343b25467c312ce Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 8 Feb 2022 13:15:03 -0800 Subject: [PATCH 1/4] Support wrapping constructs even in the presence of some recoverable errors --- .../Wrapping/ParameterWrappingTests.cs | 17 +++++++++++ .../Wrapping/ParameterWrappingTests.vb | 14 ++++++++++ .../CSharpArgumentWrapper.cs | 28 +++++++++---------- .../CSharpParameterWrapper.cs | 12 +++++--- .../Core/Portable/Wrapping/AbstractWrapper.cs | 21 +++++++------- ...AbstractWrappingCodeRefactoringProvider.cs | 19 ++++++------- .../AbstractBinaryExpressionWrapper.cs | 15 ++++------ .../AbstractChainedExpressionWrapper.cs | 21 +++++--------- .../Core/Portable/Wrapping/ISyntaxWrapper.cs | 2 +- .../AbstractSeparatedSyntaxListWrapper.cs | 12 ++------ .../VisualBasicArgumentWrapper.vb | 7 +++-- .../VisualBasicParameterWrapper.vb | 14 ++++++++-- 12 files changed, 103 insertions(+), 79 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Wrapping/ParameterWrappingTests.cs b/src/EditorFeatures/CSharpTest/Wrapping/ParameterWrappingTests.cs index e6f1d1c514d27..d20d0c72cdbe2 100644 --- a/src/EditorFeatures/CSharpTest/Wrapping/ParameterWrappingTests.cs +++ b/src/EditorFeatures/CSharpTest/Wrapping/ParameterWrappingTests.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.CSharp.Wrapping; using Microsoft.CodeAnalysis.Test.Utilities; +using Roslyn.Test.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Wrapping @@ -810,6 +811,22 @@ public C(int i, }"); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsWrapping)] + [WorkItem(38986, "https://github.com/dotnet/roslyn/issues/38986")] + public async Task TestInConstructorWithSyntaxErrorAfter() + { + await TestInRegularAndScript1Async( +@"class C { + public [||]C(int i, int j) : base(,) { + } +}", +@"class C { + public C(int i, + int j) : base(,) { + } +}"); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsWrapping)] public async Task TestInIndexer() { diff --git a/src/EditorFeatures/VisualBasicTest/Wrapping/ParameterWrappingTests.vb b/src/EditorFeatures/VisualBasicTest/Wrapping/ParameterWrappingTests.vb index d82120db9ab6b..1077a396d550b 100644 --- a/src/EditorFeatures/VisualBasicTest/Wrapping/ParameterWrappingTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Wrapping/ParameterWrappingTests.vb @@ -22,6 +22,20 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Wrapping end class") End Function + + Public Async Function TestAvailableWithSyntaxErrorAfter() As Task + Await TestInRegularAndScript1Async( +"class C + function Goobar([||]i as integer, j as integer) as + end function +end class", +"class C + function Goobar(i as integer, + j as integer) as + end function +end class") + End Function + Public Async Function TestMissingWithSelection() As Task Await TestMissingAsync( diff --git a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs index 8fc81459f781f..080aca774a786 100644 --- a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs +++ b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs @@ -2,11 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Linq; using Microsoft.CodeAnalysis.CSharp.LanguageServices; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -27,7 +26,7 @@ internal partial class CSharpArgumentWrapper protected override SeparatedSyntaxList GetListItems(BaseArgumentListSyntax listSyntax) => listSyntax.Arguments; - protected override BaseArgumentListSyntax TryGetApplicableList(SyntaxNode node) + protected override BaseArgumentListSyntax? TryGetApplicableList(SyntaxNode node) => node switch { InvocationExpressionSyntax invocationExpression => invocationExpression.ArgumentList, @@ -38,18 +37,23 @@ protected override BaseArgumentListSyntax TryGetApplicableList(SyntaxNode node) }; protected override bool PositionIsApplicable( - SyntaxNode root, int position, - SyntaxNode declaration, BaseArgumentListSyntax listSyntax) + SyntaxNode root, + int position, + SyntaxNode declaration, + bool containsSyntaxError, + BaseArgumentListSyntax listSyntax) { + if (containsSyntaxError) + return false; + var startToken = listSyntax.GetFirstToken(); - if (declaration is InvocationExpressionSyntax or - ElementAccessExpressionSyntax) + if (declaration is InvocationExpressionSyntax or ElementAccessExpressionSyntax) { // If we have something like Foo(...) or this.Foo(...) allow anywhere in the Foo(...) // section. var expr = (declaration as InvocationExpressionSyntax)?.Expression ?? - (declaration as ElementAccessExpressionSyntax).Expression; + ((ElementAccessExpressionSyntax)declaration).Expression; var name = TryGetInvokedName(expr); startToken = name == null ? listSyntax.GetFirstToken() : name.GetFirstToken(); @@ -68,21 +72,17 @@ protected override bool PositionIsApplicable( var endToken = listSyntax.GetLastToken(); var span = TextSpan.FromBounds(startToken.SpanStart, endToken.Span.End); if (!span.IntersectsWith(position)) - { return false; - } // allow anywhere in the arg list, as long we don't end up walking through something // complex like a lambda/anonymous function. var token = root.FindToken(position); - if (token.Parent.Ancestors().Contains(listSyntax)) + if (token.GetRequiredParent().Ancestors().Contains(listSyntax)) { - for (var current = token.Parent; current != listSyntax; current = current.Parent) + for (var current = token.Parent; current != listSyntax; current = current?.Parent) { if (CSharpSyntaxFacts.Instance.IsAnonymousFunctionExpression(current)) - { return false; - } } } diff --git a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpParameterWrapper.cs b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpParameterWrapper.cs index 53c9e84d0e2c2..bc3d8acb38eaf 100644 --- a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpParameterWrapper.cs +++ b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpParameterWrapper.cs @@ -31,14 +31,12 @@ protected override SeparatedSyntaxList GetListItems(BaseParamet => node.GetParameterList(); protected override bool PositionIsApplicable( - SyntaxNode root, int position, SyntaxNode declaration, BaseParameterListSyntax listSyntax) + SyntaxNode root, int position, SyntaxNode declaration, bool containsSyntaxError, BaseParameterListSyntax listSyntax) { // CSharpSyntaxGenerator.GetParameterList synthesizes a parameter list for simple-lambdas. // In that case, we're not applicable in that list. if (declaration.Kind() == SyntaxKind.SimpleLambdaExpression) - { return false; - } var generator = CSharpSyntaxGenerator.Instance; var attributes = generator.GetAttributes(declaration); @@ -52,7 +50,13 @@ protected override bool PositionIsApplicable( var lastToken = listSyntax.GetLastToken(); var headerSpan = TextSpan.FromBounds(firstToken.SpanStart, lastToken.Span.End); - return headerSpan.IntersectsWith(position); + if (!headerSpan.IntersectsWith(position)) + return false; + + if (containsSyntaxError && ContainsOverlappingSyntaxErrror(declaration, headerSpan)) + return false; + + return true; } } } diff --git a/src/Features/Core/Portable/Wrapping/AbstractWrapper.cs b/src/Features/Core/Portable/Wrapping/AbstractWrapper.cs index c8f2ab51cda50..d3ab4b123d15f 100644 --- a/src/Features/Core/Portable/Wrapping/AbstractWrapper.cs +++ b/src/Features/Core/Portable/Wrapping/AbstractWrapper.cs @@ -2,8 +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. -#nullable disable - using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -11,7 +9,9 @@ namespace Microsoft.CodeAnalysis.Wrapping { + using System.Linq; using Microsoft.CodeAnalysis.Indentation; + using Microsoft.CodeAnalysis.Text; /// /// Common implementation of all . This type takes care of a lot of common logic for @@ -32,7 +32,8 @@ internal abstract partial class AbstractSyntaxWrapper : ISyntaxWrapper protected AbstractSyntaxWrapper(IIndentationService indentationService) => IndentationService = indentationService; - public abstract Task TryCreateComputerAsync(Document document, int position, SyntaxNode node, CancellationToken cancellationToken); + public abstract Task TryCreateComputerAsync( + Document document, int position, SyntaxNode node, bool containsSyntaxError, CancellationToken cancellationToken); protected static async Task ContainsUnformattableContentAsync( Document document, IEnumerable nodesAndTokens, CancellationToken cancellationToken) @@ -46,25 +47,23 @@ protected static async Task ContainsUnformattableContentAsync( var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); foreach (var item in nodesAndTokens) { - if (item == null || - item.Span.IsEmpty) - { + if (item == null || item.Span.IsEmpty) return true; - } - var firstToken = item.IsToken ? item.AsToken() : item.AsNode().GetFirstToken(); - var lastToken = item.IsToken ? item.AsToken() : item.AsNode().GetLastToken(); + var firstToken = item.IsToken ? item.AsToken() : item.AsNode()!.GetFirstToken(); + var lastToken = item.IsToken ? item.AsToken() : item.AsNode()!.GetLastToken(); // Note: we check if things are on the same line, even in the case of a single token. // This is so that we don't try to wrap multiline tokens either (like a multi-line // string). if (!sourceText.AreOnSameLine(firstToken, lastToken)) - { return true; - } } return false; } + + protected static bool ContainsOverlappingSyntaxErrror(SyntaxNode declaration, TextSpan headerSpan) + => declaration.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error && d.Location.SourceSpan.OverlapsWith(headerSpan)); } } diff --git a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs index 31c6733fe33f0..f3f38786ec27d 100644 --- a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis.RemoveUnusedVariable; namespace Microsoft.CodeAnalysis.Wrapping { @@ -44,12 +45,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte foreach (var node in token.Parent.AncestorsAndSelf()) { - // Make sure we don't have any syntax errors here. Don't want to format if we don't - // really understand what's going on. - if (node.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error)) - { - return; - } + var containsSyntaxError = node.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error); // Check if any wrapper can handle this node. If so, then we're done, otherwise // keep walking up. @@ -58,22 +54,23 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte cancellationToken.ThrowIfCancellationRequested(); var computer = await wrapper.TryCreateComputerAsync( - document, position, node, cancellationToken).ConfigureAwait(false); + document, position, node, containsSyntaxError, cancellationToken).ConfigureAwait(false); if (computer == null) - { continue; - } var actions = await computer.GetTopLevelCodeActionsAsync().ConfigureAwait(false); if (actions.IsDefaultOrEmpty) - { continue; - } context.RegisterRefactorings(actions); return; } + + // if we hit a syntax error and the computer couldn't handle it, then bail out. Don't want to format if + // we don't really understand what's going on. + if (containsSyntaxError) + return; } } } diff --git a/src/Features/Core/Portable/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs b/src/Features/Core/Portable/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs index c33a371833560..fd7c82f566459 100644 --- a/src/Features/Core/Portable/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs +++ b/src/Features/Core/Portable/Wrapping/BinaryExpression/AbstractBinaryExpressionWrapper.cs @@ -2,8 +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. -#nullable disable - using System.Collections.Immutable; using System.Threading; using System.Threading.Tasks; @@ -41,19 +39,18 @@ protected AbstractBinaryExpressionWrapper( /// protected abstract SyntaxTriviaList GetNewLineBeforeOperatorTrivia(SyntaxTriviaList newLine); - public sealed override async Task TryCreateComputerAsync( - Document document, int position, SyntaxNode node, CancellationToken cancellationToken) + public sealed override async Task TryCreateComputerAsync( + Document document, int position, SyntaxNode node, bool containsSyntaxError, CancellationToken cancellationToken) { + if (containsSyntaxError) + return null; + if (node is not TBinaryExpressionSyntax binaryExpr) - { return null; - } var precedence = _precedenceService.GetPrecedenceKind(binaryExpr); if (precedence == PrecedenceKind.Other) - { return null; - } // Don't process this binary expression if it's in a parent binary expr of the same or // lower precedence. We'll just allow our caller to walk up to that and call back into @@ -87,9 +84,7 @@ public sealed override async Task TryCreateComputerAsync( document, exprsAndOperators, cancellationToken).ConfigureAwait(false); if (containsUnformattableContent) - { return null; - } var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/Features/Core/Portable/Wrapping/ChainedExpression/AbstractChainedExpressionWrapper.cs b/src/Features/Core/Portable/Wrapping/ChainedExpression/AbstractChainedExpressionWrapper.cs index f7b9a5e6b1bc2..70476fe96690b 100644 --- a/src/Features/Core/Portable/Wrapping/ChainedExpression/AbstractChainedExpressionWrapper.cs +++ b/src/Features/Core/Portable/Wrapping/ChainedExpression/AbstractChainedExpressionWrapper.cs @@ -2,8 +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. -#nullable disable - using System.Collections.Generic; using System.Collections.Immutable; using System.Threading; @@ -71,22 +69,21 @@ protected AbstractChainedExpressionWrapper( /// protected abstract SyntaxTriviaList GetNewLineBeforeOperatorTrivia(SyntaxTriviaList newLine); - public sealed override async Task TryCreateComputerAsync( - Document document, int position, SyntaxNode node, CancellationToken cancellationToken) + public sealed override async Task TryCreateComputerAsync( + Document document, int position, SyntaxNode node, bool containsSyntaxError, CancellationToken cancellationToken) { + if (containsSyntaxError) + return null; + // We have to be on a chain part. If not, there's nothing to do here at all. if (!IsDecomposableChainPart(node)) - { return null; - } // Has to be the topmost chain part. If we're not on the topmost, then just // bail out here. Our caller will continue walking upwards until it hits the // topmost node. if (IsDecomposableChainPart(node.Parent)) - { return null; - } // We're at the top of something that looks like it could be part of a chained // expression. Break it into the individual chunks. We need to have at least @@ -96,9 +93,7 @@ public sealed override async Task TryCreateComputerAsync( // wrap when we have this.Goo(...).Bar(...). var chunks = GetChainChunks(node); if (chunks.Length <= 1) - { return null; - } // If any of these chunk parts are unformattable, then we don't want to offer anything // here as we may make formatting worse for this construct. @@ -107,9 +102,7 @@ public sealed override async Task TryCreateComputerAsync( var unformattable = await ContainsUnformattableContentAsync( document, chunk, cancellationToken).ConfigureAwait(false); if (unformattable) - { return null; - } } // Looks good. Create the action computer which will actually determine @@ -246,7 +239,7 @@ private static ImmutableArray GetSubRange( return result.ToImmutable(); } - private bool IsDecomposableChainPart(SyntaxNode node) + private bool IsDecomposableChainPart(SyntaxNode? node) { // This is the effective set of language constructs that can 'chain' // off of a call .M(...). They are: @@ -295,7 +288,7 @@ private void Decompose(SyntaxNode node, ArrayBuilder pieces) continue; } - var currentNode = nodeOrToken.AsNode(); + var currentNode = nodeOrToken.AsNode()!; if (!IsDecomposableChainPart(currentNode)) { // We've hit some node that can't be decomposed further (like an argument list, or name node). diff --git a/src/Features/Core/Portable/Wrapping/ISyntaxWrapper.cs b/src/Features/Core/Portable/Wrapping/ISyntaxWrapper.cs index d0ee81eccdc87..106c9df08c757 100644 --- a/src/Features/Core/Portable/Wrapping/ISyntaxWrapper.cs +++ b/src/Features/Core/Portable/Wrapping/ISyntaxWrapper.cs @@ -26,6 +26,6 @@ internal interface ISyntaxWrapper /// node passed in. Returns if this Wrapper cannot wrap this node. /// Task TryCreateComputerAsync( - Document document, int position, SyntaxNode node, CancellationToken cancellationToken); + Document document, int position, SyntaxNode node, bool containsSyntaxError, CancellationToken cancellationToken); } } diff --git a/src/Features/Core/Portable/Wrapping/SeparatedSyntaxList/AbstractSeparatedSyntaxListWrapper.cs b/src/Features/Core/Portable/Wrapping/SeparatedSyntaxList/AbstractSeparatedSyntaxListWrapper.cs index cf1328975c29b..c71e064642445 100644 --- a/src/Features/Core/Portable/Wrapping/SeparatedSyntaxList/AbstractSeparatedSyntaxListWrapper.cs +++ b/src/Features/Core/Portable/Wrapping/SeparatedSyntaxList/AbstractSeparatedSyntaxListWrapper.cs @@ -43,22 +43,18 @@ protected AbstractSeparatedSyntaxListWrapper(IIndentationService indentationServ protected abstract TListSyntax? TryGetApplicableList(SyntaxNode node); protected abstract SeparatedSyntaxList GetListItems(TListSyntax listSyntax); protected abstract bool PositionIsApplicable( - SyntaxNode root, int position, SyntaxNode declaration, TListSyntax listSyntax); + SyntaxNode root, int position, SyntaxNode declaration, bool containsSyntaxError, TListSyntax listSyntax); public override async Task TryCreateComputerAsync( - Document document, int position, SyntaxNode declaration, CancellationToken cancellationToken) + Document document, int position, SyntaxNode declaration, bool containsSyntaxError, CancellationToken cancellationToken) { var listSyntax = TryGetApplicableList(declaration); if (listSyntax == null) - { return null; - } var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - if (!PositionIsApplicable(root, position, declaration, listSyntax)) - { + if (!PositionIsApplicable(root, position, declaration, containsSyntaxError, listSyntax)) return null; - } var listItems = GetListItems(listSyntax); if (listItems.Count <= 1) @@ -73,9 +69,7 @@ protected abstract bool PositionIsApplicable( document, listItems.GetWithSeparators(), cancellationToken).ConfigureAwait(false); if (containsUnformattableContent) - { return null; - } var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); diff --git a/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicArgumentWrapper.vb b/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicArgumentWrapper.vb index b7d691078acba..379f90e6ea051 100644 --- a/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicArgumentWrapper.vb +++ b/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicArgumentWrapper.vb @@ -29,8 +29,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Wrapping.SeparatedSyntaxList End Function Protected Overrides Function PositionIsApplicable( - root As SyntaxNode, position As Integer, - declaration As SyntaxNode, listSyntax As ArgumentListSyntax) As Boolean + root As SyntaxNode, position As Integer, declaration As SyntaxNode, containsSyntaxError As Boolean, listSyntax As ArgumentListSyntax) As Boolean + + If containsSyntaxError Then + Return False + End If Dim startToken = listSyntax.GetFirstToken() diff --git a/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicParameterWrapper.vb b/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicParameterWrapper.vb index a79bf96d4a2db..3c55cccfc0111 100644 --- a/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicParameterWrapper.vb +++ b/src/Features/VisualBasic/Portable/Wrapping/SeparatedSyntaxList/VisualBasicParameterWrapper.vb @@ -29,8 +29,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Wrapping.SeparatedSyntaxList End Function Protected Overrides Function PositionIsApplicable( - root As SyntaxNode, position As Integer, - declaration As SyntaxNode, listSyntax As ParameterListSyntax) As Boolean + root As SyntaxNode, position As Integer, declaration As SyntaxNode, + containsSyntaxError As Boolean, listSyntax As ParameterListSyntax) As Boolean Dim generator = VisualBasicSyntaxGenerator.Instance Dim attributes = generator.GetAttributes(declaration) @@ -44,7 +44,15 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Wrapping.SeparatedSyntaxList Dim lastToken = listSyntax.GetLastToken() Dim headerSpan = TextSpan.FromBounds(firstToken.SpanStart, lastToken.Span.End) - Return headerSpan.IntersectsWith(position) + If Not headerSpan.IntersectsWith(position) Then + Return False + End If + + If containsSyntaxError AndAlso ContainsOverlappingSyntaxErrror(declaration, headerSpan) Then + Return False + End If + + Return True End Function End Class End Namespace From 62dde579727355ebd1d9093c28c23950b6cd2ff1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 8 Feb 2022 13:16:28 -0800 Subject: [PATCH 2/4] Cleanup --- .../Wrapping/AbstractWrappingCodeRefactoringProvider.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs index f3f38786ec27d..46e63b1a7de0b 100644 --- a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs @@ -2,13 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Immutable; using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeRefactorings; -using Microsoft.CodeAnalysis.RemoveUnusedVariable; namespace Microsoft.CodeAnalysis.Wrapping { From 26a78c96f09462e51e020dcce6bf707dea7c9b88 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 8 Feb 2022 13:17:22 -0800 Subject: [PATCH 3/4] NRT --- .../Wrapping/AbstractWrappingCodeRefactoringProvider.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs index 46e63b1a7de0b..e8154b7ec6117 100644 --- a/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/Wrapping/AbstractWrappingCodeRefactoringProvider.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeRefactorings; +using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.Wrapping { @@ -32,15 +33,13 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte { var (document, span, cancellationToken) = context; if (!span.IsEmpty) - { return; - } var position = span.Start; - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var token = root.FindToken(position); - foreach (var node in token.Parent.AncestorsAndSelf()) + foreach (var node in token.GetRequiredParent().AncestorsAndSelf()) { var containsSyntaxError = node.GetDiagnostics().Any(d => d.Severity == DiagnosticSeverity.Error); From fa69a6c447f71d0bdaaba633ead07110ee3e4ee0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 8 Feb 2022 18:28:35 -0800 Subject: [PATCH 4/4] NRT --- .../Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs index 080aca774a786..0945812c934a1 100644 --- a/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs +++ b/src/Features/CSharp/Portable/Wrapping/SeparatedSyntaxList/CSharpArgumentWrapper.cs @@ -89,19 +89,15 @@ protected override bool PositionIsApplicable( return true; } - private static ExpressionSyntax TryGetInvokedName(ExpressionSyntax expr) + private static ExpressionSyntax? TryGetInvokedName(ExpressionSyntax expr) { // `Foo(...)`. Allow up through the 'Foo' portion if (expr is NameSyntax name) - { return name; - } // `this[...]`. Allow up through the 'this' token. if (expr is ThisExpressionSyntax or BaseExpressionSyntax) - { return expr; - } // expr.Foo(...) or expr?.Foo(...) // All up through the 'Foo' portion.