Skip to content

Commit 5601472

Browse files
Update 'use simple using statement' to support global statements (#75921)
2 parents 77d5fe7 + 99cc6e3 commit 5601472

File tree

4 files changed

+321
-74
lines changed

4 files changed

+321
-74
lines changed

src/Analyzers/CSharp/Analyzers/UseSimpleUsingStatement/UseSimpleUsingStatementDiagnosticAnalyzer.cs

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,20 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Collections.Generic;
56
using System.Collections.Immutable;
6-
using System.Linq;
77
using System.Threading;
88
using Microsoft.CodeAnalysis.CodeStyle;
99
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
1010
using Microsoft.CodeAnalysis.CSharp.Extensions;
11+
using Microsoft.CodeAnalysis.CSharp.LanguageService;
1112
using Microsoft.CodeAnalysis.CSharp.Syntax;
1213
using Microsoft.CodeAnalysis.Diagnostics;
1314
using Microsoft.CodeAnalysis.Operations;
15+
using Microsoft.CodeAnalysis.PooledObjects;
1416
using Microsoft.CodeAnalysis.Shared.Extensions;
17+
using Microsoft.CodeAnalysis.Utilities;
18+
using Roslyn.Utilities;
1519

1620
namespace Microsoft.CodeAnalysis.CSharp.UseSimpleUsingStatement;
1721

@@ -48,17 +52,14 @@ namespace Microsoft.CodeAnalysis.CSharp.UseSimpleUsingStatement;
4852
/// semantics will not change.
4953
/// </summary>
5054
[DiagnosticAnalyzer(LanguageNames.CSharp)]
51-
internal class UseSimpleUsingStatementDiagnosticAnalyzer : AbstractBuiltInCodeStyleDiagnosticAnalyzer
55+
internal sealed class UseSimpleUsingStatementDiagnosticAnalyzer()
56+
: AbstractBuiltInCodeStyleDiagnosticAnalyzer(
57+
IDEDiagnosticIds.UseSimpleUsingStatementDiagnosticId,
58+
EnforceOnBuildValues.UseSimpleUsingStatement,
59+
CSharpCodeStyleOptions.PreferSimpleUsingStatement,
60+
new LocalizableResourceString(nameof(CSharpAnalyzersResources.Use_simple_using_statement), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
61+
new LocalizableResourceString(nameof(CSharpAnalyzersResources.using_statement_can_be_simplified), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)))
5262
{
53-
public UseSimpleUsingStatementDiagnosticAnalyzer()
54-
: base(IDEDiagnosticIds.UseSimpleUsingStatementDiagnosticId,
55-
EnforceOnBuildValues.UseSimpleUsingStatement,
56-
CSharpCodeStyleOptions.PreferSimpleUsingStatement,
57-
new LocalizableResourceString(nameof(CSharpAnalyzersResources.Use_simple_using_statement), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
58-
new LocalizableResourceString(nameof(CSharpAnalyzersResources.using_statement_can_be_simplified), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)))
59-
{
60-
}
61-
6263
public override DiagnosticAnalyzerCategory GetAnalyzerCategory()
6364
=> DiagnosticAnalyzerCategory.SemanticSpanAnalysis;
6465

@@ -83,12 +84,14 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
8384
var outermostUsing = (UsingStatementSyntax)context.Node;
8485
var semanticModel = context.SemanticModel;
8586

86-
if (outermostUsing.Parent is not BlockSyntax parentBlock)
87-
{
88-
// Don't offer on a using statement that is parented by another using statement. We'll just offer on the
89-
// topmost using statement.
87+
var parentBlockLike = outermostUsing.Parent;
88+
if (parentBlockLike is GlobalStatementSyntax)
89+
parentBlockLike = parentBlockLike.Parent;
90+
91+
// Don't offer on a using statement that is parented by another using statement. We'll just offer on the
92+
// topmost using statement.
93+
if (parentBlockLike is not BlockSyntax and not CompilationUnitSyntax)
9094
return;
91-
}
9295

9396
var innermostUsing = outermostUsing;
9497

@@ -102,15 +105,15 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
102105
}
103106

104107
// Verify that changing this using-statement into a using-declaration will not change semantics.
105-
if (!PreservesSemantics(semanticModel, parentBlock, outermostUsing, innermostUsing, cancellationToken))
108+
if (!PreservesSemantics(semanticModel, parentBlockLike, outermostUsing, innermostUsing, cancellationToken))
106109
return;
107110

108111
// Converting a using-statement to a using-variable-declaration will cause the using's variables to now be
109112
// pushed up to the parent block's scope. This is also true for any local variables in the innermost using's
110113
// block. These may then collide with other variables in the block, causing an error. Check for that and
111114
// bail if this happens.
112115
if (CausesVariableCollision(
113-
context.SemanticModel, parentBlock,
116+
context.SemanticModel, parentBlockLike,
114117
outermostUsing, innermostUsing, cancellationToken))
115118
{
116119
return;
@@ -122,52 +125,69 @@ private void AnalyzeSyntax(SyntaxNodeAnalysisContext context)
122125
outermostUsing.UsingKeyword.GetLocation(),
123126
option.Notification,
124127
context.Options,
125-
additionalLocations: ImmutableArray.Create(outermostUsing.GetLocation()),
128+
additionalLocations: [outermostUsing.GetLocation()],
126129
properties: null));
127130
}
128131

