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 S3358 FP: Nested ternary operator is in a lambda #5774

Merged
merged 7 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,21 @@ public sealed class DoNotNestTernaryOperators : DoNotNestTernaryOperatorsBase
{
private const string MessageFormat = "Extract this nested ternary operation into an independent statement.";

private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);

protected override void Initialize(SonarAnalysisContext context)
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterSyntaxNodeActionInNonGenerated(c =>
{
if (c.Node.Ancestors().OfType<ConditionalExpressionSyntax>().Any())
if (c.Node.Ancestors()
.TakeWhile(x => !x.IsAnyKind(SyntaxKind.ParenthesizedLambdaExpression, SyntaxKind.SimpleLambdaExpression))
.OfType<ConditionalExpressionSyntax>()
.Any())
{
c.ReportIssue(Diagnostic.Create(rule, c.Node.GetLocation()));
c.ReportIssue(Diagnostic.Create(Rule, c.Node.GetLocation()));
}
},
SyntaxKind.ConditionalExpression);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,24 @@ namespace SonarAnalyzer.Rules.VisualBasic
public sealed class DoNotNestTernaryOperators : DoNotNestTernaryOperatorsBase
{
private const string MessageFormat = "Extract this nested If operator into independent If...Then...Else statements.";
private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(rule);
private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context)
{
context.RegisterSyntaxNodeActionInNonGenerated(
c =>
protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterSyntaxNodeActionInNonGenerated(c =>
{
if (c.Node.Ancestors().OfType<TernaryConditionalExpressionSyntax>().Any())
if (c.Node.Ancestors()
.TakeWhile(x => !x.IsAnyKind(
SyntaxKind.MultiLineFunctionLambdaExpression,
SyntaxKind.SingleLineFunctionLambdaExpression,
SyntaxKind.MultiLineSubLambdaExpression,
SyntaxKind.SingleLineSubLambdaExpression))
.OfType<TernaryConditionalExpressionSyntax>()
.Any())
{
c.ReportIssue(Diagnostic.Create(rule, c.Node.GetLocation()));
c.ReportIssue(Diagnostic.Create(Rule, c.Node.GetLocation()));
}
},
SyntaxKind.TernaryConditionalExpression);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ namespace SonarAnalyzer.UnitTest.Rules
[TestClass]
public class DoNotNestTernaryOperatorsTest
{
private readonly VerifierBuilder verifierCS = new VerifierBuilder<CS.DoNotNestTernaryOperators>();
private readonly VerifierBuilder verifierVB = new VerifierBuilder<VB.DoNotNestTernaryOperators>();

[TestMethod]
public void DoNotNestTernaryOperators_CS() =>
OldVerifier.VerifyAnalyzer(@"TestCases\DoNotNestTernaryOperators.cs",
new CS.DoNotNestTernaryOperators());
verifierCS.AddPaths("DoNotNestTernaryOperators.cs").Verify();

[TestMethod]
public void DoNotNestTernaryOperators_VB() =>
OldVerifier.VerifyAnalyzer(@"TestCases\DoNotNestTernaryOperators.vb",
new VB.DoNotNestTernaryOperators());
verifierVB.AddPaths("DoNotNestTernaryOperators.vb").Verify();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ public bool FooImpl(bool isMale, bool isMarried)
? "Mrs. "
: "Miss ";

var lambda1 = true // Compliant. Nested in Lambda is valid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add noncompliant

var lambda3 = new Func<string, string>(s => isMale ? "Mr. " : isMarried ? "Mrs. " : "Miss ")

? new Func<string>(() => true ? "a" : "b")
: new Func<string>(() => true ? "c" : "d");

var lambda2 = true
? new Func<string, string>(s => true ? "a" : "b")
: new Func<string, string>(s => true ? "c" : "d");

return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ Namespace Tests.TestCases
Dim x3 = If(isMale, "Mr. ",
If(isMarried, "Mrs. ", "Miss "))
' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Noncompliant

Dim x4 = If(isMale, Function() "Mr.", Function() If(isMarried, "Mrs.", "Miss")) ' Compliant. Ternary expressions in lambdas are not considered nested.
Dim x5 = If(isMale, Sub() PrintGreeting("Mr."), Sub() PrintGreeting(If(isMarried, "Mrs.", "Miss")))
Dim x6 = If(isMale, Function()
Return "Mr."
End Function,
Function()
Return If(isMarried, "Mrs.", "Miss")
End Function)
Dim x7 = If(isMale, Sub()
PrintGreeting("Mr.")
End Sub,
Sub()
PrintGreeting(If(isMarried, "Mrs.", "Miss"))
End Sub)

End Function

Private Sub PrintGreeting(greeting As String)
End Sub
End Class
End Namespace