diff --git a/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json b/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json index f67b384a996..9aec1f2025d 100644 --- a/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json +++ b/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json @@ -1,5 +1,11 @@ { "Issues": [ + { + "Id": "S1854", + "Message": "Remove this useless assignment to local variable \u0027person\u0027.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/CSharpLatest/CSharpLatest/CSharp11Features/RequiredMembers.cs#L20", + "Location": "Line 20 Position 9-51" + }, { "Id": "S1854", "Message": "Remove this useless assignment to local variable \u0027a\u0027.", @@ -35,6 +41,18 @@ "Message": "Remove this useless assignment to local variable \u0027result\u0027.", "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/CSharpLatest/CSharpLatest/CSharp9Features/S2437.cs#L10", "Location": "Line 10 Position 9-30" + }, + { + "Id": "S1854", + "Message": "Remove this useless assignment to local variable \u0027a\u0027.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/CSharpLatest/CSharpLatest/CSharp9Features/S3240.cs#L12", + "Location": "Line 12 Position 9-32" + }, + { + "Id": "S1854", + "Message": "Remove this useless assignment to local variable \u0027y\u0027.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/CSharpLatest/CSharpLatest/CSharp9Features/S3440.cs#L8-L12", + "Location": "Lines 8-12 Position 9-10" } ] } \ No newline at end of file diff --git a/analyzers/its/expected/akka.net/S1854-Akka-netstandard2.0.json b/analyzers/its/expected/akka.net/S1854-Akka-netstandard2.0.json index 84612762811..cd96b6c41be 100644 --- a/analyzers/its/expected/akka.net/S1854-Akka-netstandard2.0.json +++ b/analyzers/its/expected/akka.net/S1854-Akka-netstandard2.0.json @@ -1,5 +1,11 @@ { "Issues": [ + { + "Id": "S1854", + "Message": "Remove this useless assignment to local variable \u0027sender\u0027.", + "Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/akka.net/src/core/Akka/Actor/Futures.cs#L353", + "Location": "Line 353 Position 13-50" + }, { "Id": "S1854", "Message": "Remove this useless assignment to local variable \u0027actor\u0027.", diff --git a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs index 0e35932a984..3e8ffc6a8b5 100644 --- a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs +++ b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs @@ -48,15 +48,16 @@ public RoslynLiveVariableAnalysis(ControlFlowGraph cfg, CancellationToken cancel Analyze(); } - public ISymbol ParameterOrLocalSymbol(IOperation operation) + public IEnumerable ParameterOrLocalSymbols(IOperation operation) { - ISymbol candidate = operation switch + var candidates = operation switch { - _ when operation.AsParameterReference() is { } parameterReference => parameterReference.Parameter, - _ when operation.AsLocalReference() is { } localReference => localReference.Local, - _ => null + _ when operation.AsParameterReference() is { } parameterReference => [parameterReference.Parameter], + _ when operation.AsLocalReference() is { } localReference => [localReference.Local], + _ when operation.AsFlowCaptureReference() is { } flowCaptureReference => flowCaptures.TryGetValue(flowCaptureReference.Id, out var symbols) ? symbols : [], + _ => [] }; - return IsLocal(candidate) ? candidate : null; + return candidates.Where(x => IsLocal(x)); } public override bool IsLocal(ISymbol symbol) => @@ -90,15 +91,15 @@ private void ResolveCaptures() if (flowCapture.Value.AsFlowCaptureReference() is { } captureReference && flowCaptures.TryGetValue(captureReference.Id, out var symbols)) { - AppendFlowCaptureReference(flowCapture.Id, symbols.ToArray()); + AppendFlowCaptureReference(flowCapture.Id, symbols); } - else if (ParameterOrLocalSymbol(flowCapture.Value) is { } symbol) + else { - AppendFlowCaptureReference(flowCapture.Id, symbol); + AppendFlowCaptureReference(flowCapture.Id, ParameterOrLocalSymbols(flowCapture.Value)); } } - void AppendFlowCaptureReference(CaptureId id, params ISymbol[] symbols) + void AppendFlowCaptureReference(CaptureId id, IEnumerable symbols) { if (!flowCaptures.TryGetValue(id, out var list)) { @@ -281,27 +282,23 @@ private void ProcessOperation(ControlFlowGraph cfg, IOperation operation) private void ProcessParameterOrLocalReference(IOperationWrapper reference) { - if (owner.ParameterOrLocalSymbol(reference.WrappedOperation) is { } symbol) + var symbols = owner.ParameterOrLocalSymbols(reference.WrappedOperation); + if (reference.IsOutArgument()) { - if (reference.IsOutArgument()) - { - Assigned.Add(symbol); - UsedBeforeAssigned.Remove(symbol); - } - else if (!reference.IsAssignmentTarget()) - { - UsedBeforeAssigned.Add(symbol); - } + Assigned.UnionWith(symbols); + UsedBeforeAssigned.ExceptWith(symbols); + } + else if (!reference.IsAssignmentTarget()) + { + UsedBeforeAssigned.UnionWith(symbols); } } private void ProcessSimpleAssignment(ISimpleAssignmentOperationWrapper assignment) { - if (owner.ParameterOrLocalSymbol(assignment.Target) is { } localTarget) - { - Assigned.Add(localTarget); - UsedBeforeAssigned.Remove(localTarget); - } + var targets = owner.ParameterOrLocalSymbols(assignment.Target); + Assigned.UnionWith(targets); + UsedBeforeAssigned.ExceptWith(targets); } private void ProcessFlowAnonymousFunction(ControlFlowGraph cfg, IFlowAnonymousFunctionOperationWrapper anonymousFunction) @@ -316,9 +313,10 @@ private void ProcessCaptured(ControlFlowGraph cfg) { foreach (var operation in cfg.Blocks.SelectMany(x => x.OperationsAndBranchValue).SelectMany(x => x.DescendantsAndSelf())) { - if (owner.ParameterOrLocalSymbol(operation) is { } symbol) + var symbols = owner.ParameterOrLocalSymbols(operation); + if (symbols.Any()) { - Captured.Add(symbol); + Captured.UnionWith(symbols); } else { diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.RoslynCfg.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.RoslynCfg.cs index 642da53e992..eda24f4fa92 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.RoslynCfg.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.RoslynCfg.cs @@ -73,25 +73,21 @@ public override void AnalyzeBlock() private void ProcessParameterOrLocalReference(IOperationWrapper reference) { - if (owner.lva.ParameterOrLocalSymbol(reference.WrappedOperation) is { } symbol && IsSymbolRelevant(symbol)) + var symbols = owner.lva.ParameterOrLocalSymbols(reference.WrappedOperation).Where(x => IsSymbolRelevant(x)); + if (reference.IsOutArgument()) { - if (reference.IsOutArgument()) - { - liveOut.Remove(symbol); - } - else if (!reference.IsAssignmentTarget() && !reference.IsCompoundAssignmentTarget()) - { - liveOut.Add(symbol); - } + liveOut.ExceptWith(symbols); + } + else if (!reference.IsAssignmentTarget() && !reference.IsCompoundAssignmentTarget()) + { + liveOut.UnionWith(symbols); } } private void ProcessSimpleAssignment(ISimpleAssignmentOperationWrapper assignment) { - if (ProcessAssignment(assignment, assignment.Target, assignment.Value) is { } localTarget) - { - liveOut.Remove(localTarget); - } + var targets = ProcessAssignment(assignment, assignment.Target, assignment.Value); + liveOut.ExceptWith(targets); } private void ProcessCompoundAssignment(ICompoundAssignmentOperationWrapper assignment) => @@ -106,43 +102,38 @@ private void ProcessDeconstructionAssignment(IDeconstructionAssignmentOperationW { foreach (var tupleElement in ITupleOperationWrapper.FromOperation(deconstructionAssignment.Target).AllElements()) { - if (ProcessAssignment(deconstructionAssignment, tupleElement) is { } target) - { - liveOut.Remove(target); - } + var targets = ProcessAssignment(deconstructionAssignment, tupleElement); + liveOut.ExceptWith(targets); } } } - private ISymbol ProcessAssignment(IOperationWrapper operation, IOperation target, IOperation value = null) + private ISymbol[] ProcessAssignment(IOperationWrapper operation, IOperation target, IOperation value = null) { - if (owner.lva.ParameterOrLocalSymbol(target) is { } localTarget && IsSymbolRelevant(localTarget)) + var targets = owner.lva.ParameterOrLocalSymbols(target).Where(IsSymbolRelevant).ToArray(); + foreach (var localTarget in targets) { - if (TargetRefKind() == RefKind.None - && !IsConst() + if (TargetRefKind(localTarget) == RefKind.None + && !IsConst(localTarget) && !liveOut.Contains(localTarget) - && !IsAllowedInitialization() + && !IsAllowedInitialization(localTarget) && !ICaughtExceptionOperationWrapper.IsInstance(value) && !target.Syntax.Parent.IsKind(SyntaxKind.ForEachStatement) && !IsMuted(target.Syntax, localTarget)) { ReportIssue(operation.WrappedOperation.Syntax.GetLocation(), localTarget); } - return localTarget; - } - else - { - return null; } + return targets; - bool IsConst() => + static bool IsConst(ISymbol localTarget) => localTarget is ILocalSymbol local && local.IsConst; - RefKind TargetRefKind() => + static RefKind TargetRefKind(ISymbol localTarget) => // Only ILocalSymbol and IParameterSymbol can be returned by ParameterOrLocalSymbol localTarget is ILocalSymbol local ? local.RefKind() : ((IParameterSymbol)localTarget).RefKind; - bool IsAllowedInitialization() => + bool IsAllowedInitialization(ISymbol localTarget) => operation.WrappedOperation.Syntax is VariableDeclaratorSyntax variableDeclarator && variableDeclarator.Initializer != null // Avoid collision with S1481: Unused is allowed. Used only in local function is also unused in current CFG. diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.cs index eed3d3db207..861d4c6dfe0 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/DeadStores.cs @@ -57,6 +57,7 @@ protected override void Initialize(SonarAnalysisContext context) SyntaxKind.OperatorDeclaration, SyntaxKind.RemoveAccessorDeclaration, SyntaxKind.SetAccessorDeclaration, + SyntaxKindEx.CoalesceAssignmentExpression, SyntaxKindEx.InitAccessorDeclaration, SyntaxKindEx.LocalFunctionStatement); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs index 8ba931111a6..255c240fb07 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs @@ -302,7 +302,7 @@ public void Assignment(DeadStores sender) string coa = SomeString(); coa ??= SomeString(); Use(coa); - coa ??= SomeString(); // Roslyn CFG FN: Branching with FlowCaptureOperation + coa ??= SomeString(); // Noncompliant } public void Unary() @@ -650,9 +650,9 @@ private void ConditionalEvaluation(bool b1, bool b2, object coalesce, object coa var x = false; // Compliant ignored value x = true; // Roslyn CFG FN: Consequence of inaccurate LVA state below x = b1 && b2; // Roslyn CFG FN: Branching with FlowCaptureOperation - x = b1 || b2; // Roslyn CFG FN: Branching with FlowCaptureOperation - coalesce = coalesce ?? "Value"; // Roslyn CFG FN: Branching with FlowCaptureOperation - coalesceAssignment ??= "Value"; // Roslyn CFG FN: Branching with FlowCaptureOperation + x = b1 || b2; // Noncompliant + coalesce = coalesce ?? "Value"; // Noncompliant + coalesceAssignment ??= "Value"; // Noncompliant } private void SimpleAssignment() @@ -776,7 +776,7 @@ int[] Compute(int[] arr) var lst = arr; //Compliant var unused = arr; lst ??= new int[0]; - unused ??= new int[0]; // Roslyn CFG FN: Branching with FlowCaptureOperation + unused ??= new int[0]; // Noncompliant return lst; } }