129132
private static bool CausesVariableCollision(
130-
SemanticModel semanticModel, BlockSyntax parentBlock,
131-
UsingStatementSyntax outermostUsing, UsingStatementSyntax innermostUsing,
133+
SemanticModel semanticModel,
134+
SyntaxNode parentBlockLike,
135+
UsingStatementSyntax outermostUsing,
136+
UsingStatementSyntax innermostUsing,
132137
CancellationToken cancellationToken)
133138
{
134-
var symbolNameToExistingSymbol = semanticModel.GetExistingSymbols(parentBlock, cancellationToken).ToLookup(s => s.Name);
139+
using var _ = PooledDictionary<string, ArrayBuilder<ISymbol>>.GetInstance(out var symbolNameToExistingSymbol);
135140

136-
for (var current = outermostUsing; current != null; current = current.Statement as UsingStatementSyntax)
141+
try
137142
{
138-
// Check if the using statement itself contains variables that will collide with other variables in the
139-
// block.
140-
var usingOperation = (IUsingOperation)semanticModel.GetRequiredOperation(current, cancellationToken);
141-
if (DeclaredLocalCausesCollision(symbolNameToExistingSymbol, usingOperation.Locals))
142-
return true;
143+
foreach (var statement in CSharpBlockFacts.Instance.GetExecutableBlockStatements(parentBlockLike))
144+
{
145+
foreach (var symbol in semanticModel.GetExistingSymbols(statement, cancellationToken))
146+
symbolNameToExistingSymbol.MultiAdd(symbol.Name, symbol);
147+
}
148+
149+
for (var current = outermostUsing; current != null; current = current.Statement as UsingStatementSyntax)
150+
{
151+
// Check if the using statement itself contains variables that will collide with other variables in the
152+
// block.
153+
var usingOperation = (IUsingOperation)semanticModel.GetRequiredOperation(current, cancellationToken);
154+
if (DeclaredLocalCausesCollision(symbolNameToExistingSymbol, usingOperation.Locals))
155+
return true;
156+
}
157+
158+
var innerUsingOperation = (IUsingOperation)semanticModel.GetRequiredOperation(innermostUsing, cancellationToken);
159+
if (innerUsingOperation.Body is IBlockOperation innerUsingBlock)
160+
return DeclaredLocalCausesCollision(symbolNameToExistingSymbol, innerUsingBlock.Locals);
161+
162+
return false;
163+
}
164+
finally
165+
{
166+
symbolNameToExistingSymbol.FreeValues();
143167
}
144-
145-
var innerUsingOperation = (IUsingOperation)semanticModel.GetRequiredOperation(innermostUsing, cancellationToken);
146-
if (innerUsingOperation.Body is IBlockOperation innerUsingBlock)
147-
return DeclaredLocalCausesCollision(symbolNameToExistingSymbol, innerUsingBlock.Locals);
148-
149-
return false;
150168
}
151169

152-
private static bool DeclaredLocalCausesCollision(ILookup<string, ISymbol> symbolNameToExistingSymbol, ImmutableArray<ILocalSymbol> locals)
153-
=> locals.Any(static (local, symbolNameToExistingSymbol) => symbolNameToExistingSymbol[local.Name].Any(otherLocal => !local.Equals(otherLocal)), symbolNameToExistingSymbol);
170+
private static bool DeclaredLocalCausesCollision(Dictionary<string, ArrayBuilder<ISymbol>> symbolNameToExistingSymbol, ImmutableArray<ILocalSymbol> locals)
171+
=> locals.Any(static (local, symbolNameToExistingSymbol) =>
172+
symbolNameToExistingSymbol.TryGetValue(local.Name, out var symbols) &&
173+
symbols.Any(otherLocal => !local.Equals(otherLocal)), symbolNameToExistingSymbol);
154174

