Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S4507 FP: Top-level statements #8572

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,57 @@
*/

using SonarAnalyzer.CFG.Roslyn;
using StyleCop.Analyzers.Lightup;

namespace SonarAnalyzer.Extensions
{
internal static partial class SyntaxNodeExtensions
{
private static readonly ControlFlowGraphCache CfgCache = new();
private static readonly SyntaxKind[] ParenthesizedNodeKinds = new[] { SyntaxKind.ParenthesizedExpression, SyntaxKindEx.ParenthesizedPattern };
private static readonly SyntaxKind[] ParenthesizedNodeKinds = [SyntaxKind.ParenthesizedExpression, SyntaxKindEx.ParenthesizedPattern];

private static readonly SyntaxKind[] EnclosingScopeSyntaxKinds = [
SyntaxKind.AddAccessorDeclaration,
SyntaxKind.AnonymousMethodExpression,
SyntaxKind.BaseConstructorInitializer,
SyntaxKind.ConstructorDeclaration,
SyntaxKind.ConversionOperatorDeclaration,
SyntaxKind.DestructorDeclaration,
SyntaxKind.EqualsValueClause,
SyntaxKind.GetAccessorDeclaration,
SyntaxKind.GlobalStatement,
SyntaxKindEx.InitAccessorDeclaration,
SyntaxKindEx.LocalFunctionStatement,
SyntaxKind.MethodDeclaration,
SyntaxKind.OperatorDeclaration,
SyntaxKind.ParenthesizedLambdaExpression,
SyntaxKindEx.PrimaryConstructorBaseType,
SyntaxKind.RemoveAccessorDeclaration,
SyntaxKind.SetAccessorDeclaration,
SyntaxKind.SimpleLambdaExpression,
SyntaxKind.ThisConstructorInitializer];
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved

private static readonly SyntaxKind[] NegationOrConditionEnclosingSyntaxKinds = [
SyntaxKind.AnonymousMethodExpression,
SyntaxKind.BitwiseNotExpression,
SyntaxKind.ConditionalExpression,
SyntaxKind.IfStatement,
SyntaxKind.MethodDeclaration,
SyntaxKind.ParenthesizedLambdaExpression,
SyntaxKind.SimpleLambdaExpression,
SyntaxKind.WhileStatement];

public static ControlFlowGraph CreateCfg(this SyntaxNode node, SemanticModel model, CancellationToken cancel) =>
CfgCache.FindOrCreate(node, model, cancel);

public static bool ContainsConditionalConstructs(this SyntaxNode node) =>
node != null &&
node.DescendantNodes()
.Any(descendant => descendant.IsAnyKind(SyntaxKind.IfStatement,
SyntaxKind.ConditionalExpression,
SyntaxKind.CoalesceExpression,
SyntaxKind.SwitchStatement,
SyntaxKindEx.SwitchExpression,
SyntaxKindEx.CoalesceAssignmentExpression));
.Any(descendant => descendant.Kind() is SyntaxKind.IfStatement
or SyntaxKind.ConditionalExpression
or SyntaxKind.CoalesceExpression
or SyntaxKind.SwitchStatement
or SyntaxKindEx.SwitchExpression
or SyntaxKindEx.CoalesceAssignmentExpression);

public static object FindConstantValue(this SyntaxNode node, SemanticModel semanticModel) =>
new CSharpConstantValueFinder(semanticModel).FindConstant(node);
Expand All @@ -49,7 +79,7 @@ public static string FindStringConstant(this SyntaxNode node, SemanticModel sema

public static bool IsPartOfBinaryNegationOrCondition(this SyntaxNode node)
{
if (!(node.Parent is MemberAccessExpressionSyntax))
if (node.Parent is not MemberAccessExpressionSyntax)
{
return false;
}
Expand All @@ -61,12 +91,8 @@ public static bool IsPartOfBinaryNegationOrCondition(this SyntaxNode node)
}

var current = topNode;
while (!current.Parent?.IsAnyKind(SyntaxKind.BitwiseNotExpression,
SyntaxKind.IfStatement,
SyntaxKind.WhileStatement,
SyntaxKind.ConditionalExpression,
SyntaxKind.MethodDeclaration,
SyntaxKind.SimpleLambdaExpression) ?? false)
while (current.Parent != null
&& !NegationOrConditionEnclosingSyntaxKinds.Contains(current.Parent.Kind()))
{
current = current.Parent;
}
Expand Down Expand Up @@ -400,14 +426,13 @@ public static ConditionalAccessExpressionSyntax GetParentConditionalAccessExpres

// Effectively, if we're on the RHS of the ? we have to walk up the RHS spine first until we hit the first
// conditional access.
while (current.IsAnyKind(
SyntaxKind.InvocationExpression,
SyntaxKind.ElementAccessExpression,
SyntaxKind.SimpleMemberAccessExpression,
SyntaxKind.MemberBindingExpression,
SyntaxKind.ElementBindingExpression,
while ((current.Kind() is SyntaxKind.InvocationExpression
or SyntaxKind.ElementAccessExpression
or SyntaxKind.SimpleMemberAccessExpression
or SyntaxKind.MemberBindingExpression
or SyntaxKind.ElementBindingExpression
// Optional exclamations might follow the conditional operation. For example: a.b?.$$c!!!!()
SyntaxKindEx.SuppressNullableWarningExpression) &&
or SyntaxKindEx.SuppressNullableWarningExpression) &&
current.Parent is not ConditionalAccessExpressionSyntax)
{
current = current.Parent;
Expand Down Expand Up @@ -539,6 +564,9 @@ public static bool IsFalse(this SyntaxNode node) =>
_ => false,
};

public static SyntaxNode EnclosingScope(this SyntaxNode node) =>
node.Ancestors().FirstOrDefault(x => x.IsAnyKind(EnclosingScopeSyntaxKinds));

private readonly record struct PathPosition(int Index, int TupleLength);

private sealed class ControlFlowGraphCache : ControlFlowGraphCacheBase
Expand All @@ -547,7 +575,10 @@ protected override bool IsLocalFunction(SyntaxNode node) =>
node.IsKind(SyntaxKindEx.LocalFunctionStatement);

protected override bool HasNestedCfg(SyntaxNode node) =>
node.IsAnyKind(SyntaxKindEx.LocalFunctionStatement, SyntaxKind.SimpleLambdaExpression, SyntaxKind.AnonymousMethodExpression, SyntaxKind.ParenthesizedLambdaExpression);
node.Kind() is SyntaxKindEx.LocalFunctionStatement
or SyntaxKind.SimpleLambdaExpression
or SyntaxKind.AnonymousMethodExpression
or SyntaxKind.ParenthesizedLambdaExpression;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ public DeliveringDebugFeaturesInProduction() : this(AnalyzerConfiguration.Hotspo

// For simplicity we will avoid creating noise if the validation is invoked in the same method (https://github.com/SonarSource/sonar-dotnet/issues/5032)
protected override bool IsDevelopmentCheckInvoked(SyntaxNode node, SemanticModel semanticModel) =>
node.FirstAncestorOrSelf<MethodDeclarationSyntax>() is { } parentMethodDeclaration
&& parentMethodDeclaration.DescendantNodes().Any(x => IsDevelopmentCheck(x, semanticModel));
node.EnclosingScope()
.DescendantNodes()
.Any(x => IsDevelopmentCheck(x, semanticModel));

protected override bool IsInDevelopmentContext(SyntaxNode node) =>
node.Ancestors()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,51 @@ public class Derived(int i) {{baseType}} { }
}
}

[DataTestMethod]
[DataRow("""event EventHandler SomeEvent { add { $$int x = 42;$$ } remove { int x = 42; } }""", SyntaxKind.AddAccessorDeclaration)]
[DataRow("""int Method() { Func<int, int, int> add = delegate (int a, int b) { return $$a + b$$; }; return add(1, 2); }""", SyntaxKind.AnonymousMethodExpression)]
[DataRow("""Derived(int arg) : base($$arg$$) { }""", SyntaxKind.BaseConstructorInitializer)]
[DataRow("""Derived() { $$var x = 42;$$ }""", SyntaxKind.ConstructorDeclaration)]
[DataRow("""public static implicit operator int(Derived d) => $$42$$;""", SyntaxKind.ConversionOperatorDeclaration)]
[DataRow("""~Derived() { $$var x = 42;$$ }""", SyntaxKind.DestructorDeclaration)]
[DataRow("""int field = $$int.Parse("42")$$;""", SyntaxKind.EqualsValueClause)]
[DataRow("""int Property { get; set; } = $$int.Parse("42")$$;""", SyntaxKind.EqualsValueClause)]
[DataRow("""int Property { set { $$_ = value;$$ } }""", SyntaxKind.SetAccessorDeclaration)]
[DataRow("""int Property { set { $$_ = value;$$ } }""", SyntaxKind.SetAccessorDeclaration)]
[DataRow("""int Method() { return LocalFunction(); int LocalFunction() { $$return 42;$$ } }""", SyntaxKindEx.LocalFunctionStatement)]
[DataRow("""int Method() { return LocalFunction(); int LocalFunction() => $$42$$; }""", SyntaxKindEx.LocalFunctionStatement)]
[DataRow("""int Method() { $$return 42;$$ }""", SyntaxKind.MethodDeclaration)]
[DataRow("""int Method() => $$42$$;""", SyntaxKind.MethodDeclaration)]
[DataRow("""public static Derived operator +(Derived d) => $$d$$;""", SyntaxKind.OperatorDeclaration)]
[DataRow("""int Method() { var lambda = () => $$42$$; return lambda(); }""", SyntaxKind.ParenthesizedLambdaExpression)]
[DataRow("""int Method() { Func<int, int> lambda = x => $$x + 1$$; return lambda(42); }""", SyntaxKind.SimpleLambdaExpression)]
[DataRow("""event EventHandler SomeEvent { add { int x = 42; } remove { $$int x = 42;$$ } }""", SyntaxKind.RemoveAccessorDeclaration)]
[DataRow("""Derived(int arg) : this($$arg.ToString()$$) { }""", SyntaxKind.ThisConstructorInitializer)]
martin-strecker-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
#if NET
[DataRow("""int Property { init { $$_ = value;$$ } }""", SyntaxKindEx.InitAccessorDeclaration)]
[DataRow("""record BaseRec(int I); record DerivedRec(int I): BaseRec($$I++$$);""", SyntaxKindEx.PrimaryConstructorBaseType)]
#endif
public void EnclosingScope_Members(string member, SyntaxKind expectedSyntaxKind)
{
var node = NodeBetweenMarkers($$"""
using System;

public class Base
{
public Base() { }
public Base(int arg) { }
}

public class Derived: Base
{
Derived(string arg) { }
{{member}}
}
""", LanguageNames.CSharp);
var actual = ExtensionsCS.EnclosingScope(node)?.Kind() ?? SyntaxKind.None;
actual.Should().Be(expectedSyntaxKind);
}

private static SyntaxNode NodeBetweenMarkers(string code, string language, bool getInnermostNodeForTie = false)
{
var position = code.IndexOf("$$");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

// https://github.com/SonarSource/sonar-dotnet/issues/6772
if (app.Environment.IsDevelopment())
{
app.UseDeveloperExceptionPage(); // Noncompliant FP https://github.com/SonarSource/sonar-dotnet/issues/6772
app.UseDeveloperExceptionPage();
}

app.Run();
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,61 @@ namespace Tests.Diagnostics
public class Startup
{
// for coverage
private IHostingEnvironment env;

private IApplicationBuilder foo;
public IApplicationBuilder Foo
{
get
{
if (env.IsDevelopment())
{
foo.UseDeveloperExceptionPage(); // Compliant
}
return foo;
}
set
{
var x = value.UseDeveloperExceptionPage(); // Noncompliant
var x = value.UseDeveloperExceptionPage(); // Noncompliant
foo = value;
}
}

event EventHandler SomeEvent
{
add { foo.UseDeveloperExceptionPage(); } // Noncompliant
remove
{
if (env.IsDevelopment())
{
foo.UseDeveloperExceptionPage(); // Compliant
}
}
}

public Startup()
{
foo.UseDeveloperExceptionPage(); // Noncompliant
}

public void Lambda()
{
Action action1 = () =>
{
foo.UseDeveloperExceptionPage(); // Noncompliant
};
action1();

Action action2 = () =>
{
if (env.IsDevelopment())
{
foo.UseDeveloperExceptionPage(); // Compliant
}
};
action2();
}

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
// Invoking as extension methods
Expand Down
Loading