diff --git a/src/Faithlife.Analyzers/NullTestTernariesAnalyzer.cs b/src/Faithlife.Analyzers/NullTestTernariesAnalyzer.cs new file mode 100644 index 0000000..de1de40 --- /dev/null +++ b/src/Faithlife.Analyzers/NullTestTernariesAnalyzer.cs @@ -0,0 +1,68 @@ +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace Faithlife.Analyzers +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + public sealed class NullTestTernariesAnalyzer : DiagnosticAnalyzer + { + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + + context.RegisterCompilationStartAction(compilationStartAnalysisContext => + { + compilationStartAnalysisContext.RegisterSyntaxNodeAction(AnalyzeSyntax, SyntaxKind.ConditionalExpression); + }); + } + + private static void AnalyzeSyntax(SyntaxNodeAnalysisContext context) + { + var expression = (ConditionalExpressionSyntax) context.Node; + var falseValue = expression.WhenFalse; + var trueValue = expression.WhenTrue; + + if (expression.Condition is BinaryExpressionSyntax binaryExpression) + { + var lExpression = binaryExpression.Left; + var rExpression = binaryExpression.Right; + + if ((falseValue.Kind() == SyntaxKind.NullLiteralExpression || trueValue.Kind() == SyntaxKind.NullLiteralExpression) && (lExpression.Kind() == SyntaxKind.NullLiteralExpression || rExpression.Kind() == SyntaxKind.NullLiteralExpression)) + { + context.ReportDiagnostic(Diagnostic.Create(s_rule, expression.GetLocation())); + } + } + else if (expression.Condition is MemberAccessExpressionSyntax memberAccessExpression) + { + if (memberAccessExpression.Kind() == SyntaxKind.SimpleMemberAccessExpression && memberAccessExpression.Name.Identifier.Text == "HasValue") + { + context.ReportDiagnostic(Diagnostic.Create(s_rule, expression.GetLocation())); + } + } + else if (expression.Condition is PrefixUnaryExpressionSyntax prefixUnaryExpression && expression.Condition.Kind() == SyntaxKind.LogicalNotExpression) + { + if (((MemberAccessExpressionSyntax) prefixUnaryExpression.Operand).Name.Identifier.Text == "HasValue") + { + context.ReportDiagnostic(Diagnostic.Create(s_rule, expression.GetLocation())); + } + } + } + + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(s_rule); + + public const string DiagnosticId = "FL0015"; + + private static readonly DiagnosticDescriptor s_rule = new( + id: DiagnosticId, + title: "Null Checking Ternaries Usage", + messageFormat: "Prefer null conditional operators over ternaries explicitly checking for null", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + helpLinkUri: $"https://github.com/Faithlife/FaithlifeAnalyzers/wiki/{DiagnosticId}" + ); + } +} diff --git a/tests/Faithlife.Analyzers.Tests/NullTestTernariesTests.cs b/tests/Faithlife.Analyzers.Tests/NullTestTernariesTests.cs new file mode 100644 index 0000000..875f8c2 --- /dev/null +++ b/tests/Faithlife.Analyzers.Tests/NullTestTernariesTests.cs @@ -0,0 +1,102 @@ +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using NUnit.Framework; + +namespace Faithlife.Analyzers.Tests +{ + [TestFixture] + public class NullTestTernariesTests : CodeFixVerifier + { + [TestCase("Person person = null;", "var firstName = person?.FirstName;")] + [TestCase("var person = new Person { FirstName = \"Bob\", LastName = \"Dole\" };", "var firstName = person.Age?.BirthYear;")] + public void ValidUsage(string declaration, string ternary) + { + string validProgram = $@" +namespace TestApplication +{{ + public class Person + {{ + public string FirstName {{ get; set; }} + public string LastName {{ get; set; }} + public Age? Age {{ get; set; }} + }} + + public struct Age + {{ + public int? InYears {{ get; set; }} + + public int? BirthYear + {{ + get + {{ + return System.DateTime.Now.Year - InYears; + }} + }} + }} + + internal static class TestClass + {{ + public static void UtilityMethod() + {{ + {declaration} + {ternary} + }} + }} +}}"; + + VerifyCSharpDiagnostic(validProgram); + } + + [TestCase("var firstName = person != null ? person.FirstName : null;")] + [TestCase("var firstName = person == null ? null : person.FirstName;")] + [TestCase("var firstName = person.Age.HasValue ? person.Age.Value.BirthYear : null;")] + [TestCase("var firstName = !person.Age.HasValue ? null : person.Age.Value.BirthYear;")] + public void InvalidUsage(string badExample) + { + var brokenProgram = $@" +namespace TestApplication +{{ + public class Person + {{ + public string FirstName {{ get; set; }} + public string LastName {{ get; set; }} + public Age? Age {{ get; set; }} + }} + + public struct Age + {{ + public int? InYears {{ get; set; }} + + public int? BirthYear + {{ + get + {{ + return System.DateTime.Now.Year - InYears; + }} + }} + }} + + internal static class TestClass + {{ + public static void UtilityMethod() + {{ + var person = new Person {{ FirstName = ""Bob"", LastName = ""Dole"" }}; + {badExample} + }} + }} +}}"; + + var expected = new DiagnosticResult + { + Id = NullTestTernariesAnalyzer.DiagnosticId, + Message = "Prefer null conditional operators over ternaries explicitly checking for null", + Severity = DiagnosticSeverity.Warning, + Locations = new[] { new DiagnosticResultLocation("Test0.cs", 29, 20) }, + }; + + VerifyCSharpDiagnostic(brokenProgram, expected); + } + + protected override DiagnosticAnalyzer GetCSharpDiagnosticAnalyzer() => new NullTestTernariesAnalyzer(); + } +}