Skip to content

Commit e02c25d

Browse files
pavel-mikula-sonarsourcesonartech
pavel-mikula-sonarsource
authored and
sonartech
committed
NET-865 S1172 FP: Parameter used as extension delegate target
1 parent 26ddcbf commit e02c25d

File tree

14 files changed

+333
-42
lines changed

14 files changed

+333
-42
lines changed

‎analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
using SonarAnalyzer.CFG.Roslyn;
18+
using SonarAnalyzer.CFG.Syntax.Utilities;
1819

1920
namespace SonarAnalyzer.CFG.LiveVariableAnalysis;
2021

@@ -23,14 +24,16 @@ public sealed class RoslynLiveVariableAnalysis : LiveVariableAnalysisBase<Contro
2324
private readonly Dictionary<CaptureId, List<ISymbol>> flowCaptures = [];
2425
private readonly Dictionary<int, List<BasicBlock>> blockPredecessors = [];
2526
private readonly Dictionary<int, List<BasicBlock>> blockSuccessors = [];
27+
private readonly SyntaxClassifierBase syntaxClassifier;
2628

2729
internal ImmutableDictionary<int, List<BasicBlock>> BlockPredecessors => blockPredecessors.ToImmutableDictionary();
2830

2931
protected override BasicBlock ExitBlock => Cfg.ExitBlock;
3032

31-
public RoslynLiveVariableAnalysis(ControlFlowGraph cfg, CancellationToken cancel)
33+
public RoslynLiveVariableAnalysis(ControlFlowGraph cfg, SyntaxClassifierBase syntaxClassifier, CancellationToken cancel)
3234
: base(cfg, OriginalDeclaration(cfg.OriginalOperation), cancel)
3335
{
36+
this.syntaxClassifier = syntaxClassifier;
3437
foreach (var ordinal in cfg.Blocks.Select(x => x.Ordinal))
3538
{
3639
blockPredecessors.Add(ordinal, []);
@@ -53,7 +56,7 @@ _ when operation.AsLocalReference() is { } localReference => [localReference.Loc
5356
_ when operation.AsFlowCaptureReference() is { } flowCaptureReference => flowCaptures.TryGetValue(flowCaptureReference.Id, out var symbols) ? symbols : [],
5457
_ => []
5558
};
56-
return candidates.Where(x => IsLocal(x));
59+
return candidates.Where(IsLocal);
5760
}
5861

5962
public override bool IsLocal(ISymbol symbol) =>
@@ -290,19 +293,33 @@ private void ProcessOperation(ControlFlowGraph cfg, IOperation operation)
290293
break;
291294
case OperationKindEx.MethodReference:
292295
ProcessLocalFunction(cfg, IMethodReferenceOperationWrapper.FromOperation(operation).Method);
296+
// For .Select(variable.MethodReference), there's no LocalReferenceOperation in the CFG for variable, so we handle it from the syntax
297+
if (owner.syntaxClassifier.MemberAccessExpression(operation.Syntax) is { } expression)
298+
{
299+
var symbol = owner.Cfg.OriginalOperation.ToSonar().SemanticModel.GetSymbolInfo(expression).Symbol;
300+
if (symbol is ILocalSymbol or IParameterSymbol && owner.IsLocal(symbol))
301+
{
302+
ProcessParameterOrLocalSymbols([symbol], symbol is IParameterSymbol { RefKind: RefKind.Out }, false);
303+
}
304+
}
293305
break;
294306
}
295307
}
296308

