PR feedback cleanup, simplified getting NumericLiteralToken, etc.

src/Features/CSharp/Portable/CodeRefactorings/CSharpRefactoringHelpersService.cs

@@ -13,9 +13,9 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings
     [ExportLanguageService(typeof(IRefactoringHelpersService), LanguageNames.CSharp), Shared]
     internal class CSharpRefactoringHelpersService : AbstractRefactoringHelpersService<PropertyDeclarationSyntax, ParameterSyntax, MethodDeclarationSyntax, LocalDeclarationStatementSyntax>
     {
-        protected override IEnumerable<SyntaxNode> ExtractNodeOfHeader(SyntaxNode node, ISyntaxFactsService syntaxFacts)
+        protected override IEnumerable<SyntaxNode> ExtractNodesOfHeader(SyntaxNode node, ISyntaxFactsService syntaxFacts)
         {
-            foreach (var baseExtraction in base.ExtractNodeOfHeader(node, syntaxFacts))
+            foreach (var baseExtraction in base.ExtractNodesOfHeader(node, syntaxFacts))
             {
                 yield return baseExtraction;
             }
@@ -28,13 +28,13 @@ protected override IEnumerable<SyntaxNode> ExtractNodeOfHeader(SyntaxNode node,
 
         private bool IsInLocalFunctionHeader(SyntaxNode node, ISyntaxFactsService syntaxFacts)
         {
-            var containingMethod = node.GetAncestorOrThis<LocalFunctionStatementSyntax>();
-            if (containingMethod == null)
+            var containingLocalFunction = node.GetAncestorOrThis<LocalFunctionStatementSyntax>();
+            if (containingLocalFunction == null)
             {
                 return false;
             }
 
-            return syntaxFacts.IsInHeader(node, containingMethod, containingMethod.ParameterList);
+            return syntaxFacts.IsInHeader(node, containingLocalFunction, containingLocalFunction.ParameterList);
         }
     }
 }

src/Features/CSharp/Portable/ConvertNumericLiteral/CSharpConvertNumericLiteralCodeRefactoringProvider.cs

 // 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.Composition;
-using System.Threading;
-using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeRefactorings;
 using Microsoft.CodeAnalysis.ConvertNumericLiteral;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.CSharp.ConvertNumericLiteral
 {
     [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = nameof(CSharpConvertNumericLiteralCodeRefactoringProvider)), Shared]
-    internal sealed class CSharpConvertNumericLiteralCodeRefactoringProvider : AbstractConvertNumericLiteralCodeRefactoringProvider
+    internal sealed class CSharpConvertNumericLiteralCodeRefactoringProvider : AbstractConvertNumericLiteralCodeRefactoringProvider<LiteralExpressionSyntax>
     {
         [ImportingConstructor]
         public CSharpConvertNumericLiteralCodeRefactoringProvider()
@@ -19,13 +16,5 @@ public CSharpConvertNumericLiteralCodeRefactoringProvider()
         }
 
         protected override (string hexPrefix, string binaryPrefix) GetNumericLiteralPrefixes() => (hexPrefix: "0x", binaryPrefix: "0b");
-
-        internal override async Task<SyntaxToken> GetNumericTokenAsync(Document document, TextSpan span, CancellationToken cancellationToken)
-        {
-            var expressionNode = await GetNumericLiteralExpression<LiteralExpressionSyntax>(document, span, cancellationToken).ConfigureAwait(false);
-            return expressionNode != null
-                ? expressionNode.Token
-                : default;
-        }
     }
 }

src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.LanguageServices;
@@ -44,8 +45,8 @@ internal abstract class AbstractRefactoringHelpersService<TPropertyDeclaration,
         /// of tokens gracefully. Over-selection containing leading comments is also handled correctly. 
         /// </para>
         /// </summary>
-        protected Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan selection, Predicate<SyntaxNode> predicate, CancellationToken cancellationToken)
-            => TryGetSelectedNodeAsync(document, selection, predicate, DefaultNodeExtractor, cancellationToken);
+        protected Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan selection, Func<SyntaxNode, bool> predicate, CancellationToken cancellationToken)
+            => TryGetSelectedNodeAsync(document, selection, predicate, DefaultNodesExtractor, cancellationToken);
 
         /// <summary>
         /// <para>
