From d53c1f47b526923e1e93d546ae8b11e8d3d708a4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 6 Oct 2020 13:51:12 -0700 Subject: [PATCH] Suppress inline hints in certain clear cases. --- .../AbstractInlineParameterNameHintsTests.vb | 8 +- .../CSharpInlineParameterNameHintsTests.vb | 150 ++++++++++++++++++ .../CSharpInlineParameterNameHintsService.cs | 62 ++++++-- ...AbstractInlineParameterNameHintsService.cs | 62 +++++++- ...ualBasicInlineParameterNameHintsService.vb | 8 +- 5 files changed, 264 insertions(+), 26 deletions(-) diff --git a/src/EditorFeatures/Test2/InlineHints/AbstractInlineParameterNameHintsTests.vb b/src/EditorFeatures/Test2/InlineHints/AbstractInlineParameterNameHintsTests.vb index 48d40b53d4e..a69fbed16a5 100644 --- a/src/EditorFeatures/Test2/InlineHints/AbstractInlineParameterNameHintsTests.vb +++ b/src/EditorFeatures/Test2/InlineHints/AbstractInlineParameterNameHintsTests.vb @@ -10,10 +10,16 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.InlineHints <[UseExportProvider]> Public MustInherit Class AbstractInlineParameterNameHintsTests - Protected Async Function VerifyParamHints(test As XElement, Optional optionIsEnabled As Boolean = True) As Tasks.Task + Protected Async Function VerifyParamHints(test As XElement, Optional optionIsEnabled As Boolean = True) As Task Using workspace = TestWorkspace.Create(test) WpfTestRunner.RequireWpfFact($"{NameOf(AbstractInlineParameterNameHintsTests)}.{NameOf(Me.VerifyParamHints)} creates asynchronous taggers") + workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions( + workspace.Options.WithChangedOption( + InlineHintsOptions.EnabledForParameters, + workspace.CurrentSolution.Projects.Single().Language, + optionIsEnabled))) + Dim hostDocument = workspace.Documents.Single() Dim snapshot = hostDocument.GetTextBuffer().CurrentSnapshot Dim document = workspace.CurrentSolution.GetDocument(hostDocument.Id) diff --git a/src/EditorFeatures/Test2/InlineHints/CSharpInlineParameterNameHintsTests.vb b/src/EditorFeatures/Test2/InlineHints/CSharpInlineParameterNameHintsTests.vb index b02dc559df8..b2fc18293a5 100644 --- a/src/EditorFeatures/Test2/InlineHints/CSharpInlineParameterNameHintsTests.vb +++ b/src/EditorFeatures/Test2/InlineHints/CSharpInlineParameterNameHintsTests.vb @@ -398,5 +398,155 @@ class Derived : Base Await VerifyParamHints(input) End Function + + + + Public Async Function TestNotOnEnableDisableBoolean1() As Task + Dim input = + + + +class A +{ + void EnableLogging(bool value) + { + } + + void Main() + { + EnableLogging(true); + } +} + + + + + Await VerifyParamHints(input) + End Function + + + + Public Async Function TestNotOnEnableDisableBoolean2() As Task + Dim input = + + + +class A +{ + void DisableLogging(bool value) + { + } + + void Main() + { + DisableLogging(true); + } +} + + + + + Await VerifyParamHints(input) + End Function + + + + Public Async Function TestOnEnableDisableNonBoolean1() As Task + Dim input = + + + +class A +{ + void EnableLogging(string value) + { + } + + void Main() + { + EnableLogging({|value:"IO"|}); + } +} + + + + + Await VerifyParamHints(input) + End Function + + + + Public Async Function TestOnEnableDisableNonBoolean2() As Task + Dim input = + + + +class A +{ + void DisableLogging(string value) + { + } + + void Main() + { + DisableLogging({|value:"IO"|}); + } +} + + + + + Await VerifyParamHints(input) + End Function + + + + Public Async Function TestOnSetMethodWithClearContext() As Task + Dim input = + + + +class A +{ + void SetClassification(string classification) + { + } + + void Main() + { + SetClassification("IO"); + } +} + + + + + Await VerifyParamHints(input) + End Function + + + + Public Async Function TestOnSetMethodWithUnclearContext() As Task + Dim input = + + + +class A +{ + void SetClassification(string values) + { + } + + void Main() + { + SetClassification({|values:"IO"|}); + } +} + + + + + Await VerifyParamHints(input) + End Function End Class End Namespace diff --git a/src/Features/CSharp/Portable/InlineHints/CSharpInlineParameterNameHintsService.cs b/src/Features/CSharp/Portable/InlineHints/CSharpInlineParameterNameHintsService.cs index 74ae0b69f32..0d07c6d63dc 100644 --- a/src/Features/CSharp/Portable/InlineHints/CSharpInlineParameterNameHintsService.cs +++ b/src/Features/CSharp/Portable/InlineHints/CSharpInlineParameterNameHintsService.cs @@ -3,14 +3,12 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections.Generic; using System.Composition; using System.Threading; using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.InlineHints; -using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.CSharp.InlineHints { @@ -28,29 +26,61 @@ public CSharpInlineParameterNameHintsService() } protected override void AddAllParameterNameHintLocations( - SemanticModel semanticModel, SyntaxNode node, - Action addHint, CancellationToken cancellationToken) + SemanticModel semanticModel, + SyntaxNode node, + Action addHint, + bool hideForParametersThatDifferBySuffix, + bool hideForParametersThatMatchMethodIntent, + CancellationToken cancellationToken) { if (node is ArgumentSyntax argument) { - if (argument.NameColon == null) - { - var param = argument.DetermineParameter(semanticModel, cancellationToken: cancellationToken); - if (!string.IsNullOrEmpty(param?.Name)) - addHint(new InlineParameterHint(param.GetSymbolKey(cancellationToken), param.Name, argument.SpanStart, GetKind(argument.Expression))); - } + if (argument.NameColon != null) + return; + + var parameter = argument.DetermineParameter(semanticModel, cancellationToken: cancellationToken); + if (string.IsNullOrEmpty(parameter?.Name)) + return; + + if (hideForParametersThatMatchMethodIntent && MatchesMethodIntent(argument, parameter)) + return; + + addHint(new InlineParameterHint(parameter.GetSymbolKey(cancellationToken), parameter.Name, argument.Span.Start, GetKind(argument.Expression))); } else if (node is AttributeArgumentSyntax attribute) { - if (attribute.NameEquals == null && attribute.NameColon == null) - { - var param = attribute.DetermineParameter(semanticModel, cancellationToken: cancellationToken); - if (!string.IsNullOrEmpty(param?.Name)) - addHint(new InlineParameterHint(param.GetSymbolKey(cancellationToken), param.Name, attribute.SpanStart, GetKind(attribute.Expression))); - } + if (attribute.NameEquals != null || attribute.NameColon != null) + return; + + var parameter = attribute.DetermineParameter(semanticModel, cancellationToken: cancellationToken); + if (string.IsNullOrEmpty(parameter?.Name)) + return; + + addHint(new InlineParameterHint(parameter.GetSymbolKey(cancellationToken), parameter.Name, attribute.SpanStart, GetKind(attribute.Expression))); } } + private static bool MatchesMethodIntent(ArgumentSyntax argument, IParameterSymbol parameter) + { + // Methods like `SetColor(color: "y")` `FromResult(result: "x")` `Enable/DisablePolling(bool)` don't need + // parameter names to improve clarity. The parameter is clear from the context of the method name. + if (argument.Parent is not ArgumentListSyntax argumentList) + return false; + + if (argumentList.Arguments[0] != argument) + return false; + + if (argumentList.Parent is not InvocationExpressionSyntax invocationExpression) + return false; + + var invokedExpression = invocationExpression.Expression; + var rightMostName = invokedExpression.GetRightmostName(); + if (rightMostName == null) + return false; + + return MatchesMethodIntent(rightMostName.Identifier.ValueText, parameter); + } + private static InlineParameterHintKind GetKind(ExpressionSyntax arg) => arg switch { diff --git a/src/Features/Core/Portable/InlineHints/AbstractInlineParameterNameHintsService.cs b/src/Features/Core/Portable/InlineHints/AbstractInlineParameterNameHintsService.cs index bddd6f8dd2c..32e9172ccbc 100644 --- a/src/Features/Core/Portable/InlineHints/AbstractInlineParameterNameHintsService.cs +++ b/src/Features/Core/Portable/InlineHints/AbstractInlineParameterNameHintsService.cs @@ -2,11 +2,9 @@ // 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; -using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.PooledObjects; @@ -19,7 +17,9 @@ namespace Microsoft.CodeAnalysis.InlineHints internal abstract class AbstractInlineParameterNameHintsService : IInlineParameterNameHintsService { protected abstract void AddAllParameterNameHintLocations( - SemanticModel semanticModel, SyntaxNode node, Action addHint, CancellationToken cancellationToken); + SemanticModel semanticModel, SyntaxNode node, Action addHint, + bool hideForParametersThatDifferBySuffix, bool hideForParametersThatMatchMethodIntent, + CancellationToken cancellationToken); public async Task> GetInlineParameterNameHintsAsync(Document document, TextSpan textSpan, CancellationToken cancellationToken) { @@ -35,16 +35,24 @@ public async Task> GetInlineParameterNameHin if (!literalParameters && !objectCreationParameters && !otherParameters) return ImmutableArray.Empty; + var hideForParametersThatDifferBySuffix = options.GetOption(InlineHintsOptions.HideForParametersThatDifferBySuffix); + var hideForParametersThatMatchMethodIntent = options.GetOption(InlineHintsOptions.HideForParametersThatMatchMethodIntent); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); - using var _ = ArrayBuilder.GetInstance(out var result); + using var _1 = ArrayBuilder.GetInstance(out var result); + Action addHint = AddHint; foreach (var node in root.DescendantNodes(textSpan)) { cancellationToken.ThrowIfCancellationRequested(); - AddAllParameterNameHintLocations(semanticModel, node, addHint, cancellationToken); + AddAllParameterNameHintLocations( + semanticModel, node, addHint, + hideForParametersThatDifferBySuffix, + hideForParametersThatMatchMethodIntent, + cancellationToken); } return result.ToImmutable(); @@ -64,5 +72,47 @@ private static bool HintMatches(InlineParameterHint hint, bool literalParameters InlineParameterHintKind.Other => otherParameters, _ => throw ExceptionUtilities.UnexpectedValue(hint.Kind), }; + + protected static bool MatchesMethodIntent(string methodName, IParameterSymbol parameter) + { + // Check for something like `EnableLogging(true)` + if (TryGetIntent("Enable", methodName, out _) || + TryGetIntent("Disable", methodName, out _)) + { + return parameter.Type.SpecialType == SpecialType.System_Boolean; + } + + // More names can be added here if we find other patterns like this. + if (TryGetIntent("Set", methodName, out var methodIntent) || + TryGetIntent("From", methodName, out methodIntent)) + { + return IntentNameMatchesParameterName(methodIntent.Value, parameter.Name); + } + + return false; + + static bool TryGetIntent(string prefix, string nameValue, [NotNullWhen(true)] out ReadOnlyMemory? result) + { + if (nameValue.Length > prefix.Length && + nameValue.StartsWith(prefix) && + char.IsUpper(nameValue[prefix.Length])) + { + result = nameValue.AsMemory().Slice(prefix.Length); + return true; + } + + result = null; + return false; + } + + static bool IntentNameMatchesParameterName(ReadOnlyMemory intent, string parameterName) + { + // Method's name will be something like 'FromResult', so 'intent' will be 'Result' and parameterName + // will be 'result'. So we check if the first letters differ on case and the rest of the method + // matches. + return char.ToLower(intent.Span[0]) == parameterName[0] && + intent.Span.Slice(1).Equals(parameterName.AsSpan().Slice(1), StringComparison.Ordinal); + } + } } } diff --git a/src/Features/VisualBasic/Portable/InlineParameterNameHints/VisualBasicInlineParameterNameHintsService.vb b/src/Features/VisualBasic/Portable/InlineParameterNameHints/VisualBasicInlineParameterNameHintsService.vb index adf4dbb0a9a..063d73faa8d 100644 --- a/src/Features/VisualBasic/Portable/InlineParameterNameHints/VisualBasicInlineParameterNameHintsService.vb +++ b/src/Features/VisualBasic/Portable/InlineParameterNameHints/VisualBasicInlineParameterNameHintsService.vb @@ -23,14 +23,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.InlineParameterNameHints semanticModel As SemanticModel, node As SyntaxNode, addHint As Action(Of InlineParameterHint), - cancellationToken As CancellationToken) + hideForParametersThatDifferBySuffix As Boolean, + hideForParametersThatMatchMethodIntent As Boolean, + CancellationToken As CancellationToken) Dim simpleArgument = TryCast(node, SimpleArgumentSyntax) If simpleArgument?.Expression IsNot Nothing Then If Not simpleArgument.IsNamed AndAlso simpleArgument.NameColonEquals Is Nothing Then - Dim param = simpleArgument.DetermineParameter(semanticModel, allowParamArray:=False, cancellationToken) + Dim param = simpleArgument.DetermineParameter(semanticModel, allowParamArray:=False, CancellationToken) If Not String.IsNullOrEmpty(param?.Name) Then - addHint(New InlineParameterHint(param.GetSymbolKey(cancellationToken), param.Name, simpleArgument.Span.Start, GetKind(simpleArgument.Expression))) + addHint(New InlineParameterHint(param.GetSymbolKey(CancellationToken), param.Name, simpleArgument.Span.Start, GetKind(simpleArgument.Expression))) End If End If End If -- GitLab