Refactor Field and Fields (#8156)

Author: flobernd

Diff for: src/Elastic.Clients.Elasticsearch.Shared/Core/Extensions/Extensions.cs

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Reflection;
 using System.Runtime.ExceptionServices;
@@ -172,13 +173,14 @@ internal static bool HasAny<T>(this IEnumerable<T> list, Func<T, bool> predicate
 
 	internal static bool HasAny<T>(this IEnumerable<T> list) => list != null && list.Any();
 
-	internal static bool IsEmpty<T>(this IEnumerable<T> list)
+	internal static bool IsNullOrEmpty<T>(this IEnumerable<T>? list)
 	{
-		if (list == null)
+		if (list is null)
 			return true;
 
 		var enumerable = list as T[] ?? list.ToArray();
-		return !enumerable.Any() || enumerable.All(t => t == null);
+
+		return (enumerable.Length == 0) || enumerable.All(x => x is null);
 	}
 
 	internal static void ThrowIfNull<T>(this T value, string name, string message = null)
@@ -189,9 +191,9 @@ internal static void ThrowIfNull<T>(this T value, string name, string message =
 			throw new ArgumentNullException(name, "Argument can not be null when " + message);
 	}
 
-	internal static bool IsNullOrEmpty(this string value) => string.IsNullOrWhiteSpace(value);
+	internal static bool IsNullOrEmpty(this string? value) => string.IsNullOrWhiteSpace(value);
 
-	internal static bool IsNullOrEmptyCommaSeparatedList(this string value, out string[] split)
+	internal static bool IsNullOrEmptyCommaSeparatedList(this string? value, [NotNullWhen(false)] out string[]? split)
 	{
 		split = null;
 		if (string.IsNullOrWhiteSpace(value))

Diff for: src/Elastic.Clients.Elasticsearch.Shared/Core/Infer/Field/Field.cs

@@ -4,24 +4,29 @@
 
 using System;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Text.Json.Serialization;
+
 using Elastic.Transport;
 
 #if ELASTICSEARCH_SERVERLESS
 namespace Elastic.Clients.Elasticsearch.Serverless;
 #else
+
 namespace Elastic.Clients.Elasticsearch;
 #endif
 
 [JsonConverter(typeof(FieldConverter))]
-[DebuggerDisplay("{" + nameof(DebugDisplay) + ",nq}")]
-public sealed class Field : IEquatable<Field>, IUrlParameter
+[DebuggerDisplay($"{nameof(DebuggerDisplay)},nq")]
+public sealed class Field :
+	IEquatable<Field>,
+	IUrlParameter
 {
 	private readonly object _comparisonValue;
-	private readonly Type _type;
+	private readonly Type? _type;
 
 	// Pseudo and metadata fields
 
@@ -30,143 +35,173 @@ public sealed class Field : IEquatable<Field>, IUrlParameter
 	public static Field KeyField = new("_key");
 	public static Field CountField = new("_count");
 
-	public Field(string name) : this(name, null, null) { }
+	/// <summary>
+	///     The name of the field
+	/// </summary>
+	public string? Name { get; }
+
+	/// <summary>
+	///     An expression from which the name of the field can be inferred
+	/// </summary>
+	public Expression? Expression { get; }
+
+	/// <summary>
+	///     A property from which the name of the field can be inferred
+	/// </summary>
+	public PropertyInfo? Property { get; }
+
+	/// <summary>
+	///     A boost to apply to the field
+	/// </summary>
+	public double? Boost { get; set; }
+
+	/// <summary>
+	///     A format to apply to the field.
+	/// </summary>
+	/// <remarks>
+	///     Can be used only for Doc Value Fields Elasticsearch 6.4.0+
+	/// </remarks>
+	public string? Format { get; set; }
+
+	internal bool CachableExpression { get; }
+
+	#region Constructors
+
+	public Field(string name) : this(name, null, null)
+	{
+	}
 
-	public Field(string name, double boost) : this(name, boost, null) { }
+	public Field(string name, double boost) : this(name, boost, null)
+	{
+	}
 
-	public Field(string name, string format) : this(name, null, format) { }
+	public Field(string name, string format) : this(name, null, format)
+	{
+	}
 
 	public Field(string name, double? boost, string? format)
 	{
-		name.ThrowIfNullOrEmpty(nameof(name));
+		if (string.IsNullOrEmpty(name))
+			throw new ArgumentException($"{name} can not be null or empty.", nameof(name));
+
 		Name = ParseFieldName(name, out var b);
 		Boost = b ?? boost;
 		Format = format;
+
 		_comparisonValue = Name;
 	}
 
-	public Field(Expression expression, double? boost = null, string format = null)
+	public Field(Expression expression, double? boost = null, string? format = null)
 	{
 		Expression = expression ?? throw new ArgumentNullException(nameof(expression));
 		Boost = boost;
 		Format = format;
+
 		_comparisonValue = expression.ComparisonValueFromExpression(out var type, out var cachable);
 		_type = type;
+
 		CachableExpression = cachable;
 	}
 
-	public Field(PropertyInfo property, double? boost = null, string format = null)
+	public Field(PropertyInfo property, double? boost = null, string? format = null)
 	{
 		Property = property ?? throw new ArgumentNullException(nameof(property));
 		Boost = boost;
 		Format = format;
+
 		_comparisonValue = property;
 		_type = property.DeclaringType;
 	}
 
-	/// <summary>
-	///     A boost to apply to the field
-	/// </summary>
-	public double? Boost { get; set; }
+	#endregion Constructors
 
-	/// <summary>
-	///     A format to apply to the field.
-	/// </summary>
-	/// <remarks>
-	///     Can be used only for Doc Value Fields Elasticsearch 6.4.0+
-	/// </remarks>
-	public string? Format { get; set; }
+	#region Factory Methods
 
-	internal bool CachableExpression { get; }
+	public static Field? FromString(string? name) => string.IsNullOrEmpty(name) ? null : new Field(name);
 
-	/// <summary>
-	///     An expression from which the name of the field can be inferred
-	/// </summary>
-	public Expression Expression { get; }
+	public static Field? FromExpression(Expression? expression) => expression is null ? null : new Field(expression);
 
-	/// <summary>
-	///     The name of the field
-	/// </summary>
-	public string Name { get; }
+	public static Field? FromProperty(PropertyInfo? property) => property is null ? null : new Field(property);
 
-	/// <summary>
-	///     A property from which the name of the field can be inferred
-	/// </summary>
-	public PropertyInfo Property { get; }
+	#endregion Factory Methods
 
-	internal string DebugDisplay =>
-		$"{Expression?.ToString() ?? PropertyDebug ?? Name}{(Boost.HasValue ? "^" + Boost.Value : string.Empty)}"
-		+ $"{(!string.IsNullOrEmpty(Format) ? " format: " + Format : string.Empty)}"
-		+ $"{(_type == null ? string.Empty : " typeof: " + _type.Name)}";
+	#region Conversion Operators
 
-	private string PropertyDebug => Property == null ? null : $"PropertyInfo: {Property.Name}";
+	public static implicit operator Field?(string? name) => FromString(name);
 
-	public bool Equals(Field other) => _type != null
-		? other != null && _type == other._type && _comparisonValue.Equals(other._comparisonValue)
-		: other != null && _comparisonValue.Equals(other._comparisonValue);
+	public static implicit operator Field?(Expression? expression) => FromExpression(expression);
 
-	string IUrlParameter.GetString(ITransportConfiguration settings)
+	public static implicit operator Field?(PropertyInfo? property) => FromProperty(property);
+
+	#endregion Conversion Operators
+
+	#region Combinator Methods
+
+	public Fields And(Field field)
 	{
-		if (settings is not IElasticsearchClientSettings elasticsearchSettings)
-		{
-			throw new ArgumentNullException(nameof(settings),
-				$"Can not resolve {nameof(Field)} if no {nameof(IElasticsearchClientSettings)} is provided");
-		}
+		if (field is null)
+			throw new ArgumentNullException(nameof(field));
 
-		return GetStringCore(elasticsearchSettings);
+		return new([this, field]);
 	}
 
-	private string GetStringCore(IElasticsearchClientSettings settings)
+	public Fields And<T, TValue>(Expression<Func<T, TValue>> expression, double? boost = null, string? format = null)
 	{
-		if (settings is null)
-		{
-			throw new ArgumentNullException(nameof(settings),
-				$"Can not resolve {nameof(Field)} if no {nameof(IElasticsearchClientSettings)} is provided");
-		}
+		if (expression is null)
+			throw new ArgumentNullException(nameof(expression));
 
-		return settings.Inferrer.Field(this);
+		return new([this, new Field(expression, boost, format)]);
 	}
 
-	public override string ToString() => DebugDisplay;
+	public Fields And<T>(Expression<Func<T, object>> expression, double? boost = null, string? format = null)
+	{
+		if (expression is null)
+			throw new ArgumentNullException(nameof(expression));
 
-	public Fields And(Field field) => new(new[] { this, field });
+		return new([this, new Field(expression, boost, format)]);
+	}
 
-	public Fields And<T, TValue>(Expression<Func<T, TValue>> field, double? boost = null, string format = null)
-		where T : class =>
-		new(new[] { this, new Field(field, boost, format) });
+	public Fields And(string field, double? boost = null, string? format = null)
+	{
+		if (field is null)
+			throw new ArgumentNullException(nameof(field));
 
-	public Fields And<T>(Expression<Func<T, object>> field, double? boost = null, string format = null)
-		where T : class =>
-		new(new[] { this, new Field(field, boost, format) });
+		return new([this, new Field(field, boost, format)]);
+	}
 
-	public Fields And(string field, double? boost = null, string format = null) =>
-		new(new[] { this, new Field(field, boost, format) });
+	public Fields And(PropertyInfo property, double? boost = null, string? format = null)
+	{
+		if (property is null)
+			throw new ArgumentNullException(nameof(property));
 
-	public Fields And(PropertyInfo property, double? boost = null, string format = null) =>
-		new(new[] { this, new Field(property, boost, format) });
+		return new([this, new Field(property, boost, format)]);
+	}
 
-	private static string ParseFieldName(string name, out double? boost)
-	{
-		boost = null;
-		if (name == null)
-			return null;
+	#endregion Combinator Methods
 
-		var caretIndex = name.IndexOf('^');
-		if (caretIndex == -1)
-			return name;
+	#region Equality
 
-		var parts = name.Split(new[] { '^' }, 2, StringSplitOptions.RemoveEmptyEntries);
-		name = parts[0];
-		boost = double.Parse(parts[1], CultureInfo.InvariantCulture);
-		return name;
-	}
+	public static bool operator ==(Field? a, Field? b) => Equals(a, b);
 
-	public static implicit operator Field(string name) => name.IsNullOrEmpty() ? null : new Field(name);
+	public static bool operator !=(Field? a, Field? b) => !Equals(a, b);
 
-	public static implicit operator Field(Expression expression) =>
-		expression == null ? null : new Field(expression);
+	public bool Equals(Field? other) =>
+		other switch
+		{
+			not null when _type is not null => (_type == other._type) && _comparisonValue.Equals(other._comparisonValue),
+			not null when _type is null => _comparisonValue.Equals(other._comparisonValue),
+			_ => false
+		};
 
-	public static implicit operator Field(PropertyInfo property) => property == null ? null : new Field(property);
+	public override bool Equals(object? obj) =>
+		obj switch
+		{
+			Field f => Equals(f),
+			string s => Equals(s),
+			Expression e => Equals(e),
+			PropertyInfo p => Equals(p),
+			_ => false
+		};
 
 	public override int GetHashCode()
 	{
@@ -178,22 +213,50 @@ public override int GetHashCode()
 		}
 	}
 
-	public override bool Equals(object obj)
+	#endregion Equality
+
+	#region IUrlParameter
+
+	string IUrlParameter.GetString(ITransportConfiguration settings)
 	{
-		switch (obj)
+		if (settings is not IElasticsearchClientSettings elasticsearchSettings)
 		{
-			case string s:
-				return Equals(s);
-			case PropertyInfo p:
-				return Equals(p);
-			case Field f:
-				return Equals(f);
-			default:
-				return false;
+			throw new ArgumentNullException(nameof(settings),
+				$"Can not resolve {nameof(Field)} if no {nameof(IElasticsearchClientSettings)} is provided");
 		}
+
+		return elasticsearchSettings.Inferrer.Field(this);
 	}
 
-	public static bool operator ==(Field x, Field y) => Equals(x, y);
+	#endregion IUrlParameter
+
+	#region Debugging
+
+	public override string ToString() =>
+		$"{Expression?.ToString() ?? PropertyDebug ?? Name}{(Boost.HasValue ? "^" + Boost.Value : string.Empty)}" +
+		$"{(!string.IsNullOrEmpty(Format) ? " format: " + Format : string.Empty)}" +
+		$"{(_type == null ? string.Empty : " typeof: " + _type.Name)}";
+
+	internal string DebuggerDisplay => ToString();
+
+	private string? PropertyDebug => Property is null ? null : $"PropertyInfo: {Property.Name}";
 
-	public static bool operator !=(Field x, Field y) => !Equals(x, y);
+	#endregion Debugging
+
+	[return: NotNullIfNotNull(nameof(name))]
+	private static string? ParseFieldName(string? name, out double? boost)
+	{
+		boost = null;
+		if (name is null)
+			return null;
+
+		var caretIndex = name.IndexOf('^');
+		if (caretIndex == -1)
+			return name;
+
+		var parts = name.Split(new[] { '^' }, 2, StringSplitOptions.RemoveEmptyEntries);
+		name = parts[0];
+		boost = double.Parse(parts[1], CultureInfo.InvariantCulture);
+		return name;
+	}
 }