Skip to content

Commit

Permalink
Merge pull request #75258 from CyrusNajmabadi/inlineDeclarationTopLevel
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Sep 30, 2024
2 parents d979bbd + c02384a commit 5bf36f9
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -30,7 +31,7 @@ namespace Microsoft.CodeAnalysis.CSharp.InlineDeclaration;
///
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
internal class CSharpInlineDeclarationDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer
internal sealed class CSharpInlineDeclarationDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer
{
private const string CS0165 = nameof(CS0165); // Use of unassigned local variable 's'

Expand Down Expand Up @@ -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))
{
Expand Down Expand Up @@ -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;
}
Expand All @@ -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<GlobalStatementSyntax>().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:
Expand All @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand All @@ -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
{
Expand Down Expand Up @@ -238,6 +237,20 @@ private static SyntaxNode ReplaceIdentifierWithInlineDeclaration(
return editor.GetChangedRoot();
}

private static ImmutableArray<StatementSyntax> 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<GlobalStatementSyntax>().Select(g => g.Statement).ToImmutableArray();

return [];
}

public static TypeSyntax GenerateTypeSyntaxOrVar(
ITypeSymbol symbol, CSharpSimplifierOptions options)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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))
{
}
}
}
}
""");
}
}

0 comments on commit 5bf36f9

Please sign in to comment.