diff --git a/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json b/analyzers/its/expected/CSharpLatest/S1854-CSharpLatest-net8.0.json index 9aec1f2025d..b4881f2675a 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#L19", + "Location": "Line 19 Position 13-40" + }, { "Id": "S1854", "Message": "Remove this useless assignment to local variable \u0027person\u0027.", diff --git a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs index cc467b52fa2..838930a8141 100644 --- a/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs +++ b/analyzers/src/SonarAnalyzer.CFG/LiveVariableAnalysis/RoslynLiveVariableAnalysis.cs @@ -75,7 +75,7 @@ protected override IEnumerable Successors(BasicBlock block) => protected override State ProcessBlock(BasicBlock block) { var ret = new RoslynState(this); - ret.ProcessBlock(Cfg, block, flowCaptures); + ret.ProcessBlock(Cfg, block); return ret; } @@ -245,18 +245,11 @@ private sealed class RoslynState : State public RoslynState(RoslynLiveVariableAnalysis owner) => this.owner = owner; - public void ProcessBlock(ControlFlowGraph cfg, BasicBlock block, Dictionary> flowCaptureOperations) + public void ProcessBlock(ControlFlowGraph cfg, BasicBlock block) { foreach (var operation in block.OperationsAndBranchValue.ToReversedExecutionOrder().Select(x => x.Instance)) { - if (operation.AsFlowCaptureReference() is { } flowCaptureReference && flowCaptureOperations.TryGetValue(flowCaptureReference.Id, out var symbols)) - { - UsedBeforeAssigned.UnionWith(symbols); - } - else - { - ProcessOperation(cfg, operation); - } + ProcessOperation(cfg, operation); } } @@ -271,6 +264,9 @@ private void ProcessOperation(ControlFlowGraph cfg, IOperation operation) case OperationKindEx.ParameterReference: ProcessParameterOrLocalReference(IParameterReferenceOperationWrapper.FromOperation(operation)); break; + case OperationKindEx.FlowCaptureReference: + ProcessParameterOrLocalReference(IFlowCaptureReferenceOperationWrapper.FromOperation(operation)); + break; case OperationKindEx.SimpleAssignment: ProcessSimpleAssignment(ISimpleAssignmentOperationWrapper.FromOperation(operation)); break; @@ -294,7 +290,7 @@ private void ProcessParameterOrLocalReference(IOperationWrapper reference) Assigned.UnionWith(symbols); UsedBeforeAssigned.ExceptWith(symbols); } - else if (!reference.IsAssignmentTarget()) + else if (!reference.IsAssignmentTarget() && reference.ToSonar().Parent?.Kind != OperationKindEx.FlowCapture) { UsedBeforeAssigned.UnionWith(symbols); } @@ -359,7 +355,7 @@ private void ProcessLocalFunction(ControlFlowGraph cfg, IMethodSymbol method) var localFunctionCfg = cfg.FindLocalFunctionCfgInScope(localFunction, owner.Cancel); foreach (var block in localFunctionCfg.Blocks.Reverse()) // Simplified approach, ignoring branching and try/catch/finally flows { - ProcessBlock(localFunctionCfg, block, []); + ProcessBlock(localFunctionCfg, block); } } } diff --git a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.FlowCaptureOperation.cs b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.FlowCaptureOperation.cs index 272eed969e3..5415b029630 100644 --- a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.FlowCaptureOperation.cs +++ b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.FlowCaptureOperation.cs @@ -41,8 +41,8 @@ public void FlowCaptrure_NullCoalescingAssignment() var context = CreateContextCS(code, additionalParameters: "string param"); context.ValidateEntry(LiveIn("param"), LiveOut("param")); context.Validate(context.Cfg.Blocks[1], LiveIn("param"), LiveOut("param")); - context.Validate(context.Cfg.Blocks[2], LiveIn("param"), LiveOut("param")); - context.Validate(context.Cfg.Blocks[3], LiveIn("param")); + context.Validate(context.Cfg.Blocks[2], LiveIn("param")); + context.Validate(context.Cfg.Blocks[3]); context.ValidateExit(); } @@ -214,20 +214,20 @@ public void FlowCaptrure_NullCoalescingOperator_ConsequentCalls_Assignment() { const string code = """var result = s1 ??= s2 = s3 ??= s4 ?? "End";"""; var context = CreateContextCS(code, additionalParameters: "string s1, string s2, string s3, string s4"); - context.ValidateEntry(LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); - context.Validate(context.Cfg.Blocks[1], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 1: #0=s1 - context.Validate(context.Cfg.Blocks[2], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 2: #1=#0; if #1 is null - context.Validate(context.Cfg.Blocks[3], LiveIn("s1"), LiveOut("s1")); // 3: F: #2=#1 - context.Validate(context.Cfg.Blocks[4], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 4: T: #3=s2 - context.Validate(context.Cfg.Blocks[5], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 5: #4=s3 - context.Validate(context.Cfg.Blocks[6], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 6: #5=#4; if #5 is null - context.Validate(context.Cfg.Blocks[7], LiveIn("s1", "s2", "s3"), LiveOut("s1", "s2", "s3")); // 7: F: #6=#5 - context.Validate(context.Cfg.Blocks[8], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 8: T: #7=s4; if #7 is null - context.Validate(context.Cfg.Blocks[9], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 9: F: #8=#7 - context.Validate(context.Cfg.Blocks[10], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3", "s4")); // 10: #7=null; #8="End" - context.Validate(context.Cfg.Blocks[11], LiveIn("s1", "s2", "s3", "s4"), LiveOut("s1", "s2", "s3")); // 11: #6= (#4=#8) - context.Validate(context.Cfg.Blocks[12], LiveIn("s1", "s2", "s3"), LiveOut("s1")); // 12: #2= (#0 = (#3=#6) ) - context.Validate(context.Cfg.Blocks[13], LiveIn("s1")); // 13: result=#2 + context.ValidateEntry(LiveIn("s1", "s3", "s4"), LiveOut("s1", "s3", "s4")); + context.Validate(context.Cfg.Blocks[1], LiveIn("s1", "s3", "s4"), LiveOut("s1", "s3", "s4")); // 1: #0=s1 + context.Validate(context.Cfg.Blocks[2], LiveIn("s1", "s3", "s4"), LiveOut("s1", "s3", "s4")); // 2: #1=#0; if #1 is null + context.Validate(context.Cfg.Blocks[3], LiveIn("s1"), LiveOut("s1")); // 3: F: #2=#1 + context.Validate(context.Cfg.Blocks[4], LiveIn("s3", "s4"), LiveOut("s3", "s4")); // 4: T: #3=s2 + context.Validate(context.Cfg.Blocks[5], LiveIn("s3", "s4"), LiveOut("s3", "s4")); // 5: #4=s3 + context.Validate(context.Cfg.Blocks[6], LiveIn("s3", "s4"), LiveOut("s3", "s4")); // 6: #5=#4; if #5 is null + context.Validate(context.Cfg.Blocks[7], LiveIn("s3"), LiveOut("s3")); // 7: F: #6=#5 + context.Validate(context.Cfg.Blocks[8], LiveIn("s4"), LiveOut("s4")); // 8: T: #7=s4; if #7 is null + context.Validate(context.Cfg.Blocks[9], LiveIn("s4"), LiveOut("s4")); // 9: F: #8=#7 + context.Validate(context.Cfg.Blocks[10], LiveIn("s4"), LiveOut("s4")); // 10: #7=null; #8="End" + context.Validate(context.Cfg.Blocks[11], LiveIn("s4"), LiveOut("s3")); // 11: #6= (#4=#8) + context.Validate(context.Cfg.Blocks[12], LiveIn("s3"), LiveOut("s1")); // 12: #2= (#0 = (#3=#6) ) + context.Validate(context.Cfg.Blocks[13], LiveIn("s1")); // 13: result=#2 context.ValidateExit(); } @@ -301,12 +301,13 @@ public void FlowCaptrure_NullCoalescingOperator_Overwrite() */ const string code = """s1 = (s1 = "overwrite") ?? "value";"""; var context = CreateContextCS(code, additionalParameters: "string s1"); - context.ValidateEntry(LiveIn("s1"), LiveOut("s1")); - context.Validate(context.Cfg.Blocks[1], LiveIn("s1")); - context.Validate(context.Cfg.Blocks[2], LiveOut("s1")); // This should have LiveIn("s1") and LiveOut("s1") but #1 gets as value all the assignment operation. - context.Validate(context.Cfg.Blocks[3], LiveIn("s1"), LiveOut("s1")); - context.Validate(context.Cfg.Blocks[4], LiveIn("s1"), LiveOut("s1")); - context.Validate(context.Cfg.Blocks[5], LiveIn("s1")); + // s1 is never read. The assignment returns its r-value, which is used for further calculation. + context.ValidateEntry(); + context.Validate(context.Cfg.Blocks[1]); + context.Validate(context.Cfg.Blocks[2]); + context.Validate(context.Cfg.Blocks[3]); + context.Validate(context.Cfg.Blocks[4]); + context.Validate(context.Cfg.Blocks[5]); context.ValidateExit(); } diff --git a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.LocalFunction.cs b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.LocalFunction.cs index 80ee21068bc..609515ec0a3 100644 --- a/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.LocalFunction.cs +++ b/analyzers/tests/SonarAnalyzer.Test/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.LocalFunction.cs @@ -233,7 +233,12 @@ public void LocalFunctionInvocation_Recursive_LiveIn() return; LocalFunction(10); - int LocalFunction(int cnt) => variable + (cnt == 0 ? 0 : LocalFunction(cnt - 1)); + int LocalFunction(int cnt) + { + if (cnt == 0) + return 0; + return variable + LocalFunction(cnt - 1); + } """; var context = CreateContextCS(code); context.ValidateEntry(LiveIn("boolParameter"), LiveOut("boolParameter")); @@ -241,6 +246,23 @@ public void LocalFunctionInvocation_Recursive_LiveIn() context.Validate("LocalFunction(10);", LiveIn("variable")); } + [TestMethod] + public void LocalFunctionInvocation_Recursive_FlowCapture() + { + var code = """ + var variable = 42; + if (boolParameter) + return; + LocalFunction(10); + + int LocalFunction(int cnt) => variable + (cnt == 0 ? 0 : LocalFunction(cnt - 1)); + """; + var context = CreateContextCS(code); + context.ValidateEntry(LiveIn("boolParameter"), LiveOut("boolParameter")); + context.Validate("boolParameter", LiveIn("boolParameter")); // should be LiveOut("variable") but FlowCaptures inside of local functions are not resolved + context.Validate("LocalFunction(10);"); // should be Livein("variable") + } + [TestMethod] public void LocalFunctionInvocation_Recursive_WhenAnalyzingLocalFunctionItself_LiveIn() { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs index 507318064f0..e7f56fbfff1 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/DeadStores.RoslynCfg.cs @@ -648,11 +648,22 @@ public void Unused() private void ConditionalEvaluation(bool b1, bool b2, object coalesce, object coalesceAssignment) { 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 = true; // Noncompliant + x = b1 && b2; // Noncompliant x = b1 || b2; // Noncompliant coalesce = coalesce ?? "Value"; // Noncompliant coalesceAssignment ??= "Value"; // Noncompliant + + DeadStores lst; + lst = new DeadStores // Noncompliant + { + Property = 42 + }; + lst = new DeadStores + { + Property = 42 + }; + lst.ToString(); } private void SimpleAssignment()