Skip to content

Commit

Permalink
refactor: enable nullable ref type checking (#425)
Browse files Browse the repository at this point in the history
  • Loading branch information
natemcmaster authored Jan 16, 2021
1 parent db2b121 commit e0b6a2c
Show file tree
Hide file tree
Showing 31 changed files with 98 additions and 64 deletions.
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<WarningsNotAsErrors>$(WarningsNotAsErrors);1591</WarningsNotAsErrors>
<LangVersion>9.0</LangVersion>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<Nullable>annotations</Nullable>
<Nullable>enable</Nullable>
<AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)src\StrongName.snk</AssemblyOriginatorKeyFile>
<SignAssembly>true</SignAssembly>

Expand Down
4 changes: 2 additions & 2 deletions src/CommandLineUtils/Attributes/AllowedValuesAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ or StringComparison.InvariantCultureIgnoreCase
}

/// <inheritdoc />
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
{
if (value is string str && _allowedValues.Any(t => str.Equals(t, Comparer)))
{
return ValidationResult.Success;
}

return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ internal FilePathExistsAttributeBase(FilePathType filePathType)
}

/// <inheritdoc />
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
{
if (value is not string path || path.Length == 0 || path.IndexOfAny(Path.GetInvalidPathChars()) >= 0)
{
return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}

if (!Path.IsPathRooted(path)
Expand All @@ -50,7 +50,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
return ValidationResult.Success;
}

return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}

private static string GetDefaultErrorMessage(FilePathType filePathType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ internal FilePathNotExistsAttributeBase(FilePathType filePathType)
}

/// <inheritdoc />
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
{
if (value is not string path || path.Length == 0 || path.IndexOfAny(Path.GetInvalidPathChars()) >= 0)
{
return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}

if (!Path.IsPathRooted(path)
Expand All @@ -55,7 +55,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
return ValidationResult.Success;
}

return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}

private static string GetDefaultErrorMessage(FilePathType filePathType)
Expand Down
4 changes: 2 additions & 2 deletions src/CommandLineUtils/Attributes/LegalFilePathAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public LegalFilePathAttribute()
}

/// <inheritdoc />
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
{
if (value is string path)
{
Expand All @@ -37,7 +37,7 @@ protected override ValidationResult IsValid(object value, ValidationContext vali
}
}

return new ValidationResult(FormatErrorMessage(value as string));
return new ValidationResult(FormatErrorMessage(value?.ToString() ?? string.Empty));
}
}
}
2 changes: 1 addition & 1 deletion src/CommandLineUtils/CommandArgument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public IReadOnlyList<string?> Values
/// <summary>
/// Defines the underlying type of the argument for the help-text-generator
/// </summary>
internal Type UnderlyingType { get; set; }
internal Type? UnderlyingType { get; set; }

/// <summary>
/// Try to add a value to this argument.
Expand Down
6 changes: 3 additions & 3 deletions src/CommandLineUtils/CommandArgument{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ public IReadOnlyList<T> ParsedValues
((IInternalCommandParamOfT)this).Parse(CultureInfo.CurrentCulture);
}

if (_parsedValues.Count == 0 && _hasDefaultValue)
if (_parsedValues.Count == 0 && _hasDefaultValue && DefaultValue != null)
{
return new[] { DefaultValue };
return new T[] { DefaultValue };
}

return _parsedValues;
Expand Down Expand Up @@ -85,7 +85,7 @@ void IInternalCommandParamOfT.Parse(CultureInfo culture)
}
}

