diff --git a/src/EditorFeatures/CSharpTest/UseDeconstruction/UseDeconstructionTests.cs b/src/EditorFeatures/CSharpTest/UseDeconstruction/UseDeconstructionTests.cs new file mode 100644 index 0000000000000000000000000000000000000000..4c98495dfd64a2e8761ff57a405995b763287888 --- /dev/null +++ b/src/EditorFeatures/CSharpTest/UseDeconstruction/UseDeconstructionTests.cs @@ -0,0 +1,445 @@ +// 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.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.UseDeconstruction; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Diagnostics; +using Roslyn.Test.Utilities; +using Xunit; + +namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.UseDeconstruction +{ + public class UseDeconstructionTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest + { + internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) + => (new CSharpUseDeconstructionDiagnosticAnalyzer(), new CSharpUseDeconstructionCodeFixProvider()); + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestVar() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (name, age) = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfNameInInnerScope() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + { + int age; + } + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfNameInOuterScope() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + int age; + + void M() + { + var [|t1|] = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestUpdateReference() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1.name + "" "" + t1.age); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (name, age) = GetPerson(); + Console.WriteLine(name + "" "" + age); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestTupleType() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + (int name, int age) [|t1|] = GetPerson(); + Console.WriteLine(t1.name + "" "" + t1.age); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + (int name, int age) = GetPerson(); + Console.WriteLine(name + "" "" + age); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestVarInForEach() + { + await TestInRegularAndScriptAsync( +@"using System.Collections.Generic; + +class C +{ + void M() + { + foreach (var [|t1|] in GetPeople()) + Console.WriteLine(t1.name + "" "" + t1.age); + } + + IEnumerable<(string name, int age)> GetPeople() => default; +}", +@"using System.Collections.Generic; + +class C +{ + void M() + { + foreach (var (name, age) in GetPeople()) + Console.WriteLine(name + "" "" + age); + } + + IEnumerable<(string name, int age)> GetPeople() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestTupleTypeInForEach() + { + await TestInRegularAndScriptAsync( +@"using System.Collections.Generic; + +class C +{ + void M() + { + foreach ((string name, int age) [|t1|] in GetPeople()) + Console.WriteLine(t1.name + "" "" + t1.age); + } + + IEnumerable<(string name, int age)> GetPeople() => default; +}", +@"using System.Collections.Generic; + +class C +{ + void M() + { + foreach ((string name, int age) in GetPeople()) + Console.WriteLine(name + "" "" + age); + } + + IEnumerable<(string name, int age)> GetPeople() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestFixAll1() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var {|FixAllInDocument:t1|} = GetPerson(); + var t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (name, age) = GetPerson(); + var t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestFixAll2() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var {|FixAllInDocument:t1|} = GetPerson(); + } + + void M2() + { + var t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (name, age) = GetPerson(); + } + + void M2() + { + var (name, age) = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestFixAll3() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + (string name1, int age1) {|FixAllInDocument:t1|} = GetPerson(); + (string name2, int age2) t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + (string name1, int age1) = GetPerson(); + (string name2, int age2) = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestFixAll4() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + (string name, int age) {|FixAllInDocument:t1|} = GetPerson(); + (string name, int age) t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}", +@"class C +{ + void M() + { + (string name, int age) = GetPerson(); + (string name, int age) t2 = GetPerson(); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfDefaultTupleNameWithVar() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + } + + (string, int) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestWithUserNamesThatMatchDefaultTupleNameWithVar1() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + } + + (string Item1, int Item2) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (Item1, Item2) = GetPerson(); + } + + (string Item1, int Item2) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestWithUserNamesThatMatchDefaultTupleNameWithVar2() + { + await TestInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1.Item1); + } + + (string Item1, int Item2) GetPerson() => default; +}", +@"class C +{ + void M() + { + var (Item1, Item2) = GetPerson(); + Console.WriteLine(Item1); + } + + (string Item1, int Item2) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfDefaultTupleNameWithTupleType() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + (string, int) [|t1|] = GetPerson(); + } + + (string, int) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfTupleIsUsed() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfTupleMethodIsUsed() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1.ToString()); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfTupleDefaultElementNameUsed() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1.Item1); + } + + (string name, int age) GetPerson() => default; +}"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseDeconstruction)] + public async Task TestNotIfTupleRandomNameUsed() + { + await TestMissingInRegularAndScriptAsync( +@"class C +{ + void M() + { + var [|t1|] = GetPerson(); + Console.WriteLine(t1.Unknown); + } + + (string name, int age) GetPerson() => default; +}"); + } + } +} diff --git a/src/EditorFeatures/TestUtilities/Traits.cs b/src/EditorFeatures/TestUtilities/Traits.cs index 9b74f127df849c13b6971d6fa4492812a369b391..c9ea889faa0baa74b4169361d84407245120ff0d 100644 --- a/src/EditorFeatures/TestUtilities/Traits.cs +++ b/src/EditorFeatures/TestUtilities/Traits.cs @@ -102,6 +102,7 @@ public static class Features public const string CodeActionsUseAutoProperty = "CodeActions.UseAutoProperty"; public const string CodeActionsUseCoalesceExpression = "CodeActions.UseCoalesceExpression"; public const string CodeActionsUseCollectionInitializer = "CodeActions.UseCollectionInitializer"; + public const string CodeActionsUseDeconstruction= "CodeActions.UseDeconstruction"; public const string CodeActionsUseDefaultLiteral = "CodeActions.UseDefaultLiteral"; public const string CodeActionsUseInferredMemberName = "CodeActions.UseInferredMemberName"; public const string CodeActionsUseExpressionBody = "CodeActions.UseExpressionBody"; diff --git a/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionCodeFixProvider.cs b/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionCodeFixProvider.cs index 870cbd19cdbf29bc4ec5ab791c28658dd2563968..67b2fdd5d2b960ddd8c626d4a8f2b8fb14738c56 100644 --- a/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionCodeFixProvider.cs @@ -68,7 +68,11 @@ private SyntaxNode UpdateRoot(SemanticModel semanticModel, SyntaxNode root, Synt { editor.ReplaceNode( variableDeclaration.Parent, - CreateDeconstructionStatement(tupleType, (LocalDeclarationStatementSyntax)variableDeclaration.Parent, variableDeclarator)); + (current, _) => + { + var currentDeclarationStatement = (LocalDeclarationStatementSyntax)current; + return CreateDeconstructionStatement(tupleType, currentDeclarationStatement, currentDeclarationStatement.Declaration.Variables[0]); + }); } } else if (node is ForEachStatementSyntax forEachStatement) @@ -80,13 +84,19 @@ private SyntaxNode UpdateRoot(SemanticModel semanticModel, SyntaxNode root, Synt { editor.ReplaceNode( forEachStatement, - CreateForEachVariableStatement(tupleType, forEachStatement)); + (current, _) => CreateForEachVariableStatement(tupleType, (ForEachStatementSyntax)current)); } } foreach (var memberAccess in memberAccessExpressions.NullToEmpty()) { - editor.ReplaceNode(memberAccess, memberAccess.Name.WithTriviaFrom(memberAccess)); + editor.ReplaceNode( + memberAccess, + (current, _) => + { + var currentMemberAccess = (MemberAccessExpressionSyntax)current; + return currentMemberAccess.Name.WithTriviaFrom(currentMemberAccess); + }); } return editor.GetChangedRoot(); diff --git a/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionDiagnosticAnalyzer.cs index a646e744a67164b81a0e3af97e5651a01e25cdb3..fd3c4883b83788656fa04c8c283fd0079c2f3bcc 100644 --- a/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/UseDeconstruction/CSharpUseDeconstructionDiagnosticAnalyzer.cs @@ -177,6 +177,15 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context) return false; } + // All tuple elements must have been explicitly provided by the user. + foreach (var element in tupleType.TupleElements) + { + if (element.IsImplicitlyDeclared) + { + return false; + } + } + var variableName = identifier.ValueText; var references = ArrayBuilder.GetInstance(); @@ -278,7 +287,7 @@ private bool IsViableTupleTypeSyntax(TypeSyntax type) return false; } - if (field.CorrespondingTupleField == field) + if (field.IsImplicitlyDeclared) { // They're referring to .Item1-.ItemN. We can't update this to refer to the local return false;