Prefer null conditional over ternaries

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<DiagnosticDescriptor> 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}"
+		);
+	}
+}

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();
+	}
+}