From 63fe88c83679af2260fbb22d392218a794f45df0 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Mar 2023 20:03:36 +0100 Subject: [PATCH 01/16] Fix FPs in LocksReleasedAllPaths.Mutex.NetFx.cs --- .../Roslyn/Extensions/IOperationExtensions.cs | 1 + .../RuleChecks/LocksReleasedAllPathsBase.cs | 37 +++++++++++-------- .../LocksReleasedAllPaths.Mutex.NetFx.cs | 2 +- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index ced4b6eba2a..edcbc59aeb1 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -31,6 +31,7 @@ OperationKindEx.FieldReference when operation.ToFieldReference() is var fieldRef OperationKindEx.LocalReference => operation.ToLocalReference().Local, OperationKindEx.ParameterReference => operation.ToParameterReference().Parameter, OperationKindEx.Argument => operation.ToArgument().Value.TrackedSymbol(), + OperationKindEx.DeclarationExpression => IDeclarationExpressionOperationWrapper.FromOperation(operation).Expression.TrackedSymbol(), _ => null }; diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs index e4cd44b82cc..71f15d85600 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs @@ -94,6 +94,27 @@ public override ProgramState[] PostProcess(SymbolicContext context) }; } } + else if (context.Operation.Instance.AsObjectCreation() is { } objectCreation + && objectCreation.Type.Is(KnownType.System_Threading_Mutex) + && context.Operation.Parent.AsAssignment() is { } assignment + && assignment.Target.TrackedSymbol() is { } symbol + && objectCreation.Arguments.Length > 0 + && objectCreation.Arguments.ToDictionary(x => x.ToArgument().Parameter.Name, x => x.ToArgument().Value) is var arguments + && arguments.TryGetValue("initiallyOwned", out var initiallyOwned) + && context.State[initiallyOwned] is { } initiallyOwnedValue + && initiallyOwnedValue.HasConstraint(BoolConstraint.True)) + { + lastSymbolLock[symbol] = objectCreation.ToSonar(); + return arguments.TryGetValue("createdNew", out var createdNew) + && createdNew.TrackedSymbol() is { } trackedCreatedNew + ? new[] + { + AddLock(context, symbol).SetSymbolConstraint(trackedCreatedNew, BoolConstraint.True), + context.State.SetSymbolConstraint(trackedCreatedNew, BoolConstraint.False), + } + : AddLock(context, objectCreation.WrappedOperation).Preserve(symbol).ToArray(); + } + return base.PostProcess(context); } @@ -131,21 +152,7 @@ ProgramState ProcessCondition(ISymbol lockSymbol) protected override ProgramState PostProcessSimple(SymbolicContext context) { - if (context.Operation.Instance.AsObjectCreation() is { } objectCreation) - { - if (objectCreation.Type.Is(KnownType.System_Threading_Mutex) - && context.Operation.Parent.AsAssignment() is { } assignment - && objectCreation.Arguments.Length > 0 - && objectCreation.Arguments.First().ToArgument().Value is { } firstArgument - && context.State[firstArgument] is { } firstArgumentValue - && firstArgumentValue.HasConstraint(BoolConstraint.True) - && assignment.Target.TrackedSymbol() is { } symbol) - { - lastSymbolLock[symbol] = objectCreation.ToSonar(); - return AddLock(context, objectCreation.WrappedOperation).Preserve(symbol); - } - } - else if (context.Operation.Instance.AsInvocation() is { } invocation) + if (context.Operation.Instance.AsInvocation() is { } invocation) { // ToDo: we ignore the number of parameters for now. if (invocation.TargetMethod.IsAny(KnownType.System_Threading_Monitor, "Enter", "TryEnter")) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs index cb268ad80c6..fa7fbb85f57 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs @@ -14,7 +14,7 @@ public class MutexTest public static void Noncompliant(MutexSecurity mutexSecurity, bool cond) { // Note that Dispose() closes the underlying WaitHandle, but does not release the mutex - var m0 = new Mutex(true, "foo", out var mutexWasCreated, mutexSecurity); // Noncompliant + var m0 = new Mutex(true, "foo", out var mutexWasCreated, mutexSecurity); if (cond) { m0.ReleaseMutex(); From d3aca0f2e4eaacb94ac8700f05a4e7052749fb12 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 22 Mar 2023 21:05:03 +0100 Subject: [PATCH 02/16] Fix tests --- .../Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs | 6 +++--- .../Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs | 2 +- .../SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs | 3 --- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs index 71f15d85600..16f210c6387 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs @@ -109,8 +109,8 @@ public override ProgramState[] PostProcess(SymbolicContext context) && createdNew.TrackedSymbol() is { } trackedCreatedNew ? new[] { - AddLock(context, symbol).SetSymbolConstraint(trackedCreatedNew, BoolConstraint.True), - context.State.SetSymbolConstraint(trackedCreatedNew, BoolConstraint.False), + AddLock(context, objectCreation.WrappedOperation).Preserve(symbol).SetSymbolConstraint(trackedCreatedNew, BoolConstraint.True), + context.SetSymbolConstraint(trackedCreatedNew, BoolConstraint.False), } : AddLock(context, objectCreation.WrappedOperation).Preserve(symbol).ToArray(); } @@ -198,7 +198,7 @@ private ProgramState AddLock(SymbolicContext context, ISymbol symbol) } lastSymbolLock[symbol] = context.Operation; - return context.SetSymbolConstraint(symbol, LockConstraint.Held).Preserve(symbol); + return context.SetSymbolConstraint(symbol, LockConstraint.Held).SetSymbolConstraint(symbol, ObjectConstraint.NotNull).Preserve(symbol); } private ProgramState RemoveLock(SymbolicContext context, ISymbol symbol) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs index fa7fbb85f57..cb268ad80c6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs @@ -14,7 +14,7 @@ public class MutexTest public static void Noncompliant(MutexSecurity mutexSecurity, bool cond) { // Note that Dispose() closes the underlying WaitHandle, but does not release the mutex - var m0 = new Mutex(true, "foo", out var mutexWasCreated, mutexSecurity); + var m0 = new Mutex(true, "foo", out var mutexWasCreated, mutexSecurity); // Noncompliant if (cond) { m0.ReleaseMutex(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs index c503d7fd517..ff009150faf 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs @@ -23,8 +23,6 @@ public void Noncompliant(Foo foo) var m3 = Mutex.OpenExisting("x"); m3.WaitOne(); // Noncompliant - var m4 = new Mutex(); - foo.instanceMutex.WaitOne(); // FN Foo.staticMutex.WaitOne(); // Noncompliant @@ -35,7 +33,6 @@ public void Noncompliant(Foo foo) m1.ReleaseMutex(); m2.ReleaseMutex(); m3.ReleaseMutex(); - m4.ReleaseMutex(); foo.instanceMutex.ReleaseMutex(); Foo.staticMutex.ReleaseMutex(); } From 60e7d4ea3408fd358592e55c06980a9c91a1bd15 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 17:14:17 +0200 Subject: [PATCH 03/16] Add test cases for wasCreated and initiallyOwned handling. --- .../Roslyn/LocksReleasedAllPaths.Mutex.cs | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs index ff009150faf..cd7d053e9b5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs @@ -44,6 +44,34 @@ public void Noncompliant(Foo foo) m3.Dispose(); } + public void MutexWasCreatedAndReleased(bool arg) + { + var m = new Mutex(initiallyOwned: true, "bar", out var wasCreated); // Noncompliant + if (wasCreated && arg) + { + m.ReleaseMutex(); + } + } + + public void MutexWasCreatedAndNotReleased(bool arg) + { + var m = new Mutex(initiallyOwned: true, "bar", out var wasCreated); // Compliant + if (!wasCreated && arg) + { + // wasCreated is false here, indicating that the lock is NOT (yet) held + m.ReleaseMutex(); // This is a release without a previous lock. This is not covered by this rule. + } + } + + public void MutexWasCreatedWithoutOwnership(bool arg) + { + var m = new Mutex(initiallyOwned: false, "bar", out var wasCreated); // Compliant. The lock must still be held by requesting it. + if (wasCreated && arg) + { + m.ReleaseMutex(); + } + } + public void Noncompliant2(Mutex paramMutex, Mutex paramMutex2) { // 'true' means it owns the mutex if no exception gets thrown @@ -336,11 +364,11 @@ public void MutextAquireByConstructor_MultipleVariableDeclaration() public void MutextAquireByConstructor_SwitchExpression(int x) { var m = x switch - { - 1 => new Mutex(), - 2 => new Mutex(new bool()), - 3 => new Mutex(true, "bar", out var mutextCreated), // FN - }; + { + 1 => new Mutex(), + 2 => new Mutex(new bool()), + 3 => new Mutex(true, "bar", out var mutextCreated), // FN + }; if (cond) { From 1ef9f24197ba87fea3c733213a51ad4c6596def4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 18:04:14 +0200 Subject: [PATCH 04/16] Add test for TrackedSymbol_DeclarationExpression --- .../Roslyn/IOperationExtensionsTest.cs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs index 71a8977a9d2..df569af5892 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs @@ -59,6 +59,23 @@ public void TrackedSymbol_FieldReference_IsFieldSymbol(string assignment) assignmentTarget.TrackedSymbol().Should().Be(fieldReferenceSymbol); } + [DataTestMethod] + [DataRow(@"(int i, int j) = (1, 1)")] + [DataRow(@"(var i, var j) = (1, 1)")] + [DataRow(@"int.TryParse(string.Empty, out int value)")] + [DataRow(@"int.TryParse(string.Empty, out var value)")] + public void TrackedSymbol_DeclarationExpression(string assignment) + { + var code = $"public class C {{ void Method() {{ {assignment}; }} }}"; + var graph = TestHelper.CompileCfgCS(code); + var allDeclarations = graph.Blocks[1].Operations.SelectMany(x => x.DescendantsAndSelf()).Where(x => x.Kind == OperationKindEx.DeclarationExpression).Select(IDeclarationExpressionOperationWrapper.FromOperation).ToArray(); + allDeclarations.Should().NotBeEmpty(); + allDeclarations.Should().AllSatisfy(x => + x.WrappedOperation.TrackedSymbol().Should().NotBeNull().And.BeAssignableTo() + .Which.GetSymbolType().Should().NotBeNull().And.BeAssignableTo() + .Which.SpecialType.Should().Be(SpecialType.System_Int32)); + } + [TestMethod] public void TrackedSymbol_SimpleAssignment_IsNull() { From 9a67d724a5c32c74803e5255268bdb016d1afa03 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 18:04:48 +0200 Subject: [PATCH 05/16] Add ArgumentValue extension method for ImmutableArray --- .../Roslyn/Extensions/IOperationExtensions.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index edcbc59aeb1..c4ab315d4c3 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -103,6 +103,19 @@ public static IOperation UnwrapConversion(this IOperation operation) return operation; } + public static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) + { + foreach (var argument in arguments) + { + var argumenentOperation = IArgumentOperationWrapper.FromOperation(argument); + if (argumenentOperation.Parameter.Name == parameterName) + { + return argumenentOperation.Value; + } + } + return null; + } + private static T? As(this IOperation operation, OperationKind kind, Func fromOperation) where T : struct => operation.Kind == kind ? fromOperation(operation) : null; } From 41f979dce8c442b999579b74886a85901b7258ea Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:27:22 +0200 Subject: [PATCH 06/16] Add ArgumentValue and tests --- .../IInvocationOperationExtensions.cs | 4 + .../IObjectCreationOperationExtensions.cs | 28 ++++++ .../Roslyn/Extensions/IOperationExtensions.cs | 4 + .../IInvocationOperationExtensionsTest.cs | 45 ++++++++++ .../Roslyn/IObjectCreationExtensionsTest.cs | 86 +++++++++++++++++++ 5 files changed, 167 insertions(+) create mode 100644 analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs index 4f7f770ab50..fd5304fd8fe 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs @@ -42,5 +42,9 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation, || (invocation.TargetMethod.IsExtensionMethod && !invocation.Arguments.IsEmpty && state.ResolveCapture(invocation.Arguments[0].ToArgument().Value.UnwrapConversion()).Kind == OperationKindEx.InstanceReference); + + /// + public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) + => invocation.Arguments.ArgumentValue(parameterName); } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs new file mode 100644 index 00000000000..b8e479f7016 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs @@ -0,0 +1,28 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +namespace SonarAnalyzer.SymbolicExecution.Roslyn; + +public static class IObjectCreationOperationExtensions +{ + /// + public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) + => objectCreation.Arguments.ArgumentValue(parameterName); +} diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index c4ab315d4c3..459fd548495 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -103,6 +103,10 @@ public static IOperation UnwrapConversion(this IOperation operation) return operation; } + /// + /// Returns the argument value corresponding to . For parameter an + /// IArrayCreationExpression is returned. + /// public static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) { foreach (var argument in arguments) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs new file mode 100644 index 00000000000..edddf865c85 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -0,0 +1,45 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using Microsoft.CodeAnalysis.CSharp.Syntax; +using SonarAnalyzer.SymbolicExecution.Roslyn; +using SonarAnalyzer.UnitTest; +using StyleCop.Analyzers.Lightup; + +[TestClass] +public class IInvocationOperationExtensionsTest +{ + [TestMethod] + public void ArgumentValue() + { + var testClass = $$""" + class Test + { + static void M(string stringParam) + => M("param"); + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var invocation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocation)); + var argument = operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param"); + } + +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs new file mode 100644 index 00000000000..f9abeae0f48 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs @@ -0,0 +1,86 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2023 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using Microsoft.CodeAnalysis.CSharp.Syntax; +using SonarAnalyzer.SymbolicExecution.Roslyn; +using StyleCop.Analyzers.Lightup; + +namespace SonarAnalyzer.UnitTest.SymbolicExecution.Roslyn; + +[TestClass] +public class IObjectCreationExtensionsTest +{ + [DataTestMethod] + [DataRow("""1, "Test" """, "intParam", 1)] + [DataRow("""1, "Test" """, "stringParam", "Test")] + [DataRow("""1, "Test" """, "optionalBoolParam", true)] + [DataRow("""1, "Test" """, "optionalIntParam", 1)] + [DataRow("""1, "Test" """, "stringParams", null)] + [DataRow("""1, "Test", true """, "optionalBoolParam", true)] + [DataRow("""stringParam: "Test", intParam: 1 """, "stringParam", "Test")] + [DataRow("""stringParam: "Test", intParam: 1, optionalIntParam: 2 """, "optionalIntParam", 2)] + [DataRow("""1, "Test", true, 0, "param1", "param2" """, "stringParams", null)] + public void ArgumentValue(string objectCreationArguments, string parameterName, object expected) + { + var testClass = $$""" + class Test + { + Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1, params string[] stringParams) + { + } + + static void Create() + { + new Test({{objectCreationArguments}}); + } + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); + operation.ArgumentValue(parameterName).Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(expected); + } + + [TestMethod] + public void ArgumentValue_Params() + { + var testClass = $$""" + class Test + { + Test(params string[] stringParams) + { + } + + static void Create() + { + new Test("param1", "param2"); + } + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); + var argument = operation.ArgumentValue("stringParams").Should().NotBeNull().And.BeAssignableTo().Subject; + var argumentArray = IArrayCreationOperationWrapper.FromOperation(argument); + argumentArray.Initializer.ElementValues.Should().SatisfyRespectively( + x => x.ConstantValue.Value.Should().Be("param1"), + x => x.ConstantValue.Value.Should().Be("param2")); + } +} From ad2ea041f332120cecfd65917895b1ff38d3cc29 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:29:49 +0200 Subject: [PATCH 07/16] Use ArgumentValue --- .../Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs index 16f210c6387..80efcc64dc2 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs @@ -98,14 +98,12 @@ public override ProgramState[] PostProcess(SymbolicContext context) && objectCreation.Type.Is(KnownType.System_Threading_Mutex) && context.Operation.Parent.AsAssignment() is { } assignment && assignment.Target.TrackedSymbol() is { } symbol - && objectCreation.Arguments.Length > 0 - && objectCreation.Arguments.ToDictionary(x => x.ToArgument().Parameter.Name, x => x.ToArgument().Value) is var arguments - && arguments.TryGetValue("initiallyOwned", out var initiallyOwned) + && objectCreation.ArgumentValue("initiallyOwned") is { } initiallyOwned && context.State[initiallyOwned] is { } initiallyOwnedValue && initiallyOwnedValue.HasConstraint(BoolConstraint.True)) { lastSymbolLock[symbol] = objectCreation.ToSonar(); - return arguments.TryGetValue("createdNew", out var createdNew) + return objectCreation.ArgumentValue("createdNew") is { } createdNew && createdNew.TrackedSymbol() is { } trackedCreatedNew ? new[] { From e37bece5ce66832889bdacaf4d08e8e9e9c0f45c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:36:12 +0200 Subject: [PATCH 08/16] Fix doc comment --- .../SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index 459fd548495..9458eeda8a9 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -105,7 +105,7 @@ public static IOperation UnwrapConversion(this IOperation operation) /// /// Returns the argument value corresponding to . For parameter an - /// IArrayCreationExpression is returned. + /// IArrayCreationOperation is returned. /// public static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) { From 61041cdc0eafd9f17f240158618e5917ff4cb733 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:37:28 +0200 Subject: [PATCH 09/16] Fix typo --- .../Roslyn/Extensions/IOperationExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index 9458eeda8a9..e91bba1a4eb 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -111,10 +111,10 @@ public static IOperation ArgumentValue(this ImmutableArray arguments { foreach (var argument in arguments) { - var argumenentOperation = IArgumentOperationWrapper.FromOperation(argument); - if (argumenentOperation.Parameter.Name == parameterName) + var argumentOperation = IArgumentOperationWrapper.FromOperation(argument); + if (argumentOperation.Parameter.Name == parameterName) { - return argumenentOperation.Value; + return argumentOperation.Value; } } return null; From 96d7e5201fa66b07bff8e73c9650828945735e0c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:44:09 +0200 Subject: [PATCH 10/16] More params tests --- .../Roslyn/IObjectCreationExtensionsTest.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs index f9abeae0f48..b8de78306ee 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs @@ -32,17 +32,15 @@ public class IObjectCreationExtensionsTest [DataRow("""1, "Test" """, "stringParam", "Test")] [DataRow("""1, "Test" """, "optionalBoolParam", true)] [DataRow("""1, "Test" """, "optionalIntParam", 1)] - [DataRow("""1, "Test" """, "stringParams", null)] [DataRow("""1, "Test", true """, "optionalBoolParam", true)] [DataRow("""stringParam: "Test", intParam: 1 """, "stringParam", "Test")] [DataRow("""stringParam: "Test", intParam: 1, optionalIntParam: 2 """, "optionalIntParam", 2)] - [DataRow("""1, "Test", true, 0, "param1", "param2" """, "stringParams", null)] public void ArgumentValue(string objectCreationArguments, string parameterName, object expected) { var testClass = $$""" class Test { - Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1, params string[] stringParams) + Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) { } @@ -58,8 +56,11 @@ static void Create() operation.ArgumentValue(parameterName).Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(expected); } - [TestMethod] - public void ArgumentValue_Params() + [DataTestMethod] + [DataRow(""" """)] + [DataRow(""" "param1", "param2" """, "param1", "param2")] + [DataRow(""" null, null """, null, null)] + public void ArgumentValue_Params(string arguments, params string[] expected) { var testClass = $$""" class Test @@ -70,7 +71,7 @@ class Test static void Create() { - new Test("param1", "param2"); + new Test({{arguments}}); } } """; @@ -79,8 +80,7 @@ static void Create() var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); var argument = operation.ArgumentValue("stringParams").Should().NotBeNull().And.BeAssignableTo().Subject; var argumentArray = IArrayCreationOperationWrapper.FromOperation(argument); - argumentArray.Initializer.ElementValues.Should().SatisfyRespectively( - x => x.ConstantValue.Value.Should().Be("param1"), - x => x.ConstantValue.Value.Should().Be("param2")); + var result = argumentArray.Initializer.ElementValues.Select(x => x.ConstantValue.Value).ToArray(); + result.Should().BeEquivalentTo(expected); } } From b1e6ae69ea88cfdb6afc49b6249176d4c805a9c0 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 29 Mar 2023 19:49:27 +0200 Subject: [PATCH 11/16] Smaller fixes --- .../Roslyn/IInvocationOperationExtensionsTest.cs | 2 +- .../SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs index edddf865c85..a8ab5f5d669 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -29,7 +29,7 @@ public class IInvocationOperationExtensionsTest [TestMethod] public void ArgumentValue() { - var testClass = $$""" + const string testClass = """ class Test { static void M(string stringParam) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs index cd7d053e9b5..769963b62ee 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.cs @@ -65,7 +65,7 @@ public void MutexWasCreatedAndNotReleased(bool arg) public void MutexWasCreatedWithoutOwnership(bool arg) { - var m = new Mutex(initiallyOwned: false, "bar", out var wasCreated); // Compliant. The lock must still be held by requesting it. + var m = new Mutex(initiallyOwned: false, "bar", out var wasCreated); // Compliant. The lock must still be acquired by requesting it. if (wasCreated && arg) { m.ReleaseMutex(); From 87aa44e14af121d2a89afc0fe8d6747ec9af9253 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 30 Mar 2023 09:06:15 +0200 Subject: [PATCH 12/16] Fix code smells --- .../Roslyn/IInvocationOperationExtensionsTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs index a8ab5f5d669..36bb6cf288a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -23,6 +23,8 @@ using SonarAnalyzer.UnitTest; using StyleCop.Analyzers.Lightup; +namespace SonarAnalyzer.UnitTest.SymbolicExecution.Roslyn; + [TestClass] public class IInvocationOperationExtensionsTest { @@ -39,7 +41,6 @@ static void M(string stringParam) var (tree, model) = TestHelper.CompileCS(testClass); var invocation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocation)); - var argument = operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param"); + operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param"); } - } From 938affc96da164c806146d1237c186dccf90e345 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 31 Mar 2023 10:48:13 +0200 Subject: [PATCH 13/16] Code review --- .../IInvocationOperationExtensions.cs | 4 ++-- .../IObjectCreationOperationExtensions.cs | 4 ++-- .../Roslyn/Extensions/IOperationExtensions.cs | 11 +++++------ .../IInvocationOperationExtensionsTest.cs | 3 +-- .../Roslyn/IObjectCreationExtensionsTest.cs | 17 +++++------------ .../Roslyn/IOperationExtensionsTest.cs | 13 +++++++++++++ 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs index fd5304fd8fe..c3b1244842c 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs @@ -44,7 +44,7 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation, && state.ResolveCapture(invocation.Arguments[0].ToArgument().Value.UnwrapConversion()).Kind == OperationKindEx.InstanceReference); /// - public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) - => invocation.Arguments.ArgumentValue(parameterName); + public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) => + invocation.Arguments.ArgumentValue(parameterName); } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs index b8e479f7016..2633981a247 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs @@ -23,6 +23,6 @@ namespace SonarAnalyzer.SymbolicExecution.Roslyn; public static class IObjectCreationOperationExtensions { /// - public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) - => objectCreation.Arguments.ArgumentValue(parameterName); + public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) => + objectCreation.Arguments.ArgumentValue(parameterName); } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index e91bba1a4eb..4108119c6c5 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -104,17 +104,16 @@ public static IOperation UnwrapConversion(this IOperation operation) } /// - /// Returns the argument value corresponding to . For parameter an - /// IArrayCreationOperation is returned. + /// Returns the argument value corresponding to . For parameter an IArrayCreationOperation is returned. /// public static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) { - foreach (var argument in arguments) + foreach (var operation in arguments) { - var argumentOperation = IArgumentOperationWrapper.FromOperation(argument); - if (argumentOperation.Parameter.Name == parameterName) + var argument = operation.ToArgument(); + if (argument.Parameter.Name == parameterName) { - return argumentOperation.Value; + return argument.Value; } } return null; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs index 36bb6cf288a..c9bdd3eb56d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -34,8 +34,7 @@ public void ArgumentValue() const string testClass = """ class Test { - static void M(string stringParam) - => M("param"); + static void M(string stringParam) => M("param"); } """; var (tree, model) = TestHelper.CompileCS(testClass); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs index b8de78306ee..587e93b80c0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs @@ -40,14 +40,10 @@ public void ArgumentValue(string objectCreationArguments, string parameterName, var testClass = $$""" class Test { - Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) - { - } + Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) { } - static void Create() - { + static void Create() => new Test({{objectCreationArguments}}); - } } """; var (tree, model) = TestHelper.CompileCS(testClass); @@ -60,19 +56,16 @@ static void Create() [DataRow(""" """)] [DataRow(""" "param1", "param2" """, "param1", "param2")] [DataRow(""" null, null """, null, null)] + [DataRow(""" new[] {"param1", "param2"} """, "param1", "param2")] public void ArgumentValue_Params(string arguments, params string[] expected) { var testClass = $$""" class Test { - Test(params string[] stringParams) - { - } + Test(params string[] stringParams) { } - static void Create() - { + static void Create() => new Test({{arguments}}); - } } """; var (tree, model) = TestHelper.CompileCS(testClass); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs index df569af5892..51b22500822 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs @@ -76,6 +76,19 @@ public void TrackedSymbol_DeclarationExpression(string assignment) .Which.SpecialType.Should().Be(SpecialType.System_Int32)); } + [TestMethod] + public void TrackedSymbol_DeclarationExpression_Tuple() + { + var code = $"public class C {{ void Method() {{ var (i, j) = (1, 1); }} }}"; + var graph = TestHelper.CompileCfgCS(code); + var allDeclarations = graph.Blocks[1].Operations.SelectMany(x => x.DescendantsAndSelf()).Where(x => x.Kind == OperationKindEx.DeclarationExpression).Select(IDeclarationExpressionOperationWrapper.FromOperation).ToArray(); + var declaration = allDeclarations.Should().ContainSingle().Which.WrappedOperation; + declaration.Kind.Should().Be(OperationKindEx.DeclarationExpression); + var declarationExpression = IDeclarationExpressionOperationWrapper.FromOperation(declaration).Expression; + declarationExpression.Kind.Should().Be(OperationKindEx.Tuple); + declaration.TrackedSymbol().Should().BeNull(); + } + [TestMethod] public void TrackedSymbol_SimpleAssignment_IsNull() { From c5f260a020b358a677aa82855114bfd97adb7f53 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 3 Apr 2023 13:00:40 +0200 Subject: [PATCH 14/16] Move extension to IOperation and add extension methods for property and RaiseEvent. --- .../IInvocationOperationExtensions.cs | 4 - .../IObjectCreationOperationExtensions.cs | 28 ----- .../Roslyn/Extensions/IOperationExtensions.cs | 18 +++- .../IInvocationOperationExtensionsTest.cs | 45 -------- .../Roslyn/IObjectCreationExtensionsTest.cs | 79 -------------- .../Roslyn/IOperationExtensionsTest.cs | 100 ++++++++++++++++++ 6 files changed, 117 insertions(+), 157 deletions(-) delete mode 100644 analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs delete mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs index c3b1244842c..4f7f770ab50 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs @@ -42,9 +42,5 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation, || (invocation.TargetMethod.IsExtensionMethod && !invocation.Arguments.IsEmpty && state.ResolveCapture(invocation.Arguments[0].ToArgument().Value.UnwrapConversion()).Kind == OperationKindEx.InstanceReference); - - /// - public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) => - invocation.Arguments.ArgumentValue(parameterName); } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs deleted file mode 100644 index 2633981a247..00000000000 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IObjectCreationOperationExtensions.cs +++ /dev/null @@ -1,28 +0,0 @@ -/* - * SonarAnalyzer for .NET - * Copyright (C) 2015-2023 SonarSource SA - * mailto: contact AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -namespace SonarAnalyzer.SymbolicExecution.Roslyn; - -public static class IObjectCreationOperationExtensions -{ - /// - public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) => - objectCreation.Arguments.ArgumentValue(parameterName); -} diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index 4108119c6c5..3ab9e4227c1 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -103,10 +103,26 @@ public static IOperation UnwrapConversion(this IOperation operation) return operation; } + /// + public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) => + invocation.Arguments.ArgumentValue(parameterName); + + /// + public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) => + objectCreation.Arguments.ArgumentValue(parameterName); + + /// + public static IOperation ArgumentValue(this IPropertyReferenceOperationWrapper propertyReference, string parameterName) => + propertyReference.Arguments.ArgumentValue(parameterName); + + /// + public static IOperation ArgumentValue(this IRaiseEventOperationWrapper raiseEvent, string parameterName) => + raiseEvent.Arguments.ArgumentValue(parameterName); + /// /// Returns the argument value corresponding to . For parameter an IArrayCreationOperation is returned. /// - public static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) + private static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) { foreach (var operation in arguments) { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs deleted file mode 100644 index c9bdd3eb56d..00000000000 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ /dev/null @@ -1,45 +0,0 @@ -/* - * SonarAnalyzer for .NET - * Copyright (C) 2015-2023 SonarSource SA - * mailto: contact AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -using Microsoft.CodeAnalysis.CSharp.Syntax; -using SonarAnalyzer.SymbolicExecution.Roslyn; -using SonarAnalyzer.UnitTest; -using StyleCop.Analyzers.Lightup; - -namespace SonarAnalyzer.UnitTest.SymbolicExecution.Roslyn; - -[TestClass] -public class IInvocationOperationExtensionsTest -{ - [TestMethod] - public void ArgumentValue() - { - const string testClass = """ - class Test - { - static void M(string stringParam) => M("param"); - } - """; - var (tree, model) = TestHelper.CompileCS(testClass); - var invocation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); - var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocation)); - operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param"); - } -} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs deleted file mode 100644 index 587e93b80c0..00000000000 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IObjectCreationExtensionsTest.cs +++ /dev/null @@ -1,79 +0,0 @@ -/* - * SonarAnalyzer for .NET - * Copyright (C) 2015-2023 SonarSource SA - * mailto: contact AT sonarsource DOT com - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -using Microsoft.CodeAnalysis.CSharp.Syntax; -using SonarAnalyzer.SymbolicExecution.Roslyn; -using StyleCop.Analyzers.Lightup; - -namespace SonarAnalyzer.UnitTest.SymbolicExecution.Roslyn; - -[TestClass] -public class IObjectCreationExtensionsTest -{ - [DataTestMethod] - [DataRow("""1, "Test" """, "intParam", 1)] - [DataRow("""1, "Test" """, "stringParam", "Test")] - [DataRow("""1, "Test" """, "optionalBoolParam", true)] - [DataRow("""1, "Test" """, "optionalIntParam", 1)] - [DataRow("""1, "Test", true """, "optionalBoolParam", true)] - [DataRow("""stringParam: "Test", intParam: 1 """, "stringParam", "Test")] - [DataRow("""stringParam: "Test", intParam: 1, optionalIntParam: 2 """, "optionalIntParam", 2)] - public void ArgumentValue(string objectCreationArguments, string parameterName, object expected) - { - var testClass = $$""" - class Test - { - Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) { } - - static void Create() => - new Test({{objectCreationArguments}}); - } - """; - var (tree, model) = TestHelper.CompileCS(testClass); - var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); - var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); - operation.ArgumentValue(parameterName).Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(expected); - } - - [DataTestMethod] - [DataRow(""" """)] - [DataRow(""" "param1", "param2" """, "param1", "param2")] - [DataRow(""" null, null """, null, null)] - [DataRow(""" new[] {"param1", "param2"} """, "param1", "param2")] - public void ArgumentValue_Params(string arguments, params string[] expected) - { - var testClass = $$""" - class Test - { - Test(params string[] stringParams) { } - - static void Create() => - new Test({{arguments}}); - } - """; - var (tree, model) = TestHelper.CompileCS(testClass); - var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); - var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); - var argument = operation.ArgumentValue("stringParams").Should().NotBeNull().And.BeAssignableTo().Subject; - var argumentArray = IArrayCreationOperationWrapper.FromOperation(argument); - var result = argumentArray.Initializer.ElementValues.Select(x => x.ConstantValue.Value).ToArray(); - result.Should().BeEquivalentTo(expected); - } -} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs index 51b22500822..510e2a99ab4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/IOperationExtensionsTest.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Operations; using SonarAnalyzer.SymbolicExecution.Roslyn; using StyleCop.Analyzers.Lightup; @@ -95,5 +96,104 @@ public void TrackedSymbol_SimpleAssignment_IsNull() var simpleAssignment = TestHelper.CompileCfgBodyCS("var a = true; bool b; b = a;").Blocks[1].Operations[0]; simpleAssignment.TrackedSymbol().Should().BeNull(); } + + [DataTestMethod] + [DataRow("""1, "Test" """, "intParam", 1)] + [DataRow("""1, "Test" """, "stringParam", "Test")] + [DataRow("""1, "Test" """, "optionalBoolParam", true)] + [DataRow("""1, "Test" """, "optionalIntParam", 1)] + [DataRow("""1, "Test", true """, "optionalBoolParam", true)] + [DataRow("""stringParam: "Test", intParam: 1 """, "stringParam", "Test")] + [DataRow("""stringParam: "Test", intParam: 1, optionalIntParam: 2 """, "optionalIntParam", 2)] + public void ArgumentValue_ObjectCreation(string objectCreationArguments, string parameterName, object expected) + { + var testClass = $$""" + class Test + { + Test(int intParam, string stringParam, bool optionalBoolParam = true, int optionalIntParam = 1) { } + + static void Create() => + new Test({{objectCreationArguments}}); + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); + operation.ArgumentValue(parameterName).Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(expected); + } + + [DataTestMethod] + [DataRow(""" """)] + [DataRow(""" "param1", "param2" """, "param1", "param2")] + [DataRow(""" null, null """, null, null)] + [DataRow(""" new[] {"param1", "param2"} """, "param1", "param2")] + public void ArgumentValue_ObjectCreation_Params(string arguments, params string[] expected) + { + var testClass = $$""" + class Test + { + Test(params string[] stringParams) { } + + static void Create() => + new Test({{arguments}}); + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var objectCreation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IObjectCreationOperationWrapper.FromOperation(model.GetOperation(objectCreation)); + var argument = operation.ArgumentValue("stringParams").Should().NotBeNull().And.BeAssignableTo().Subject; + var argumentArray = IArrayCreationOperationWrapper.FromOperation(argument); + var result = argumentArray.Initializer.ElementValues.Select(x => x.ConstantValue.Value).ToArray(); + result.Should().BeEquivalentTo(expected); + } + + [TestMethod] + public void ArgumentValue_Invocation() + { + const string testClass = """ + class Test + { + static void M(string stringParam) => M("param"); + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var invocation = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocation)); + operation.ArgumentValue("stringParam").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be("param"); + } + + [TestMethod] + public void ArgumentValue_PropertyReference() + { + const string testClass = """ + class Test + { + int this[int index] => this[1]; + } + """; + var (tree, model) = TestHelper.CompileCS(testClass); + var elementAccess = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IPropertyReferenceOperationWrapper.FromOperation(model.GetOperation(elementAccess)); + operation.ArgumentValue("index").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().Be(1); + } + + [TestMethod] + public void ArgumentValue_RaiseEvent() + { + const string testClass = """ + Imports System + Public Class C + Event SomeEvent As EventHandler + Public Sub M() + RaiseEvent SomeEvent(Nothing, EventArgs.Empty) + End Sub + End Class + """; + var (tree, model) = TestHelper.CompileVB(testClass); + var raiseEvent = tree.GetRoot().DescendantNodesAndSelf().OfType().Single(); + var operation = IRaiseEventOperationWrapper.FromOperation(model.GetOperation(raiseEvent)); + operation.ArgumentValue("sender").Should().NotBeNull().And.BeAssignableTo().Which.ConstantValue.Value.Should().BeNull(); + operation.ArgumentValue("e").Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(OperationKindEx.FieldReference); + } } } From 22e1d6ad065a98cabbe171646e0ec0882cfba73b Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 3 Apr 2023 13:16:39 +0200 Subject: [PATCH 15/16] Remove // Noncompliant FP comment --- .../Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs index cb268ad80c6..463861374eb 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Roslyn/LocksReleasedAllPaths.Mutex.NetFx.cs @@ -94,7 +94,7 @@ public static void CompliantFromDocs(string mutexName, MutexSecurity mutexSecuri // Enter the mutex, and hold it until the program exits. try { - m.WaitOne(); // Noncompliant FP + m.WaitOne(); } catch (UnauthorizedAccessException ex) { From 2f034441e0178d1525517bcc6c181eb08d878bbb Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 3 Apr 2023 14:51:58 +0200 Subject: [PATCH 16/16] private ArgumentValue should not be an extension method. --- .../Roslyn/Extensions/IOperationExtensions.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs index 3ab9e4227c1..a8b6aff7ca1 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IOperationExtensions.cs @@ -105,24 +105,24 @@ public static IOperation UnwrapConversion(this IOperation operation) /// public static IOperation ArgumentValue(this IInvocationOperationWrapper invocation, string parameterName) => - invocation.Arguments.ArgumentValue(parameterName); + ArgumentValue(invocation.Arguments, parameterName); /// public static IOperation ArgumentValue(this IObjectCreationOperationWrapper objectCreation, string parameterName) => - objectCreation.Arguments.ArgumentValue(parameterName); + ArgumentValue(objectCreation.Arguments, parameterName); /// public static IOperation ArgumentValue(this IPropertyReferenceOperationWrapper propertyReference, string parameterName) => - propertyReference.Arguments.ArgumentValue(parameterName); + ArgumentValue(propertyReference.Arguments, parameterName); /// public static IOperation ArgumentValue(this IRaiseEventOperationWrapper raiseEvent, string parameterName) => - raiseEvent.Arguments.ArgumentValue(parameterName); + ArgumentValue(raiseEvent.Arguments, parameterName); /// /// Returns the argument value corresponding to . For parameter an IArrayCreationOperation is returned. /// - private static IOperation ArgumentValue(this ImmutableArray arguments, string parameterName) + private static IOperation ArgumentValue(ImmutableArray arguments, string parameterName) { foreach (var operation in arguments) {