From 9b87b4155c9f5c9fc89b58946e5e5b873e372733 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini <114916336+cristian-ambrosini-sonarsource@users.noreply.github.com> Date: Fri, 18 Nov 2022 09:48:05 +0100 Subject: [PATCH] Fix S2225 FP: Support static virtual/abstract interface methods (#6335) --- .../Facade/CSharpSyntaxFacade.cs | 3 ++ .../Facade/SyntaxFacade.cs | 1 + .../Rules/ToStringShouldNotReturnNullBase.cs | 3 +- .../Extensions/MethodBlockSyntaxExtensions.cs | 27 ++++++++++ .../Extensions/SyntaxNodeExtensions.cs | 2 +- .../Facade/VisualBasicSyntaxFacade.cs | 3 ++ .../ToStringShouldNotReturnNull.CSharp11.cs | 6 +-- .../TestCases/ToStringShouldNotReturnNull.cs | 8 +++ .../TestCases/ToStringShouldNotReturnNull.vb | 52 +++++++++++-------- 9 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 analyzers/src/SonarAnalyzer.VisualBasic/Extensions/MethodBlockSyntaxExtensions.cs diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index 6888451634c..63f6dab7a2b 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -119,4 +119,7 @@ public override SyntaxNode RemoveParentheses(SyntaxNode node) => public override bool TryGetGetInterpolatedTextValue(SyntaxNode node, SemanticModel semanticModel, out string interpolatedValue) => Cast(node).TryGetGetInterpolatedTextValue(semanticModel, out interpolatedValue); + + public override bool IsStatic(SyntaxNode node) => + Cast(node).IsStatic(); } diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs index 58c5cc2d69e..7ccc6781aee 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs @@ -47,6 +47,7 @@ public abstract class SyntaxFacade public abstract SyntaxNode RemoveParentheses(SyntaxNode node); public abstract string NodeStringTextValue(SyntaxNode node, SemanticModel semanticModel); public abstract bool TryGetGetInterpolatedTextValue(SyntaxNode node, SemanticModel semanticModel, out string interpolatedValue); + public abstract bool IsStatic(SyntaxNode node); protected static T Cast(SyntaxNode node) where T : SyntaxNode => node as T ?? throw new InvalidCastException($"A {node.GetType().Name} node can not be cast to a {typeof(T).Name} node."); diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ToStringShouldNotReturnNullBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ToStringShouldNotReturnNullBase.cs index 712d5bd8eec..085de3b6244 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ToStringShouldNotReturnNullBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ToStringShouldNotReturnNullBase.cs @@ -63,5 +63,6 @@ private bool WithinToString(SyntaxNode node) => node.Ancestors() .TakeWhile(x => !IsLocalOrLambda(x)) .Any(x => Language.Syntax.IsKind(x, MethodKind) - && nameof(ToString).Equals(Language.Syntax.NodeIdentifier(x)?.ValueText, Language.NameComparison)); + && nameof(ToString).Equals(Language.Syntax.NodeIdentifier(x)?.ValueText, Language.NameComparison) + && !Language.Syntax.IsStatic(x)); } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/MethodBlockSyntaxExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/MethodBlockSyntaxExtensions.cs new file mode 100644 index 00000000000..0c967190029 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/MethodBlockSyntaxExtensions.cs @@ -0,0 +1,27 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 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.Extensions; + +public static class MethodBlockSyntaxExtensions +{ + public static bool IsShared(this MethodBlockSyntax methodBlock) => + methodBlock.SubOrFunctionStatement.Modifiers.Any(SyntaxKind.SharedKeyword); +} diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs index d9edb61c589..3e141b1cbd2 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs @@ -38,7 +38,7 @@ public static ControlFlowGraph CreateCfg(this SyntaxNode block, SemanticModel mo public static bool IsPartOfBinaryNegationOrCondition(this SyntaxNode node) { - if (!(node.Parent is MemberAccessExpressionSyntax)) + if (node.Parent is not MemberAccessExpressionSyntax) { return false; } diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index 90013bdbe11..34e8862d809 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -121,4 +121,7 @@ public override SyntaxNode RemoveParentheses(SyntaxNode node) => public override bool TryGetGetInterpolatedTextValue(SyntaxNode node, SemanticModel semanticModel, out string interpolatedValue) => Cast(node).TryGetGetInterpolatedTextValue(semanticModel, out interpolatedValue); + + public override bool IsStatic(SyntaxNode node) => + Cast(node).IsShared(); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.CSharp11.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.CSharp11.cs index 571ba24199e..045e0bcd948 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.CSharp11.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.CSharp11.cs @@ -2,7 +2,7 @@ { static virtual string ToString() { - return null; // Noncompliant FP + return null; // Compliant } } @@ -11,10 +11,10 @@ public interface SomeOtherInterface static abstract string ToString(); } -public class SomeClass: SomeOtherInterface +public class SomeClass : SomeOtherInterface { public static string ToString() { - return null; // Noncompliant FP + return null; // Compliant } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.cs index a790efb47ce..bb889105179 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.cs @@ -131,4 +131,12 @@ public override string ToString() return null; // Noncompliant } } + + public class SomeClass + { + public static string ToString() + { + return null; // Compliant + } + } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.vb index 11b0a4a64ff..d39949034b6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ToStringShouldNotReturnNull.vb @@ -63,22 +63,30 @@ Namespace Compliant Return String.Empty End Function End Structure - - Class ToString - - Public Function SomeMethod() As String + + Class ToString + + Public Function SomeMethod() As String Return Nothing 'Compliant End Function - - End Class - - Class BinaryConditionalExpressionNotSupported - - Public Function SomeMethod() As String + + End Class + + Class BinaryConditionalExpressionNotSupported + + Public Function SomeMethod() As String Return If(Nothing, Nothing) ' Not supported, doesn't make sense End Function - - End Class + + End Class + + Class ToStringSharedMethod + + Public Shared Function ToString() As String + Return Nothing + End Function + + End Class End Namespace @@ -96,8 +104,8 @@ Namespace Noncompliant End Class Public Class ReturnsNothingConditionaly - - Public Overrides Function ToString() As String + + Public Overrides Function ToString() As String If Condition.[When]() Then Return Nothing ' Noncompliant End If @@ -111,15 +119,15 @@ Namespace Noncompliant Return If(Condition.[When](), Nothing, "") ' Noncompliant End Function End Class - - Public Class ReturnsNullViaNestedTenary - - Public Overrides Function ToString() As String + + Public Class ReturnsNullViaNestedTenary + + Public Overrides Function ToString() As String Return If(Condition.When(), ' Noncompliant - If(Condition.When(), Nothing , "something"), - If(Condition.When(), "something", Nothing)) - End Function - + If(Condition.When(), Nothing, "something"), + If(Condition.When(), "something", Nothing)) + End Function + End Class Structure StructReturnsNothing