diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs index b9b4b854dc2..1ab29d7d498 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs @@ -34,7 +34,6 @@ public class SymbolicExecutionRunner : SymbolicExecutionRunnerBase // ToDo: This should be migrated to SymbolicExecutionRunnerBase.AllRules. private static readonly ImmutableArray SonarRules = ImmutableArray.Create( new SonarRules.ObjectsShouldNotBeDisposedMoreThanOnce(), - new SonarRules.PublicMethodArgumentsShouldBeCheckedForNull(), new SonarRules.EmptyCollectionsShouldNotBeEnumerated(), new SonarRules.ConditionEvaluatesToConstant(), new SonarRules.InvalidCastToInterfaceSymbolicExecution(), @@ -49,7 +48,8 @@ public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { } protected override ImmutableDictionary AllRules { get; } = ImmutableDictionary.Empty .Add(LocksReleasedAllPaths.S2222, CreateFactory()) .Add(NullPointerDereference.S2259, CreateFactory()) - .Add(EmptyNullableValueAccess.S3655, CreateFactory()); + .Add(EmptyNullableValueAccess.S3655, CreateFactory()) + .Add(PublicMethodArgumentsShouldBeCheckedForNull.S3900, CreateFactory()); public override ImmutableArray SupportedDiagnostics => base.SupportedDiagnostics.Concat(SonarRules.SelectMany(x => x.SupportedDiagnostics)).ToImmutableArray(); diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.cs new file mode 100644 index 00000000000..c618337cc7e --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/PublicMethodArgumentsShouldBeCheckedForNull.cs @@ -0,0 +1,33 @@ +/* + * 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.RuleChecks.CSharp; + +public class PublicMethodArgumentsShouldBeCheckedForNull : SymbolicRuleCheck +{ + private const string DiagnosticId = "S3900"; + private const string MessageFormat = "Refactor this method to add validation of parameter '{0}' before using it."; + + internal static readonly DiagnosticDescriptor S3900 = DescriptorFactory.Create(DiagnosticId, MessageFormat); + + protected override DiagnosticDescriptor Rule => S3900; + + public override bool ShouldExecute() => false; +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/PublicMethodArgumentsShouldBeCheckedForNullTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/PublicMethodArgumentsShouldBeCheckedForNullTest.cs index e1be0f1ea1d..ad5de0f76d0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/PublicMethodArgumentsShouldBeCheckedForNullTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/SymbolicExecution/PublicMethodArgumentsShouldBeCheckedForNullTest.cs @@ -18,41 +18,81 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using SonarAnalyzer.Rules.CSharp; -using SonarAnalyzer.SymbolicExecution.Sonar.Analyzers; +using SonarAnalyzer.Common; +using ChecksCS = SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks.CSharp; +using CS = SonarAnalyzer.Rules.CSharp; namespace SonarAnalyzer.UnitTest.Rules { [TestClass] public class PublicMethodArgumentsShouldBeCheckedForNullTest { - private readonly VerifierBuilder sonar = new VerifierBuilder().WithBasePath(@"SymbolicExecution\Sonar") - .WithOnlyDiagnostics(PublicMethodArgumentsShouldBeCheckedForNull.S3900); + private readonly VerifierBuilder sonar = new VerifierBuilder() + .AddAnalyzer(() => new CS.SymbolicExecutionRunner(AnalyzerConfiguration.AlwaysEnabledWithSonarCfg)) + .WithBasePath(@"SymbolicExecution\Sonar") + .WithOnlyDiagnostics(ChecksCS.PublicMethodArgumentsShouldBeCheckedForNull.S3900); + + private readonly VerifierBuilder roslynCS = new VerifierBuilder() + .AddAnalyzer(() => new CS.SymbolicExecutionRunner(AnalyzerConfiguration.AlwaysEnabled)) + .WithBasePath(@"SymbolicExecution\Roslyn") + .WithOnlyDiagnostics(ChecksCS.PublicMethodArgumentsShouldBeCheckedForNull.S3900); [DataTestMethod] [DataRow(ProjectType.Product)] [DataRow(ProjectType.Test)] - public void PublicMethodArgumentsShouldBeCheckedForNull_CS(ProjectType projectType) => - sonar.AddReferences(TestHelper.ProjectTypeReference(projectType).Concat(MetadataReferenceFacade.NETStandard21)) + public void PublicMethodArgumentsShouldBeCheckedForNull_Sonar_CS(ProjectType projectType) => + sonar.AddReferences(TestHelper.ProjectTypeReference(projectType)) + .AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.cs") + .Verify(); + + [DataTestMethod] + [DataRow(ProjectType.Product)] + [DataRow(ProjectType.Test)] + public void PublicMethodArgumentsShouldBeCheckedForNull_Roslyn_CS(ProjectType projectType) => + roslynCS.AddReferences(TestHelper.ProjectTypeReference(projectType)) .AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.cs") - .WithOptions(ParseOptionsHelper.FromCSharp8) .Verify(); #if NET [TestMethod] - public void PublicMethodArgumentsShouldBeCheckedForNull_CSharp9() => + public void PublicMethodArgumentsShouldBeCheckedForNull_Sonar_CSharp8() => + sonar.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs") + .WithOptions(ParseOptionsHelper.FromCSharp8) + .Verify(); + + [TestMethod] + public void PublicMethodArgumentsShouldBeCheckedForNull_Roslyn_CSharp8() => + roslynCS.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs") + .WithOptions(ParseOptionsHelper.FromCSharp8) + .Verify(); + + [TestMethod] + public void PublicMethodArgumentsShouldBeCheckedForNull_Sonar_CSharp9() => sonar.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp9.cs") .AddReferences(NuGetMetadataReference.MicrosoftAspNetCoreMvcCore(Constants.NuGetLatestVersion)) .WithOptions(ParseOptionsHelper.FromCSharp9) .Verify(); [TestMethod] - public void PublicMethodArgumentsShouldBeCheckedForNull_CSharp11() => + public void PublicMethodArgumentsShouldBeCheckedForNull_Roslyn_CSharp9() => + roslynCS.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp9.cs") + .AddReferences(NuGetMetadataReference.MicrosoftAspNetCoreMvcCore(Constants.NuGetLatestVersion)) + .WithOptions(ParseOptionsHelper.FromCSharp9) + .Verify(); + + [TestMethod] + public void PublicMethodArgumentsShouldBeCheckedForNull_Sonar_CSharp11() => sonar.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp11.cs") .WithOptions(ParseOptionsHelper.FromCSharp11) .Verify(); + [TestMethod] + public void PublicMethodArgumentsShouldBeCheckedForNull_Roslyn_CSharp11() => + roslynCS.AddPaths("PublicMethodArgumentsShouldBeCheckedForNull.CSharp11.cs") + .WithOptions(ParseOptionsHelper.FromCSharp11) + .Verify(); + #endif } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs new file mode 100644 index 00000000000..7c80fd68e1b --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.CSharp8.cs @@ -0,0 +1,209 @@ +public class NullCoalescenceAssignment +{ + public void NullCoalescenceAssignment_NotNull(string s1) + { + s1 ??= "N/A"; + s1.ToString(); // Compliant + } + + public void NullCoalescenceAssignment_Null(string s1) + { + s1 ??= null; + s1.ToString(); // Noncompliant + } + + public void InsideIf(string str) + { + if (str != null) + { + str ??= null; + str.ToString(); // Compliant, we know str is not null + } + + if (str == null) + { + str ??= "foo"; + str.ToString(); // Compliant, assigned foo + } + } +} + +public interface IWithDefaultMembers +{ + decimal Count { get; set; } + decimal Price { get; set; } + + void Reset(string s) + { + s.ToString(); // Noncompliant + } +} + +public class LocalStaticFunctions +{ + public void Method(object arg) + { + string LocalFunction(object o) + { + return o.ToString(); // Compliant - FN: local functions are not supported by the CFG + } + + static string LocalStaticFunction(object o) + { + return o.ToString(); // Compliant - FN: local functions are not supported by the CFG + } + } +} + +public class Address +{ + public string Name { get; } + + public string State { get; } + + public void Deconstruct(out string name, out string state) => + (name, state) = (Name, State); +} + +public class Person +{ + public string Name { get; } + + public Address Address { get; } + + public void Deconstruct(out string name, out Address address) => + (name, address) = (Name, Address); +} + +public class SwitchExpressions +{ + public void OnlyDiscardBranch_Noncompliant(string s, bool b) + { + var result = b switch + { + _ => s.ToString() // Noncompliant + }; + } + + public void MultipleBranches_Noncompliant(string s, int val) + { + var result = val switch + { + 1 => "a", + 2 => s.ToString(), // Noncompliant + _ => "b" + }; + } + + public void Nested_Noncompliant(string s, int val, bool condition) + { + var result = val switch + { + 1 => "a", + 2 => condition switch + { + _ => s.ToString() // Noncompliant + }, + _ => "b" + }; + } + + public void MultipleBranches_HandleNull(string s, int val) + { + var result = s switch + { + null => s.ToString(), // Noncompliant + _ => s.ToString() // Compliant as null was already handled + }; + } + + public void MultipleBranches_Compliant(string s, int val) + { + var result = val switch + { + 1 => "a", + 2 => s == null ? string.Empty : s.ToString(), + _ => "b" + }; + } + + public string MultipleBranches_PropertyPattern(Address address, string s) + { + return address switch + { + { State: "WA" } addr => s.ToString(), // Noncompliant + _ => string.Empty + }; + } + + public string MultipleBranches_PropertyPattern_FP(string s) + { + return s switch + { + { Length: 5 } => s.ToString(), // Noncompliant - FP we know that the length is 5 so the string cannot be null + _ => string.Empty + }; + } + + public string MultipleBranches_RecursivePattern(Person person, string s) + { + return person switch + { + { Address: { State: "WA" } } pers => s.ToString(), // Noncompliant + _ => string.Empty + }; + } + + public string MultipleBranches_TuplePattern(Address address, string s) + { + return address switch + { + var (name, state) => s.ToString(), // Compliant - FN + _ => string.Empty + }; + } + + public string MultipleBranches_WhenClause(Address address, string s) + { + return address switch + { + Address addr when addr.Name.Length > 0 => s.ToString(), // Noncompliant + Address addr when addr.Name.Length == s.Length => string.Empty, // Noncompliant + _ => string.Empty + }; + } + + public string MultipleBranches_VarDeclaration(Address address, string s) + { + return address switch + { + Address addr => s.ToString(), // Noncompliant + _ => string.Empty + }; + } + + public string TwoBranches_NoDefault(bool condition, string s) + { + return condition switch + { + true => s.ToString(), // Noncompliant + false => s.ToString() // Noncompliant + }; + } +} + +public class SwitchStatement +{ + public void Test(string s) + { + switch (s) + { + case null: + break; + + default: + s.ToString(); // Compliant - the null is handled by the case null branch. + break; + } + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.cs index 3d02195aa44..b309cd589ae 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/SymbolicExecution/Sonar/PublicMethodArgumentsShouldBeCheckedForNull.cs @@ -360,219 +360,6 @@ public static string SanitiseIMDbId(string imdbId) } } -namespace CSharp8 -{ - public class NullCoalescenceAssignment - { - public void NullCoalescenceAssignment_NotNull(string s1) - { - s1 ??= "N/A"; - s1.ToString(); // Compliant - } - - public void NullCoalescenceAssignment_Null(string s1) - { - s1 ??= null; - s1.ToString(); // Noncompliant - } - - public void InsideIf(string str) - { - if (str != null) - { - str ??= null; - str.ToString(); // Compliant, we know str is not null - } - - if (str == null) - { - str ??= "foo"; - str.ToString(); // Compliant, assigned foo - } - } - } - - public interface IWithDefaultMembers - { - decimal Count { get; set; } - decimal Price { get; set; } - - void Reset(string s) - { - s.ToString(); // Noncompliant - } - } - - public class LocalStaticFunctions - { - public void Method(object arg) - { - string LocalFunction(object o) - { - return o.ToString(); // Compliant - FN: local functions are not supported by the CFG - } - - static string LocalStaticFunction(object o) - { - return o.ToString(); // Compliant - FN: local functions are not supported by the CFG - } - } - } - - public class Address - { - public string Name { get; } - - public string State { get; } - - public void Deconstruct(out string name, out string state) => - (name, state) = (Name, State); - } - - public class Person - { - public string Name { get; } - - public Address Address { get; } - - public void Deconstruct(out string name, out Address address) => - (name, address) = (Name, Address); - } - - public class SwitchExpressions - { - public void OnlyDiscardBranch_Noncompliant(string s, bool b) - { - var result = b switch - { - _ => s.ToString() // Noncompliant - }; - } - - public void MultipleBranches_Noncompliant(string s, int val) - { - var result = val switch - { - 1 => "a", - 2 => s.ToString(), // Noncompliant - _ => "b" - }; - } - - public void Nested_Noncompliant(string s, int val, bool condition) - { - var result = val switch - { - 1 => "a", - 2 => condition switch - { - _ => s.ToString() // Noncompliant - }, - _ => "b" - }; - } - - public void MultipleBranches_HandleNull(string s, int val) - { - var result = s switch - { - null => s.ToString(), // Noncompliant - _ => s.ToString() // Compliant as null was already handled - }; - } - - public void MultipleBranches_Compliant(string s, int val) - { - var result = val switch - { - 1 => "a", - 2 => s == null ? string.Empty : s.ToString(), - _ => "b" - }; - } - - public string MultipleBranches_PropertyPattern(Address address, string s) - { - return address switch - { - { State: "WA" } addr => s.ToString(), // Noncompliant - _ => string.Empty - }; - } - - public string MultipleBranches_PropertyPattern_FP(string s) - { - return s switch - { - { Length: 5 } => s.ToString(), // Noncompliant - FP we know that the length is 5 so the string cannot be null - _ => string.Empty - }; - } - - public string MultipleBranches_RecursivePattern(Person person, string s) - { - return person switch - { - { Address: { State: "WA" } } pers => s.ToString(), // Noncompliant - _ => string.Empty - }; - } - - public string MultipleBranches_TuplePattern(Address address, string s) - { - return address switch - { - var (name, state) => s.ToString(), // Compliant - FN - _ => string.Empty - }; - } - - public string MultipleBranches_WhenClause(Address address, string s) - { - return address switch - { - Address addr when addr.Name.Length > 0 => s.ToString(), // Noncompliant - Address addr when addr.Name.Length == s.Length => string.Empty, // Noncompliant - _ => string.Empty - }; - } - - public string MultipleBranches_VarDeclaration(Address address, string s) - { - return address switch - { - Address addr => s.ToString(), // Noncompliant - _ => string.Empty - }; - } - - public string TwoBranches_NoDefault(bool condition, string s) - { - return condition switch - { - true => s.ToString(), // Noncompliant - false => s.ToString() // Noncompliant - }; - } - } - - public class SwitchStatement - { - public void Test(string s) - { - switch (s) - { - case null: - break; - - default: - s.ToString(); // Compliant - the null is handled by the case null branch. - break; - } - } - } -} - // https://github.com/SonarSource/sonar-dotnet/issues/3400 namespace Repro_3400 {