void SetBaseDefaultValue(T value)
void SetBaseDefaultValue(T? value)
{
if (!ReflectionHelper.IsSpecialValueTupleType(typeof(T), out _))
{
Expand Down
2 changes: 1 addition & 1 deletion src/CommandLineUtils/CommandLineApplication.Validation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public Func<ValidationResult, int> ValidationErrorHandler
/// Validates arguments and options.
/// </summary>
/// <returns>The first validation result that is not <see cref="ValidationResult.Success"/> if there is an error.</returns>
public ValidationResult GetValidationResult()
public ValidationResult? GetValidationResult()
{
if (Parent != null)
{
Expand Down
2 changes: 1 addition & 1 deletion src/CommandLineUtils/CommandLineApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ public async Task<int> ExecuteAsync(string[] args, CancellationToken cancellatio
}

var validationResult = command.GetValidationResult();
if (validationResult != ValidationResult.Success)
if (validationResult != null)
{
return command.ValidationErrorHandler(validationResult);
}
Expand Down
2 changes: 2 additions & 0 deletions src/CommandLineUtils/CommandLineApplication{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Diagnostics.CodeAnalysis;
using McMaster.Extensions.CommandLineUtils.Abstractions;
using McMaster.Extensions.CommandLineUtils.Conventions;
using McMaster.Extensions.CommandLineUtils.HelpText;
Expand All @@ -15,6 +16,7 @@ namespace McMaster.Extensions.CommandLineUtils
public class CommandLineApplication<TModel> : CommandLineApplication, IModelAccessor
where TModel : class
{
[AllowNull] // this is initialized in common method called in all constructors
private Lazy<TModel> _lazy;
private Func<TModel> _modelFactory = DefaultModelFactory;

Expand Down
2 changes: 1 addition & 1 deletion src/CommandLineUtils/CommandOption.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public IReadOnlyList<string?> Values
/// <summary>
/// Defines the underlying type of the option for the help-text-generator
/// </summary>
internal Type UnderlyingType { get; set; }
internal Type? UnderlyingType { get; set; }

/// <summary>
/// A collection of validators that execute before invoking <see cref="CommandLineApplication.OnExecute(Func{int})"/>.
Expand Down
4 changes: 2 additions & 2 deletions src/CommandLineUtils/CommandOption{T}.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public IReadOnlyList<T> ParsedValues
((IInternalCommandParamOfT)this).Parse(CultureInfo.CurrentCulture);
}

if (_parsedValues.Count == 0 && _hasDefaultValue)
if (_parsedValues.Count == 0 && _hasDefaultValue && DefaultValue != null)
{
return new[] { DefaultValue };
}
Expand Down Expand Up @@ -86,7 +86,7 @@ void IInternalCommandParamOfT.Parse(CultureInfo culture)
}
}

private void SetBaseDefaultValue(T value)
private void SetBaseDefaultValue(T? value)
{
if (!ReflectionHelper.IsSpecialValueTupleType(typeof(T), out _))
{
Expand Down
8 changes: 4 additions & 4 deletions src/CommandLineUtils/Conventions/ExecuteMethodConvention.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ private async Task<int> OnExecute(ConventionContext context, CancellationToken c
MethodInfo? asyncMethod;
try
{
method = context.ModelType.GetMethod("OnExecute", binding);
asyncMethod = context.ModelType.GetMethod("OnExecuteAsync", binding);
method = context.ModelType?.GetMethod("OnExecute", binding);
asyncMethod = context.ModelType?.GetMethod("OnExecuteAsync", binding);
}
catch (AmbiguousMatchException ex)
{
Expand Down Expand Up @@ -75,7 +75,7 @@ private async Task<int> OnExecute(ConventionContext context, CancellationToken c
throw new InvalidOperationException(Strings.InvalidOnExecuteReturnType(method.Name));
}

private async Task<int> InvokeAsync(MethodInfo method, object instance, object[] arguments)
private async Task<int> InvokeAsync(MethodInfo method, object instance, object?[] arguments)
{
try
{
Expand All @@ -95,7 +95,7 @@ private async Task<int> InvokeAsync(MethodInfo method, object instance, object[]
return 0;
}

private int Invoke(MethodInfo method, object instance, object[] arguments)
private int Invoke(MethodInfo method, object instance, object?[] arguments)
{
try
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static readonly MethodInfo s_addSubcommandMethod
private void AddSubcommandImpl<TSubCommand>(ConventionContext context)
where TSubCommand : class
{
context.Application.Command<TSubCommand>(null, null);
context.Application.Command<TSubCommand>(null!, null!); // Hmm, should probably rethink this...
}
}
}
2 changes: 1 addition & 1 deletion src/CommandLineUtils/HelpText/DefaultHelpTextGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ protected virtual string Format(CommandOption option)
}
}

private string[] ExtractNamesFromEnum(Type type)
private string[] ExtractNamesFromEnum(Type? type)
{
if (type == null)
{
Expand Down
5 changes: 4 additions & 1 deletion src/CommandLineUtils/Internal/CommandLineProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,10 @@ private bool ProcessUnexpectedArg(string argTypeName, string? argValue = null)

case UnrecognizedArgumentHandling.CollectAndContinue:
var arg = _enumerator.Current;
_currentCommand._remainingArguments.Add(arg.Raw);
if (arg != null)
{
_currentCommand._remainingArguments.Add(arg.Raw);
}
return true;

case UnrecognizedArgumentHandling.StopParsingAndCollect:
Expand Down
11 changes: 6 additions & 5 deletions src/CommandLineUtils/Internal/ReflectionHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using System.Reflection;
using System.Threading;
Expand Down Expand Up @@ -78,10 +79,10 @@ public static MemberInfo[] GetMembers(Type type)
return GetAllMembers(type).ToArray();
}

public static object[] BindParameters(MethodInfo method, CommandLineApplication command, CancellationToken cancellationToken)
public static object?[] BindParameters(MethodInfo method, CommandLineApplication command, CancellationToken cancellationToken)
{
var methodParams = method.GetParameters();
var arguments = new object[methodParams.Length];
var arguments = new object?[methodParams.Length];

for (var i = 0; i < methodParams.Length; i++)
{
Expand Down Expand Up @@ -117,15 +118,15 @@ public static object[] BindParameters(MethodInfo method, CommandLineApplication
return arguments;
}

public static bool IsNullableType(Type type, out Type? wrappedType)
public static bool IsNullableType(Type type, [NotNullWhen(true)] out Type? wrappedType)
{
var result = type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>);
wrappedType = result ? type.GetGenericArguments().First() : null;

return result;
}

public static bool IsSpecialValueTupleType(Type type, out Type? wrappedType)
public static bool IsSpecialValueTupleType(Type type, [NotNullWhen(true)] out Type? wrappedType)
{
var result = type.IsGenericType &&
type.GetGenericTypeDefinition() == typeof(ValueTuple<,>) &&
Expand All @@ -135,7 +136,7 @@ public static bool IsSpecialValueTupleType(Type type, out Type? wrappedType)
return result;
}

public static bool IsSpecialTupleType(Type type, out Type? wrappedType)
public static bool IsSpecialTupleType(Type type, [NotNullWhen(true)] out Type? wrappedType)
{
var result = type.IsGenericType &&
type.GetGenericTypeDefinition() == typeof(Tuple<,>) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;

namespace McMaster.Extensions.CommandLineUtils.Abstractions
Expand All @@ -13,7 +14,7 @@ namespace McMaster.Extensions.CommandLineUtils.Abstractions
/// </summary>
internal class TypeDescriptorValueParserFactory
{
public bool TryGetParser<T>(out IValueParser parser)
public bool TryGetParser<T>([NotNullWhen(true)] out IValueParser? parser)
{
var targetType = typeof(T);
var converter = TypeDescriptor.GetConverter(targetType);
Expand All @@ -39,7 +40,7 @@ public TypeConverterValueParser(Type targetType, TypeConverter typeConverter)

private TypeConverter TypeConverter { get; }

public T Parse(string argName, string value, CultureInfo culture)
public T Parse(string? argName, string? value, CultureInfo culture)
{
try
{
Expand All @@ -52,7 +53,7 @@ public T Parse(string argName, string value, CultureInfo culture)
}
}

object IValueParser.Parse(string argName, string value, CultureInfo culture)
object? IValueParser.Parse(string? argName, string? value, CultureInfo culture)
=> Parse(argName, value, culture);
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/CommandLineUtils/Properties/NullabilityHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) Nate McMaster.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

// Files here are for simplify annotations of nullable code and are not functional in .NET Standard 2.0
#if NETSTANDARD2_0 || NET45
namespace System.Diagnostics.CodeAnalysis
{
// https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.notnullwhenattribute?
[AttributeUsage(AttributeTargets.Parameter, Inherited = false)]
internal sealed class NotNullWhenAttribute : Attribute
{
public NotNullWhenAttribute(bool returnValue)
{
}
}

// https://docs.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.allownullattribute
[System.AttributeUsage(System.AttributeTargets.Field | System.AttributeTargets.Parameter | System.AttributeTargets.Property, Inherited=false)]
internal sealed class AllowNullAttribute : Attribute { }
}
#endif
Loading

0 comments on commit e0b6a2c

Please sign in to comment.