diff --git a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs index dada7e67b052556aaca5686e951fdb1e1a73e2d8..3110f941cccba09081614256dc41d754b6ecd95c 100644 --- a/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs +++ b/src/EditorFeatures/CSharpTest/InlineDeclaration/CSharpInlineDeclarationTests.cs @@ -979,6 +979,76 @@ void M() void Baz(out string s) { } void Bar(Action a) { } +}"); + } + + [WorkItem(15408, "https://github.com/dotnet/roslyn/issues/15408")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestDataFlow1() + { + await TestMissingAsync( +@" +using System; + +class C +{ + void Foo(string x) + { + object [|s|] = null; + if (x != null || TryBaz(out s)) + { + Console.WriteLine(s); + } + } + + private bool TryBaz(out object s) + { + throw new NotImplementedException(); + } +}"); + } + + [WorkItem(15408, "https://github.com/dotnet/roslyn/issues/15408")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] + public async Task TestDataFlow2() + { + await TestAsync( +@" +using System; + +class C +{ + void Foo(string x) + { + object [|s|] = null; + if (x != null && TryBaz(out s)) + { + Console.WriteLine(s); + } + } + + private bool TryBaz(out object s) + { + throw new NotImplementedException(); + } +}", +@" +using System; + +class C +{ + void Foo(string x) + { + if (x != null && TryBaz(out object s)) + { + Console.WriteLine(s); + } + } + + private bool TryBaz(out object s) + { + throw new NotImplementedException(); + } }"); } } diff --git a/src/EditorFeatures/CSharpTest/UseObjectInitializer/UseObjectInitializerTests.cs b/src/EditorFeatures/CSharpTest/UseObjectInitializer/UseObjectInitializerTests.cs index e0ba7d7cfa0650fbdfe8e5093ec4989062c0e611..a9e10f8bc237f12cce3f89f585d9d5db4bccede2 100644 --- a/src/EditorFeatures/CSharpTest/UseObjectInitializer/UseObjectInitializerTests.cs +++ b/src/EditorFeatures/CSharpTest/UseObjectInitializer/UseObjectInitializerTests.cs @@ -454,5 +454,20 @@ void M() }", compareTokens: false); } + + [WorkItem(15459, "https://github.com/dotnet/roslyn/issues/15459")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseObjectInitializer)] + public async Task TestMissingInNonTopLevelObjectInitializer() + { + await TestMissingAsync( +@"class C { + int a; + C Add(int x) { + var c = Add([||]new int()); + c.a = 1; + return c; + } +}"); + } } } \ No newline at end of file diff --git a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs index 063551fc2284e2ddf19d1b2f514163bd06006125..bd82873c41f8ca956254838dc4589bbc222e4a7e 100644 --- a/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -8,6 +7,7 @@ using Microsoft.CodeAnalysis.CSharp.Extensions; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.CSharp.InlineDeclaration { @@ -26,6 +26,8 @@ namespace Microsoft.CodeAnalysis.CSharp.InlineDeclaration [DiagnosticAnalyzer(LanguageNames.CSharp)] internal class CSharpInlineDeclarationDiagnosticAnalyzer : AbstractCodeStyleDiagnosticAnalyzer { + private const string CS0165 = nameof(CS0165); // Use of unassigned local variable 's' + public CSharpInlineDeclarationDiagnosticAnalyzer() : base(IDEDiagnosticIds.InlineDeclarationDiagnosticId, new LocalizableResourceString(nameof(FeaturesResources.Inline_variable_declaration), FeaturesResources.ResourceManager, typeof(FeaturesResources)), @@ -186,6 +188,15 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) return; } + // See if inlining this variable would make it so that some variables were no + // longer definitely assigned. + if (WouldCauseDefiniteAssignmentErrors( + semanticModel, localDeclarator, enclosingBlockOfLocalStatement, + outSymbol, cancellationToken)) + { + return; + } + // Collect some useful nodes for the fix provider to use so it doesn't have to // find them again. var allLocations = ImmutableArray.Create( @@ -206,6 +217,79 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context) additionalLocations: allLocations)); } + private bool WouldCauseDefiniteAssignmentErrors( + SemanticModel semanticModel, VariableDeclaratorSyntax localDeclarator, + BlockSyntax enclosingBlock, ISymbol outSymbol, CancellationToken cancellationToken) + { + // See if we have something like: + // + // int i = 0; + // if (Foo() || Bar(out i)) + // { + // Console.WriteLine(i); + // } + // + // In this case, inlining the 'i' would cause it to longer be definitely + // assigned in the WriteLine invocation. + + if (localDeclarator.Initializer == null) + { + // Don't need to examine this unless the variable has an initializer. + return false; + } + + // Find all the current read-references to the local. + var query = from t in enclosingBlock.DescendantTokens() + where t.Kind() == SyntaxKind.IdentifierToken + where t.ValueText == outSymbol.Name + let id = t.Parent as IdentifierNameSyntax + where id != null + where !id.IsOnlyWrittenTo() + let symbol = semanticModel.GetSymbolInfo(id).GetAnySymbol() + where outSymbol.Equals(symbol) + select id; + + var references = query.ToImmutableArray(); + + var root = semanticModel.SyntaxTree.GetCompilationUnitRoot(cancellationToken); + + // Ensure we can track the references and the local variable as we make edits + // to the tree. + var rootWithTrackedNodes = root.TrackNodes(references.Concat(localDeclarator).Concat(enclosingBlock)); + + // Now, take the local variable and remove it's initializer. Then go to all + // the locations where we read from it. If they're definitely assigned, then + // that means the out-var did it's work and assigned the variable across all + // paths. If it's not definitely assigned, then we can't inline this variable. + var currentLocalDeclarator = rootWithTrackedNodes.GetCurrentNode(localDeclarator); + var rootWithoutInitializer = rootWithTrackedNodes.ReplaceNode( + currentLocalDeclarator, + currentLocalDeclarator.WithInitializer(null)); + + // Fork the compilation so we can do this analysis. + var newCompilation = semanticModel.Compilation.ReplaceSyntaxTree( + root.SyntaxTree, rootWithoutInitializer.SyntaxTree); + var newSemanticModel = newCompilation.GetSemanticModel(rootWithoutInitializer.SyntaxTree); + + // NOTE: there is no current compiler API to determine if a variable is definitely + // assigned or not. So, for now, we just get diagnostics for this block and see if + // we get any definite assigment errors where we have a reference to the symbol. If + // so, then we don't offer the fix. + + var currentBlock = rootWithoutInitializer.GetCurrentNode(enclosingBlock); + var diagnostics = newSemanticModel.GetDiagnostics(currentBlock.Span, cancellationToken); + + var diagnosticSpans = diagnostics.Where(d => d.Id == CS0165) + .Select(d => d.Location.SourceSpan) + .Distinct(); + + var newReferenceSpans = rootWithoutInitializer.GetCurrentNodes(references) + .Select(n => n.Span) + .Distinct(); + + return diagnosticSpans.Intersect(newReferenceSpans).Any(); + } + private SyntaxNode GetOutArgumentScope(SyntaxNode argumentExpression) { for (var current = argumentExpression; current != null; current = current.Parent) diff --git a/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs b/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs index e7b272b202377aab798bbc8d7c7d2a38edf6d6ee..b1d8a87fe8ec1f248876ac1edeede50e931c423c 100644 --- a/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs +++ b/src/Features/Core/Portable/UseCollectionInitializer/ObjectCreationExpressionAnalyzer.cs @@ -238,7 +238,7 @@ private bool TryInitializeVariableDeclarationCase() return false; } - var containingDeclarator = _objectCreationExpression.FirstAncestorOrSelf(); + var containingDeclarator = _objectCreationExpression.Parent.Parent as TVariableDeclaratorSyntax; if (containingDeclarator == null) { return false; diff --git a/src/Features/Core/Portable/UseObjectInitializer/ObjectCreationExpressionAnalyzer.cs b/src/Features/Core/Portable/UseObjectInitializer/ObjectCreationExpressionAnalyzer.cs index 406790df6b9ca61e9b22d2d2c40c388b4ee5aaf2..efbfd83047821c37b47d6998e910a9e951f4a0c6 100644 --- a/src/Features/Core/Portable/UseObjectInitializer/ObjectCreationExpressionAnalyzer.cs +++ b/src/Features/Core/Portable/UseObjectInitializer/ObjectCreationExpressionAnalyzer.cs @@ -248,7 +248,7 @@ private bool TryInitializeVariableDeclarationCase() return false; } - var containingDeclarator = _objectCreationExpression.FirstAncestorOrSelf(); + var containingDeclarator = _objectCreationExpression.Parent.Parent as TVariableDeclaratorSyntax; if (containingDeclarator == null) { return false; diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpFlagsEnumGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpFlagsEnumGenerator.cs index d1889fba38ce78176713ba8b8b498b3bcf441c63..f4e15146543633306f9e34a63a6309ee7c24d1df 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpFlagsEnumGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpFlagsEnumGenerator.cs @@ -20,7 +20,7 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeGeneration internal class CSharpFlagsEnumGenerator : AbstractFlagsEnumGenerator { internal static readonly CSharpFlagsEnumGenerator Instance = new CSharpFlagsEnumGenerator(); - private static readonly SyntaxGenerator s_generatorInstance = new CSharpSyntaxGenerator(); + private static readonly SyntaxGenerator s_generatorInstance = CSharpSyntaxGenerator.Instance; private CSharpFlagsEnumGenerator() { @@ -41,8 +41,7 @@ private CSharpFlagsEnumGenerator() return expression; } - var factory = new CSharpSyntaxGenerator(); - return factory.CastExpression(enumType, expression); + return CSharpSyntaxGenerator.Instance.CastExpression(enumType, expression); } protected override SyntaxGenerator GetSyntaxGenerator() diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs index 6ce6a8944137b09b46e02e459e4ff73225ea1c99..6308a5fd04b19c55c97a1a5c489520e112fd41f8 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs @@ -21,6 +21,8 @@ internal class CSharpSyntaxGenerator : SyntaxGenerator { internal override SyntaxTrivia CarriageReturnLineFeed => SyntaxFactory.CarriageReturnLineFeed; + public static readonly SyntaxGenerator Instance = new CSharpSyntaxGenerator(); + #region Declarations public override SyntaxNode CompilationUnit(IEnumerable declarations) { diff --git a/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs b/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs index 7eea6502840242e9128951a3a49c463149bbee4a..32991f062def8dd7d404e93b31cff8265824f6dc 100644 --- a/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs +++ b/src/Workspaces/Core/Portable/Editing/SyntaxEditor.cs @@ -29,6 +29,13 @@ public SyntaxEditor(SyntaxNode root, Workspace workspace) _changes = new List(); } + internal SyntaxEditor(SyntaxNode root, SyntaxGenerator generator) + { + OriginalRoot = root ?? throw new ArgumentNullException(nameof(root)); + _generator = generator; + _changes = new List(); + } + /// /// The that was specified when the was constructed. /// @@ -280,4 +287,4 @@ public override SyntaxNode Apply(SyntaxNode root, SyntaxGenerator generator) } } } -} +} \ No newline at end of file diff --git a/src/Workspaces/Core/Portable/Editing/SyntaxEditorExtensions.cs b/src/Workspaces/Core/Portable/Editing/SyntaxEditorExtensions.cs index 8f1266d38cf1c8795a02d5f1763c3ffe0a0831e3..69f7e46dcf70ea62255617939a6c1ce459128ba9 100644 --- a/src/Workspaces/Core/Portable/Editing/SyntaxEditorExtensions.cs +++ b/src/Workspaces/Core/Portable/Editing/SyntaxEditorExtensions.cs @@ -106,4 +106,4 @@ public static void AddBaseType(this SyntaxEditor editor, SyntaxNode declaration, editor.ReplaceNode(declaration, (d, g) => g.AddBaseType(d, baseType)); } } -} +} \ No newline at end of file diff --git a/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.StringPiece.vb b/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.StringPiece.vb index 5b0a1c2f57f15c22304f2b71253c0812df1d000a..b1012e6fc5430e09a8a5f8a13923ebb72277a3c3 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.StringPiece.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.StringPiece.vb @@ -74,7 +74,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration End Function Private Shared Function GenerateStringConstantExpression(name As String) As MemberAccessExpressionSyntax - Dim factory = New VisualBasicSyntaxGenerator() Dim result = GenerateMemberAccessExpression("Microsoft", "VisualBasic", "Constants", name) Return result.WithAdditionalAnnotations(Simplifier.Annotation) diff --git a/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.vb b/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.vb index 40fa8dd2d48a6cca9b3a48f129e8f07abad6c273..cbc7d0a9b18e2174a440cf2c8c7ee58cb390fe19 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeGeneration/ExpressionGenerator.vb @@ -142,7 +142,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration End Function Private Function GenerateChrWExpression(c As Char) As InvocationExpressionSyntax - Dim factory = New VisualBasicSyntaxGenerator() Dim access = GenerateMemberAccessExpression("Microsoft", "VisualBasic", "Strings", "ChrW") Dim value = AscW(c) diff --git a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicFlagsEnumGenerator.vb b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicFlagsEnumGenerator.vb index 106c0f3463f7bcd1bf8ee2a6047726314adfd5dc..50b95bbc09d96008dc83713c6469fc966524e9a2 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicFlagsEnumGenerator.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicFlagsEnumGenerator.vb @@ -17,7 +17,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration Inherits AbstractFlagsEnumGenerator Public Shared ReadOnly Instance As VisualBasicFlagsEnumGenerator = New VisualBasicFlagsEnumGenerator - Private Shared ReadOnly s_syntaxGeneratorInstance As SyntaxGenerator = New VisualBasicSyntaxGenerator + Private Shared ReadOnly s_syntaxGeneratorInstance As SyntaxGenerator = VisualBasicSyntaxGenerator.Instance Private Sub New() End Sub @@ -32,8 +32,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration Return expression End If - Dim factory = New VisualBasicSyntaxGenerator() - Return factory.ConvertExpression(enumType, expression) + Return VisualBasicSyntaxGenerator.Instance.ConvertExpression(enumType, expression) End Function Protected Overrides Function GetSyntaxGenerator() As SyntaxGenerator