diff --git a/src/Analyzers/CSharp/Analyzers/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs index cc1998d92fddc..0c301b610c1da 100644 --- a/src/Analyzers/CSharp/Analyzers/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/InlineDeclaration/CSharpInlineDeclarationDiagnosticAnalyzer.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Immutable; using System.Linq; using System.Linq.Expressions; @@ -30,7 +31,7 @@ namespace Microsoft.CodeAnalysis.CSharp.InlineDeclaration; /// /// [DiagnosticAnalyzer(LanguageNames.CSharp)] -internal class CSharpInlineDeclarationDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer +internal sealed class CSharpInlineDeclarationDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer { private const string CS0165 = nameof(CS0165); // Use of unassigned local variable 's' @@ -179,10 +180,9 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // for references to the local to make sure that no reads/writes happen before // the out-argument. If there are any reads/writes we can't inline as those // accesses will become invalid. - if (localStatement.Parent is not BlockSyntax enclosingBlockOfLocalStatement) - { + var enclosingBlockOfLocalStatement = GetEnclosingPseudoBlock(localStatement.Parent); + if (enclosingBlockOfLocalStatement is null) return; - } if (argumentExpression.IsInExpressionTree(semanticModel, expressionType, cancellationToken)) { @@ -223,8 +223,8 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb // See if inlining this variable would make it so that some variables were no // longer definitely assigned. - if (WouldCauseDefiniteAssignmentErrors(semanticModel, localStatement, - enclosingBlockOfLocalStatement, outLocalSymbol)) + if (WouldCauseDefiniteAssignmentErrors( + semanticModel, localStatement, enclosingBlockOfLocalStatement, outLocalSymbol)) { return; } @@ -251,10 +251,38 @@ private void AnalyzeSyntaxNode(SyntaxNodeAnalysisContext context, INamedTypeSymb properties: null)); } + public static SyntaxNode? GetEnclosingPseudoBlock(SyntaxNode? parent) + { + if (parent is BlockSyntax) + return parent; + + if (parent is SwitchSectionSyntax) + return parent; + + if (parent is GlobalStatementSyntax) + return parent.Parent as CompilationUnitSyntax; + + return null; + } + + private static StatementSyntax GetLastStatement(SyntaxNode enclosingBlock) + { + if (enclosingBlock is BlockSyntax block) + return block.Statements.Last(); + + if (enclosingBlock is SwitchSectionSyntax switchSection) + return switchSection.Statements.Last(); + + if (enclosingBlock is CompilationUnitSyntax compilationUnit) + return compilationUnit.Members.OfType().Last().Statement; + + throw ExceptionUtilities.Unreachable(); + } + private static bool WouldCauseDefiniteAssignmentErrors( SemanticModel semanticModel, LocalDeclarationStatementSyntax localStatement, - BlockSyntax enclosingBlock, + SyntaxNode enclosingBlock, ILocalSymbol outLocalSymbol) { // See if we have something like: @@ -272,7 +300,7 @@ private static bool WouldCauseDefiniteAssignmentErrors( var dataFlow = semanticModel.AnalyzeDataFlow( nextStatement, - enclosingBlock.Statements.Last()); + GetLastStatement(enclosingBlock)); Contract.ThrowIfNull(dataFlow); return dataFlow.DataFlowsIn.Contains(outLocalSymbol); } @@ -332,7 +360,7 @@ private static bool WouldCauseDefiniteAssignmentErrors( private static bool IsAccessed( SemanticModel semanticModel, ISymbol outSymbol, - BlockSyntax enclosingBlockOfLocalStatement, + SyntaxNode enclosingBlockOfLocalStatement, LocalDeclarationStatementSyntax localStatement, ArgumentSyntax argumentNode, CancellationToken cancellationToken) diff --git a/src/Analyzers/CSharp/CodeFixes/InlineDeclaration/CSharpInlineDeclarationCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/InlineDeclaration/CSharpInlineDeclarationCodeFixProvider.cs index 106b78d3d4ca1..934b1cc4b4b3b 100644 --- a/src/Analyzers/CSharp/CodeFixes/InlineDeclaration/CSharpInlineDeclarationCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/InlineDeclaration/CSharpInlineDeclarationCodeFixProvider.cs @@ -115,24 +115,21 @@ private static SyntaxNode ReplaceIdentifierWithInlineDeclaration( // this local statement will be moved to be above the statement containing // the out-var. var localDeclarationStatement = (LocalDeclarationStatementSyntax)declaration.Parent; - var block = (BlockSyntax)localDeclarationStatement.Parent; - var declarationIndex = block.Statements.IndexOf(localDeclarationStatement); + var block = CSharpInlineDeclarationDiagnosticAnalyzer.GetEnclosingPseudoBlock(localDeclarationStatement.Parent); + var statements = GetStatements(block); + var declarationIndex = statements.IndexOf(localDeclarationStatement); // Try to find a predecessor Statement on the same line that isn't going to be removed StatementSyntax priorStatementSyntax = null; var localDeclarationToken = localDeclarationStatement.GetFirstToken(); for (var i = declarationIndex - 1; i >= 0; i--) { - var statementSyntax = block.Statements[i]; + var statementSyntax = statements[i]; if (declarationsToRemove.Contains(statementSyntax)) - { continue; - } if (sourceText.AreOnSameLine(statementSyntax.GetLastToken(), localDeclarationToken)) - { priorStatementSyntax = statementSyntax; - } break; } @@ -158,9 +155,9 @@ private static SyntaxNode ReplaceIdentifierWithInlineDeclaration( // We initialize this to null here but we must see at least the statement // into which the declaration is going to be inlined so this will be not null StatementSyntax nextStatementSyntax = null; - for (var i = declarationIndex + 1; i < block.Statements.Count; i++) + for (var i = declarationIndex + 1; i < statements.Length; i++) { - var statement = block.Statements[i]; + var statement = statements[i]; if (!declarationsToRemove.Contains(statement)) { nextStatementSyntax = statement; @@ -175,7 +172,9 @@ private static SyntaxNode ReplaceIdentifierWithInlineDeclaration( } // The above code handled the moving of trivia. So remove the node, keeping around no trivia from it. - editor.RemoveNode(localDeclarationStatement, SyntaxRemoveOptions.KeepNoTrivia); + editor.RemoveNode(localDeclarationStatement.Parent is GlobalStatementSyntax globalStatement + ? globalStatement + : localDeclarationStatement, SyntaxRemoveOptions.KeepNoTrivia); } else { @@ -238,6 +237,20 @@ private static SyntaxNode ReplaceIdentifierWithInlineDeclaration( return editor.GetChangedRoot(); } + private static ImmutableArray GetStatements(SyntaxNode pseudoBlock) + { + if (pseudoBlock is BlockSyntax block) + return [.. block.Statements]; + + if (pseudoBlock is SwitchSectionSyntax switchSection) + return [.. switchSection.Statements]; + + if (pseudoBlock is CompilationUnitSyntax compilationUnit) + return compilationUnit.Members.OfType().Select(g => g.Statement).ToImmutableArray(); + + return []; + } + public static TypeSyntax GenerateTypeSyntaxOrVar( ITypeSymbol symbol, CSharpSimplifierOptions options) { diff --git a/src/Analyzers/CSharp/Tests/InlineDeclaration/CSharpInlineDeclarationTests.cs b/src/Analyzers/CSharp/Tests/InlineDeclaration/CSharpInlineDeclarationTests.cs index 97f431170bbe6..fffaef75f7d51 100644 --- a/src/Analyzers/CSharp/Tests/InlineDeclaration/CSharpInlineDeclarationTests.cs +++ b/src/Analyzers/CSharp/Tests/InlineDeclaration/CSharpInlineDeclarationTests.cs @@ -18,13 +18,9 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.InlineDeclaration; [Trait(Traits.Feature, Traits.Features.CodeActionsInlineDeclaration)] -public partial class CSharpInlineDeclarationTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor +public sealed partial class CSharpInlineDeclarationTests(ITestOutputHelper logger) + : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest_NoEditor(logger) { - public CSharpInlineDeclarationTests(ITestOutputHelper logger) - : base(logger) - { - } - internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) => (new CSharpInlineDeclarationDiagnosticAnalyzer(), new CSharpInlineDeclarationCodeFixProvider()); @@ -2433,15 +2429,21 @@ void M(out C c2) """); } - [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/44429")] + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/44429")] + [WorkItem("https://github.com/dotnet/roslyn/issues/74736")] public async Task TopLevelStatement() { - await TestMissingAsync(""" + await TestAsync(""" [|int|] i; if (int.TryParse(v, out i)) { } - """, new TestParameters(TestOptions.Regular)); + """, """ + if (int.TryParse(v, out int i)) + { + } + """, CSharpParseOptions.Default); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/47041")] @@ -2512,4 +2514,41 @@ private void TestMethod(out int hello) } """); } + + [Fact] + public async Task TestInSwitchSection() + { + await TestInRegularAndScript1Async( + """ + class C + { + void M(object o) + { + switch (o) + { + case string s: + [|int|] i; + if (int.TryParse(v, out i)) + { + } + } + } + } + """, + """ + class C + { + void M(object o) + { + switch (o) + { + case string s: + if (int.TryParse(v, out int i)) + { + } + } + } + } + """); + } }