Skip to content

Commit 4402e33

Browse files
sharwellsharwell
authored
Merge pull request #36272 from sharwell/track-ref-returns
Track ref returns for Make Field Readonly
2 parents de59c37 + f5a49d3 commit 4402e33

File tree

8 files changed

+509
-31
lines changed

8 files changed

+509
-31
lines changed

‎src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs‎

Lines changed: 442 additions & 0 deletions
Large diffs are not rendered by default.

‎src/Analyzers/Core/Analyzers/MakeFieldReadonly/MakeFieldReadonlyDiagnosticAnalyzer.cs‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ void UpdateFieldStateOnWrite(IFieldSymbol field)
156156
private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol)
157157
{
158158
// Check if the underlying member is being written or a writable reference to the member is taken.
159-
var valueUsageInfo = fieldReference.GetValueUsageInfo();
159+
var valueUsageInfo = fieldReference.GetValueUsageInfo(owningSymbol);
160160
if (!valueUsageInfo.IsWrittenTo())
161161
{
162162
return false;
@@ -184,7 +184,7 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo
184184
if (instanceFieldWrittenInCtor || staticFieldWrittenInStaticCtor)
185185
{
186186
// Finally, ensure that the write is not inside a lambda or local function.
187-
if (!IsInAnonymousFunctionOrLocalFunction(fieldReference))
187+
if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is null)
188188
{
189189
// It is safe to ignore this write.
190190
return false;
@@ -195,24 +195,6 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo
195195
return true;
196196
}
197197

198-
private static bool IsInAnonymousFunctionOrLocalFunction(IOperation operation)
199-
{
200-
operation = operation.Parent;
201-
while (operation != null)
202-
{
203-
switch (operation.Kind)
204-
{
205-
case OperationKind.AnonymousFunction:
206-
case OperationKind.LocalFunction:
207-
return true;
208-
}
209-
210-
operation = operation.Parent;
211-
}
212-
213-
return false;
214-
}
215-
216198
private static CodeStyleOption2<bool> GetCodeStyleOption(IFieldSymbol field, AnalyzerOptions options, CancellationToken cancellationToken)
217199
=> options.GetOption(CodeStyleOptions2.PreferReadonly, field.Language, field.Locations[0].SourceTree, cancellationToken);
218200
}

‎src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ private void AnalyzeMemberReferenceOperation(OperationAnalysisContext operationC
268268
if (IsCandidateSymbol(memberSymbol))
269269
{
270270
// Get the value usage info.
271-
var valueUsageInfo = memberReference.GetValueUsageInfo();
271+
var valueUsageInfo = memberReference.GetValueUsageInfo(operationContext.ContainingSymbol);
272272

273273
if (valueUsageInfo == ValueUsageInfo.ReadWrite)
274274
{

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public static bool IsTargetOfObjectMemberInitializer(this IOperation operation)
2222
/// Returns the <see cref="ValueUsageInfo"/> for the given operation.
2323
/// This extension can be removed once https://github.com/dotnet/roslyn/issues/25057 is implemented.
2424
/// </summary>
25-
public static ValueUsageInfo GetValueUsageInfo(this IOperation operation)
25+
public static ValueUsageInfo GetValueUsageInfo(this IOperation operation, ISymbol containingSymbol)
2626
{
2727
/*
2828
| code | Read | Write | ReadableRef | WritableRef | NonReadWriteRef |
@@ -126,7 +126,7 @@ where void Foo(in T v)
126126
// Note: IParenthesizedOperation is specific to VB, where the parens cause a copy, so this cannot be classified as a write.
127127
Debug.Assert(parenthesizedOperation.Language == LanguageNames.VisualBasic);
128128

129-
return parenthesizedOperation.GetValueUsageInfo() &
129+
return parenthesizedOperation.GetValueUsageInfo(containingSymbol) &
130130
~(ValueUsageInfo.Write | ValueUsageInfo.Reference);
131131
}
132132
else if (operation.Parent is INameOfOperation ||
@@ -152,6 +152,29 @@ operation.Parent is ITypeOfOperation ||
152152
return ValueUsageInfo.Read;
153153
}
154154
}
155+
else if (operation.Parent is IReturnOperation)
156+
{
157+
var containingMethod = TryGetContainingAnonymousFunctionOrLocalFunction(operation)
158+
?? (containingSymbol as IMethodSymbol);
159+
return (containingMethod?.RefKind) switch
160+
{
161+
RefKind.RefReadOnly => ValueUsageInfo.ReadableReference,
162+
RefKind.Ref => ValueUsageInfo.ReadableWritableReference,
163+
_ => ValueUsageInfo.Read,
164+
};
165+
}
166+
else if (operation.Parent is IConditionalOperation conditionalOperation)
167+
{
168+
if (operation == conditionalOperation.WhenTrue
169+
|| operation == conditionalOperation.WhenFalse)
170+
{
171+
return GetValueUsageInfo(conditionalOperation, containingSymbol);
172+
}
173+
else
174+
{
175+
return ValueUsageInfo.Read;
176+
}
177+
}
155178
else if (operation.Parent is IReDimClauseOperation reDimClauseOperation &&
156179
reDimClauseOperation.Operand == operation)
157180
{
@@ -161,7 +184,7 @@ operation.Parent is ITypeOfOperation ||
161184
}
162185
else if (operation.Parent is IDeclarationExpressionOperation declarationExpression)
163186
{
164-
return declarationExpression.GetValueUsageInfo();
187+
return declarationExpression.GetValueUsageInfo(containingSymbol);
165188
}
166189
else if (operation.IsInLeftOfDeconstructionAssignment(out _))
167190
{
@@ -185,6 +208,26 @@ operation.Parent is ITypeOfOperation ||
185208
return ValueUsageInfo.Read;
186209
}
187210

211+
public static IMethodSymbol TryGetContainingAnonymousFunctionOrLocalFunction(this IOperation operation)
212+
{
213+
operation = operation.Parent;
214+
while (operation != null)
215+
{
216+
switch (operation.Kind)
217+
{
218+
case OperationKind.AnonymousFunction:
219+
return ((IAnonymousFunctionOperation)operation).Symbol;
220+
221+
case OperationKind.LocalFunction:
222+
return ((ILocalFunctionOperation)operation).Symbol;
223+
}
224+
225+
operation = operation.Parent;
226+
}
227+
228+
return null;
229+
}
230+
188231
public static bool IsInLeftOfDeconstructionAssignment(this IOperation operation, out IDeconstructionAssignmentOperation deconstructionAssignment)
189232
{
190233
deconstructionAssignment = null;

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/FlowAnalysis/SymbolUsageAnalysis/SymbolUsageAnalysis.DataFlowAnalyzer.FlowGraphAnalysisData.cs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ private sealed class FlowGraphAnalysisData : AnalysisData
6767

6868
private FlowGraphAnalysisData(
6969
ControlFlowGraph controlFlowGraph,
70+
ISymbol owningSymbol,
7071
ImmutableArray<IParameterSymbol> parameters,
7172
PooledDictionary<BasicBlock, BasicBlockAnalysisData> analysisDataByBasicBlockMap,
7273
PooledDictionary<(ISymbol symbol, IOperation operation), bool> symbolsWriteMap,
@@ -78,6 +79,7 @@ private FlowGraphAnalysisData(
7879
PooledDictionary<IFlowAnonymousFunctionOperation, ControlFlowGraph> lambdaTargetsToAccessingCfgMap)
7980
{
8081
ControlFlowGraph = controlFlowGraph;
82+
OwningSymbol = owningSymbol;
8183
_parameters = parameters;
8284
_analysisDataByBasicBlockMap = analysisDataByBasicBlockMap;
8385
_analyzeLocalFunctionOrLambdaInvocation = analyzeLocalFunctionOrLambdaInvocation;
@@ -96,6 +98,8 @@ private FlowGraphAnalysisData(
9698
_symbolWritesInsideBlockRangeMap = PooledDictionary<(int firstBlockOrdinal, int lastBlockOrdinal), PooledHashSet<(ISymbol, IOperation)>>.GetInstance();
9799
}
98100

101+
public ISymbol OwningSymbol { get; }
102+
99103
protected override PooledHashSet<ISymbol> SymbolsReadBuilder { get; }
100104

101105
protected override PooledDictionary<(ISymbol symbol, IOperation operation), bool> SymbolsWriteBuilder { get; }
@@ -112,6 +116,7 @@ public static FlowGraphAnalysisData Create(
112116
var parameters = owningSymbol.GetParameters();
113117
return new FlowGraphAnalysisData(
114118
cfg,
119+
owningSymbol,
115120
parameters,
116121
analysisDataByBasicBlockMap: CreateAnalysisDataByBasicBlockMap(cfg),
117122
symbolsWriteMap: CreateSymbolsWriteMap(parameters),
@@ -135,6 +140,7 @@ public static FlowGraphAnalysisData Create(
135140
var parameters = lambdaOrLocalFunction.GetParameters();
136141
return new FlowGraphAnalysisData(
137142
cfg,
143+
lambdaOrLocalFunction,
138144
parameters,
139145
analysisDataByBasicBlockMap: CreateAnalysisDataByBasicBlockMap(cfg),
140146
symbolsWriteMap: UpdateSymbolsWriteMap(parentAnalysisData.SymbolsWriteBuilder, parameters),

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/FlowAnalysis/SymbolUsageAnalysis/SymbolUsageAnalysis.DataFlowAnalyzer.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ private static BasicBlockAnalysisData AnalyzeLocalFunctionOrLambdaInvocation(
8686
public override BasicBlockAnalysisData AnalyzeBlock(BasicBlock basicBlock, CancellationToken cancellationToken)
8787
{
8888
BeforeBlockAnalysis();
89-
Walker.AnalyzeOperationsAndUpdateData(basicBlock.Operations, _analysisData, cancellationToken);
89+
Walker.AnalyzeOperationsAndUpdateData(_analysisData.OwningSymbol, basicBlock.Operations, _analysisData, cancellationToken);
9090
AfterBlockAnalysis();
9191
return _analysisData.CurrentBlockAnalysisData;
9292

@@ -151,7 +151,7 @@ private BasicBlockAnalysisData AnalyzeBranch(
151151

152152
// Analyze the branch value
153153
var operations = SpecializedCollections.SingletonEnumerable(basicBlock.BranchValue);
154-
Walker.AnalyzeOperationsAndUpdateData(operations, _analysisData, cancellationToken);
154+
Walker.AnalyzeOperationsAndUpdateData(_analysisData.OwningSymbol, operations, _analysisData, cancellationToken);
155155
ProcessOutOfScopeLocals();
156156
return _analysisData.CurrentBlockAnalysisData;
157157

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/FlowAnalysis/SymbolUsageAnalysis/SymbolUsageAnalysis.Walker.cs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ internal static partial class SymbolUsageAnalysis
2222
private sealed class Walker : OperationWalker
2323
{
2424
private AnalysisData _currentAnalysisData;
25+
private ISymbol _currentContainingSymbol;
2526
private IOperation _currentRootOperation;
2627
private CancellationToken _cancellationToken;
2728
private PooledDictionary<IAssignmentOperation, PooledHashSet<(ISymbol, IOperation)>> _pendingWritesMap;
@@ -30,30 +31,33 @@ private sealed class Walker : OperationWalker
3031
private Walker() { }
3132

3233
public static void AnalyzeOperationsAndUpdateData(
34+
ISymbol containingSymbol,
3335
IEnumerable<IOperation> operations,
3436
AnalysisData analysisData,
3537
CancellationToken cancellationToken)
3638
{
3739
var visitor = s_visitorPool.Allocate();
3840
try
3941
{
40-
visitor.Visit(operations, analysisData, cancellationToken);
42+
visitor.Visit(containingSymbol, operations, analysisData, cancellationToken);
4143
}
4244
finally
4345
{
4446
s_visitorPool.Free(visitor);
4547
}
4648
}
4749

48-
private void Visit(IEnumerable<IOperation> operations, AnalysisData analysisData, CancellationToken cancellationToken)
50+
private void Visit(ISymbol containingSymbol, IEnumerable<IOperation> operations, AnalysisData analysisData, CancellationToken cancellationToken)
4951
{
52+
Debug.Assert(_currentContainingSymbol == null);
5053
Debug.Assert(_currentAnalysisData == null);
5154
Debug.Assert(_currentRootOperation == null);
5255
Debug.Assert(_pendingWritesMap == null);
5356

5457
_pendingWritesMap = PooledDictionary<IAssignmentOperation, PooledHashSet<(ISymbol, IOperation)>>.GetInstance();
5558
try
5659
{
60+
_currentContainingSymbol = containingSymbol;
5761
_currentAnalysisData = analysisData;
5862
_cancellationToken = cancellationToken;
5963

@@ -67,6 +71,7 @@ private void Visit(IEnumerable<IOperation> operations, AnalysisData analysisData
6771
}
6872
finally
6973
{
74+
_currentContainingSymbol = null;
7075
_currentAnalysisData = null;
7176
_currentRootOperation = null;
7277
_cancellationToken = default;
@@ -101,7 +106,7 @@ private void OnReferenceFound(ISymbol symbol, IOperation operation)
101106
{
102107
Debug.Assert(symbol != null);
103108

104-
var valueUsageInfo = operation.GetValueUsageInfo();
109+
var valueUsageInfo = operation.GetValueUsageInfo(_currentContainingSymbol);
105110
var isReadFrom = valueUsageInfo.IsReadFrom();
106111
var isWrittenTo = valueUsageInfo.IsWrittenTo();
107112

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/FlowAnalysis/SymbolUsageAnalysis/SymbolUsageAnalysis.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public static SymbolUsageResult Run(IOperation rootOperation, ISymbol owningSymb
3333
using (analysisData = OperationTreeAnalysisData.Create(owningSymbol, AnalyzeLocalFunction))
3434
{
3535
var operations = SpecializedCollections.SingletonEnumerable(rootOperation);
36-
Walker.AnalyzeOperationsAndUpdateData(operations, analysisData, cancellationToken);
36+
Walker.AnalyzeOperationsAndUpdateData(owningSymbol, operations, analysisData, cancellationToken);
3737
return analysisData.ToResult();
3838
}
3939

@@ -47,7 +47,7 @@ BasicBlockAnalysisData AnalyzeLocalFunction(IMethodSymbol localFunction)
4747
if (localFunctionOperation != null)
4848
{
4949
var operations = SpecializedCollections.SingletonEnumerable(localFunctionOperation);
50-
Walker.AnalyzeOperationsAndUpdateData(operations, analysisData, cancellationToken);
50+
Walker.AnalyzeOperationsAndUpdateData(localFunction, operations, analysisData, cancellationToken);
5151
}
5252

5353
return analysisData.CurrentBlockAnalysisData;

0 commit comments

Comments
 (0)