@@ -60,7 +61,7 @@ protected Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan s
         /// - Whole node passing <paramref name="predicate"/> is selected.
         /// </para>
         /// <para>
-        /// The <paramref name="extractNode"/> enables testing with <paramref name="predicate"/> and potentially returning Nodes 
+        /// The <paramref name="extractNodes"/> enables testing with <paramref name="predicate"/> and potentially returning Nodes 
         /// that are under/above those that might be selected / considered (as described above). It should iterate over all candidate
         /// nodes.
         /// </para>
@@ -70,7 +71,7 @@ protected Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan s
         /// of tokens gracefully. Over-selection containing leading comments is also handled correctly. 
         /// </para>
         /// </summary>
-        protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan selectionRaw, Predicate<SyntaxNode> predicate, Func<SyntaxNode, ISyntaxFactsService, bool, IEnumerable<SyntaxNode>> extractNode, CancellationToken cancellationToken)
+        protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, TextSpan selectionRaw, Func<SyntaxNode, bool> predicate, Func<SyntaxNode, ISyntaxFactsService, bool, IEnumerable<SyntaxNode>> extractNodes, CancellationToken cancellationToken)
         {
             // Given selection is trimmed first to enable over-selection that spans multiple lines. Since trailing whitespace ends
             // at newline boundary over-selection to e.g. a line after LocalFunctionStatement would cause FindNode to find enclosing
@@ -117,7 +118,7 @@ protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, Text
             var prevNode = selectionNode;
             do
             {
-                var wantedNode = TryGetAcceptedNodeOrExtracted(selectionNode, predicate, extractNode, syntaxFacts, extractParentsOfHeader: false);
+                var wantedNode = TryGetAcceptedNodeOrExtracted(selectionNode, predicate, extractNodes, syntaxFacts, extractParentsOfHeader: false);
                 if (wantedNode != null)
                 {
                     // For selections we need to handle an edge case where only AttributeLists are within selection (e.g. `Func([|[in][out]|] arg1);`).
@@ -233,7 +234,7 @@ protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, Text
                     // Consider either a Node that is:
                     // - Parent of touched Token (location can be within) 
                     // - Ancestor Node of such Token as long as their span starts on location (it's still on the edge)
-                    var wantedNode = TryGetAcceptedNodeOrExtracted(rightNode, predicate, extractNode, syntaxFacts, extractParentsOfHeader: true);
+                    var wantedNode = TryGetAcceptedNodeOrExtracted(rightNode, predicate, extractNodes, syntaxFacts, extractParentsOfHeader: true);
                     if (wantedNode != null)
                     {
                         return wantedNode;
@@ -271,7 +272,7 @@ protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, Text
                 {
                     // Consider either a Node that is:
                     // - Ancestor Node of such Token as long as their span ends on location (it's still on the edge)
-                    var wantedNode = TryGetAcceptedNodeOrExtracted(leftNode, predicate, extractNode, syntaxFacts, extractParentsOfHeader: true);
+                    var wantedNode = TryGetAcceptedNodeOrExtracted(leftNode, predicate, extractNodes, syntaxFacts, extractParentsOfHeader: true);
                     if (wantedNode != null)
                     {
                         return wantedNode;
@@ -289,7 +290,7 @@ protected async Task<SyntaxNode> TryGetSelectedNodeAsync(Document document, Text
             // nothing found -> return null
             return null;
 
-            static SyntaxNode TryGetAcceptedNodeOrExtracted(SyntaxNode node, Predicate<SyntaxNode> predicate, Func<SyntaxNode, ISyntaxFactsService, bool, IEnumerable<SyntaxNode>> extractNode, ISyntaxFactsService syntaxFacts, bool extractParentsOfHeader)
+            static SyntaxNode TryGetAcceptedNodeOrExtracted(SyntaxNode node, Func<SyntaxNode, bool> predicate, Func<SyntaxNode, ISyntaxFactsService, bool, IEnumerable<SyntaxNode>> extractNodes, ISyntaxFactsService syntaxFacts, bool extractParentsOfHeader)
             {
                 if (node == null)
                 {
@@ -301,15 +302,7 @@ static SyntaxNode TryGetAcceptedNodeOrExtracted(SyntaxNode node, Predicate<Synta
                     return node;
                 }
 
-                foreach (var extractedNode in extractNode(node, syntaxFacts, extractParentsOfHeader))
-                {
-                    if (predicate(extractedNode))
-                    {
-                        return extractedNode;
-                    }
-                }
-
-                return null;
+                return extractNodes(node, syntaxFacts, extractParentsOfHeader).FirstOrDefault(predicate);
             }
         }
 
@@ -351,24 +344,24 @@ private static TextSpan GetSpanWithoutAttributes(SyntaxNode node, SyntaxNode roo
         /// with that and only that node. On the other hand placing cursor anywhere in header should still count as selecting the node it's header of.
         /// </para>
         /// </summary>
-        protected virtual IEnumerable<SyntaxNode> DefaultNodeExtractor(SyntaxNode node, ISyntaxFactsService syntaxFacts, bool extractParentsOfHeader)
+        protected virtual IEnumerable<SyntaxNode> DefaultNodesExtractor(SyntaxNode node, ISyntaxFactsService syntaxFacts, bool extractParentsOfHeader)
         {
             // REMARKS: 
             // The set of currently attempted extractions is in no way exhaustive and covers only cases
             // that were found to be relevant for refactorings that were moved to `TryGetSelectedNodeAsync`.
             // Feel free to extend it / refine current heuristics. 
 
-            foreach (var extractedNode in ExtractNodeSimple(node, syntaxFacts))
+            foreach (var extractedNode in ExtractNodesSimple(node, syntaxFacts))
             {
                 yield return extractedNode;
             }
 
             if (extractParentsOfHeader)
             {
-                foreach (var headerNode in ExtractNodeOfHeader(node, syntaxFacts))
+                foreach (var headerNode in ExtractNodesOfHeader(node, syntaxFacts))
                 {
                     yield return headerNode;
-                    foreach (var extractedNode in ExtractNodeSimple(headerNode, syntaxFacts))
+                    foreach (var extractedNode in ExtractNodesSimple(headerNode, syntaxFacts))
                     {
                         yield return extractedNode;
                     }
@@ -376,7 +369,7 @@ protected virtual IEnumerable<SyntaxNode> DefaultNodeExtractor(SyntaxNode node,
             }
         }
 
-        protected virtual IEnumerable<SyntaxNode> ExtractNodeSimple(SyntaxNode node, ISyntaxFactsService syntaxFacts)
+        protected virtual IEnumerable<SyntaxNode> ExtractNodesSimple(SyntaxNode node, ISyntaxFactsService syntaxFacts)
         {
             // `var a = b`;
             if (syntaxFacts.IsLocalDeclarationStatement(node))
@@ -421,7 +414,7 @@ protected virtual IEnumerable<SyntaxNode> ExtractNodeSimple(SyntaxNode node, ISy
             }
         }
 
-        protected virtual IEnumerable<SyntaxNode> ExtractNodeOfHeader(SyntaxNode node, ISyntaxFactsService syntaxFacts)
+        protected virtual IEnumerable<SyntaxNode> ExtractNodesOfHeader(SyntaxNode node, ISyntaxFactsService syntaxFacts)
         {
             // Header: [Test] `public int a` { get; set; }
             if (syntaxFacts.IsInPropertyDeclarationHeader(node))

src/Features/Core/Portable/ConvertNumericLiteral/AbstractConvertNumericLiteralCodeRefactoringProvider.cs

@@ -14,7 +14,7 @@
 
 namespace Microsoft.CodeAnalysis.ConvertNumericLiteral
 {
-    internal abstract class AbstractConvertNumericLiteralCodeRefactoringProvider : CodeRefactoringProvider
+    internal abstract class AbstractConvertNumericLiteralCodeRefactoringProvider<TNumericLiteralExpression> : CodeRefactoringProvider where TNumericLiteralExpression : SyntaxNode
     {
         protected abstract (string hexPrefix, string binaryPrefix) GetNumericLiteralPrefixes();
 
@@ -115,17 +115,19 @@ void RegisterRefactoringWithResult(string text, string title)
             }
         }
 
-        internal abstract Task<SyntaxToken> GetNumericTokenAsync(Document document, TextSpan span, CancellationToken cancellationToken);
-
-        protected async Task<TExpression> GetNumericLiteralExpression<TExpression>(Document document, TextSpan span, CancellationToken cancellationToken) where TExpression : SyntaxNode
+        internal virtual async Task<SyntaxToken> GetNumericTokenAsync(Document document, TextSpan span, CancellationToken cancellationToken)
         {
             var syntaxFacts = document.GetLanguageService<ISyntaxFactsService>();
             var refactoringService = document.GetLanguageService<IRefactoringHelpersService>();
 
-            var literalNode = await refactoringService.TryGetSelectedNodeAsync<TExpression>(document, span, cancellationToken).ConfigureAwait(false);
-            return syntaxFacts.IsNumericLiteralExpression(literalNode)
+            var literalNode = await refactoringService.TryGetSelectedNodeAsync<TNumericLiteralExpression>(document, span, cancellationToken).ConfigureAwait(false);
+            var numericLiteralExpressionNode = syntaxFacts.IsNumericLiteralExpression(literalNode)
                 ? literalNode
                 : null;
+
+            return numericLiteralExpressionNode != null
+                ? numericLiteralExpressionNode.GetFirstToken()    // We know that TNumericLiteralExpression has always only one token: NumericLiteralToken
+                : default;
         }
 
         private static (string prefix, string number, string suffix) GetNumericLiteralParts(string numericText, string hexPrefix, string binaryPrefix)

src/Features/VisualBasic/Portable/CodeRefactorings/VisualBasicRefactoringHelpersService.vb

@@ -11,8 +11,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeRefactorings
     Friend Class VisualBasicRefactoringHelpersService
         Inherits AbstractRefactoringHelpersService(Of PropertyStatementSyntax, ParameterSyntax, MethodStatementSyntax, LocalDeclarationStatementSyntax)
 
-        Protected Overrides Iterator Function ExtractNodeSimple(node As SyntaxNode, syntaxFacts As ISyntaxFactsService) As IEnumerable(Of SyntaxNode)
-            For Each baseExtraction In MyBase.ExtractNodeSimple(node, syntaxFacts)
+        Protected Overrides Iterator Function ExtractNodesSimple(node As SyntaxNode, syntaxFacts As ISyntaxFactsService) As IEnumerable(Of SyntaxNode)
+            For Each baseExtraction In MyBase.ExtractNodesSimple(node, syntaxFacts)
                 Yield baseExtraction
             Next
 

src/Features/VisualBasic/Portable/ConvertNumericLiteral/VisualBasicConvertNumericLiteralCodeRefactoringProvider.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 Imports System.Composition
-Imports System.Threading
 Imports Microsoft.CodeAnalysis.CodeRefactorings
 Imports Microsoft.CodeAnalysis.ConvertNumericLiteral
-Imports Microsoft.CodeAnalysis.Text
 Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.ConvertNumericLiteral
     <ExportCodeRefactoringProvider(LanguageNames.VisualBasic, Name:=NameOf(VisualBasicConvertNumericLiteralCodeRefactoringProvider)), [Shared]>
     Friend NotInheritable Class VisualBasicConvertNumericLiteralCodeRefactoringProvider
-        Inherits AbstractConvertNumericLiteralCodeRefactoringProvider
+        Inherits AbstractConvertNumericLiteralCodeRefactoringProvider(Of LiteralExpressionSyntax)
 
         <ImportingConstructor>
         Public Sub New()
@@ -19,10 +17,5 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ConvertNumericLiteral
         Protected Overrides Function GetNumericLiteralPrefixes() As (hexPrefix As String, binaryPrefix As String)
             Return (hexPrefix:="&H", binaryPrefix:="&B")
         End Function
-
-        Friend Overrides Async Function GetNumericTokenAsync(ByVal document As Document, ByVal span As TextSpan, ByVal cancellationToken As CancellationToken) As Task(Of SyntaxToken)
-            Dim expressionNode = Await GetNumericLiteralExpression(Of LiteralExpressionSyntax)(document, span, cancellationToken).ConfigureAwait(False)
-            Return If(expressionNode IsNot Nothing, expressionNode.Token, New SyntaxToken())
-        End Function
     End Class
 End Namespace