155175
private static bool PreservesSemantics(
156176
SemanticModel semanticModel,
157-
BlockSyntax parentBlock,
177+
SyntaxNode parentBlockLike,
158178
UsingStatementSyntax outermostUsing,
159179
UsingStatementSyntax innermostUsing,
160180
CancellationToken cancellationToken)
161181
{
162-
var statements = parentBlock.Statements;
182+
var statements = (IReadOnlyList<StatementSyntax>)CSharpBlockFacts.Instance.GetExecutableBlockStatements(parentBlockLike);
163183
var index = statements.IndexOf(outermostUsing);
164184

165185
return UsingValueDoesNotLeakToFollowingStatements(semanticModel, statements, index, cancellationToken) &&
166186
UsingStatementDoesNotInvolveJumps(statements, index, innermostUsing);
167187
}
168188

169189
private static bool UsingStatementDoesNotInvolveJumps(
170-
SyntaxList<StatementSyntax> parentStatements, int index, UsingStatementSyntax innermostUsing)
190+
IReadOnlyList<StatementSyntax> parentStatements, int index, UsingStatementSyntax innermostUsing)
171191
{
172192
// Jumps are not allowed to cross a using declaration in the forward direction, and can't go back unless
173193
// there is a curly brace between the using and the label.
@@ -204,7 +224,7 @@ private static bool IsGotoOrLabeledStatement(StatementSyntax priorStatement)
204224

205225
private static bool UsingValueDoesNotLeakToFollowingStatements(
206226
SemanticModel semanticModel,
207-
SyntaxList<StatementSyntax> statements,
227+
IReadOnlyList<StatementSyntax> statements,
208228
int index,
209229
CancellationToken cancellationToken)
210230
{

src/Analyzers/CSharp/CodeFixes/UseSimpleUsingStatement/UseSimpleUsingStatementCodeFixProvider.cs

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
#nullable disable
6-
75
using System;
86
using System.Collections.Generic;
97
using System.Collections.Immutable;
108
using System.Composition;
119
using System.Linq;
10+
using System.Linq.Expressions;
1211
using System.Threading;
1312
using System.Threading.Tasks;
1413
using Microsoft.CodeAnalysis.CodeFixes;
@@ -21,6 +20,7 @@
2120
using Microsoft.CodeAnalysis.Host.Mef;
2221
using Microsoft.CodeAnalysis.PooledObjects;
2322
using Microsoft.CodeAnalysis.Shared.Extensions;
23+
using Microsoft.CodeAnalysis.Utilities;
2424
using Roslyn.Utilities;
2525

2626
namespace Microsoft.CodeAnalysis.CSharp.UseSimpleUsingStatement;
@@ -46,41 +46,73 @@ protected override Task FixAllAsync(
4646
Document document, ImmutableArray<Diagnostic> diagnostics,
4747
SyntaxEditor editor, CancellationToken cancellationToken)
4848
{
49-
var topmostUsingStatements = diagnostics.Select(d => (UsingStatementSyntax)d.AdditionalLocations[0].FindNode(cancellationToken)).ToSet();
50-
var blocks = topmostUsingStatements.Select(u => (BlockSyntax)u.Parent);
49+
var topmostUsingStatements = diagnostics.Select(
50+
d => (UsingStatementSyntax)d.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken)).ToSet();
51+
var blockLikes = topmostUsingStatements.Select(u => u.Parent is GlobalStatementSyntax ? u.Parent.GetRequiredParent() : u.GetRequiredParent()).ToSet();
5152

5253
// Process blocks in reverse order so we rewrite from inside-to-outside with nested
5354
// usings.
5455
var root = editor.OriginalRoot;
5556
var updatedRoot = root.ReplaceNodes(
56-
blocks.OrderByDescending(b => b.SpanStart),
57+
blockLikes.OrderByDescending(b => b.SpanStart),
5758
(original, current) => RewriteBlock(original, current, topmostUsingStatements));
5859

5960
editor.ReplaceNode(root, updatedRoot);
6061

6162
return Task.CompletedTask;
6263
}
6364

64-
private static BlockSyntax RewriteBlock(
65-
BlockSyntax originalBlock, BlockSyntax currentBlock,
65+
private static SyntaxNode RewriteBlock(
66+
SyntaxNode originalBlockLike,
67+
SyntaxNode currentBlockLike,
6668
ISet<UsingStatementSyntax> topmostUsingStatements)
6769
{
68-
if (originalBlock.Statements.Count == currentBlock.Statements.Count)
70+
var originalBlockStatements = (IReadOnlyList<StatementSyntax>)CSharpBlockFacts.Instance.GetExecutableBlockStatements(originalBlockLike);
71+
var currentBlockStatements = (IReadOnlyList<StatementSyntax>)CSharpBlockFacts.Instance.GetExecutableBlockStatements(currentBlockLike);
72+
73+
if (originalBlockStatements.Count == currentBlockStatements.Count)
6974
{
70-
var statementToUpdateIndex = originalBlock.Statements.IndexOf(s => topmostUsingStatements.Contains(s));
71-
var statementToUpdate = currentBlock.Statements[statementToUpdateIndex];
75+
var statementToUpdateIndex = IndexOf(originalBlockStatements, s => topmostUsingStatements.Contains(s));
76+
var statementToUpdate = currentBlockStatements[statementToUpdateIndex];
7277

7378
if (statementToUpdate is UsingStatementSyntax usingStatement &&
7479
usingStatement.Declaration != null)
7580
{
76-
var updatedStatements = currentBlock.Statements.ReplaceRange(
77-
statementToUpdate,
78-
Expand(usingStatement));
79-
return currentBlock.WithStatements(updatedStatements);
81+
var expandedUsing = Expand(usingStatement);
82+
83+
return WithStatements(currentBlockLike, usingStatement, expandedUsing);
8084
}
8185
}
8286

83-
return currentBlock;
87+
return currentBlockLike;
88+
}
89+
90+
public static int IndexOf<T>(IReadOnlyList<T> list, Func<T, bool> predicate)
91+
{
92+
for (var i = 0; i < list.Count; i++)
93+
{
94+
if (predicate(list[i]))
95+
return i;
96+
}
97+
98+
return -1;
99+
}
100+
101+
private static SyntaxNode WithStatements(
102+
SyntaxNode currentBlockLike,
103+
UsingStatementSyntax usingStatement,
104+
ImmutableArray<StatementSyntax> expandedUsingStatements)
105+
{
106+
return currentBlockLike switch
107+
{
108+
BlockSyntax currentBlock => currentBlock.WithStatements(
109+
currentBlock.Statements.ReplaceRange(usingStatement, expandedUsingStatements)),
110+
111+
CompilationUnitSyntax compilationUnit => compilationUnit.WithMembers(
112+
compilationUnit.Members.ReplaceRange((GlobalStatementSyntax)usingStatement.GetRequiredParent(), expandedUsingStatements.Select(GlobalStatement))),
113+
114+
_ => throw ExceptionUtilities.UnexpectedValue(currentBlockLike),
115+
};
84116
}
85117

86118
private static ImmutableArray<StatementSyntax> Expand(UsingStatementSyntax usingStatement)
@@ -163,15 +195,13 @@ private static SyntaxTriviaList Expand(ArrayBuilder<StatementSyntax> result, Usi
163195
}
164196

165197
return default;
166-
}
167198

168-
private static LocalDeclarationStatementSyntax Convert(UsingStatementSyntax usingStatement)
169-
{
170-
return LocalDeclarationStatement(
171-
usingStatement.AwaitKeyword,
172-
usingStatement.UsingKeyword.WithAppendedTrailingTrivia(ElasticMarker),
173-
modifiers: default,
174-
usingStatement.Declaration,
175-
SemicolonToken).WithTrailingTrivia(usingStatement.CloseParenToken.TrailingTrivia);
199+
static LocalDeclarationStatementSyntax Convert(UsingStatementSyntax usingStatement)
200+
=> LocalDeclarationStatement(
201+
usingStatement.AwaitKeyword,
202+
usingStatement.UsingKeyword.WithAppendedTrailingTrivia(ElasticMarker),
203+
modifiers: default,
204+
usingStatement.Declaration!,
205+
SemicolonToken).WithTrailingTrivia(usingStatement.CloseParenToken.TrailingTrivia);
176206
}
177207
}

0 commit comments

Comments
 (0)