From 6dc35229e9e2e8c52e744f526913145fb19f8dd4 Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Wed, 10 Jan 2018 11:18:38 -0800 Subject: [PATCH] UseExplicitType for var in deconstruction (#23975) --- .../Portable/Binder/ForEachLoopBinder.cs | 6 +- .../Emit/CodeGen/CodeGenDeconstructTests.cs | 1 + .../UseExplicitTypeTests.cs | 169 +++++++++++++++++- ...CSharpUseExplicitTypeDiagnosticAnalyzer.cs | 12 ++ .../UseExplicitTypeCodeFixProvider.cs | 72 +++++++- 5 files changed, 250 insertions(+), 10 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs b/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs index a40f8ba19e4..b87a3b4eba0 100644 --- a/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs +++ b/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs @@ -249,7 +249,7 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics, var variables = node.Variable; if (variables.IsDeconstructionLeft()) { - var valuePlaceholder = new BoundDeconstructValuePlaceholder(_syntax.Expression, collectionEscape, iterationVariableType) { WasCompilerGenerated = true } ; + var valuePlaceholder = new BoundDeconstructValuePlaceholder(_syntax.Expression, collectionEscape, iterationVariableType).MakeCompilerGenerated(); DeclarationExpressionSyntax declaration = null; ExpressionSyntax expression = null; BoundDeconstructionAssignmentOperator deconstruction = BindDeconstruction( @@ -268,7 +268,7 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics, hasErrors = true; } - deconstructStep = new BoundForEachDeconstructStep(variables, deconstruction, valuePlaceholder); + deconstructStep = new BoundForEachDeconstructStep(variables, deconstruction, valuePlaceholder).MakeCompilerGenerated(); } else if (!node.HasErrors) { @@ -277,7 +277,7 @@ private BoundForEachStatement BindForEachPartsWorker(DiagnosticBag diagnostics, hasErrors = true; } - boundIterationVariableType = new BoundTypeExpression(variables, aliasOpt: null, type: iterationVariableType); + boundIterationVariableType = new BoundTypeExpression(variables, aliasOpt: null, type: iterationVariableType).MakeCompilerGenerated(); break; } default: diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs index a4b99f52049..d4c368b51e5 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs @@ -1227,6 +1227,7 @@ public void Deconstruct(out int a, out int b) Assert.Equal("(System.Int32 a, System.Int32 b)", tuple2.ToTestDisplayString()); var underlying2 = tuple2.TupleUnderlyingType; Assert.Equal("System.ValueTuple[missing]", underlying2.ToTestDisplayString()); + Assert.Equal("(System.Int32 a, System.Int32 b)", model.GetTypeInfo(ab).ConvertedType.ToTestDisplayString()); } [Fact] diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseExplicitTypeTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseExplicitTypeTests.cs index 119c52158f0..0a655a4ce7e 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseExplicitTypeTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/UseImplicitOrExplicitType/UseExplicitTypeTests.cs @@ -6,7 +6,6 @@ using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.CSharp.CodeStyle; using Microsoft.CodeAnalysis.CSharp.Diagnostics.TypeStyle; -using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.CSharp.TypeStyle; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Options; @@ -462,6 +461,174 @@ void Method() }", new TestParameters(options: ExplicitTypeEverywhere())); } + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnDeconstructionVar() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + [|var|] (x, y) = new Program(); + } + void Deconstruct(out int i, out string s) { i = 1; s = ""hello""; } +}", @"using System; +class Program +{ + void M() + { + (int x, string y) = new Program(); + } + void Deconstruct(out int i, out string s) { i = 1; s = ""hello""; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnNestedDeconstructionVar() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + [|var|] (x, (y, z)) = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", @"using System; +class Program +{ + void M() + { + (int x, (int y, Program z)) = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnBadlyFormattedNestedDeconstructionVar() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + [|var|](x,(y,z)) = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", @"using System; +class Program +{ + void M() + { + (int x, (int y, Program z)) = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnForeachNestedDeconstructionVar() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + foreach ([|var|] (x, (y, z)) in new[] { new Program() } { } + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", @"using System; +class Program +{ + void M() + { + foreach ((int x, (int y, Program z)) in new[] { new Program() } { } + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnNestedDeconstructionVarWithTrivia() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + /*before*/[|var|]/*after*/ (/*x1*/x/*x2*/, /*yz1*/(/*y1*/y/*y2*/, /*z1*/z/*z2*/)/*yz2*/) /*end*/ = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", @"using System; +class Program +{ + void M() + { + /*before*//*after*/(/*x1*/int x/*x2*/, /*yz1*/(/*y1*/int y/*y2*/, /*z1*/Program z/*z2*/)/*yz2*/) /*end*/ = new Program(); + } + void Deconstruct(out int i, out Program s) { i = 1; s = null; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnDeconstructionVarWithDiscard() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + [|var|] (x, _) = new Program(); + } + void Deconstruct(out int i, out string s) { i = 1; s = ""hello""; } +}", @"using System; +class Program +{ + void M() + { + (int x, string _) = new Program(); + } + void Deconstruct(out int i, out string s) { i = 1; s = ""hello""; } +}", options: ExplicitTypeEverywhere()); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] + [WorkItem(23752, "https://github.com/dotnet/roslyn/issues/23752")] + public async Task OnDeconstructionVarWithErrorType() + { + await TestInRegularAndScriptAsync( +@"using System; +class Program +{ + void M() + { + [|var|] (x, y) = new Program(); + } + void Deconstruct(out int i, out Error s) { i = 1; s = null; } +}", @"using System; +class Program +{ + void M() + { + (int x, Error y) = new Program(); + } + void Deconstruct(out int i, out Error s) { i = 1; s = null; } +}", options: ExplicitTypeEverywhere()); + } + [WpfFact, Trait(Traits.Feature, Traits.Features.CodeActionsUseExplicitType)] public async Task OnForEachVarWithExplicitType() { diff --git a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseExplicitTypeDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseExplicitTypeDiagnosticAnalyzer.cs index ff524c9acc3..92b304e7810 100644 --- a/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseExplicitTypeDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpUseExplicitTypeDiagnosticAnalyzer.cs @@ -81,6 +81,18 @@ protected override bool TryAnalyzeVariableDeclaration(TypeSyntax typeName, Seman { issueSpan = default; + // var (x, y) = e; + // foreach (var (x, y) in e) ... + if (typeName.IsParentKind(SyntaxKind.DeclarationExpression)) + { + var parent = (DeclarationExpressionSyntax)typeName.Parent; + if (parent.Designation.IsKind(SyntaxKind.ParenthesizedVariableDesignation)) + { + issueSpan = typeName.Span; + return true; + } + } + // If it is currently not var, explicit typing exists, return. // this also takes care of cases where var is mapped to a named type via an alias or a class declaration. if (!typeName.IsTypeInferred(semanticModel)) diff --git a/src/Features/CSharp/Portable/TypeStyle/UseExplicitTypeCodeFixProvider.cs b/src/Features/CSharp/Portable/TypeStyle/UseExplicitTypeCodeFixProvider.cs index d669a77a345..500f981b12b 100644 --- a/src/Features/CSharp/Portable/TypeStyle/UseExplicitTypeCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/TypeStyle/UseExplicitTypeCodeFixProvider.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CSharp.TypeStyle @@ -53,6 +54,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) var declarationContext = node.Parent; TypeSyntax typeSyntax = null; + ParenthesizedVariableDesignationSyntax parensDesignation = null; if (declarationContext is VariableDeclarationSyntax varDecl) { typeSyntax = varDecl.Type; @@ -64,21 +66,79 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) else if (declarationContext is DeclarationExpressionSyntax declarationExpression) { typeSyntax = declarationExpression.Type; + if (declarationExpression.Designation.IsKind(SyntaxKind.ParenthesizedVariableDesignation)) + { + parensDesignation = (ParenthesizedVariableDesignationSyntax)declarationExpression.Designation; + } } else { Contract.Fail($"unhandled kind {declarationContext.Kind().ToString()}"); } - var typeSymbol = semanticModel.GetTypeInfo(typeSyntax).ConvertedType; + if (parensDesignation is null) + { + var typeSymbol = semanticModel.GetTypeInfo(typeSyntax).ConvertedType; + var typeName = typeSymbol.GenerateTypeSyntax() + .WithLeadingTrivia(node.GetLeadingTrivia()) + .WithTrailingTrivia(node.GetTrailingTrivia()); + Debug.Assert(!typeName.ContainsDiagnostics, "Explicit type replacement likely introduced an error in code"); + + editor.ReplaceNode(node, typeName); + } + else + { + var tupleTypeSymbol = semanticModel.GetTypeInfo(typeSyntax.Parent).ConvertedType; + + var leadingTrivia = node.GetLeadingTrivia() + .Concat(parensDesignation.GetAllPrecedingTriviaToPreviousToken().Where(t => !t.IsWhitespace()).Select(t => t.WithoutAnnotations(SyntaxAnnotation.ElasticAnnotation))); + + var tupleDeclaration = GenerateTupleDeclaration(tupleTypeSymbol, parensDesignation).WithLeadingTrivia(leadingTrivia); + + editor.ReplaceNode(declarationContext, tupleDeclaration); + } + } + + private ExpressionSyntax GenerateTupleDeclaration(ITypeSymbol typeSymbol, ParenthesizedVariableDesignationSyntax parensDesignation) + { + Debug.Assert(typeSymbol.IsTupleType); + var elements = ((INamedTypeSymbol)typeSymbol).TupleElements; + Debug.Assert(elements.Length == parensDesignation.Variables.Count); - var typeName = typeSymbol.GenerateTypeSyntax() - .WithLeadingTrivia(node.GetLeadingTrivia()) - .WithTrailingTrivia(node.GetTrailingTrivia()); + var builder = ArrayBuilder.GetInstance(elements.Length); + for (int i = 0; i < elements.Length; i++) + { + var designation = parensDesignation.Variables[i]; + var type = elements[i].Type; + ExpressionSyntax newDeclaration; + switch (designation.Kind()) + { + case SyntaxKind.SingleVariableDesignation: + case SyntaxKind.DiscardDesignation: + var typeName = type.GenerateTypeSyntax(); + newDeclaration = SyntaxFactory.DeclarationExpression(typeName, designation); + break; + case SyntaxKind.ParenthesizedVariableDesignation: + newDeclaration = GenerateTupleDeclaration(type, (ParenthesizedVariableDesignationSyntax)designation); + break; + default: + throw ExceptionUtilities.UnexpectedValue(designation.Kind()); + } + + newDeclaration = newDeclaration + .WithLeadingTrivia(designation.GetAllPrecedingTriviaToPreviousToken()) + .WithTrailingTrivia(designation.GetTrailingTrivia()); + + builder.Add(SyntaxFactory.Argument(newDeclaration)); + } - Debug.Assert(!typeName.ContainsDiagnostics, "Explicit type replacement likely introduced an error in code"); + var separatorBuilder = ArrayBuilder.GetInstance(builder.Count - 1, SyntaxFactory.Token(leading: default, SyntaxKind.CommaToken, trailing: default)); - editor.ReplaceNode(node, typeName); + return SyntaxFactory.TupleExpression( + SyntaxFactory.Token(SyntaxKind.OpenParenToken).WithTrailingTrivia(), + SyntaxFactory.SeparatedList(builder.ToImmutableAndFree(), separatorBuilder.ToImmutableAndFree()), + SyntaxFactory.Token(SyntaxKind.CloseParenToken)) + .WithTrailingTrivia(parensDesignation.GetTrailingTrivia()); } private class MyCodeAction : CodeAction.DocumentChangeAction -- GitLab