From fc63c345f0db11345c691dc781553b8575020265 Mon Sep 17 00:00:00 2001 From: Gen Lu Date: Fri, 16 Aug 2019 12:03:52 -0700 Subject: [PATCH] Revert "Adding Null Checks For All Parameters" --- .../AddParameterCheckTests.cs | 258 ------------------ .../AddParameterCheckTests.vb | 56 ---- .../Portable/FeaturesResources.Designer.cs | 9 - .../Core/Portable/FeaturesResources.resx | 3 - ...ddParameterCheckCodeRefactoringProvider.cs | 168 +++--------- ...erCodeRefactoringProviderMemberCreation.cs | 7 +- ...tializeParameterCodeRefactoringProvider.cs | 130 ++------- .../Portable/xlf/FeaturesResources.cs.xlf | 5 - .../Portable/xlf/FeaturesResources.de.xlf | 5 - .../Portable/xlf/FeaturesResources.es.xlf | 5 - .../Portable/xlf/FeaturesResources.fr.xlf | 5 - .../Portable/xlf/FeaturesResources.it.xlf | 5 - .../Portable/xlf/FeaturesResources.ja.xlf | 5 - .../Portable/xlf/FeaturesResources.ko.xlf | 5 - .../Portable/xlf/FeaturesResources.pl.xlf | 5 - .../Portable/xlf/FeaturesResources.pt-BR.xlf | 5 - .../Portable/xlf/FeaturesResources.ru.xlf | 5 - .../Portable/xlf/FeaturesResources.tr.xlf | 5 - .../xlf/FeaturesResources.zh-Hans.xlf | 5 - .../xlf/FeaturesResources.zh-Hant.xlf | 5 - .../CodeGeneration/CSharpSyntaxGenerator.cs | 6 +- 21 files changed, 61 insertions(+), 641 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/InitializeParameter/AddParameterCheckTests.cs b/src/EditorFeatures/CSharpTest/InitializeParameter/AddParameterCheckTests.cs index d5b71865c81..b140f1a9959 100644 --- a/src/EditorFeatures/CSharpTest/InitializeParameter/AddParameterCheckTests.cs +++ b/src/EditorFeatures/CSharpTest/InitializeParameter/AddParameterCheckTests.cs @@ -304,248 +304,6 @@ public C(string s) }"); } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableParameters() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C([||]string a, string b, string c) - { - } -}", -@" -using System; - -class C -{ - public C(string a, string b, string c) - { - if (string.IsNullOrEmpty(a)) - { - throw new ArgumentException(""message"", nameof(a)); - } - - if (string.IsNullOrEmpty(b)) - { - throw new ArgumentException(""message"", nameof(b)); - } - - if (string.IsNullOrEmpty(c)) - { - throw new ArgumentException(""message"", nameof(c)); - } - } -}", index: 3); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestCursorNotOnParameters() - { - await TestMissingInRegularAndScriptAsync( - -@" -using System; - -class C -{ - public C(string a[|,|] string b, string c) - { - } -}" -); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableWithCursorOnNonNullable() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C(string a, [||]bool b, string c) - { - } -}", -@" -using System; - -class C -{ - public C(string a, bool b, string c) - { - if (string.IsNullOrEmpty(a)) - { - throw new ArgumentException(""message"", nameof(a)); - } - - if (string.IsNullOrEmpty(c)) - { - throw new ArgumentException(""message"", nameof(c)); - } - } -}", index: 0); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableNonNullable() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C([||]string a, bool b, string c) - { - } -}", -@" -using System; - -class C -{ - public C(string a, bool b, string c) - { - if (string.IsNullOrEmpty(a)) - { - throw new ArgumentException(""message"", nameof(a)); - } - - if (string.IsNullOrEmpty(c)) - { - throw new ArgumentException(""message"", nameof(c)); - } - } -}", index: 3); - - } - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableStringsAndObjects() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C([||]string a, object b, string c) - { - } -}", -@" -using System; - -class C -{ - public C(string a, object b, string c) - { - if (string.IsNullOrEmpty(a)) - { - throw new ArgumentException(""message"", nameof(a)); - } - - if (b is null) - { - throw new ArgumentNullException(nameof(b)); - } - - if (string.IsNullOrEmpty(c)) - { - throw new ArgumentException(""message"", nameof(c)); - } - } -}", index: 3); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableObjects() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C([||]object a, object b, object c) - { - } -}", -@" -using System; - -class C -{ - public C(object a, object b, object c) - { - if (a is null) - { - throw new ArgumentNullException(nameof(a)); - } - - if (b is null) - { - throw new ArgumentNullException(nameof(b)); - } - - if (c is null) - { - throw new ArgumentNullException(nameof(c)); - } - } -}", index: 1); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestMultiNullableStructs() - { - await TestInRegularAndScript1Async( - -@" -using System; - -class C -{ - public C([||]int ? a, bool ? b, double ? c) - { - } -}", -@" -using System; - -class C -{ - public C(int ? a, bool ? b, double ? c) - { - if (a is null) - { - throw new ArgumentNullException(nameof(a)); - } - - if (b is null) - { - throw new ArgumentNullException(nameof(b)); - } - - if (c is null) - { - throw new ArgumentNullException(nameof(c)); - } - } -}", index: 1); - } - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] public async Task TestUpdateExistingPropertyAssignment() { @@ -1359,22 +1117,6 @@ class C } }"); } - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] - public async Task TestNotOnIndexerParameters() - { - await TestMissingAsync( -@" -class C -{ - int this[[|object a|], object b, object c] - { - get - { - return 0; - } - } -}"); - } [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInitializeParameter)] public async Task TestSpecialStringCheck1() diff --git a/src/EditorFeatures/VisualBasicTest/InitializeParameter/AddParameterCheckTests.vb b/src/EditorFeatures/VisualBasicTest/InitializeParameter/AddParameterCheckTests.vb index 98e6772ee08..80d6a3c9c0b 100644 --- a/src/EditorFeatures/VisualBasicTest/InitializeParameter/AddParameterCheckTests.vb +++ b/src/EditorFeatures/VisualBasicTest/InitializeParameter/AddParameterCheckTests.vb @@ -466,62 +466,6 @@ class C end class", index:=2) End Function - - Public Async Function TestMultiNullableParameters() As Task - Await TestInRegularAndScript1Async( -" -Imports System - -class C - public sub new([||]a as string, b as string, c as string) - end sub -end class", -" -Imports System - -class C - public sub new(a as string, b as string, c as string) - If String.IsNullOrEmpty(a) Then - Throw New ArgumentException(""message"", NameOf(a)) - End If - - If String.IsNullOrEmpty(b) Then - Throw New ArgumentException(""message"", NameOf(b)) - End If - - If String.IsNullOrEmpty(c) Then - Throw New ArgumentException(""message"", NameOf(c)) - End If - end sub -end class", index:=3) - End Function - - - Public Async Function TestMultiNullableWithCursorOnNonNullable() As Task - Await TestInRegularAndScript1Async( -" -Imports System - -class C - public sub new([||]a as boolean, b as string, c as object) - end sub -end class", -" -Imports System - -class C - public sub new(a as boolean, b as string, c as object) - If String.IsNullOrEmpty(b) Then - Throw New ArgumentException(""message"", NameOf(b)) - End If - - If c Is Nothing Then - Throw New ArgumentNullException(NameOf(c)) - End If - end sub -end class", index:=0) - End Function - Public Async Function TestSimpleReferenceTypeWithParameterNameSelected1() As Task diff --git a/src/Features/Core/Portable/FeaturesResources.Designer.cs b/src/Features/Core/Portable/FeaturesResources.Designer.cs index 23f4244be95..d9aec65fc9b 100644 --- a/src/Features/Core/Portable/FeaturesResources.Designer.cs +++ b/src/Features/Core/Portable/FeaturesResources.Designer.cs @@ -232,15 +232,6 @@ internal class FeaturesResources { } } - /// - /// Looks up a localized string similar to Add null checks for all parameters. - /// - internal static string Add_null_checks_for_all_parameters { - get { - return ResourceManager.GetString("Add_null_checks_for_all_parameters", resourceCulture); - } - } - /// /// Looks up a localized string similar to Add optional parameter to constructor. /// diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index 7efdf07d115..0ff88bc7a65 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -1693,7 +1693,4 @@ This version used in: {2} Warning: Semantics may change when converting statement. - - Add null checks for all parameters - \ No newline at end of file diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index 785734d32ce..800732f5b11 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -1,7 +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; -using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -30,53 +29,54 @@ internal abstract partial class AbstractAddParameterCheckCodeRefactoringProvider where TExpressionSyntax : SyntaxNode where TBinaryExpressionSyntax : TExpressionSyntax { - protected override async Task> GetRefactoringsForAllParametersAsync( - Document document, SyntaxNode functionDeclaration, IMethodSymbol methodSymbol, - IBlockOperation blockStatementOpt, ImmutableArray listOfParameterNodes, int position, CancellationToken cancellationToken) + protected override async Task> GetRefactoringsAsync( + Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method, + IBlockOperation blockStatementOpt, CancellationToken cancellationToken) { + // Only should provide null-checks for reference types and nullable types. + if (!parameter.Type.IsReferenceType && + !parameter.Type.IsNullable()) + { + return ImmutableArray.Empty; + } - // List to keep track of the valid parameters - var listOfParametersOrdinals = new List(); + var syntaxFacts = document.GetLanguageService(); var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - foreach (var parameterNode in listOfParameterNodes) + // Look for an existing "if (p == null)" statement, or "p ?? throw" check. If we already + // have one, we don't want to offer to generate a new null check. + // + // Note: we only check the top level statements of the block. I think that's sufficient + // as this will catch the 90% case, while not being that bad an experience even when + // people do strange things in their constructors. + if (blockStatementOpt != null) { - var parameter = (IParameterSymbol)semanticModel.GetDeclaredSymbol(parameterNode, cancellationToken); - if (ParameterValidForNullCheck(document, parameter, semanticModel, blockStatementOpt, cancellationToken)) + if (!CanOffer(blockStatementOpt.Syntax)) { - listOfParametersOrdinals.Add(parameter.Ordinal); + return ImmutableArray.Empty; } - } - // Min 2 parameters to offer the refactoring - if (listOfParametersOrdinals.Count() < 2) - { - return ImmutableArray.Empty; - } - - // Great. The list has parameters that need null checks. Offer to add null checks for all. - return ImmutableArray.Create(new MyCodeAction( - FeaturesResources.Add_null_checks_for_all_parameters, - c => UpdateDocumentForRefactoringAsync(document, functionDeclaration, blockStatementOpt, listOfParametersOrdinals, position, c))); - } - - protected override async Task> GetRefactoringsForSingleParameterAsync( - Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol methodSymbol, - IBlockOperation blockStatementOpt, CancellationToken cancellationToken) - { - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + foreach (var statement in blockStatementOpt.Operations) + { + if (IsIfNullCheck(statement, parameter)) + { + return ImmutableArray.Empty; + } - // Only should provide null-checks for reference types and nullable types. - if (!ParameterValidForNullCheck(document, parameter, semanticModel, blockStatementOpt, cancellationToken)) - { - return ImmutableArray.Empty; + if (ContainsNullCoalesceCheck( + syntaxFacts, semanticModel, statement, + parameter, cancellationToken)) + { + return ImmutableArray.Empty; + } + } } // Great. There was no null check. Offer to add one. var result = ArrayBuilder.GetInstance(); result.Add(new MyCodeAction( FeaturesResources.Add_null_check, - c => AddNullCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, c))); + c => AddNullCheckAsync(document, parameter, functionDeclaration, method, blockStatementOpt, c))); // Also, if this was a string, offer to add the special checks to // string.IsNullOrEmpty and string.IsNullOrWhitespace. @@ -84,11 +84,11 @@ internal abstract partial class AbstractAddParameterCheckCodeRefactoringProvider { result.Add(new MyCodeAction( FeaturesResources.Add_string_IsNullOrEmpty_check, - c => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, nameof(string.IsNullOrEmpty), c))); + c => AddStringCheckAsync(document, parameter, functionDeclaration, method, blockStatementOpt, nameof(string.IsNullOrEmpty), c))); result.Add(new MyCodeAction( FeaturesResources.Add_string_IsNullOrWhiteSpace_check, - c => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, nameof(string.IsNullOrWhiteSpace), c))); + c => AddStringCheckAsync(document, parameter, functionDeclaration, method, blockStatementOpt, nameof(string.IsNullOrWhiteSpace), c))); } return result.ToImmutableAndFree(); @@ -96,61 +96,6 @@ internal abstract partial class AbstractAddParameterCheckCodeRefactoringProvider protected abstract bool CanOffer(SyntaxNode body); - private async Task UpdateDocumentForRefactoringAsync( - Document document, - SyntaxNode functionDeclaration, - IBlockOperation blockStatementOpt, - List listOfParametersOrdinals, - int position, - CancellationToken cancellationToken) - { - foreach (var index in listOfParametersOrdinals) - { - // Updates functionDeclaration and uses it to get the first valid ParameterNode using the ordinals (index). - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var token = root.FindToken(position); - var firstparameterNode = GetParameterNode(token, position); - functionDeclaration = firstparameterNode.FirstAncestorOrSelf(IsFunctionDeclaration); - var generator = SyntaxGenerator.GetGenerator(document); - var parameterNodes = generator.GetParameters(functionDeclaration); - var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var parameter = GetParameterAtOrdinal(index, parameterNodes, semanticModel, cancellationToken); - var syntaxFacts = document.GetLanguageService(); - - if (!CanOfferRefactoring(functionDeclaration, semanticModel, syntaxFacts, cancellationToken, out blockStatementOpt)) - { - continue; - } - - // If parameter is a string, default check would be IsNullOrEmpty. This is because IsNullOrEmpty is more commonly used in this regard according to telemetry and UX testing. - if (parameter.Type.SpecialType == SpecialType.System_String) - { - document = await AddStringCheckAsync(document, parameter, functionDeclaration, (IMethodSymbol)parameter.ContainingSymbol, blockStatementOpt, nameof(string.IsNullOrEmpty), cancellationToken).ConfigureAwait(false); - continue; - } - - // For all other parameters, add null check - updates document - document = await AddNullCheckAsync(document, parameter, functionDeclaration, - (IMethodSymbol)parameter.ContainingSymbol, blockStatementOpt, cancellationToken).ConfigureAwait(false); - } - - return document; - } - - private IParameterSymbol GetParameterAtOrdinal(int index, IReadOnlyList parameterNodes, SemanticModel semanticModel, CancellationToken cancellationToken) - { - foreach (var parameterNode in parameterNodes) - { - var parameter = (IParameterSymbol)semanticModel.GetDeclaredSymbol(parameterNode, cancellationToken); - if (index == parameter.Ordinal) - { - return parameter; - } - } - - return null; - } - private bool ContainsNullCoalesceCheck( ISyntaxFactsService syntaxFacts, SemanticModel semanticModel, IOperation statement, IParameterSymbol parameter, @@ -211,49 +156,6 @@ private bool IsIfNullCheck(IOperation statement, IParameterSymbol parameter) return false; } - protected bool ParameterValidForNullCheck(Document document, IParameterSymbol parameter, SemanticModel semanticModel, - IBlockOperation blockStatementOpt, CancellationToken cancellationToken) - { - if (!parameter.Type.IsReferenceType && - !parameter.Type.IsNullable()) - { - return false; - } - - var syntaxFacts = document.GetLanguageService(); - - // Look for an existing "if (p == null)" statement, or "p ?? throw" check. If we already - // have one, we don't want to offer to generate a new null check. - // - // Note: we only check the top level statements of the block. I think that's sufficient - // as this will catch the 90% case, while not being that bad an experience even when - // people do strange things in their constructors. - if (blockStatementOpt != null) - { - if (!CanOffer(blockStatementOpt.Syntax)) - { - return false; - } - - foreach (var statement in blockStatementOpt.Operations) - { - if (IsIfNullCheck(statement, parameter)) - { - return false; - } - - if (ContainsNullCoalesceCheck( - syntaxFacts, semanticModel, statement, - parameter, cancellationToken)) - { - return false; - } - } - } - - return true; - } - private bool IsStringCheck(IOperation condition, IParameterSymbol parameter) { if (condition is IInvocationOperation invocation && diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractInitializeMemberFromParameterCodeRefactoringProviderMemberCreation.cs b/src/Features/Core/Portable/InitializeParameter/AbstractInitializeMemberFromParameterCodeRefactoringProviderMemberCreation.cs index b1d440fb171..22a12a6e1cc 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractInitializeMemberFromParameterCodeRefactoringProviderMemberCreation.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractInitializeMemberFromParameterCodeRefactoringProviderMemberCreation.cs @@ -34,12 +34,7 @@ internal abstract partial class AbstractInitializeMemberFromParameterCodeRefacto protected abstract Accessibility DetermineDefaultPropertyAccessibility(); - protected override Task> GetRefactoringsForAllParametersAsync(Document document, SyntaxNode functionDeclaration, IMethodSymbol method, IBlockOperation blockStatementOpt, ImmutableArray listOfParameterNodes, int position, CancellationToken cancellationToken) - { - return Task.FromResult(ImmutableArray.Empty); - } - - protected override async Task> GetRefactoringsForSingleParameterAsync( + protected override async Task> GetRefactoringsAsync( Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method, IBlockOperation blockStatementOpt, CancellationToken cancellationToken) { diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractInitializeParameterCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractInitializeParameterCodeRefactoringProvider.cs index dac3a0b5b14..83307e93749 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractInitializeParameterCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractInitializeParameterCodeRefactoringProvider.cs @@ -10,7 +10,6 @@ using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Operations; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.InitializeParameter @@ -29,126 +28,62 @@ internal abstract partial class AbstractInitializeParameterCodeRefactoringProvid protected abstract SyntaxNode GetBody(SyntaxNode functionDeclaration); protected abstract SyntaxNode GetTypeBlock(SyntaxNode node); - protected abstract Task> GetRefactoringsForAllParametersAsync( - Document document, - SyntaxNode functionDeclaration, - IMethodSymbol method, - IBlockOperation blockStatementOpt, - ImmutableArray listOfParameterNodes, - int position, - CancellationToken cancellationToken); - - protected abstract Task> GetRefactoringsForSingleParameterAsync( - Document document, - IParameterSymbol parameter, - SyntaxNode functionDeclaration, - IMethodSymbol methodSymbol, - IBlockOperation blockStatementOpt, - CancellationToken cancellationToken); - protected abstract void InsertStatement( SyntaxEditor editor, SyntaxNode functionDeclaration, IMethodSymbol method, SyntaxNode statementToAddAfterOpt, TStatementSyntax statement); + protected abstract Task> GetRefactoringsAsync( + Document document, IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method, + IBlockOperation blockStatementOpt, CancellationToken cancellationToken); + public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) { var (document, textSpan, cancellationToken) = context; - var position = context.Span.Start; var syntaxTree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var token = root.FindToken(position); - var selectedParameter = await context.TryGetRelevantNodeAsync().ConfigureAwait(false); - if (selectedParameter == null) + + var parameterNode = await context.TryGetRelevantNodeAsync().ConfigureAwait(false); + if (parameterNode == null) { return; } - var functionDeclaration = selectedParameter.FirstAncestorOrSelf(IsFunctionDeclaration); - if (functionDeclaration is null) + var functionDeclaration = parameterNode.FirstAncestorOrSelf(IsFunctionDeclaration); + if (functionDeclaration == null) { return; } - var generator = SyntaxGenerator.GetGenerator(document); - var parameterNodes = generator.GetParameters(functionDeclaration); var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - var syntaxFacts = document.GetLanguageService(); - - // Only offered when there isn't a selection, or the selection exactly selects a parameter name. - if (!context.Span.IsEmpty) - { - var parameterName = syntaxFacts.GetNameOfParameter(selectedParameter); - if (parameterName == null || parameterName.Value.Span != context.Span) - { - return; - } - } - - var parameterDefault = syntaxFacts.GetDefaultOfParameter(selectedParameter); - - // Don't offer inside the "=initializer" of a parameter - if (parameterDefault?.Span.Contains(position) == true) - { - return; - } // we can't just call GetDeclaredSymbol on functionDeclaration because it could an anonymous function, // so first we have to get the parameter symbol and then its containing method symbol - if (!TryGetParameterSymbol(selectedParameter, semanticModel, out var parameter, cancellationToken)) + var parameter = (IParameterSymbol)semanticModel.GetDeclaredSymbol(parameterNode, cancellationToken); + if (parameter == null || parameter.Name == "") { return; } - var methodSymbol = (IMethodSymbol)parameter.ContainingSymbol; - if (methodSymbol.IsAbstract || - methodSymbol.IsExtern || - methodSymbol.PartialImplementationPart != null || - methodSymbol.ContainingType.TypeKind == TypeKind.Interface) + var method = (IMethodSymbol)parameter.ContainingSymbol; + if (method.IsAbstract || + method.IsExtern || + method.PartialImplementationPart != null || + method.ContainingType.TypeKind == TypeKind.Interface) { return; } + var syntaxFacts = document.GetLanguageService(); if (CanOfferRefactoring(functionDeclaration, semanticModel, syntaxFacts, cancellationToken, out var blockStatementOpt)) { - // Ok. Looks like the selected parameter could be refactored. Defer to subclass to - // actually determine if there are any viable refactorings here. - context.RegisterRefactorings(await GetRefactoringsForSingleParameterAsync( - document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, cancellationToken).ConfigureAwait(false)); - } - - // List with parameterNodes that pass all checks - var listOfPotentiallyValidParametersNodes = ArrayBuilder.GetInstance(); - foreach (var parameterNode in parameterNodes) - { - if (!TryGetParameterSymbol(parameterNode, semanticModel, out parameter, cancellationToken)) - { - return; - } - - // Update the list of valid parameter nodes - listOfPotentiallyValidParametersNodes.Add(parameterNode); - } - - if (listOfPotentiallyValidParametersNodes.Count > 1) - { - // Looks like we can offer a refactoring for more than one parameter. Defer to subclass to + // Ok. Looks like a reasonable parameter to analyze. Defer to subclass to // actually determine if there are any viable refactorings here. - context.RegisterRefactorings(await GetRefactoringsForAllParametersAsync( - document, functionDeclaration, methodSymbol, blockStatementOpt, listOfPotentiallyValidParametersNodes.ToImmutableAndFree(), position, cancellationToken).ConfigureAwait(false)); - } - - static bool TryGetParameterSymbol( - SyntaxNode parameterNode, - SemanticModel semanticModel, - out IParameterSymbol parameter, - CancellationToken cancellationToken) - { - parameter = (IParameterSymbol)semanticModel.GetDeclaredSymbol(parameterNode, cancellationToken); - return parameter.Name != ""; + context.RegisterRefactorings(await GetRefactoringsAsync( + document, parameter, functionDeclaration, method, blockStatementOpt, cancellationToken).ConfigureAwait(false)); } } - protected bool CanOfferRefactoring(SyntaxNode functionDeclaration, SemanticModel semanticModel, ISyntaxFactsService syntaxFacts, CancellationToken cancellationToken, out IBlockOperation blockStatementOpt) + private bool CanOfferRefactoring(SyntaxNode functionDeclaration, SemanticModel semanticModel, ISyntaxFactsService syntaxFacts, CancellationToken cancellationToken, out IBlockOperation blockStatementOpt) { blockStatementOpt = null; @@ -163,7 +98,6 @@ protected bool CanOfferRefactoring(SyntaxNode functionDeclaration, SemanticModel // In order to get the block operation for the body of an anonymous function, we need to // get it via `IAnonymousFunctionOperation.Body` instead of getting it directly from the body syntax. - var operation = semanticModel.GetOperation( syntaxFacts.IsAnonymousFunction(functionDeclaration) ? functionDeclaration : functionBody, cancellationToken); @@ -188,27 +122,9 @@ protected bool CanOfferRefactoring(SyntaxNode functionDeclaration, SemanticModel return true; } - protected TParameterSyntax GetParameterNode(SyntaxToken token, int position) - { - var parameterNode = token.Parent?.FirstAncestorOrSelf(); - if (parameterNode != null) - { - return parameterNode; - } - - // We may be on the comma of a param list. Try the position before us. - token = token.GetPreviousToken(); - if (position == token.FullSpan.End) - { - return token.Parent?.FirstAncestorOrSelf(); - } - - return null; - } - protected static bool IsParameterReference(IOperation operation, IParameterSymbol parameter) - => UnwrapImplicitConversion(operation) is IParameterReferenceOperation parameterReference && - parameter.Equals(parameterReference.Parameter); + => UnwrapImplicitConversion(operation) is IParameterReferenceOperation parameterReference && + parameter.Equals(parameterReference.Parameter); protected static IOperation UnwrapImplicitConversion(IOperation operation) => operation is IConversionOperation conversion && conversion.IsImplicit diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf index e685ee2abb1..b4e9fe1a63b 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf @@ -7,11 +7,6 @@ Přidat název členu - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf index c91f21672c5..5f8fd827c68 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf @@ -7,11 +7,6 @@ Membername hinzufügen - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf index f55d8f8feca..fa76a22fe00 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf @@ -7,11 +7,6 @@ Agregar nombre de miembro - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf index 727d521bd64..d588a202409 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf @@ -7,11 +7,6 @@ Ajouter le nom du membre - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf index bd69f19cf09..b82a3a4f4b2 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf @@ -7,11 +7,6 @@ Aggiungi nome del membro - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf index b047fa0277a..524a2ccfa47 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf @@ -7,11 +7,6 @@ メンバー名を追加します - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf index 52465f9c1ef..a6e487beff0 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf @@ -7,11 +7,6 @@ 멤버 이름 추가 - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf index a68e7e6a045..e630162c5ea 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf @@ -7,11 +7,6 @@ Dodaj nazwę składowej - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf index 28b34df2de2..780d7f73b65 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf @@ -7,11 +7,6 @@ Adicionar o nome do membro - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf index e1f0fd5328d..c8c18718607 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf @@ -7,11 +7,6 @@ Добавить имя элемента - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf index e5da9b952a8..47b0ba9a737 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf @@ -7,11 +7,6 @@ Üye adı Ekle - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf index d7b07dc4779..9da85811fa1 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf @@ -7,11 +7,6 @@ 添加成员名称 - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf index db6109eeb50..63aaa9dafa4 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf @@ -7,11 +7,6 @@ 新增成員名稱 - - Add null checks for all parameters - Add null checks for all parameters - - Add optional parameter to constructor Add optional parameter to constructor diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs index c623f3b5b91..1ae65751174 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs @@ -2295,9 +2295,7 @@ public override IReadOnlyList GetParameters(SyntaxNode declaration) var list = GetParameterList(declaration); return list != null ? list.Parameters - : declaration is SimpleLambdaExpressionSyntax simpleLambda - ? new[] { simpleLambda.Parameter } - : SpecializedCollections.EmptyReadOnlyList(); + : SpecializedCollections.EmptyReadOnlyList(); } public override SyntaxNode InsertParameters(SyntaxNode declaration, int index, IEnumerable parameters) @@ -2387,8 +2385,8 @@ internal static BaseParameterListSyntax GetParameterList(SyntaxNode declaration) SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)declaration).ParameterList, SyntaxKind.IndexerDeclaration => ((IndexerDeclarationSyntax)declaration).ParameterList, SyntaxKind.ParenthesizedLambdaExpression => ((ParenthesizedLambdaExpressionSyntax)declaration).ParameterList, + SyntaxKind.SimpleLambdaExpression => SyntaxFactory.ParameterList(SyntaxFactory.SingletonSeparatedList(((SimpleLambdaExpressionSyntax)declaration).Parameter)), SyntaxKind.LocalFunctionStatement => ((LocalFunctionStatementSyntax)declaration).ParameterList, - SyntaxKind.AnonymousMethodExpression => ((AnonymousMethodExpressionSyntax)declaration).ParameterList, _ => (BaseParameterListSyntax)null, }; -- GitLab