From bd9ddabf6ef597e229d59d093b0c0c8e86e7be72 Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Tue, 9 Jul 2024 15:57:24 +0200 Subject: [PATCH 1/7] Add GetInstance tetsts and use the GetInstance method where it makes sense --- .../Roslyn/HardcodedBytesRuleBase.cs | 4 +- ...SecureRandomSeedsShouldNotBePredictable.cs | 4 +- .../OperationProcessors/CollectionTracker.cs | 5 +- .../Roslyn/OperationProcessors/Invocation.cs | 2 +- .../IInvocationOperationExtensionsTest.cs | 63 +++++++++++++++++++ 5 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs index 2c7b7bd0c25..86aa00c5555 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs @@ -61,7 +61,7 @@ protected ProgramState ProcessArraySetValue(ProgramState state, IInvocationOpera { if (invocation.TargetMethod.Name == nameof(Array.SetValue) && invocation.TargetMethod.ContainingType.Is(KnownType.System_Array) - && invocation.Instance.TrackedSymbol(state) is { } array) + && invocation.GetInstance(state).TrackedSymbol(state) is { } array) { return invocation.ArgumentValue("value") is { ConstantValue.HasValue: true } ? state @@ -74,7 +74,7 @@ protected ProgramState ProcessArraySetValue(ProgramState state, IInvocationOpera protected ProgramState ProcessArrayInitialize(ProgramState state, IInvocationOperationWrapper invocation) => invocation.TargetMethod.Name == nameof(Array.Initialize) && invocation.TargetMethod.ContainingType.Is(KnownType.System_Array) - && invocation.Instance.TrackedSymbol(state) is { } array + && invocation.GetInstance(state).TrackedSymbol(state) is { } array ? state.SetSymbolConstraint(array, Hardcoded) : null; diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs index 4187979f60b..621abf19e8d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs @@ -119,7 +119,7 @@ private static ProgramState ProcessSecureRandomGetInstance(ProgramState state, I private static ProgramState ProcessSeedingMethods(ProgramState state, IInvocationOperationWrapper invocation) { return (IsSetSeed() || IsAddSeedMaterial()) - && invocation.Instance.TrackedSymbol(state) is { } instance + && invocation.GetInstance(state).TrackedSymbol(state) is { } instance // If it is already unpredictable, do nothing. // Seeding methods do not overwrite the state, but _mix_ it with the new value. && state[instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true @@ -143,7 +143,7 @@ bool IsAddSeedMaterial() => private ProgramState ProcessNextMethods(ProgramState state, IInvocationOperationWrapper invocation) { if ((IsSecureRandomMethod() || IsRandomGeneratorMethod()) - && invocation.Instance is { } instance + && invocation.GetInstance(state) is { } instance && state[instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true) { ReportIssue(invocation.WrappedOperation); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs index 170188f4dd5..1a8410ea7df 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs @@ -149,7 +149,7 @@ public static ProgramState LearnFrom(ProgramState state, IInvocationOperationWra { return state.SetOperationConstraint(invocation, constraint); } - if (invocation.Instance is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) + if (invocation.GetInstance(state) is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) { var targetMethod = invocation.TargetMethod; var symbolValue = state[instance] ?? SymbolicValue.Empty; @@ -182,8 +182,7 @@ private static NumberConstraint EnumerableCountConstraint(ProgramState state, II { if (invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))) { - var instance = invocation.Instance ?? invocation.Arguments[0].ToArgument().Value; - if (instance.TrackedSymbol(state) is { } symbol + if (invocation.GetInstance(state).TrackedSymbol(state) is { } symbol && state[symbol]?.Constraint() is { } collection) { if (collection == CollectionConstraint.Empty) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs index 677b92022dc..7dc9871f98d 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs @@ -113,7 +113,7 @@ protected override ProgramState[] Process(SymbolicContext context, IInvocationOp if (invocation.TargetMethod.IsExtensionMethod && invocation.TargetMethod.ReducedFrom is { } reducedFrom // VB reduces method symbol to 'instance.Extension()' without annotated ArgumentOperation && reducedFrom.Parameters.First().HasNotNullAttribute() - && invocation.Instance.TrackedSymbol(state) is { } instanceSymbol) + && invocation.GetInstance(state).TrackedSymbol(state) is { } instanceSymbol) { state = state.SetSymbolConstraint(instanceSymbol, ObjectConstraint.NotNull); } diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs new file mode 100644 index 00000000000..beb3f9e164e --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -0,0 +1,63 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2024 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.Test.SymbolicExecution.Roslyn; + +[TestClass] +public class IInvocationOperationExtensionsTest +{ + [DataTestMethod] + [DataRow("public void Method() {}", "sample.Method()", OperationKind.FieldReference)] + [DataRow("", "this.Method()", OperationKind.InstanceReference)] + [DataRow("", "sample.ExtensionMethod()", OperationKind.FieldReference)] + [DataRow("public void Method() {}", "sample.GetSample().Method()", OperationKind.Invocation)] + + public void Invocation_GetInstance_ReturnsSymbol(string definition, string invocation, OperationKind kind) + { + var code = $$""" + class Test + { + Sample sample = new Sample(); + void Method() {} + void M() => {{invocation}}; + } + + public class Sample + { + {{definition}} + public Sample GetSample() => new Sample(); + } + + public static class Extensions + { + public static void ExtensionMethod(this Sample sample) {} + } + """; + var (tree, model) = TestHelper.CompileCS(code); + var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); + var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocationSyntax)); + var d = operation.GetInstance(ProgramState.Empty); + d.Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); + } +} From faf9464c3c58b82d005f8d1a8f7844b718a2f2ea Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Thu, 11 Jul 2024 11:59:59 +0200 Subject: [PATCH 2/7] Empty-Commit for CI From e673c54ab3a81ca1e97b080da9467b4dbc54728e Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Thu, 11 Jul 2024 14:53:02 +0200 Subject: [PATCH 3/7] apply comments --- .../Roslyn/HardcodedBytesRuleBase.cs | 4 +- ...SecureRandomSeedsShouldNotBePredictable.cs | 4 +- .../IInvocationOperationExtensions.cs | 2 +- .../OperationProcessors/CollectionTracker.cs | 4 +- .../Invocation.Enumerable.cs | 4 +- .../Roslyn/OperationProcessors/Invocation.cs | 2 +- ...ptyCollectionsShouldNotBeEnumeratedBase.cs | 2 +- .../IInvocationOperationExtensionsTest.cs | 70 ++++++++++++++----- .../EmptyCollectionsShouldNotBeEnumerated.cs | 17 +++++ .../EmptyCollectionsShouldNotBeEnumerated.vb | 2 +- 10 files changed, 83 insertions(+), 28 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs index 86aa00c5555..2c7b7bd0c25 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/HardcodedBytesRuleBase.cs @@ -61,7 +61,7 @@ protected ProgramState ProcessArraySetValue(ProgramState state, IInvocationOpera { if (invocation.TargetMethod.Name == nameof(Array.SetValue) && invocation.TargetMethod.ContainingType.Is(KnownType.System_Array) - && invocation.GetInstance(state).TrackedSymbol(state) is { } array) + && invocation.Instance.TrackedSymbol(state) is { } array) { return invocation.ArgumentValue("value") is { ConstantValue.HasValue: true } ? state @@ -74,7 +74,7 @@ protected ProgramState ProcessArraySetValue(ProgramState state, IInvocationOpera protected ProgramState ProcessArrayInitialize(ProgramState state, IInvocationOperationWrapper invocation) => invocation.TargetMethod.Name == nameof(Array.Initialize) && invocation.TargetMethod.ContainingType.Is(KnownType.System_Array) - && invocation.GetInstance(state).TrackedSymbol(state) is { } array + && invocation.Instance.TrackedSymbol(state) is { } array ? state.SetSymbolConstraint(array, Hardcoded) : null; diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs index 621abf19e8d..4187979f60b 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs @@ -119,7 +119,7 @@ private static ProgramState ProcessSecureRandomGetInstance(ProgramState state, I private static ProgramState ProcessSeedingMethods(ProgramState state, IInvocationOperationWrapper invocation) { return (IsSetSeed() || IsAddSeedMaterial()) - && invocation.GetInstance(state).TrackedSymbol(state) is { } instance + && invocation.Instance.TrackedSymbol(state) is { } instance // If it is already unpredictable, do nothing. // Seeding methods do not overwrite the state, but _mix_ it with the new value. && state[instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true @@ -143,7 +143,7 @@ bool IsAddSeedMaterial() => private ProgramState ProcessNextMethods(ProgramState state, IInvocationOperationWrapper invocation) { if ((IsSecureRandomMethod() || IsRandomGeneratorMethod()) - && invocation.GetInstance(state) is { } instance + && invocation.Instance is { } instance && state[instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true) { ReportIssue(invocation.WrappedOperation); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs index 749308fd438..ff2b29ef8c5 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs @@ -43,7 +43,7 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation, && !invocation.Arguments.IsEmpty && state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value).Kind == OperationKindEx.InstanceReference); - public static IOperation GetInstance(this IInvocationOperationWrapper invocation, ProgramState state) => + public static IOperation EffectiveInstance(this IInvocationOperationWrapper invocation, ProgramState state) => invocation.Instance ?? (invocation.TargetMethod.IsExtensionMethod ? state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs index 1a8410ea7df..8b66d5b988b 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs @@ -149,7 +149,7 @@ public static ProgramState LearnFrom(ProgramState state, IInvocationOperationWra { return state.SetOperationConstraint(invocation, constraint); } - if (invocation.GetInstance(state) is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) + if (invocation.EffectiveInstance(state) is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) { var targetMethod = invocation.TargetMethod; var symbolValue = state[instance] ?? SymbolicValue.Empty; @@ -182,7 +182,7 @@ private static NumberConstraint EnumerableCountConstraint(ProgramState state, II { if (invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))) { - if (invocation.GetInstance(state).TrackedSymbol(state) is { } symbol + if (invocation.EffectiveInstance(state).TrackedSymbol(state) is { } symbol && state[symbol]?.Constraint() is { } collection) { if (collection == CollectionConstraint.Empty) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs index 224ca090cd5..0377a0d8cf0 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs @@ -98,7 +98,7 @@ private static ProgramState[] ProcessLinqEnumerableAndQueryable(ProgramState sta static IEnumerable ProcessElementOrDefaultMethods(ProgramState state, IInvocationOperationWrapper invocation) { - var constraint = invocation.GetInstance(state).TrackedSymbol(state) is { } instanceSymbol + var constraint = invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol && GetElementType(instanceSymbol) is { } elementType && (elementType.IsReferenceType || elementType.IsNullableValueType()) ? state[instanceSymbol]?.Constraint() @@ -119,7 +119,7 @@ static IEnumerable ProcessElementOrDefaultMethods(ProgramState sta private static ProgramState[] ProcessElementExistsCheckMethods(ProgramState state, IInvocationOperationWrapper invocation) { - if (ElementExistsCheckMethods.Contains(invocation.TargetMethod.Name) && invocation.GetInstance(state).TrackedSymbol(state) is { } instanceSymbol) + if (ElementExistsCheckMethods.Contains(invocation.TargetMethod.Name) && invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol) { return state[instanceSymbol]?.Constraint() switch { diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs index 7dc9871f98d..c23f35acde4 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs @@ -113,7 +113,7 @@ protected override ProgramState[] Process(SymbolicContext context, IInvocationOp if (invocation.TargetMethod.IsExtensionMethod && invocation.TargetMethod.ReducedFrom is { } reducedFrom // VB reduces method symbol to 'instance.Extension()' without annotated ArgumentOperation && reducedFrom.Parameters.First().HasNotNullAttribute() - && invocation.GetInstance(state).TrackedSymbol(state) is { } instanceSymbol) + && invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol) { state = state.SetSymbolConstraint(instanceSymbol, ObjectConstraint.NotNull); } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs index fc2bdda8532..9be80460501 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs @@ -97,7 +97,7 @@ public override void ExecutionCompleted() protected override ProgramState PreProcessSimple(SymbolicContext context) { if (context.Operation.Instance.AsInvocation() is { } invocation - && invocation.Instance is { } instance + && invocation.EffectiveInstance(context.State) is { } instance && RaisingMethods.Contains(invocation.TargetMethod.Name)) { if (context.State[instance]?.HasConstraint(CollectionConstraint.Empty) is true) diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs index beb3f9e164e..25b61f7ff28 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -18,9 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis.CSharp.Syntax; using SonarAnalyzer.SymbolicExecution.Roslyn; using StyleCop.Analyzers.Lightup; +using SyntaxCS = Microsoft.CodeAnalysis.CSharp.Syntax; +using SyntaxVB = Microsoft.CodeAnalysis.VisualBasic.Syntax; namespace SonarAnalyzer.Test.SymbolicExecution.Roslyn; @@ -28,24 +29,23 @@ namespace SonarAnalyzer.Test.SymbolicExecution.Roslyn; public class IInvocationOperationExtensionsTest { [DataTestMethod] - [DataRow("public void Method() {}", "sample.Method()", OperationKind.FieldReference)] - [DataRow("", "this.Method()", OperationKind.InstanceReference)] - [DataRow("", "sample.ExtensionMethod()", OperationKind.FieldReference)] - [DataRow("public void Method() {}", "sample.GetSample().Method()", OperationKind.Invocation)] + [DataRow("sample.Method()", OperationKind.FieldReference)] + [DataRow("this.Method()", OperationKind.InstanceReference)] + [DataRow("sample.ExtensionMethod()", OperationKind.FieldReference)] + [DataRow("sample.GetSample().Method()", OperationKind.Invocation)] - public void Invocation_GetInstance_ReturnsSymbol(string definition, string invocation, OperationKind kind) + public void Invocation_GetInstance_ReturnsSymbol_CS(string invocation, OperationKind kind) { var code = $$""" - class Test - { - Sample sample = new Sample(); - void Method() {} - void M() => {{invocation}}; - } public class Sample { - {{definition}} + Sample sample = new Sample(); + + public void Method() {} + + void M() => {{invocation}}; + public Sample GetSample() => new Sample(); } @@ -55,9 +55,47 @@ public static void ExtensionMethod(this Sample sample) {} } """; var (tree, model) = TestHelper.CompileCS(code); - var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); + var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); + var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocationSyntax)); + operation.EffectiveInstance(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); + } + + [DataTestMethod] + [DataRow("sample.Method()", OperationKind.FieldReference)] + [DataRow("Me.Method()", OperationKind.InstanceReference)] + [DataRow("sample.ExtensionMethod()", OperationKind.FieldReference)] + [DataRow("sample.GetSample().Method()", OperationKind.Invocation)] + + public void Invocation_GetInstance_ReturnsSymbol_VB(string invocation, OperationKind kind) + { + var code = $$""" + Public Class Sample + Private sample As Sample = New Sample() + + Public Sub Method() + End Sub + + Private Sub M() + {{invocation}} + End Sub + + Public Function GetSample() As Sample + Return New Sample() + End Function + + End Class + + Public Module Extensions + + + Public Sub ExtensionMethod(sample As Sample) + End Sub + + End Module + """; + var (tree, model) = TestHelper.CompileVB(code); + var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocationSyntax)); - var d = operation.GetInstance(ProgramState.Empty); - d.Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); + operation.EffectiveInstance(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs index 015130ac6aa..860fd8da343 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.cs @@ -1001,6 +1001,23 @@ private static int GetChar(string s, int v1, int v2) } } +namespace Extensions +{ + class ExtensionMethods + { + public void Test() + { + var list = new List(); + list.Clear(3); // Noncompliant + } + } + + public static class Extensions + { + public static void Clear(this List list, int i) { } + } +} + // See https://github.com/SonarSource/sonar-dotnet/issues/1002 class Program { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb index 5b6ba2c3ad6..770fc7cc6a2 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SymbolicExecution/Roslyn/EmptyCollectionsShouldNotBeEnumerated.vb @@ -264,7 +264,7 @@ Public Class AdvancedTests Dim List As New List(Of Integer) List.All(Function(X) True) ' FN List.Any() ' FN - Enumerable.Reverse(List) ' FN + Enumerable.Reverse(List) ' Noncompliant List.Clear() ' Noncompliant End Sub From 09a5a4af3b108efe86bc267451b9895cbbdbaa9d Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Thu, 11 Jul 2024 16:18:04 +0200 Subject: [PATCH 4/7] call instance property as method makes no sense in VB context --- .../SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs index c23f35acde4..677b92022dc 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.cs @@ -113,7 +113,7 @@ protected override ProgramState[] Process(SymbolicContext context, IInvocationOp if (invocation.TargetMethod.IsExtensionMethod && invocation.TargetMethod.ReducedFrom is { } reducedFrom // VB reduces method symbol to 'instance.Extension()' without annotated ArgumentOperation && reducedFrom.Parameters.First().HasNotNullAttribute() - && invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol) + && invocation.Instance.TrackedSymbol(state) is { } instanceSymbol) { state = state.SetSymbolConstraint(instanceSymbol, ObjectConstraint.NotNull); } From 1c95755743d67f4d1158cee2a842cb0c3f1d847a Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Thu, 11 Jul 2024 17:13:57 +0200 Subject: [PATCH 5/7] Rename extension method --- .../Roslyn/Extensions/IInvocationOperationExtensions.cs | 2 +- .../Roslyn/OperationProcessors/CollectionTracker.cs | 4 ++-- .../Roslyn/OperationProcessors/Invocation.Enumerable.cs | 4 ++-- .../RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs | 2 +- .../Roslyn/IInvocationOperationExtensionsTest.cs | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs index ff2b29ef8c5..a5cc6a13b66 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/Extensions/IInvocationOperationExtensions.cs @@ -43,7 +43,7 @@ public static bool HasThisReceiver(this IInvocationOperationWrapper invocation, && !invocation.Arguments.IsEmpty && state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value).Kind == OperationKindEx.InstanceReference); - public static IOperation EffectiveInstance(this IInvocationOperationWrapper invocation, ProgramState state) => + public static IOperation Target(this IInvocationOperationWrapper invocation, ProgramState state) => invocation.Instance ?? (invocation.TargetMethod.IsExtensionMethod ? state.ResolveCaptureAndUnwrapConversion(invocation.Arguments[0].ToArgument().Value) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs index 8b66d5b988b..d2a4c2bfaee 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/CollectionTracker.cs @@ -149,7 +149,7 @@ public static ProgramState LearnFrom(ProgramState state, IInvocationOperationWra { return state.SetOperationConstraint(invocation, constraint); } - if (invocation.EffectiveInstance(state) is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) + if (invocation.Target(state) is { } instance && instance.Type.DerivesOrImplementsAny(CollectionTypes)) { var targetMethod = invocation.TargetMethod; var symbolValue = state[instance] ?? SymbolicValue.Empty; @@ -182,7 +182,7 @@ private static NumberConstraint EnumerableCountConstraint(ProgramState state, II { if (invocation.TargetMethod.Is(KnownType.System_Linq_Enumerable, nameof(Enumerable.Count))) { - if (invocation.EffectiveInstance(state).TrackedSymbol(state) is { } symbol + if (invocation.Target(state).TrackedSymbol(state) is { } symbol && state[symbol]?.Constraint() is { } collection) { if (collection == CollectionConstraint.Empty) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs index 0377a0d8cf0..95704728886 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/OperationProcessors/Invocation.Enumerable.cs @@ -98,7 +98,7 @@ private static ProgramState[] ProcessLinqEnumerableAndQueryable(ProgramState sta static IEnumerable ProcessElementOrDefaultMethods(ProgramState state, IInvocationOperationWrapper invocation) { - var constraint = invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol + var constraint = invocation.Target(state).TrackedSymbol(state) is { } instanceSymbol && GetElementType(instanceSymbol) is { } elementType && (elementType.IsReferenceType || elementType.IsNullableValueType()) ? state[instanceSymbol]?.Constraint() @@ -119,7 +119,7 @@ static IEnumerable ProcessElementOrDefaultMethods(ProgramState sta private static ProgramState[] ProcessElementExistsCheckMethods(ProgramState state, IInvocationOperationWrapper invocation) { - if (ElementExistsCheckMethods.Contains(invocation.TargetMethod.Name) && invocation.EffectiveInstance(state).TrackedSymbol(state) is { } instanceSymbol) + if (ElementExistsCheckMethods.Contains(invocation.TargetMethod.Name) && invocation.Target(state).TrackedSymbol(state) is { } instanceSymbol) { return state[instanceSymbol]?.Constraint() switch { diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs index 9be80460501..499d8a3fcf8 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/EmptyCollectionsShouldNotBeEnumeratedBase.cs @@ -97,7 +97,7 @@ public override void ExecutionCompleted() protected override ProgramState PreProcessSimple(SymbolicContext context) { if (context.Operation.Instance.AsInvocation() is { } invocation - && invocation.EffectiveInstance(context.State) is { } instance + && invocation.Target(context.State) is { } instance && RaisingMethods.Contains(invocation.TargetMethod.Name)) { if (context.State[instance]?.HasConstraint(CollectionConstraint.Empty) is true) diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs index 25b61f7ff28..7578cea74d7 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/IInvocationOperationExtensionsTest.cs @@ -57,7 +57,7 @@ public static void ExtensionMethod(this Sample sample) {} var (tree, model) = TestHelper.CompileCS(code); var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocationSyntax)); - operation.EffectiveInstance(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); + operation.Target(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); } [DataTestMethod] @@ -96,6 +96,6 @@ End Module var (tree, model) = TestHelper.CompileVB(code); var invocationSyntax = tree.GetRoot().DescendantNodesAndSelf().OfType().First(); var operation = IInvocationOperationWrapper.FromOperation(model.GetOperation(invocationSyntax)); - operation.EffectiveInstance(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); + operation.Target(ProgramState.Empty).Should().NotBeNull().And.BeAssignableTo().Which.Kind.Should().Be(kind); } } From 56271438356ee5b0831881d1a3447f029b4b1eb2 Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Fri, 12 Jul 2024 10:40:01 +0200 Subject: [PATCH 6/7] add UT --- .../RoslynSymbolicExecutionTest.Invocation.Enumerable.cs | 4 +++- .../TestFramework/SymbolicExecution/SETestContext.cs | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.Enumerable.cs b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.Enumerable.cs index 7614619df1c..b3d31178bff 100644 --- a/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.Enumerable.cs +++ b/analyzers/tests/SonarAnalyzer.Test/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Invocation.Enumerable.cs @@ -711,7 +711,8 @@ public void Invocation_CollectionMethods_SetCollectionConstraint() list.RemoveAt(0); tag = "remove"; - void Invoke(Action action) { } + list.Add(1, 2); // Extension method + tag = "addExtension"; """; var validator = SETestContext.CreateCS(code, "List list", new PreserveTestCheck("list")).Validator; validator.ValidateContainsOperation(OperationKind.Invocation); @@ -720,6 +721,7 @@ void Invoke(Action action) { } Verify("clear", ObjectConstraint.NotNull, CollectionConstraint.Empty); Verify("add", ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); Verify("remove", ObjectConstraint.NotNull); + Verify("addExtension", ObjectConstraint.NotNull, CollectionConstraint.NotEmpty); void Verify(string state, params SymbolicConstraint[] constraints) => validator.TagValue(state, "list").Should().HaveOnlyConstraints(constraints); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs index 1437522d721..1b2311dd566 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs @@ -183,6 +183,11 @@ public static void Tag(string name, object arg = null) {{ }} public static void Tag(this object o, string name) {{ }} public static T Unknown() => default; }} + +public static class ExtensionMethods +{{ + public static void Add(this List list, int a, int b) {{ }} +}} "; private static string PrependComma(string additionalParameters) => From e2cb15fd8042b73ec59d5f8e977f4bf8fa1305b5 Mon Sep 17 00:00:00 2001 From: mary-georgiou-sonarsource Date: Fri, 12 Jul 2024 10:52:36 +0200 Subject: [PATCH 7/7] cleanup --- .../SymbolicExecution/SETestContext.cs | 211 +++++++++--------- 1 file changed, 106 insertions(+), 105 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs index 1b2311dd566..ae56ef557a6 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestFramework/SymbolicExecution/SETestContext.cs @@ -60,135 +60,136 @@ public static SETestContext CreateCSLambda(string methodBody, string lambdaFragm new(ClassCodeCS(methodBody, null), AnalyzerLanguage.CSharp, additionalChecks, null, lambdaFragment); public static SETestContext CreateCSMethod(string method, params SymbolicCheck[] additionalChecks) => - new($@" -using System; + new($$""" + using System; -public class Sample -{{ - {method} + public class Sample + { + {{method}} - private static void Tag(string name) {{ }} - private static void Tag(string name, object arg) {{ }} - private static T Unknown() => default; -}} + private static void Tag(string name) { } + private static void Tag(string name, object arg) { } + private static T Unknown() => default; + } -public class Deconstructable -{{ - public void Deconstruct(out int A, out int B) {{ A = 1; B = 2; }} -}}", AnalyzerLanguage.CSharp, additionalChecks); + public class Deconstructable + { + public void Deconstruct(out int A, out int B) { A = 1; B = 2; } + } + """, AnalyzerLanguage.CSharp, additionalChecks); public static SETestContext CreateVB(string methodBody, params SymbolicCheck[] additionalChecks) => CreateVB(methodBody, null, additionalChecks); public static SETestContext CreateVB(string methodBody, string additionalParameters, params SymbolicCheck[] additionalChecks) { - var code = $@" -Public Class Sample + var code = $""" + Public Class Sample - Private Readonly Property Condition As Boolean - Get - Return Environment.ProcessorCount = 42 ' Something that cannot have constraint - End Get - End Property + Private Readonly Property Condition As Boolean + Get + Return Environment.ProcessorCount = 42 ' Something that cannot have constraint + End Get + End Property - Private FieldArray() As Integer + Private FieldArray() As Integer - Public Sub Main(BoolParameter As Boolean{PrependComma(additionalParameters)}) - {methodBody} - End Sub + Public Sub Main(BoolParameter As Boolean{PrependComma(additionalParameters)}) + {methodBody} + End Sub - Private Shared Sub Tag(Name As String, Optional Arg As Object = Nothing) - End Sub + Private Shared Sub Tag(Name As String, Optional Arg As Object = Nothing) + End Sub - Private Shared Function Unknown(Of T)() As T - End Function + Private Shared Function Unknown(Of T)() As T + End Function -End Class"; + End Class + """; return new(code, AnalyzerLanguage.VisualBasic, additionalChecks); } public static SETestContext CreateVBMethod(string method, params SymbolicCheck[] additionalChecks) => - new($@" -Public Class Sample + new($""" + Public Class Sample - {method} + {method} - Private Shared Sub Tag(Name As String, Optional Arg As Object = Nothing) - End Sub + Private Shared Sub Tag(Name As String, Optional Arg As Object = Nothing) + End Sub -End Class", AnalyzerLanguage.VisualBasic, additionalChecks); + End Class + """, AnalyzerLanguage.VisualBasic, additionalChecks); private static string ClassCodeCS(string methodBody, string additionalParameters) => - $@" -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; - -using static Tagger; - -public unsafe class Sample -{{ - public object ObjectField; - public static int StaticField; - public static object StaticObjectField; - public static int StaticProperty {{ get; set; }} - public static event EventHandler StaticEvent; - public event EventHandler Event; - public int Property {{ get; set; }} - public Sample SampleProperty {{ get; set; }} - public int? NullableProperty {{ get; set; }} - public object AutoProperty {{ get; set; }} - public object FullProperty {{ get => ObjectField; set => ObjectField = value; }} - public static object StaticAutoProperty {{ get; set; }} - public static object StaticFullProperty {{ get => StaticObjectField; set => StaticObjectField = value; }} - public NotImplementedException PropertyException {{ get; set; }} - public int this[int index] {{get => 42; set {{ }} }} - private int field; - private NotImplementedException fieldException; - - private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint - - public Sample(){{ }} - public Sample(int i){{ }} - - public void Main(bool boolParameter{PrependComma(additionalParameters)}) - {{ - {methodBody} - }} - - public NotImplementedException CreateException() => new NotImplementedException(); - public void InstanceMethod(object parameter = null) {{ }} - public void InstanceMethodWithRefParam(ref object parameter) {{ }} - public void InstanceMethodWithOutParam(out object parameter) {{ parameter = null; }} - public static void StaticMethod() {{ }} -}} - -public class Person : PersonBase -{{ - public static string StaticProperty {{ get; set; }} - public string Field; - public event EventHandler Event; - public string Method() => null; - public static void StaticMethod() {{ }} -}} - -public class PersonBase -{{ -}} - -public static class Tagger -{{ - public static void Tag(string name, object arg = null) {{ }} - public static void Tag(this object o, string name) {{ }} - public static T Unknown() => default; -}} - -public static class ExtensionMethods -{{ - public static void Add(this List list, int a, int b) {{ }} -}} -"; + $$""" + using System; + using System.Collections.Generic; + using System.Diagnostics; + using System.Linq; + + using static Tagger; + + public unsafe class Sample + { + public object ObjectField; + public static int StaticField; + public static object StaticObjectField; + public static int StaticProperty { get; set; } + public static event EventHandler StaticEvent; + public event EventHandler Event; + public int Property { get; set; } + public Sample SampleProperty { get; set; } + public int? NullableProperty { get; set; } + public object AutoProperty { get; set; } + public object FullProperty { get => ObjectField; set => ObjectField = value; } + public static object StaticAutoProperty { get; set; } + public static object StaticFullProperty { get => StaticObjectField; set => StaticObjectField = value; } + public NotImplementedException PropertyException { get; set; } + public int this[int index] {get => 42; set { } } + private int field; + private NotImplementedException fieldException; + + private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint + + public Sample(){ } + public Sample(int i){ } + + public void Main(bool boolParameter{{PrependComma(additionalParameters)}}) + { + {{methodBody}} + } + + public NotImplementedException CreateException() => new NotImplementedException(); + public void InstanceMethod(object parameter = null) { } + public void InstanceMethodWithRefParam(ref object parameter) { } + public void InstanceMethodWithOutParam(out object parameter) { parameter = null; } + public static void StaticMethod() { } + } + + public class Person : PersonBase + { + public static string StaticProperty { get; set; } + public string Field; + public event EventHandler Event; + public string Method() => null; + public static void StaticMethod() { } + } + + public class PersonBase { } + + public static class Tagger + { + public static void Tag(string name, object arg = null) { } + public static void Tag(this object o, string name) { } + public static T Unknown() => default; + } + + public static class ExtensionMethods + { + public static void Add(this List list, int a, int b) { } + } + """; private static string PrependComma(string additionalParameters) => additionalParameters is null ? null : ", " + additionalParameters;