297-
private void ProcessParameterOrLocalReference(IOperationWrapper reference)
309+
private void ProcessParameterOrLocalReference(IOperationWrapper reference) =>
310+
ProcessParameterOrLocalSymbols(
311+
owner.ParameterOrLocalSymbols(reference.WrappedOperation),
312+
reference.IsOutArgument(),
313+
reference.IsAssignmentTarget() || reference.ToSonar().Parent?.Kind == OperationKindEx.FlowCapture);
314+
315+
private void ProcessParameterOrLocalSymbols(IEnumerable<ISymbol> symbols, bool isOutArgument, bool isAssignmentTarget)
298316
{
299-
var symbols = owner.ParameterOrLocalSymbols(reference.WrappedOperation);
300-
if (reference.IsOutArgument())
317+
if (isOutArgument)
301318
{
302319
Assigned.UnionWith(symbols);
303320
UsedBeforeAssigned.ExceptWith(symbols);
304321
}
305-
else if (!reference.IsAssignmentTarget() && reference.ToSonar().Parent?.Kind != OperationKindEx.FlowCapture)
322+
else if (!isAssignmentTarget)
306323
{
307324
UsedBeforeAssigned.UnionWith(symbols);
308325
}

‎analyzers/src/SonarAnalyzer.CFG/Syntax/Utilities/SyntaxClassifierBase.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright (C) 2015-2024 SonarSource SA
3+
* All rights reserved
4+
* mailto:info AT sonarsource DOT com
5+
*/
6+
7+
namespace SonarAnalyzer.CFG.Syntax.Utilities;
8+
9+
/// <summary>
10+
/// This class violates the basic principle that SE should only depend on IOperation.
11+
///
12+
/// Anything added here needs to have extremely rare reason why it exists.
13+
/// </summary>
14+
public abstract class SyntaxClassifierBase
15+
{
16+
public abstract SyntaxNode MemberAccessExpression(SyntaxNode node);
17+
protected abstract bool IsCfgBoundary(SyntaxNode node);
18+
protected abstract bool IsStatement(SyntaxNode node);
19+
protected abstract SyntaxNode ParentLoopCondition(SyntaxNode node);
20+
21+
// Detecting loops from CFG shape is not possible from the shape of CFG, because of nested loops.
22+
public bool IsInLoopCondition(SyntaxNode node)
23+
{
24+
while (node is not null)
25+
{
26+
if (ParentLoopCondition(node) == node)
27+
{
28+
return true;
29+
}
30+
if (IsStatement(node) || IsCfgBoundary(node))
31+
{
32+
return false;
33+
}
34+
else
35+
{
36+
node = node.Parent;
37+
}
38+
}
39+
return false;
40+
}
41+
}

‎analyzers/src/SonarAnalyzer.CSharp.Core/Syntax/Utilities/CSharpSyntaxClassifier.cs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (C) 2015-2024 SonarSource SA
3+
* All rights reserved
4+
* mailto:info AT sonarsource DOT com
5+
*/
6+
7+
using SonarAnalyzer.CFG.Syntax.Utilities;
8+
9+
namespace SonarAnalyzer.CSharp.Core.Syntax.Utilities;
10+
11+
public sealed class CSharpSyntaxClassifier : SyntaxClassifierBase
12+
{
13+
private static CSharpSyntaxClassifier instance;
14+
15+
public static CSharpSyntaxClassifier Instance => instance ??= new();
16+
17+
private CSharpSyntaxClassifier() { }
18+
19+
public override SyntaxNode MemberAccessExpression(SyntaxNode node) =>
20+
(node as MemberAccessExpressionSyntax)?.Expression;
21+
22+
protected override bool IsStatement(SyntaxNode node) =>
23+
node is StatementSyntax;
24+
25+
protected override SyntaxNode ParentLoopCondition(SyntaxNode node) =>
26+
node.Parent switch
27+
{
28+
DoStatementSyntax doStatement => doStatement.Condition,
29+
ForStatementSyntax forStatement => forStatement.Condition,
30+
ForEachStatementSyntax forEachStatement => forEachStatement.Expression,
31+
WhileStatementSyntax whileStatement => whileStatement.Condition,
32+
_ when node.Parent.IsKind(SyntaxKindEx.ForEachVariableStatement) => ((ForEachVariableStatementSyntaxWrapper)node.Parent).Expression,
33+
_ => null
34+
};
35+
36+
protected override bool IsCfgBoundary(SyntaxNode node) =>
37+
node is LambdaExpressionSyntax || node.IsKind(SyntaxKindEx.LocalFunctionStatement);
38+
}

‎analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
using SonarAnalyzer.CFG.LiveVariableAnalysis;
1818
using SonarAnalyzer.CFG.Sonar;
1919
using SonarAnalyzer.CSharp.Core.LiveVariableAnalysis;
20-
using SonarAnalyzer.CSharp.Walkers;
2120

2221
namespace SonarAnalyzer.Rules.CSharp
2322
{
@@ -80,7 +79,7 @@ private void CheckForDeadStores(SonarSyntaxNodeReportingContext context, ISymbol
8079
}
8180
else if (context.Node.CreateCfg(context.SemanticModel, context.Cancel) is { } cfg)
8281
{
83-
var lva = new RoslynLiveVariableAnalysis(cfg, context.Cancel);
82+
var lva = new RoslynLiveVariableAnalysis(cfg, CSharpSyntaxClassifier.Instance, context.Cancel);
8483
var checker = new RoslynChecker(context, lva);
8584
checker.Analyze(cfg.Blocks);
8685
}

‎analyzers/src/SonarAnalyzer.CSharp/Rules/MethodParameterUnused.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ public LvaResult(MethodContext declaration, IControlFlowGraph cfg)
286286

287287
public LvaResult(ControlFlowGraph cfg, CancellationToken cancel)
288288
{
289-
var lva = new RoslynLiveVariableAnalysis(cfg, cancel);
289+
var lva = new RoslynLiveVariableAnalysis(cfg, CSharpSyntaxClassifier.Instance, cancel);
290290
LiveInEntryBlock = lva.LiveIn(cfg.EntryBlock).OfType<IParameterSymbol>().ToImmutableArray();
291291
CapturedVariables = lva.CapturedVariables;
292292
}

‎analyzers/src/SonarAnalyzer.VisualBasic.Core/Syntax/Utilities/VisualBasicSyntaxClassifier.cs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright (C) 2015-2024 SonarSource SA
3+
* All rights reserved
4+
* mailto:info AT sonarsource DOT com
5+
*/
6+
7+
using SonarAnalyzer.CFG.Syntax.Utilities;
8+
9+
namespace SonarAnalyzer.VisualBasic.Core.Syntax.Utilities;
10+
11+
public sealed class VisualBasicSyntaxClassifier : SyntaxClassifierBase
12+
{
13+
private static VisualBasicSyntaxClassifier instance;
14+
15+
public static VisualBasicSyntaxClassifier Instance => instance ??= new();
16+
17+
private VisualBasicSyntaxClassifier() { }
18+
19+
public override SyntaxNode MemberAccessExpression(SyntaxNode node) =>
20+
(node as MemberAccessExpressionSyntax)?.Expression;
21+
22+
protected override bool IsStatement(SyntaxNode node) =>
23+
node is StatementSyntax;
24+
25+
protected override SyntaxNode ParentLoopCondition(SyntaxNode node) =>
26+
node.Parent switch
27+
{
28+
DoStatementSyntax doStatement => doStatement.WhileOrUntilClause,
29+
LoopStatementSyntax loopStatement => loopStatement.WhileOrUntilClause,
30+
ForBlockSyntax forBlock => forBlock.ForStatement,
31+
ForEachStatementSyntax foreachStatement => foreachStatement.Expression,
32+
WhileStatementSyntax whileStatement => whileStatement.Condition,
33+
_ => null
34+
};
35+
36+
protected override bool IsCfgBoundary(SyntaxNode node) =>
37+
node is LambdaExpressionSyntax;
38+
}

‎analyzers/tests/SonarAnalyzer.Test/CFG/Roslyn/RoslynLvaSerializerTest.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
using SonarAnalyzer.CFG;
1818
using SonarAnalyzer.CFG.LiveVariableAnalysis;
19+
using SonarAnalyzer.CSharp.Core.Syntax.Utilities;
1920

2021
namespace SonarAnalyzer.Test.CFG.Roslyn;
2122

@@ -321,6 +322,6 @@ void Use(int v) {}
321322
private static RoslynLiveVariableAnalysis CreateLva(string code)
322323
{
323324
var cfg = TestHelper.CompileCfgCS(code);
324-
return new RoslynLiveVariableAnalysis(cfg, CancellationToken.None);
325+
return new RoslynLiveVariableAnalysis(cfg, CSharpSyntaxClassifier.Instance, CancellationToken.None);
325326
}
326327
}

‎analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.cs‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
using SonarAnalyzer.CFG;
1919
using SonarAnalyzer.CFG.LiveVariableAnalysis;
2020
using SonarAnalyzer.CFG.Roslyn;
21+
using SonarAnalyzer.CFG.Syntax.Utilities;
2122
using SonarAnalyzer.CSharp.Core.Syntax.Extensions;
23+
using SonarAnalyzer.CSharp.Core.Syntax.Utilities;
24+
using SonarAnalyzer.VisualBasic.Core.Syntax.Utilities;
2225

2326
namespace SonarAnalyzer.Test.LiveVariableAnalysis;
2427

@@ -107,7 +110,7 @@ public void ProcessParameterReference_MemberBindingByReference_LiveIn()
107110
[TestMethod]
108111
public void ProcessParameterReference_MemberBindingByReference_DifferentCfgOnNetFx_LiveIn()
109112
{
110-
// This specific char/string scenario produces different CFG shape under .NET Framework build.
113+
// This specific char/string scenario produces different CFG shape under .NET Framework build. We have a syntax-based solution in place to support it.
111114
// https://github.com/dotnet/roslyn/issues/56644
112115
var code = """
113116
char[] charArray = null;
@@ -118,11 +121,7 @@ public void ProcessParameterReference_MemberBindingByReference_DifferentCfgOnNet
118121
""";
119122
var context = CreateContextCS(code);
120123
context.ValidateEntry(LiveIn("boolParameter"), LiveOut("boolParameter"));
121-
#if NET
122124
context.Validate("ret = charArray.Any(stringVariable.Contains);", LiveIn("charArray", "stringVariable"));
123-
#else
124-
context.Validate("ret = charArray.Any(stringVariable.Contains);", LiveIn("charArray"));
125-
#endif
126125
}
127126

128127
[TestMethod]
@@ -1075,7 +1074,13 @@ private class Context
10751074
public Context(string code, AnalyzerLanguage language, string localFunctionName = null)
10761075
{
10771076
Cfg = TestHelper.CompileCfg(code, language, code.Contains("// Error CS"), localFunctionName);
1078-
Lva = new RoslynLiveVariableAnalysis(Cfg, default);
1077+
SyntaxClassifierBase syntaxClassifier = language.LanguageName switch
1078+
{
1079+
LanguageNames.CSharp => CSharpSyntaxClassifier.Instance,
1080+
LanguageNames.VisualBasic => VisualBasicSyntaxClassifier.Instance,
1081+
_ => throw new UnexpectedLanguageException(language)
1082+
};
1083+
Lva = new RoslynLiveVariableAnalysis(Cfg, syntaxClassifier, default);
10791084
const string Separator = "----------";
10801085
Console.WriteLine(Separator);
10811086
Console.WriteLine(CfgSerializer.Serialize(Lva));
@@ -1087,7 +1092,7 @@ public Context(string code, SyntaxKind syntaxKind)
10871092
var (tree, model) = TestHelper.Compile(code, false, AnalyzerLanguage.CSharp);
10881093
var node = tree.GetRoot().DescendantNodes().First(x => x.RawKind == (int)syntaxKind);
10891094
Cfg = node.CreateCfg(model, default);
1090-
Lva = new RoslynLiveVariableAnalysis(Cfg, default);
1095+
Lva = new RoslynLiveVariableAnalysis(Cfg, CSharpSyntaxClassifier.Instance, default); // FIXME: null?
10911096
}
10921097

10931098
public void ValidateAllEmpty()

‎analyzers/tests/SonarAnalyzer.Test/Rules/MethodParameterUnusedTest.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,6 @@ public void MethodParameterUnused_CS_SonarCfg() =>
3333
public void MethodParameterUnused_CS_RoslynCfg() =>
3434
roslynCS.AddPaths("MethodParameterUnused.RoslynCfg.cs").Verify();
3535

36-
#if NETFRAMEWORK
37-
38-
[TestMethod]
39-
public void MethodParameterUnused_CS_RoslynCfg_NetFx() =>
40-
roslynCS.AddPaths("MethodParameterUnused.RoslynCfg.NetFx.cs").Verify();
41-
42-
#endif
43-
4436
[TestMethod]
4537
public void MethodParameterUnused_CodeFix_CS() =>
4638
roslynCS.AddPaths("MethodParameterUnused.RoslynCfg.cs")

0 commit comments

Comments
 (0)