VB Service Added - Inline Parameter Name Hints by akhera99 · Pull Request #45622 · dotnet/roslyn

akhera99 · 2020-07-02T19:39:10Z

Hello,

This pull request is to be able to merge the added Visual Basic service to the current feature branch I have out.

View of the VB hints:

Modified/Created Files:
InlineParameterNameHintsService.vb much like the service for C#, I go through and located all the SimpleArgumentSyntax types and determine if the adornment should be added.
VisualBasicInlineParameterNameHintsTests.vb essentially did all the tests I did for the C# tagger, but for VB.
ArgumentSyntaxExtensions.vb previously the DetermineParameter function made a call to the GetSymbolInfo method which had the type ExpressionSyntax as a parameter. I just changed the call to use the GetSymbolInfo method which had the SyntaxNode type as a parameter so that I would be able to get the associated parameter for expressions as well as attributes.

Please let me know if you have any questions or concerns!

...ces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Extensions/ArgumentSyntaxExtensions.vb

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

jasonmalinowski · 2020-07-02T21:01:24Z

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

+            Dim semanticModel = Await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(False)
+            Dim nodes = root.DescendantNodes()
+
+            For Each node In nodes


Something in this loop should be checking for cancellation.

jasonmalinowski · 2020-07-02T21:02:53Z

src/EditorFeatures/Test2/InlineParameterNameHints/VisualBasicInlineParameterNameHintsTests.vb

+                    <Document>
+                        Class Foo
+                            Sub Main(args As String())
+                                TestMethod({|x:CInt(5.5)|}, {|y:2.2|})


Also add tests for CType, TryCast and DirectCast.

Added, they don't resolve to the same base type so I had to add checks for it in the service as well.

src/EditorFeatures/Test2/InlineParameterNameHints/AbstractInlineParameterNameHintsTests.vb

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

CyrusNajmabadi · 2020-07-03T06:19:00Z

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

+        Implements IInlineParameterNameHintsService
+
+        <ImportingConstructor>
+        <Obsolete(MefConstruction.ImportingConstructorMessage, True)>


named parameter for true.

It does not allow me to add one?

really? weird :(

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

CyrusNajmabadi · 2020-07-08T19:27:53Z

src/Features/Core/Portable/InlineParameterNameHints/AbstractInlineParameterNameHintsService.cs

+        public async Task<IEnumerable<InlineParameterHint>> GetInlineParameterNameHintsAsync(Document document, TextSpan textSpan, CancellationToken cancellationToken)
+        {
+            var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
+            var spans = new List<InlineParameterHint>();


remove this and just do var spans = ... below.

CyrusNajmabadi · 2020-07-08T19:28:12Z

src/Features/Core/Portable/InlineParameterNameHints/AbstractInlineParameterNameHintsService.cs

+            return spans;
+        }
+
+        protected abstract List<InlineParameterHint> AddAllParameterNameHintLocations(


don't really see why this is returning a List. Either have it be IEnuemrable, or ImmutableArray.

oh wait. it's even weirder :) you both pass in teh value to add to, and you return it out as a return value. Pick one or the other :)

CyrusNajmabadi · 2020-07-08T19:30:17Z

src/Features/CSharp/Portable/InlineParameterNameHints/InlineParameterNameHintsService.cs

    /// </summary>
    [ExportLanguageService(typeof(IInlineParameterNameHintsService), LanguageNames.CSharp), Shared]
-    internal class InlineParameterNameHintsService : IInlineParameterNameHintsService
+    internal class InlineParameterNameHintsService : AbstractInlineParameterNameHintsService


once we have a VB and C# version we rename thsee. so this should be CSharpInlineParameterNameHintsService. That way it's easy to find/nav-to them since we have distinct names, not three types with the same name.

CyrusNajmabadi · 2020-07-08T19:30:25Z

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

+
+Namespace Microsoft.CodeAnalysis.VisualBasic.InlineParameterNameHints
+    <ExportLanguageService(GetType(IInlineParameterNameHintsService), LanguageNames.VisualBasic), [Shared]>
+    Friend Class InlineParameterNameHintsService


CyrusNajmabadi · 2020-07-08T19:30:49Z

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb

+                        If param IsNot Nothing AndAlso param.Name.Length > 0 Then
+                            spans.Add(New InlineParameterHint(param.Name, simpleArgument.Span.Start))
+                        End If


nit: you could share this code with c# by having a protected helper in teh base class. up to you.

CyrusNajmabadi

lgtm as long as the renames happen. Up to you about the other nits :)

jasonmalinowski · 2020-07-10T18:18:35Z

.../VisualBasic/Portable/InlineParameterNameHints/VisualBasicInlineParameterNameHintsService.vb

+            If TypeOf arg Is TryCastExpressionSyntax Then
+                Dim cast = DirectCast(arg, TryCastExpressionSyntax)
+                ' Recurse until we find a literal
+                ' If so, then we should add the adornment
+                Return IsExpressionWithNoName(cast.Expression)
+            End If
+
+            If TypeOf arg Is CTypeExpressionSyntax Then
+                Dim cast = DirectCast(arg, CTypeExpressionSyntax)
+                ' Recurse until we find a literal
+                ' If so, then we should add the adornment
+                Return IsExpressionWithNoName(cast.Expression)
+            End If
+
+            If TypeOf arg Is DirectCastExpressionSyntax Then
+                Dim cast = DirectCast(arg, DirectCastExpressionSyntax)
+                ' Recurse until we find a literal
+                ' If so, then we should add the adornment
+                Return IsExpressionWithNoName(cast.Expression)
+            End If


There's a CastExpressionSyntax base type you can use to cover all of these if you wanted.

Have the VB service working for the cases I know of

2076f7c

akhera99 requested a review from a team as a code owner July 2, 2020 19:39

Added comments

3990633

jasonmalinowski reviewed Jul 2, 2020

View reviewed changes

...ces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Extensions/ArgumentSyntaxExtensions.vb Outdated Show resolved Hide resolved

jasonmalinowski reviewed Jul 2, 2020

View reviewed changes

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Jul 3, 2020

View reviewed changes

src/Features/VisualBasic/Portable/InlineParameterNameHints/InlineParameterNameHintsService.vb Outdated Show resolved Hide resolved

Ankita Khera added 2 commits July 3, 2020 20:18

Added more tests and cast types to add adornments for

655fd1a

Abstracted the services out a bit

777db9a

sharwell added the Area-IDE label Jul 8, 2020

added abstract

8b05cc7

CyrusNajmabadi reviewed Jul 8, 2020

View reviewed changes

CyrusNajmabadi approved these changes Jul 8, 2020

View reviewed changes

Ankita Khera added 2 commits July 9, 2020 11:18

Made file name changes and redid some logic

cc904ac

Added file header

362174c

akhera99 merged commit 2f0fef4 into dotnet:features/parameter-hints Jul 10, 2020

jasonmalinowski reviewed Jul 10, 2020

View reviewed changes

akhera99 deleted the vb-service branch June 18, 2024 20:50

Conversation

akhera99 commented Jul 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akhera99 Jul 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

akhera99 commented Jul 2, 2020 •

edited

Loading

akhera99 Jul 3, 2020 •

edited

Loading