From 60e793c132f9cbdf4cda7e65d979cf61d41deb6d Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Wed, 21 Aug 2024 21:39:38 +0300 Subject: [PATCH 1/4] Do not show inline statement snippets when dotting from a type name --- ...arpConditionalBlockSnippetProviderTests.cs | 28 +++++++++++++ .../Snippets/CSharpDoSnippetProviderTests.cs | 28 +++++++++++++ .../CSharpForEachSnippetProviderTests.cs | 42 ++++++++++++++----- .../Snippets/CSharpForSnippetProviderTests.cs | 30 +++++++++++++ .../Snippets/CommonSnippetTestData.cs | 10 +++++ .../AbstractInlineStatementSnippetProvider.cs | 12 ++++++ 6 files changed, 139 insertions(+), 11 deletions(-) diff --git a/src/Features/CSharpTest/Snippets/AbstractCSharpConditionalBlockSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/AbstractCSharpConditionalBlockSnippetProviderTests.cs index 3b46139dd4bbe..299468058da10 100644 --- a/src/Features/CSharpTest/Snippets/AbstractCSharpConditionalBlockSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/AbstractCSharpConditionalBlockSnippetProviderTests.cs @@ -538,4 +538,32 @@ void M(bool flag) } """); } + + [Fact] + public async Task NoInlineSnippetForTypeItselfTest() + { + await VerifySnippetIsAbsentAsync(""" + class C + { + void M() + { + bool.$$ + } + } + """); + } + + [Fact] + public async Task NoInlineSnippetForTypeItselfTest_Parenthesized() + { + await VerifySnippetIsAbsentAsync(""" + class C + { + void M() + { + (bool).$$ + } + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CSharpDoSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpDoSnippetProviderTests.cs index 69d1e4ca499b0..01e86e3f3d04d 100644 --- a/src/Features/CSharpTest/Snippets/CSharpDoSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpDoSnippetProviderTests.cs @@ -554,4 +554,32 @@ void M(bool flag) } """); } + + [Fact] + public async Task NoInlineDoSnippetForTypeItselfTest() + { + await VerifySnippetIsAbsentAsync(""" + class C + { + void M() + { + bool.$$ + } + } + """); + } + + [Fact] + public async Task NoInlineDoSnippetForTypeItselfTest_Parenthesized() + { + await VerifySnippetIsAbsentAsync(""" + class C + { + void M() + { + (bool).$$ + } + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs index c803a34b7b6e1..ba54a5258e0cc 100644 --- a/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs @@ -224,17 +224,10 @@ await VerifySnippetAsync(""" } [Theory] - [InlineData("List")] - [InlineData("int[]")] - [InlineData("IEnumerable")] - [InlineData("ArrayList")] - [InlineData("IEnumerable")] + [MemberData(nameof(CommonSnippetTestData.CommonEnumerableTypes), MemberType = typeof(CommonSnippetTestData))] public async Task InsertInlineForEachSnippetForCorrectTypeTest(string collectionType) { await VerifySnippetAsync($$""" - using System.Collections; - using System.Collections.Generic; - class C { void M({{collectionType}} enumerable) @@ -243,9 +236,6 @@ void M({{collectionType}} enumerable) } } """, $$""" - using System.Collections; - using System.Collections.Generic; - class C { void M({{collectionType}} enumerable) @@ -710,4 +700,34 @@ void M(IAsyncEnumerable asyncInts) """, referenceAssemblies: ReferenceAssemblies.Net.Net70); } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.CommonEnumerableTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineForEachSnippetForTypeItselfTest(string collectionType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + {{collectionType}}.$$ + } + } + """); + } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.CommonEnumerableTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineForEachSnippetForTypeItselfTest_Parenthesized(string collectionType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + ({{collectionType}}).$$ + } + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs index 8e219314bd621..6f654b82a6f12 100644 --- a/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs @@ -875,4 +875,34 @@ public class MyType } """); } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.IntegerTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineForSnippetForTypeItselfTest(string integerType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + {{integerType}}.$$ + } + } + """); + } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.IntegerTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineForSnippetForTypeItselfTest_Parenthesized(string integerType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + ({{integerType}}).$$ + } + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CommonSnippetTestData.cs b/src/Features/CSharpTest/Snippets/CommonSnippetTestData.cs index 95585d2385a45..9cbd0e10a127a 100644 --- a/src/Features/CSharpTest/Snippets/CommonSnippetTestData.cs +++ b/src/Features/CSharpTest/Snippets/CommonSnippetTestData.cs @@ -38,4 +38,14 @@ public static class CommonSnippetTestData "private protected", "protected internal", }; + + public static TheoryData CommonEnumerableTypes => new() + { + "string", + "System.Collections.Generic.List", + "int[]", + "System.Collections.Generic.IEnumerable", + "System.Collections.ArrayList", + "System.Collections.IEnumerable", + }; } diff --git a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs index f29734df6fd92..1dd5c0213dbde 100644 --- a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs +++ b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs @@ -83,7 +83,19 @@ private static bool TryGetInlineExpressionInfo(SyntaxToken targetToken, ISyntaxF syntaxFacts.IsExpressionStatement(parentNode?.Parent)) { var expression = syntaxFacts.GetExpressionOfMemberAccessExpression(parentNode)!; + var typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); + var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); + + // If we get the same symbol from symbol info as expression's type + // then we are dotting from a type name itself, e.g. `string.$$`. + // Inline statement snippets are not valid in this context + if (SymbolEqualityComparer.Default.Equals(symbolInfo.Symbol, typeInfo.Type)) + { + expressionInfo = null; + return false; + } + expressionInfo = new(expression, typeInfo); return true; } From 971d915c9c9917af61c9f0386dc9a774d2146eb4 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Wed, 21 Aug 2024 21:41:31 +0300 Subject: [PATCH 2/4] Rename --- ...StatementProvider.cs => CSharpDoWhileLoopSnippetProvider.cs} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/Features/CSharp/Portable/Snippets/{CSharpDoWhileLoopStatementProvider.cs => CSharpDoWhileLoopSnippetProvider.cs} (97%) diff --git a/src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopStatementProvider.cs b/src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopSnippetProvider.cs similarity index 97% rename from src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopStatementProvider.cs rename to src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopSnippetProvider.cs index 4e8154e17b8a4..e29b74da65358 100644 --- a/src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopStatementProvider.cs +++ b/src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopSnippetProvider.cs @@ -19,7 +19,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Snippets; [ExportSnippetProvider(nameof(ISnippetProvider), LanguageNames.CSharp), Shared] [method: ImportingConstructor] [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] -internal sealed class CSharpDoWhileLoopStatementProvider() +internal sealed class CSharpDoWhileLoopSnippetProvider() : AbstractConditionalBlockSnippetProvider { public override string Identifier => CSharpSnippetIdentifiers.Do; From ccd77bf194129691c1474b2f19c13c465e937b94 Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 22 Aug 2024 22:47:55 +0300 Subject: [PATCH 3/4] Test `Color Color` case --- .../CSharpForEachSnippetProviderTests.cs | 48 +++++++++++++ .../Snippets/CSharpForSnippetProviderTests.cs | 37 ++++++++++ .../CSharpReversedForSnippetProviderTests.cs | 67 +++++++++++++++++++ 3 files changed, 152 insertions(+) diff --git a/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs index ba54a5258e0cc..0108eeffc5074 100644 --- a/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpForEachSnippetProviderTests.cs @@ -730,4 +730,52 @@ void M() } """); } + + [Theory] + [InlineData("ArrayList")] + [InlineData("IEnumerable")] + [InlineData("MyCollection")] + public async Task InsertInlineForEachSnippetForVariableNamedLikeTypeTest(string typeAndVariableName) + { + await VerifySnippetAsync($$""" + using System.Collections; + using System.Collections.Generic; + + class C + { + void M() + { + {{typeAndVariableName}} {{typeAndVariableName}} = default; + {{typeAndVariableName}}.$$ + } + } + + class MyCollection : IEnumerable + { + public IEnumerator GetEnumerator() => null; + IEnumerator IEnumerable.GetEnumerator() = null; + } + """, $$""" + using System.Collections; + using System.Collections.Generic; + + class C + { + void M() + { + {{typeAndVariableName}} {{typeAndVariableName}} = default; + foreach (var {|0:item|} in {{typeAndVariableName}}) + { + $$ + } + } + } + + class MyCollection : IEnumerable + { + public IEnumerator GetEnumerator() => null; + IEnumerator IEnumerable.GetEnumerator() = null; + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs index 6f654b82a6f12..0fa94d8d5c9c3 100644 --- a/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpForSnippetProviderTests.cs @@ -905,4 +905,41 @@ void M() } """); } + + [Fact] + public async Task InsertInlineForSnippetForVariableNamedLikeTypeTest() + { + await VerifySnippetAsync(""" + class C + { + void M() + { + MyType MyType = default; + MyType.$$ + } + } + + class MyType + { + public int Length => 0; + } + """, """ + class C + { + void M() + { + MyType MyType = default; + for (int {|0:i|} = 0; {|0:i|} < MyType.Length; {|0:i|}++) + { + $$ + } + } + } + + class MyType + { + public int Length => 0; + } + """); + } } diff --git a/src/Features/CSharpTest/Snippets/CSharpReversedForSnippetProviderTests.cs b/src/Features/CSharpTest/Snippets/CSharpReversedForSnippetProviderTests.cs index 375214bf81ddc..3912eeb524458 100644 --- a/src/Features/CSharpTest/Snippets/CSharpReversedForSnippetProviderTests.cs +++ b/src/Features/CSharpTest/Snippets/CSharpReversedForSnippetProviderTests.cs @@ -877,4 +877,71 @@ public class MyType } """); } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.IntegerTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineReversedForSnippetForTypeItselfTest(string integerType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + {{integerType}}.$$ + } + } + """); + } + + [Theory] + [MemberData(nameof(CommonSnippetTestData.IntegerTypes), MemberType = typeof(CommonSnippetTestData))] + public async Task NoInlineReversedForSnippetForTypeItselfTest_Parenthesized(string integerType) + { + await VerifySnippetIsAbsentAsync($$""" + class C + { + void M() + { + ({{integerType}}).$$ + } + } + """); + } + + [Fact] + public async Task InsertInlineReversedForSnippetForVariableNamedLikeTypeTest() + { + await VerifySnippetAsync(""" + class C + { + void M() + { + MyType MyType = default; + MyType.$$ + } + } + + class MyType + { + public int Length => 0; + } + """, """ + class C + { + void M() + { + MyType MyType = default; + for (int {|0:i|} = MyType.Length - 1; {|0:i|} >= 0; {|0:i|}--) + { + $$ + } + } + } + + class MyType + { + public int Length => 0; + } + """); + } } From 13abfd67544a1841e2bfe3b04ce0299a5052be6d Mon Sep 17 00:00:00 2001 From: DoctorKrolic Date: Thu, 22 Aug 2024 22:52:49 +0300 Subject: [PATCH 4/4] Simplify --- .../AbstractInlineStatementSnippetProvider.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs index 1dd5c0213dbde..07aea536e7e09 100644 --- a/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs +++ b/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractInlineStatementSnippetProvider.cs @@ -83,19 +83,17 @@ private static bool TryGetInlineExpressionInfo(SyntaxToken targetToken, ISyntaxF syntaxFacts.IsExpressionStatement(parentNode?.Parent)) { var expression = syntaxFacts.GetExpressionOfMemberAccessExpression(parentNode)!; - - var typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); var symbolInfo = semanticModel.GetSymbolInfo(expression, cancellationToken); - // If we get the same symbol from symbol info as expression's type - // then we are dotting from a type name itself, e.g. `string.$$`. + // Forbid a case when we are dotting of a type, e.g. `string.$$`. // Inline statement snippets are not valid in this context - if (SymbolEqualityComparer.Default.Equals(symbolInfo.Symbol, typeInfo.Type)) + if (symbolInfo.Symbol is ITypeSymbol) { expressionInfo = null; return false; } + var typeInfo = semanticModel.GetTypeInfo(expression, cancellationToken); expressionInfo = new(expression, typeInfo); return true; }