From 92d5b5b265957688c152bd4a3a50679b6ba48ea0 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 15 May 2025 13:30:51 +0300 Subject: [PATCH 1/7] Remove 'Opt' suffixes --- ...ddParameterCheckCodeRefactoringProvider.cs | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index d0fa8ba6bfb07..1d34a00ee906f 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -61,7 +61,7 @@ protected override async Task> GetRefactoringsForAllP Document document, SyntaxNode functionDeclaration, IMethodSymbol methodSymbol, - IBlockOperation? blockStatementOpt, + IBlockOperation? blockStatement, ImmutableArray listOfParameterNodes, TextSpan parameterSpan, CancellationToken cancellationToken) @@ -73,7 +73,7 @@ protected override async Task> GetRefactoringsForAllP foreach (var parameterNode in listOfParameterNodes) { var parameter = (IParameterSymbol)semanticModel.GetRequiredDeclaredSymbol(parameterNode, cancellationToken); - if (ParameterValidForNullCheck(document, parameter, semanticModel, blockStatementOpt, cancellationToken)) + if (ParameterValidForNullCheck(document, parameter, semanticModel, blockStatement, cancellationToken)) listOfParametersOrdinals.Add(parameter.Ordinal); } @@ -84,7 +84,7 @@ protected override async Task> GetRefactoringsForAllP // Great. The list has parameters that need null checks. Offer to add null checks for all. return [CodeAction.Create( FeaturesResources.Add_null_checks_for_all_parameters, - c => UpdateDocumentForRefactoringAsync(document, blockStatementOpt, listOfParametersOrdinals, parameterSpan, c), + c => UpdateDocumentForRefactoringAsync(document, blockStatement, listOfParametersOrdinals, parameterSpan, c), nameof(FeaturesResources.Add_null_checks_for_all_parameters))]; } @@ -94,13 +94,13 @@ protected override async Task> GetRefactoringsForSing IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol methodSymbol, - IBlockOperation? blockStatementOpt, + IBlockOperation? blockStatement, CancellationToken cancellationToken) { var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); // Only should provide null-checks for reference types and nullable types. - if (!ParameterValidForNullCheck(document, parameter, semanticModel, blockStatementOpt, cancellationToken)) + if (!ParameterValidForNullCheck(document, parameter, semanticModel, blockStatement, cancellationToken)) return []; var simplifierOptions = (TSimplifierOptions)await document.GetSimplifierOptionsAsync(cancellationToken).ConfigureAwait(false); @@ -109,7 +109,7 @@ protected override async Task> GetRefactoringsForSing using var result = TemporaryArray.Empty; result.Add(CodeAction.Create( FeaturesResources.Add_null_check, - cancellationToken => AddNullCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, simplifierOptions, cancellationToken), + cancellationToken => AddNullCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, simplifierOptions, cancellationToken), nameof(FeaturesResources.Add_null_check))); // Also, if this was a string, offer to add the special checks to string.IsNullOrEmpty and @@ -118,12 +118,12 @@ protected override async Task> GetRefactoringsForSing { result.Add(CodeAction.Create( FeaturesResources.Add_string_IsNullOrEmpty_check, - cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), + cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), nameof(FeaturesResources.Add_string_IsNullOrEmpty_check))); result.Add(CodeAction.Create( FeaturesResources.Add_string_IsNullOrWhiteSpace_check, - cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatementOpt, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), + cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), nameof(FeaturesResources.Add_string_IsNullOrWhiteSpace_check))); } @@ -132,7 +132,7 @@ protected override async Task> GetRefactoringsForSing private async Task UpdateDocumentForRefactoringAsync( Document document, - IBlockOperation? blockStatementOpt, + IBlockOperation? blockStatement, List listOfParametersOrdinals, TextSpan parameterSpan, CancellationToken cancellationToken) @@ -159,7 +159,7 @@ private async Task UpdateDocumentForRefactoringAsync( var syntaxFacts = document.GetRequiredLanguageService(); - if (!CanOfferRefactoring(functionDeclaration, semanticModel, syntaxFacts, cancellationToken, out blockStatementOpt)) + if (!CanOfferRefactoring(functionDeclaration, semanticModel, syntaxFacts, cancellationToken, out blockStatement)) continue; lazySimplifierOptions ??= (TSimplifierOptions)await document.GetSimplifierOptionsAsync(cancellationToken).ConfigureAwait(false); @@ -168,13 +168,13 @@ private async Task UpdateDocumentForRefactoringAsync( // 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, s_nullOrEmptySuffix, lazySimplifierOptions, cancellationToken).ConfigureAwait(false); + document = await AddStringCheckAsync(document, parameter, functionDeclaration, (IMethodSymbol)parameter.ContainingSymbol, blockStatement, s_nullOrEmptySuffix, lazySimplifierOptions, cancellationToken).ConfigureAwait(false); continue; } // For all other parameters, add null check - updates document document = await AddNullCheckAsync(document, parameter, functionDeclaration, - (IMethodSymbol)parameter.ContainingSymbol, blockStatementOpt, lazySimplifierOptions, cancellationToken).ConfigureAwait(false); + (IMethodSymbol)parameter.ContainingSymbol, blockStatement, lazySimplifierOptions, cancellationToken).ConfigureAwait(false); } return document; @@ -251,7 +251,7 @@ private static bool IsIfNullCheck(IOperation statement, IParameterSymbol paramet } protected bool ParameterValidForNullCheck(Document document, IParameterSymbol parameter, SemanticModel semanticModel, - IBlockOperation? blockStatementOpt, CancellationToken cancellationToken) + IBlockOperation? blockStatement, CancellationToken cancellationToken) { if (parameter.Type.IsReferenceType) { @@ -278,12 +278,12 @@ protected bool ParameterValidForNullCheck(Document document, IParameterSymbol pa // 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 (blockStatement != null) { - if (!CanOffer(blockStatementOpt.Syntax)) + if (!CanOffer(blockStatement.Syntax)) return false; - foreach (var statement in blockStatementOpt.Operations) + foreach (var statement in blockStatement.Operations) { if (IsIfNullCheck(statement, parameter)) return false; @@ -369,13 +369,13 @@ private async Task AddStringCheckAsync( IParameterSymbol parameter, SyntaxNode functionDeclaration, IMethodSymbol method, - IBlockOperation? blockStatementOpt, + IBlockOperation? blockStatement, string methodNameSuffix, TSimplifierOptions options, CancellationToken cancellationToken) { return await AddNullCheckStatementAsync( - document, parameter, functionDeclaration, method, blockStatementOpt, + document, parameter, functionDeclaration, method, blockStatement, (s, g) => CreateStringCheckStatement(s.Compilation, g, parameter, methodNameSuffix, options), cancellationToken).ConfigureAwait(false); } From 11701ac318b4cd04e165ea232b22c3bcafc90952 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 15 May 2025 13:37:30 +0300 Subject: [PATCH 2/7] Unify resource strings for special case checks --- .../AddParameterCheckTests.cs | 22 +++++++++---------- .../Core/Portable/FeaturesResources.resx | 9 +++----- ...ddParameterCheckCodeRefactoringProvider.cs | 8 +++---- .../Portable/xlf/FeaturesResources.cs.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.de.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.es.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.fr.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.it.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.ja.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.ko.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.pl.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.pt-BR.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.ru.xlf | 15 +++++-------- .../Portable/xlf/FeaturesResources.tr.xlf | 15 +++++-------- .../xlf/FeaturesResources.zh-Hans.xlf | 15 +++++-------- .../xlf/FeaturesResources.zh-Hant.xlf | 15 +++++-------- 16 files changed, 83 insertions(+), 151 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs index 612f249da0c30..55d9d6b543c4a 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs @@ -1977,7 +1977,7 @@ public C(string s) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2015,7 +2015,7 @@ public C(string s) } """, CodeActionIndex = 2, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrWhiteSpace_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace" }.RunAsync(); } @@ -2046,7 +2046,7 @@ public C(string s) } """, CodeActionIndex = 2, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrWhiteSpace_check), + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace", ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } @@ -2086,7 +2086,7 @@ public C(string s) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2142,7 +2142,7 @@ static void Main(String bar) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check), + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty", Options = { { CodeStyleOptions2.PreferIntrinsicPredefinedTypeKeywordInMemberAccess, CodeStyleOption2.FalseWithSuggestionEnforcement } @@ -2740,7 +2740,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2782,7 +2782,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2826,7 +2826,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2867,7 +2867,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2908,7 +2908,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } @@ -2952,7 +2952,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = nameof(FeaturesResources.Add_string_IsNullOrEmpty_check) + CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" }.RunAsync(); } diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index 1521e7452b381..3bd6140ad31b4 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -923,12 +923,6 @@ This version used in: {2} Add null check - - Add 'string.IsNullOrEmpty' check - - - Add 'string.IsNullOrWhiteSpace' check - Create and assign field '{0}' @@ -3229,4 +3223,7 @@ Zero-width positive lookbehind assertions are typically used at the beginning of Multiple handlers found for message {0}. + + Add '{0}' check + \ 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 1d34a00ee906f..3602e0899935c 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -117,14 +117,14 @@ protected override async Task> GetRefactoringsForSing if (parameter.Type.SpecialType == SpecialType.System_String) { result.Add(CodeAction.Create( - FeaturesResources.Add_string_IsNullOrEmpty_check, + string.Format(FeaturesResources.Add_0_check, "string.IsNullOrEmpty"), cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), - nameof(FeaturesResources.Add_string_IsNullOrEmpty_check))); + $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty")); result.Add(CodeAction.Create( - FeaturesResources.Add_string_IsNullOrWhiteSpace_check, + string.Format(FeaturesResources.Add_0_check, "string.IsNullOrWhiteSpace"), cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), - nameof(FeaturesResources.Add_string_IsNullOrWhiteSpace_check))); + $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace")); } return result.ToImmutableAndClear(); diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf index 14838d2f49a67..21bae01e7936f 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.cs.xlf @@ -40,6 +40,11 @@ Ujistěte se, že specifikátor tt použijete pro jazyky, pro které je nezbytn Odčítání musí být posledním prvkem ve třídě znaků. This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Přidat atribut DebuggerDisplay @@ -4835,16 +4840,6 @@ Tato verze se používá zde: {2}. Přidat kontrolu hodnot null - - Add 'string.IsNullOrEmpty' check - Přidat kontrolu metody string.IsNullOrEmpty - - - - Add 'string.IsNullOrWhiteSpace' check - Přidat kontrolu metody string.IsNullOrWhiteSpace - - Initialize field '{0}' Inicializovat pole {0} diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf index 8fe80d9447f8b..6703101cea0b5 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.de.xlf @@ -40,6 +40,11 @@ Stellen Sie sicher, dass Sie den Bezeichner "tt" für Sprachen verwenden, für d Eine Subtraktion muss das letzte Element in einer Zeichenklasse sein. This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute DebuggerDisplay-Attribut hinzufügen @@ -4835,16 +4840,6 @@ Diese Version wird verwendet in: {2} NULL-Überprüfung hinzufügen - - Add 'string.IsNullOrEmpty' check - "string.IsNullOrEmpty"-Überprüfung hinzufügen - - - - Add 'string.IsNullOrWhiteSpace' check - "string.IsNullOrWhiteSpace"-Überprüfung hinzufügen - - Initialize field '{0}' Feld "{0}" initialisieren diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf index 5a91d8d92f2bb..2ec27811bf8f2 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.es.xlf @@ -40,6 +40,11 @@ Asegúrese de usar el especificador "tt" para los idiomas para los que es necesa Una sustracción debe ser el último elemento de una clase de caracteres This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Agregar atributo de "DebuggerDisplay" @@ -4835,16 +4840,6 @@ Esta versión se utiliza en: {2} Agregar comprobación de valores null - - Add 'string.IsNullOrEmpty' check - Agregar comprobación de "string.IsNullOrEmpty" - - - - Add 'string.IsNullOrWhiteSpace' check - Agregar comprobación de "string.IsNullOrWhiteSpace" - - Initialize field '{0}' Inicializar campo "{0}" diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf index 0412ea669714d..446554fb783eb 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.fr.xlf @@ -40,6 +40,11 @@ Veillez à utiliser le spécificateur "tt" pour les langues où il est nécessai Une soustraction doit être le dernier élément dans une classe de caractères This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Ajouter l'attribut 'DebuggerDisplay' @@ -4835,16 +4840,6 @@ Version utilisée dans : {2} Ajouter un contrôle de valeur null - - Add 'string.IsNullOrEmpty' check - Ajouter le contrôle 'string.IsNullOrEmpty' - - - - Add 'string.IsNullOrWhiteSpace' check - Ajouter le contrôle 'string.IsNullOrWhiteSpace' - - Initialize field '{0}' Initialiser le champ '{0}' diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf index 3fc4da01a44f4..3b5bd9d9a834c 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.it.xlf @@ -40,6 +40,11 @@ Assicurarsi di usare l'identificatore "tt" per le lingue per le quali è necessa L'ultimo elemento di una classe di caratteri deve essere una sottrazione This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Aggiungi l'attributo 'DebuggerDisplay' @@ -4835,16 +4840,6 @@ Questa versione è usata {2} Aggiungi il controllo Null - - Add 'string.IsNullOrEmpty' check - Aggiungi il controllo 'string.IsNullOrEmpty' - - - - Add 'string.IsNullOrWhiteSpace' check - Aggiungi il controllo 'string.IsNullOrWhiteSpace' - - Initialize field '{0}' Inizializza il campo '{0}' diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf index 0cfd33e4874f0..5a8d3a2262203 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ja.xlf @@ -40,6 +40,11 @@ Make sure to use the "tt" specifier for languages for which it's necessary to ma 減算は、文字クラスの最後の要素でなければなりません This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute 'DebuggerDisplay' 属性の追加 @@ -4835,16 +4840,6 @@ This version used in: {2} null チェックを追加する - - Add 'string.IsNullOrEmpty' check - string.IsNullOrEmpty' チェックを追加する - - - - Add 'string.IsNullOrWhiteSpace' check - string.IsNullOrWhiteSpace' チェックを追加する - - Initialize field '{0}' フィールド '{0}' を初期化する diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf index b5e232d5567eb..0680c861b8bf0 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ko.xlf @@ -40,6 +40,11 @@ Make sure to use the "tt" specifier for languages for which it's necessary to ma 빼기는 문자 클래스의 마지막 요소여야 합니다. This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute 'DebuggerDisplay' 특성을 추가합니다. @@ -4835,16 +4840,6 @@ This version used in: {2} null 검사 추가 - - Add 'string.IsNullOrEmpty' check - string.IsNullOrEmpty' 검사 추가 - - - - Add 'string.IsNullOrWhiteSpace' check - string.IsNullOrWhiteSpace' 검사 추가 - - Initialize field '{0}' '{0}' 필드 초기화 diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf index 8124779fdb2f8..11f59f5c77cf1 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pl.xlf @@ -40,6 +40,11 @@ Pamiętaj, aby nie używać specyfikatora „tt” dla wszystkich języków, w k Odejmowanie musi być ostatnim elementem w klasie znaków This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Dodaj atrybut "DebuggerDisplay" @@ -4835,16 +4840,6 @@ Ta wersja jest używana wersja: {2} Dodaj sprawdzenie wartości null - - Add 'string.IsNullOrEmpty' check - Dodaj sprawdzenie elementu „string.IsNullOrEmpty” - - - - Add 'string.IsNullOrWhiteSpace' check - Dodaj sprawdzenie elementu „string.IsNullOrWhiteSpace” - - Initialize field '{0}' Inicjuj pole „{0}” diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf index 15e3df911f2ef..da7b61b3adb0a 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.pt-BR.xlf @@ -40,6 +40,11 @@ Verifique se o especificador "tt" foi usado para idiomas para os quais é necess Uma subtração deve ser o último elemento em uma classe de caracteres This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Adicionar atributo 'DebuggerDisplay' @@ -4835,16 +4840,6 @@ Essa versão é usada no: {2} Adicionar verificação nula - - Add 'string.IsNullOrEmpty' check - Adicionar a verificação 'string.IsNullOrEmpty' - - - - Add 'string.IsNullOrWhiteSpace' check - Adicionar a verificação 'string.IsNullOrWhiteSpace' - - Initialize field '{0}' Inicializar campo '{0}' diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf index 4ad9277a3551b..6cf3f47134875 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.ru.xlf @@ -40,6 +40,11 @@ Make sure to use the "tt" specifier for languages for which it's necessary to ma Вычитание должно быть последним элементом в классе символов This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute Добавить атрибут "DebuggerDisplay" @@ -4835,16 +4840,6 @@ This version used in: {2} Добавить проверку значений NULL - - Add 'string.IsNullOrEmpty' check - Добавить проверку string.IsNullOrEmpty - - - - Add 'string.IsNullOrWhiteSpace' check - Добавить проверку string.IsNullOrWhiteSpace - - Initialize field '{0}' Инициализировать поле "{0}" diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf index a828b6232fd92..90fa99a150d46 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.tr.xlf @@ -40,6 +40,11 @@ AM ve PM arasındaki farkın korunmasının gerekli olduğu diller için "tt" be Bir çıkarma bir karakter sınıfı içinde son öğe olması gerekir This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute 'DebuggerDisplay' özniteliği ekle @@ -4835,16 +4840,6 @@ Bu sürüm şurada kullanılır: {2} Null denetimi ekle - - Add 'string.IsNullOrEmpty' check - 'string.IsNullOrEmpty' denetimi ekle - - - - Add 'string.IsNullOrWhiteSpace' check - 'string.IsNullOrWhiteSpace' denetimi ekle - - Initialize field '{0}' '{0}' alanını başlat diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf index a84e76e3f896b..fd07af937e8d7 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hans.xlf @@ -40,6 +40,11 @@ Make sure to use the "tt" specifier for languages for which it's necessary to ma 负号必须是字符类中的最后一个元素 This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute 添加 "DebuggerDisplay" 属性 @@ -4835,16 +4840,6 @@ This version used in: {2} 添加 null 检查 - - Add 'string.IsNullOrEmpty' check - 添加 "string.IsNullOrEmpty" 检查 - - - - Add 'string.IsNullOrWhiteSpace' check - 添加 "string.IsNullOrWhiteSpace" 检查 - - Initialize field '{0}' 初始化字段“{0}” diff --git a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf index bf54961621c4b..95b987e61e626 100644 --- a/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf +++ b/src/Features/Core/Portable/xlf/FeaturesResources.zh-Hant.xlf @@ -40,6 +40,11 @@ Make sure to use the "tt" specifier for languages for which it's necessary to ma 減法必須是字元類別中的最後一個元素 This is an error message shown to the user when they write an invalid Regular Expression. Example: [a-[b]-c] + + Add '{0}' check + Add '{0}' check + + Add 'DebuggerDisplay' attribute 新增 'DebuggerDisplay' 屬性 @@ -4835,16 +4840,6 @@ This version used in: {2} 新增 null 檢查 - - Add 'string.IsNullOrEmpty' check - 新增 'string.IsNullOrEmpty' 檢查 - - - - Add 'string.IsNullOrWhiteSpace' check - 新增 'string.IsNullOrWhiteSpace' 檢查 - - Initialize field '{0}' 初始化欄位 '{0}' From 0cecaa16083abbf2adc11b5c10dba1c2354012e3 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 15 May 2025 21:23:04 +0300 Subject: [PATCH 3/7] Implement `Enum.IsDefined` check refactoring --- .../AddParameterCheckTests.cs | 379 ++++++++++++++++++ ...ddParameterCheckCodeRefactoringProvider.cs | 204 ++++++++-- 2 files changed, 557 insertions(+), 26 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs index 55d9d6b543c4a..e7769d658fef0 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs @@ -3203,4 +3203,383 @@ void M(string a, } """); } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestSimpleEnumIsDefinedCheck_ModernEnumIsDefinedOverload() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + + class C + { + void M(DayOfWeek [|dayOfWeek|]) + { + } + } + """, + FixedCode = """ + using System; + + class C + { + void M(DayOfWeek dayOfWeek) + { + if (!Enum.IsDefined(dayOfWeek)) + { + throw new System.ComponentModel.InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestSimpleEnumIsDefinedCheck_OldEnumIsDefinedOverload() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek [|dayOfWeek|]) + { + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek dayOfWeek) + { + if (!Enum.IsDefined(typeof(DayOfWeek), dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.NetStandard.NetStandard20 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestNoEnumIsDefinedCheckForOutEnumParameter() + { + await VerifyCS.VerifyRefactoringAsync(""" + using System; + using System.ComponentModel; + + class C + { + void M(out DayOfWeek [|result|]) + { + result = default; + } + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestNoEnumIsDefinedCheckForFlagsEnumParameter() + { + await VerifyCS.VerifyRefactoringAsync(""" + using System; + using System.ComponentModel; + + class C + { + void M(MyFlags [|myFlags|]) + { + } + } + + [Flags] + enum MyFlags + { + A = 1, + B = 2 + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestNoEnumIsDefinedCheckIfAlreadyExist_ModernEnumIsDefinedOverload() + { + var code = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek [|dayOfWeek|]) + { + if (!Enum.IsDefined(dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + } + } + """; + + await new VerifyCS.Test() + { + TestCode = code, + FixedCode = code, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestNoEnumIsDefinedCheckIfAlreadyExist_OldEnumIsDefinedOverload() + { + var code = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek [|dayOfWeek|]) + { + if (!Enum.IsDefined(typeof(DayOfWeek), dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + } + } + """; + + await new VerifyCS.Test() + { + TestCode = code, + FixedCode = code, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestEnumIsDefinedCheckAfterAnotherEnumIsDefinedCheck() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek dayOfWeek1, DayOfWeek [|dayOfWeek2|]) + { + if (!Enum.IsDefined(dayOfWeek1)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek1), (int)dayOfWeek1, typeof(DayOfWeek)); + } + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek dayOfWeek1, DayOfWeek dayOfWeek2) + { + if (!Enum.IsDefined(dayOfWeek1)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek1), (int)dayOfWeek1, typeof(DayOfWeek)); + } + + if (!Enum.IsDefined(dayOfWeek2)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek2), (int)dayOfWeek2, typeof(DayOfWeek)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestEnumIsDefinedCheckBeforeAnotherEnumIsDefinedCheck() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek [|dayOfWeek1|], DayOfWeek dayOfWeek2) + { + if (!Enum.IsDefined(dayOfWeek2)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek2), (int)dayOfWeek2, typeof(DayOfWeek)); + } + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek dayOfWeek1, DayOfWeek dayOfWeek2) + { + if (!Enum.IsDefined(dayOfWeek1)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek1), (int)dayOfWeek1, typeof(DayOfWeek)); + } + + if (!Enum.IsDefined(dayOfWeek2)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek2), (int)dayOfWeek2, typeof(DayOfWeek)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestEnumIsDefinedCheckAfterNullCheck() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(string s, DayOfWeek [|dayOfWeek|]) + { + ArgumentNullException.ThrowIfNull(s); + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(string s, DayOfWeek dayOfWeek) + { + ArgumentNullException.ThrowIfNull(s); + if (!Enum.IsDefined(dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestEnumIsDefinedCheckBeforeNullCheck() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek [|dayOfWeek|], object o) + { + if (o is null) + { + throw new ArgumentNullException(nameof(o)); + } + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(DayOfWeek dayOfWeek, object o) + { + if (!Enum.IsDefined(dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + + if (o is null) + { + throw new ArgumentNullException(nameof(o)); + } + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/66327")] + public async Task TestEnumIsDefinedCheckInBetweenChecks() + { + await new VerifyCS.Test() + { + TestCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(object o, DayOfWeek [|dayOfWeek|], string s) + { + if (o is null) + { + throw new ArgumentNullException(nameof(o)); + } + + ArgumentException.ThrowIfNullOrEmpty(s); + } + } + """, + FixedCode = """ + using System; + using System.ComponentModel; + + class C + { + void M(object o, DayOfWeek dayOfWeek, string s) + { + if (o is null) + { + throw new ArgumentNullException(nameof(o)); + } + + if (!Enum.IsDefined(dayOfWeek)) + { + throw new InvalidEnumArgumentException(nameof(dayOfWeek), (int)dayOfWeek, typeof(DayOfWeek)); + } + + ArgumentException.ThrowIfNullOrEmpty(s); + } + } + """, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80 + }.RunAsync(); + } } diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index 3602e0899935c..9155b0dc90604 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.ComponentModel; using System.Diagnostics; using System.Linq; using System.Threading; @@ -100,34 +101,47 @@ protected override async Task> GetRefactoringsForSing var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); // Only should provide null-checks for reference types and nullable types. - if (!ParameterValidForNullCheck(document, parameter, semanticModel, blockStatement, cancellationToken)) - return []; + if (ParameterValidForNullCheck(document, parameter, semanticModel, blockStatement, cancellationToken)) + { + var simplifierOptions = (TSimplifierOptions)await document.GetSimplifierOptionsAsync(cancellationToken).ConfigureAwait(false); + + // Great. There was no null check. Offer to add one. + using var result = TemporaryArray.Empty; + result.Add(CodeAction.Create( + FeaturesResources.Add_null_check, + cancellationToken => AddNullCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, simplifierOptions, cancellationToken), + nameof(FeaturesResources.Add_null_check))); - var simplifierOptions = (TSimplifierOptions)await document.GetSimplifierOptionsAsync(cancellationToken).ConfigureAwait(false); + // Also, if this was a string, offer to add the special checks to string.IsNullOrEmpty and + // string.IsNullOrWhitespace. + if (parameter.Type.SpecialType == SpecialType.System_String) + { + result.Add(CodeAction.Create( + string.Format(FeaturesResources.Add_0_check, "string.IsNullOrEmpty"), + cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), + $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty")); + + result.Add(CodeAction.Create( + string.Format(FeaturesResources.Add_0_check, "string.IsNullOrWhiteSpace"), + cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), + $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace")); + } - // Great. There was no null check. Offer to add one. - using var result = TemporaryArray.Empty; - result.Add(CodeAction.Create( - FeaturesResources.Add_null_check, - cancellationToken => AddNullCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, simplifierOptions, cancellationToken), - nameof(FeaturesResources.Add_null_check))); + return result.ToImmutableAndClear(); + } - // Also, if this was a string, offer to add the special checks to string.IsNullOrEmpty and - // string.IsNullOrWhitespace. - if (parameter.Type.SpecialType == SpecialType.System_String) + // Provide 'Enum.IsDefined' check for suitable enum parameters + if (ParameterValidForEnumIsDefinedCheck(parameter, semanticModel.Compilation, blockStatement)) { - result.Add(CodeAction.Create( - string.Format(FeaturesResources.Add_0_check, "string.IsNullOrEmpty"), - cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), - $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty")); + var action = CodeAction.Create( + string.Format(FeaturesResources.Add_0_check, "Enum.IsDefined"), + cancellationToken => AddEnumIsDefinedCheckStatementAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, cancellationToken), + $"{nameof(FeaturesResources.Add_0_check)};Enum.IsDefined"); - result.Add(CodeAction.Create( - string.Format(FeaturesResources.Add_0_check, "string.IsNullOrWhiteSpace"), - cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), - $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace")); + return [action]; } - return result.ToImmutableAndClear(); + return []; } private async Task UpdateDocumentForRefactoringAsync( @@ -250,7 +264,7 @@ private static bool IsIfNullCheck(IOperation statement, IParameterSymbol paramet return false; } - protected bool ParameterValidForNullCheck(Document document, IParameterSymbol parameter, SemanticModel semanticModel, + private bool ParameterValidForNullCheck(Document document, IParameterSymbol parameter, SemanticModel semanticModel, IBlockOperation? blockStatement, CancellationToken cancellationToken) { if (parameter.Type.IsReferenceType) @@ -303,6 +317,77 @@ protected bool ParameterValidForNullCheck(Document document, IParameterSymbol pa return true; } + private static (IMethodSymbol? GenericOverload, IMethodSymbol? NonGenericOverload) GetEnumIsDefinedMethods(Compilation compilation) + { + var enumType = compilation.GetSpecialType(SpecialType.System_Enum); + var enumIsDefinedMembers = enumType.GetMembers(nameof(Enum.IsDefined)); + var enumIsDefinedGenericMethod = (IMethodSymbol?)enumIsDefinedMembers.FirstOrDefault(m => m is IMethodSymbol { IsStatic: true, Arity: 1, Parameters.Length: 1 }); + var enumIsDefinedNonGenericMethod = (IMethodSymbol?)enumIsDefinedMembers.FirstOrDefault(m => m is IMethodSymbol { IsStatic: true, Arity: 0, Parameters.Length: 2 }); + return (enumIsDefinedGenericMethod, enumIsDefinedNonGenericMethod); + } + + private bool ParameterValidForEnumIsDefinedCheck(IParameterSymbol parameter, Compilation compilation, IBlockOperation? blockStatement) + { + if (parameter.RefKind == RefKind.Out) + return false; + + if (parameter.IsDiscard) + return false; + + var parameterType = parameter.Type; + + if (parameterType.TypeKind != TypeKind.Enum) + return false; + + var flagsAttributeType = compilation.GetBestTypeByMetadataName(typeof(FlagsAttribute).FullName!); + if (parameterType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, flagsAttributeType))) + return false; + + if (blockStatement is not null) + { + if (!CanOffer(blockStatement.Syntax)) + return false; + + var (enumIsDefinedGenericMethod, enumIsDefinedNonGenericMethod) = GetEnumIsDefinedMethods(compilation); + + foreach (var statement in blockStatement.Operations) + { + if (IsEnumIsDefinedCheck(statement, parameter, enumIsDefinedGenericMethod, enumIsDefinedNonGenericMethod)) + return false; + } + } + + return true; + } + + private static bool IsEnumIsDefinedCheck(IOperation statement, IParameterSymbol parameter, IMethodSymbol? enumIsDefinedGenericMethod, IMethodSymbol? enumIsDefinedNonGenericMethod) + { + if (statement is IConditionalOperation ifStatement) + { + var condition = ifStatement.Condition; + condition = condition.UnwrapImplicitConversion(); + + if (condition is not IUnaryOperation { OperatorKind: UnaryOperatorKind.Not, Operand: IInvocationOperation invocation }) + return false; + + var method = invocation.TargetMethod; + + if (method.OriginalDefinition.Equals(enumIsDefinedGenericMethod, SymbolEqualityComparer.Default) && + IsParameterReference(invocation.Arguments[0].Value, parameter)) + { + return true; + } + + if (method.Equals(enumIsDefinedNonGenericMethod, SymbolEqualityComparer.Default) && + IsParameterReference(invocation.Arguments[1].Value, parameter)) + { + return true; + } + } + + return false; + } + private static bool IsAnyThrowIfNullInvocation(IOperation statement, IParameterSymbol? parameter) { if (statement is IExpressionStatementOperation @@ -405,8 +490,7 @@ private static async Task AddNullCheckStatementAsync( // Find a good location to add the null check. In general, we want the order of checks // and assignments in the constructor to match the order of parameters in the method // signature. - var statementToAddAfter = GetStatementToAddNullCheckAfter( - semanticModel, parameter, blockStatement, cancellationToken); + var statementToAddAfter = GetStatementToAddCheckAfter(semanticModel, parameter, blockStatement, cancellationToken); var initializeParameterService = document.GetRequiredLanguageService(); initializeParameterService.InsertStatement(editor, functionDeclaration, method.ReturnsVoid, statementToAddAfter, nullCheckStatement); @@ -415,6 +499,69 @@ private static async Task AddNullCheckStatementAsync( return document.WithSyntaxRoot(newRoot); } + private static async Task AddEnumIsDefinedCheckStatementAsync( + Document document, + IParameterSymbol parameter, + SyntaxNode functionDeclaration, + IMethodSymbol method, + IBlockOperation? blockStatement, + CancellationToken cancellationToken) + { + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var compilation = semanticModel.Compilation; + + var enumType = compilation.GetSpecialType(SpecialType.System_Enum); + var enumIsDefinedGenericMethod = enumType.GetMembers(nameof(Enum.IsDefined)).FirstOrDefault(m => m is IMethodSymbol { IsStatic: true, Arity: 1, Parameters.Length: 1 }); + + var editor = new SyntaxEditor(root, document.Project.Solution.Services); + var generator = editor.Generator; + + var parameterIdentifierName = generator.IdentifierName(parameter.Name); + var typeOfParameterExpression = generator.TypeOfExpression(generator.TypeExpression(parameter.Type)); + + SyntaxNode enumIsDefinedInvocation; + if (enumIsDefinedGenericMethod is not null) + { + enumIsDefinedInvocation = generator.InvocationExpression( + generator.MemberAccessExpression( + generator.TypeExpression(enumType), + generator.GenericName(enumIsDefinedGenericMethod.Name, parameter.Type)), + parameterIdentifierName); + } + else + { + enumIsDefinedInvocation = generator.InvocationExpression( + generator.MemberAccessExpression( + generator.TypeExpression(enumType), + nameof(Enum.IsDefined)), + typeOfParameterExpression, + parameterIdentifierName); + } + + var finalCondition = generator.LogicalNotExpression(enumIsDefinedInvocation); + + var invalidEnumArgumentExceptionType = compilation.GetBestTypeByMetadataName(typeof(InvalidEnumArgumentException).FullName!); + Contract.ThrowIfNull(invalidEnumArgumentExceptionType); + var throwStatement = generator.ThrowStatement( + generator.ObjectCreationExpression( + invalidEnumArgumentExceptionType, + generator.NameOfExpression(parameterIdentifierName), + generator.CastExpression( + compilation.GetSpecialType(SpecialType.System_Int32), + parameterIdentifierName), + typeOfParameterExpression)); + + var enumIsDefinedCheckStatement = generator.IfStatement(finalCondition, [throwStatement]); + + var initializeParameterService = document.GetRequiredLanguageService(); + var statementToAddAfter = GetStatementToAddCheckAfter(semanticModel, parameter, blockStatement, cancellationToken); + initializeParameterService.InsertStatement(editor, functionDeclaration, method.ReturnsVoid, statementToAddAfter, enumIsDefinedCheckStatement); + + var newRoot = editor.GetChangedRoot(); + return document.WithSyntaxRoot(newRoot); + } + private TStatementSyntax CreateNullCheckStatement(SemanticModel semanticModel, SyntaxGenerator generator, IParameterSymbol parameter, TSimplifierOptions options) { var argumentNullExceptionType = semanticModel.Compilation.ArgumentNullExceptionType(); @@ -476,7 +623,7 @@ private TStatementSyntax CreateStringCheckStatement( return CreateParameterCheckIfStatement(condition, throwStatement, options); } - private static SyntaxNode? GetStatementToAddNullCheckAfter( + private static SyntaxNode? GetStatementToAddCheckAfter( SemanticModel semanticModel, IParameterSymbol parameter, IBlockOperation? blockStatement, @@ -519,7 +666,7 @@ private TStatementSyntax CreateStringCheckStatement( /// /// Tries to find an if-statement that looks like it is checking the provided parameter - /// in some way. If we find a match, we'll place our new null-check statement before/after + /// in some way. If we find a match, we'll place our new check statement before/after /// this statement as appropriate. /// private static IOperation? TryFindParameterCheckStatement( @@ -530,6 +677,8 @@ private TStatementSyntax CreateStringCheckStatement( { if (blockStatement != null) { + var (enumIsDefinedGenericMethod, enumIsDefinedNonGenericMethod) = GetEnumIsDefinedMethods(semanticModel.Compilation); + foreach (var statement in blockStatement.Operations) { // Ignore implicit code the compiler inserted at the top of the block (for example, the implicit @@ -545,6 +694,9 @@ private TStatementSyntax CreateStringCheckStatement( continue; } + if (IsEnumIsDefinedCheck(statement, parameterSymbol, enumIsDefinedGenericMethod, enumIsDefinedNonGenericMethod)) + return statement; + if (statement is IConditionalOperation ifStatement) { if (ContainsParameterReference(semanticModel, ifStatement.Condition, parameterSymbol, cancellationToken)) From 9363160868e8f51820f75892b639af1379a2fdc3 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Mon, 19 May 2025 21:47:10 +0300 Subject: [PATCH 4/7] Simplify equivalence keys --- .../AddParameterCheckTests.cs | 24 +++++++++---------- ...ddParameterCheckCodeRefactoringProvider.cs | 6 ++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs index e7769d658fef0..e2595691ffcc9 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs @@ -1977,7 +1977,7 @@ public C(string s) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2015,7 +2015,7 @@ public C(string s) } """, CodeActionIndex = 2, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace" + CodeActionEquivalenceKey = "Add_string_IsNullOrWhiteSpace_check" }.RunAsync(); } @@ -2046,7 +2046,7 @@ public C(string s) } """, CodeActionIndex = 2, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace", + CodeActionEquivalenceKey = "Add_string_IsNullOrWhiteSpace_check", ReferenceAssemblies = ReferenceAssemblies.Net.Net80, }.RunAsync(); } @@ -2086,9 +2086,9 @@ public C(string s) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); - } + } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/19173")] public async Task TestMissingOnUnboundTypeWithExistingNullCheck() @@ -2142,7 +2142,7 @@ static void Main(String bar) } """, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty", + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check", Options = { { CodeStyleOptions2.PreferIntrinsicPredefinedTypeKeywordInMemberAccess, CodeStyleOption2.FalseWithSuggestionEnforcement } @@ -2740,7 +2740,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2782,7 +2782,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2826,7 +2826,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, false }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2867,7 +2867,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2908,7 +2908,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } @@ -2952,7 +2952,7 @@ public C(string s) { CSharpCodeStyleOptions.AllowEmbeddedStatementsOnSameLine, true }, }, CodeActionIndex = 1, - CodeActionEquivalenceKey = $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty" + CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); } diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index 9155b0dc90604..9c011c571cf31 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -119,12 +119,12 @@ protected override async Task> GetRefactoringsForSing result.Add(CodeAction.Create( string.Format(FeaturesResources.Add_0_check, "string.IsNullOrEmpty"), cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrEmptySuffix, simplifierOptions, cancellationToken), - $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrEmpty")); + "Add_string_IsNullOrEmpty_check")); result.Add(CodeAction.Create( string.Format(FeaturesResources.Add_0_check, "string.IsNullOrWhiteSpace"), cancellationToken => AddStringCheckAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, s_nullOrWhiteSpaceSuffix, simplifierOptions, cancellationToken), - $"{nameof(FeaturesResources.Add_0_check)};string.IsNullOrWhiteSpace")); + "Add_string_IsNullOrWhiteSpace_check")); } return result.ToImmutableAndClear(); @@ -136,7 +136,7 @@ protected override async Task> GetRefactoringsForSing var action = CodeAction.Create( string.Format(FeaturesResources.Add_0_check, "Enum.IsDefined"), cancellationToken => AddEnumIsDefinedCheckStatementAsync(document, parameter, functionDeclaration, methodSymbol, blockStatement, cancellationToken), - $"{nameof(FeaturesResources.Add_0_check)};Enum.IsDefined"); + "Add_Enum_IsDefined_check"); return [action]; } From 643e447f7e5fb965eac5a36f752c5ca3b9a608b1 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Mon, 19 May 2025 22:04:38 +0300 Subject: [PATCH 5/7] Make refactoring resilient against absence of `InvalidEnumArgumentException` type --- ...bstractAddParameterCheckCodeRefactoringProvider.cs | 11 ++++------- .../Portable/CodeGeneration/CSharpSyntaxGenerator.cs | 3 +++ .../Core/Portable/Editing/SyntaxGenerator.cs | 2 ++ .../CodeGeneration/VisualBasicSyntaxGenerator.vb | 4 ++++ 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index 9c011c571cf31..4161595e4a889 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -541,11 +541,9 @@ private static async Task AddEnumIsDefinedCheckStatementAsync( var finalCondition = generator.LogicalNotExpression(enumIsDefinedInvocation); - var invalidEnumArgumentExceptionType = compilation.GetBestTypeByMetadataName(typeof(InvalidEnumArgumentException).FullName!); - Contract.ThrowIfNull(invalidEnumArgumentExceptionType); var throwStatement = generator.ThrowStatement( generator.ObjectCreationExpression( - invalidEnumArgumentExceptionType, + GetTypeNode(compilation, generator, typeof(InvalidEnumArgumentException)), generator.NameOfExpression(parameterIdentifierName), generator.CastExpression( compilation.GetSpecialType(SpecialType.System_Int32), @@ -772,12 +770,11 @@ private TStatementSyntax CreateStringCheckStatement( private static SyntaxNode GetTypeNode( Compilation compilation, SyntaxGenerator generator, Type type) { - var typeSymbol = compilation.GetTypeByMetadataName(type.FullName!); + var fullName = type.FullName!; + var typeSymbol = compilation.GetTypeByMetadataName(fullName); if (typeSymbol == null) { - return generator.QualifiedName( - generator.IdentifierName(nameof(System)), - generator.IdentifierName(type.Name)); + return generator.ParseTypeName(fullName); } return generator.TypeExpression(typeSymbol); diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs index 549c0aae42e7a..d45f7c8ecce6d 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs @@ -3573,5 +3573,8 @@ static IEnumerable> splitIntoLines(SyntaxTriviaList tr internal override SyntaxNode ParseExpression(string stringToParse) => SyntaxFactory.ParseExpression(stringToParse); + internal override SyntaxNode ParseTypeName(string stringToParse) + => SyntaxFactory.ParseTypeName(stringToParse); + #endregion } diff --git a/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs b/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs index ad653c85131ec..ea037cbd0e758 100644 --- a/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs +++ b/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs @@ -2414,6 +2414,8 @@ internal SyntaxNode AddParentheses(SyntaxNode expression, bool includeElasticTri /// internal abstract SyntaxNode ParseExpression(string stringToParse); + internal abstract SyntaxNode ParseTypeName(string stringToParse); + internal abstract SyntaxTrivia Trivia(SyntaxNode node); internal abstract SyntaxNode DocumentationCommentTrivia(IEnumerable nodes, SyntaxTriviaList trailingTrivia, string endOfLineString); diff --git a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb index 82b2ecf4f2907..3a3e8ca3030b7 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb @@ -617,6 +617,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration Friend Overrides Function ParseExpression(stringToParse As String) As SyntaxNode Return SyntaxFactory.ParseExpression(stringToParse) End Function + + Friend Overrides Function ParseTypeName(stringToParse As String) As SyntaxNode + Return SyntaxFactory.ParseTypeName(stringToParse) + End Function #End Region #Region "Declarations" From 736bcc47fd5f0b77d8e95f528f476235353d2fa9 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Mon, 19 May 2025 22:33:04 +0300 Subject: [PATCH 6/7] Fix formatting --- .../CodeActions/InitializeParameter/AddParameterCheckTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs index e2595691ffcc9..705834a29b7ad 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/InitializeParameter/AddParameterCheckTests.cs @@ -2088,7 +2088,7 @@ public C(string s) CodeActionIndex = 1, CodeActionEquivalenceKey = "Add_string_IsNullOrEmpty_check" }.RunAsync(); - } + } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/19173")] public async Task TestMissingOnUnboundTypeWithExistingNullCheck() From 7e21b012316f152cbfd5f86a5a9df2d91cad6a83 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Tue, 20 May 2025 08:28:11 +0300 Subject: [PATCH 7/7] Simplify --- .../AbstractAddParameterCheckCodeRefactoringProvider.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs index 4161595e4a889..b00c93a4f9f7d 100644 --- a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs @@ -772,12 +772,9 @@ private static SyntaxNode GetTypeNode( { var fullName = type.FullName!; var typeSymbol = compilation.GetTypeByMetadataName(fullName); - if (typeSymbol == null) - { - return generator.ParseTypeName(fullName); - } - - return generator.TypeExpression(typeSymbol); + return typeSymbol is null + ? generator.ParseTypeName(fullName) + : generator.TypeExpression(typeSymbol); } private static SyntaxNode CreateArgumentNullException(