Skip to content

Commit

Permalink
S3900: Dummy rule implementation (#6917)
Browse files Browse the repository at this point in the history
  • Loading branch information
zsolt-kolbay-sonarsource authored and pavel-mikula-sonarsource committed Mar 28, 2023
1 parent 3e26197 commit 0b12830
Show file tree
Hide file tree
Showing 5 changed files with 293 additions and 224 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public class SymbolicExecutionRunner : SymbolicExecutionRunnerBase
// ToDo: This should be migrated to SymbolicExecutionRunnerBase.AllRules.
private static readonly ImmutableArray<ISymbolicExecutionAnalyzer> SonarRules = ImmutableArray.Create<ISymbolicExecutionAnalyzer>(
new SonarRules.ObjectsShouldNotBeDisposedMoreThanOnce(),
new SonarRules.PublicMethodArgumentsShouldBeCheckedForNull(),
new SonarRules.EmptyCollectionsShouldNotBeEnumerated(),
new SonarRules.ConditionEvaluatesToConstant(),
new SonarRules.InvalidCastToInterfaceSymbolicExecution(),
Expand All @@ -49,7 +48,8 @@ public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { }
protected override ImmutableDictionary<DiagnosticDescriptor, RuleFactory> AllRules { get; } = ImmutableDictionary<DiagnosticDescriptor, RuleFactory>.Empty
.Add(LocksReleasedAllPaths.S2222, CreateFactory<LocksReleasedAllPaths>())
.Add(NullPointerDereference.S2259, CreateFactory<NullPointerDereference, SonarRules.NullPointerDereference>())
.Add(EmptyNullableValueAccess.S3655, CreateFactory<EmptyNullableValueAccess, SonarRules.EmptyNullableValueAccess>());
.Add(EmptyNullableValueAccess.S3655, CreateFactory<EmptyNullableValueAccess, SonarRules.EmptyNullableValueAccess>())
.Add(PublicMethodArgumentsShouldBeCheckedForNull.S3900, CreateFactory<PublicMethodArgumentsShouldBeCheckedForNull, SonarRules.PublicMethodArgumentsShouldBeCheckedForNull>());

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => base.SupportedDiagnostics.Concat(SonarRules.SelectMany(x => x.SupportedDiagnostics)).ToImmutableArray();

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<SymbolicExecutionRunner>().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

}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Loading

0 comments on commit 0b12830

Please sign in to comment.