Skip to content

Commit

Permalink
Added Guard.ThrowIfNull to throw ArgumentNullException, better than N…
Browse files Browse the repository at this point in the history
…RE (#646)
  • Loading branch information
snakefoot authored Nov 27, 2022
1 parent fb54d48 commit 8778b7d
Show file tree
Hide file tree
Showing 12 changed files with 128 additions and 30 deletions.
6 changes: 3 additions & 3 deletions src/NLog.Extensions.Hosting/Extensions/ConfigureExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace NLog.Extensions.Hosting
{
/// <summary>
/// Helpers for IHostbuilder, netcore 2.1
/// Helpers for IHostbuilder
/// </summary>
public static class ConfigureExtensions
{
Expand All @@ -24,7 +24,7 @@ public static class ConfigureExtensions
/// <returns>IHostBuilder for chaining</returns>
public static IHostBuilder UseNLog(this IHostBuilder builder)
{
if (builder == null) throw new ArgumentNullException(nameof(builder));
Guard.ThrowIfNull(builder);
return builder.UseNLog(null);
}

Expand All @@ -37,7 +37,7 @@ public static IHostBuilder UseNLog(this IHostBuilder builder)
/// <returns>IHostBuilder for chaining</returns>
public static IHostBuilder UseNLog(this IHostBuilder builder, NLogProviderOptions options)
{
if (builder == null) throw new ArgumentNullException(nameof(builder));
Guard.ThrowIfNull(builder);
#if NETSTANDARD2_0
builder.ConfigureServices((builderContext, services) => AddNLogLoggerProvider(services, builderContext.Configuration, null, options, CreateNLogLoggerProvider));
#else
Expand Down
1 change: 1 addition & 0 deletions src/NLog.Extensions.Hosting/NLog.Extensions.Hosting.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ Full changelog: https://github.com/NLog/NLog.Extensions.Logging/blob/master/CHAN
</ItemGroup>

<ItemGroup>
<Compile Include="..\NLog.Extensions.Logging\Internal\Guard.cs" Link="Internal\Guard.cs" />
<Compile Include="..\NLog.Extensions.Logging\Internal\RegisterNLogLoggingProvider.cs" Link="Internal\RegisterNLogLoggingProvider.cs" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public override IEnumerable<string> FileNamesToWatch
{
get
{
if (_autoReload && _reloadConfiguration == null)
if (_autoReload && _reloadConfiguration is null)
{
// Prepare for setting up reload notification handling
_reloadConfiguration = state => ReloadConfigurationSection((IConfigurationSection)state);
Expand Down Expand Up @@ -229,7 +229,7 @@ private IEnumerable<ILoggingConfigurationElement> GetChildren()
}
}

if (ReferenceEquals(_nameOverride, VariableKey) && _configurationSection.Value == null)
if (ReferenceEquals(_nameOverride, VariableKey) && _configurationSection.Value is null)
{
yield return new LoggingConfigurationElement(_configurationSection);
}
Expand All @@ -255,7 +255,7 @@ private IEnumerable<ILoggingConfigurationElement> GetChildren(IEnumerable<IConfi
}

var firstChildValue = child?.GetChildren()?.FirstOrDefault();
if (firstChildValue == null)
if (firstChildValue is null)
{
continue; // Simple value without children
}
Expand Down Expand Up @@ -286,7 +286,7 @@ private static string GetConfigKey(IConfigurationSection child)

private bool IsTargetsSection()
{
return !_topElement && _nameOverride == null && GetConfigKey(_configurationSection).EqualsOrdinalIgnoreCase(TargetsKey);
return !_topElement && _nameOverride is null && GetConfigKey(_configurationSection).EqualsOrdinalIgnoreCase(TargetsKey);
}

/// <summary>
Expand Down
37 changes: 24 additions & 13 deletions src/NLog.Extensions.Logging/Extensions/ConfigureExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public static ILoggerFactory AddNLog(this ILoggerFactory factory)
#endif
public static ILoggerFactory AddNLog(this ILoggerFactory factory, NLogProviderOptions options)
{
Guard.ThrowIfNull(factory);
factory.AddProvider(new NLogLoggerProvider(options));
return factory;
}
Expand All @@ -56,6 +57,7 @@ public static ILoggerFactory AddNLog(this ILoggerFactory factory, NLogProviderOp
#endif
public static ILoggerFactory AddNLog(this ILoggerFactory factory, IConfiguration configuration)
{
Guard.ThrowIfNull(factory);
var provider = CreateNLogLoggerProvider(null, configuration, null);
factory.AddProvider(provider);
return factory;
Expand All @@ -65,11 +67,11 @@ public static ILoggerFactory AddNLog(this ILoggerFactory factory, IConfiguration
/// <summary>
/// Enable NLog as logging provider for Microsoft Extension Logging
/// </summary>
/// <param name="factory"></param>
/// <param name="builder"></param>
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder factory)
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder)
{
return factory.AddNLog((NLogProviderOptions)null);
return builder.AddNLog((NLogProviderOptions)null);
}

/// <summary>
Expand All @@ -80,33 +82,36 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder factory)
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder factory, IConfiguration configuration)
{
Guard.ThrowIfNull(factory);
AddNLogLoggerProvider(factory, configuration, null, CreateNLogLoggerProvider);
return factory;
}

/// <summary>
/// Enable NLog as logging provider for Microsoft Extension Logging
/// </summary>
/// <param name="factory"></param>
/// <param name="builder"></param>
/// <param name="configuration">Override configuration and not use default Host Builder Configuration</param>
/// <param name="options">NLog Logging Provider options</param>
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder factory, IConfiguration configuration, NLogProviderOptions options)
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, IConfiguration configuration, NLogProviderOptions options)
{
AddNLogLoggerProvider(factory, configuration, options, CreateNLogLoggerProvider);
return factory;
Guard.ThrowIfNull(builder);
AddNLogLoggerProvider(builder, configuration, options, CreateNLogLoggerProvider);
return builder;
}

/// <summary>
/// Enable NLog as logging provider for Microsoft Extension Logging
/// </summary>
/// <param name="factory"></param>
/// <param name="builder"></param>
/// <param name="options">NLog Logging Provider options</param>
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder factory, NLogProviderOptions options)
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, NLogProviderOptions options)
{
AddNLogLoggerProvider(factory, null, options, CreateNLogLoggerProvider);
return factory;
Guard.ThrowIfNull(builder);
AddNLogLoggerProvider(builder, null, options, CreateNLogLoggerProvider);
return builder;
}

/// <summary>
Expand All @@ -129,6 +134,7 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, LoggingConfi
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, LoggingConfiguration configuration, NLogProviderOptions options)
{
Guard.ThrowIfNull(builder);
AddNLogLoggerProvider(builder, null, options, (serviceProvider, config, options) =>
{
var logFactory = configuration?.LogFactory ?? LogManager.LogFactory;
Expand All @@ -148,6 +154,7 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, LoggingConfi
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, string configFileRelativePath)
{
Guard.ThrowIfNull(builder);
AddNLogLoggerProvider(builder, null, null, (serviceProvider, config, options) =>
{
var provider = CreateNLogLoggerProvider(serviceProvider, config, options);
Expand All @@ -167,6 +174,8 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, string confi
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, NLogProviderOptions options, Func<IServiceProvider, LogFactory> factoryBuilder)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(factoryBuilder);
AddNLogLoggerProvider(builder, null, options, (serviceProvider, config, options) =>
{
serviceProvider.SetupNLogConfigSettings(config, LogManager.LogFactory);
Expand All @@ -187,6 +196,8 @@ public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, NLogProvider
/// <returns>ILoggingBuilder for chaining</returns>
public static ILoggingBuilder AddNLog(this ILoggingBuilder builder, Func<IServiceProvider, LogFactory> factoryBuilder)
{
Guard.ThrowIfNull(builder);
Guard.ThrowIfNull(factoryBuilder);
AddNLogLoggerProvider(builder, null, null, (serviceProvider, config, options) =>
{
serviceProvider.SetupNLogConfigSettings(config, LogManager.LogFactory);
Expand Down Expand Up @@ -240,7 +251,7 @@ public static LoggingConfiguration ConfigureNLog(this ILoggerFactory loggerFacto
/// <returns></returns>
public static NLogLoggerProvider Configure(this NLogLoggerProvider nlogProvider, IConfigurationSection configurationSection)
{
if (nlogProvider == null || configurationSection == null)
if (nlogProvider is null || configurationSection is null)
return nlogProvider;

Configure(nlogProvider.Options, configurationSection);
Expand All @@ -257,7 +268,7 @@ public static NLogProviderOptions Configure(this NLogProviderOptions options, IC
{
options = options ?? NLogProviderOptions.Default;

if (configurationSection == null || !(configurationSection.GetChildren()?.Any() ?? false))
if (configurationSection is null || !(configurationSection.GetChildren()?.Any() ?? false))
return options;

var configProps = options.GetType().GetProperties(BindingFlags.Instance | BindingFlags.Public).Where(p => p.SetMethod?.IsPublic == true).ToDictionary(p => p.Name, StringComparer.OrdinalIgnoreCase);
Expand Down
69 changes: 69 additions & 0 deletions src/NLog.Extensions.Logging/Internal/Guard.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//
// Copyright (c) 2004-2021 Jaroslaw Kowalski <jaak@jkowalski.net>, Kim Christensen, Julian Verdurmen
//
// All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions
// are met:
//
// * Redistributions of source code must retain the above copyright notice,
// this list of conditions and the following disclaimer.
//
// * Redistributions in binary form must reproduce the above copyright notice,
// this list of conditions and the following disclaimer in the documentation
// and/or other materials provided with the distribution.
//
// * Neither the name of Jaroslaw Kowalski nor the names of its
// contributors may be used to endorse or promote products derived from this
// software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
// ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
// LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
// CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
// SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
// CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
// THE POSSIBILITY OF SUCH DAMAGE.
//

#if !NETCOREAPP3_1_OR_GREATER
namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Parameter)]
sealed class CallerArgumentExpressionAttribute : Attribute
{
public CallerArgumentExpressionAttribute(string param)
{
Param = param;
}

public string Param { get; }
}
}
#endif

namespace NLog.Extensions.Logging
{
using System;
using System.Runtime.CompilerServices;
internal static class Guard
{
internal static T ThrowIfNull<T>(
T arg,
[CallerArgumentExpression("arg")] string param = "")
where T : class
{
if (arg is null)
{
throw new ArgumentNullException(string.IsNullOrEmpty(param) ? typeof(T).Name : param);
}

return arg;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ internal static NLogLoggerProvider CreateNLogLoggerProvider(this IServiceProvide

var configuration = serviceProvider.SetupNLogConfigSettings(hostConfiguration, provider.LogFactory);

if (configuration != null && (!ReferenceEquals(configuration, hostConfiguration) || options == null))
if (configuration != null && (!ReferenceEquals(configuration, hostConfiguration) || options is null))
{
provider.Configure(configuration.GetSection("Logging:NLog"));
}
Expand Down
6 changes: 3 additions & 3 deletions src/NLog.Extensions.Logging/Logging/NLogBeginScopeParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,12 @@ public static IDisposable CaptureScopeProperties(IEnumerable scopePropertyCollec
var keyValueExtractor = default(KeyValuePair<Func<object, object>, Func<object, object>>);
foreach (var property in scopePropertyCollection)
{
if (property == null)
if (property is null)
{
break;
}

if (keyValueExtractor.Key == null && !TryLookupExtractor(stateExtractor, property.GetType(), out keyValueExtractor))
if (keyValueExtractor.Key is null && !TryLookupExtractor(stateExtractor, property.GetType(), out keyValueExtractor))
{
break;
}
Expand Down Expand Up @@ -257,7 +257,7 @@ private static bool TryBuildExtractor(Type propertyType, out KeyValuePair<Func<o

var keyPropertyInfo = itemType.GetDeclaredProperty("Key");
var valuePropertyInfo = itemType.GetDeclaredProperty("Value");
if (valuePropertyInfo == null || keyPropertyInfo == null)
if (valuePropertyInfo is null || keyPropertyInfo is null)
{
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions src/NLog.Extensions.Logging/Logging/NLogLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void Log<TState>(Microsoft.Extensions.Logging.LogLevel logLevel, EventId
return;
}

if (formatter == null)
if (formatter is null)
{
throw new ArgumentNullException(nameof(formatter));
}
Expand Down Expand Up @@ -233,7 +233,7 @@ private static object[] CreatePositionalLogEventInfoParameters(NLogMessageParame
for (int i = 0; i < messageParameterCount; ++i)
{
// First positional name is the startPos
if (char.IsDigit(messageParameters[i].Name[0]) && paramsArray == null)
if (char.IsDigit(messageParameters[i].Name[0]) && paramsArray is null)
{
paramsArray = new object[maxIndex + 1];
for (int j = 0; j <= maxIndex; ++j)
Expand Down Expand Up @@ -457,7 +457,7 @@ private static LogLevel ConvertLogLevel(Microsoft.Extensions.Logging.LogLevel lo
/// <returns></returns>
public IDisposable BeginScope<TState>(TState state)
{
if (!_options.IncludeScopes || state == null)
if (!_options.IncludeScopes || state is null)
{
return NullScope.Instance;
}
Expand Down
9 changes: 9 additions & 0 deletions test/NLog.Extensions.Hosting.Tests/ExtensionMethodTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -10,6 +11,14 @@ namespace NLog.Extensions.Hosting.Tests
{
public class ExtensionMethodTests
{
[Fact]
public void UseNLog_ArgumentNullException()
{
IHostBuilder hostBuilder = null;
var argNulLException = Assert.Throws<ArgumentNullException>(() => hostBuilder.UseNLog());
Assert.Equal("builder", argNulLException.ParamName);
}

[Fact]
public void UseNLog_noParams_WorksWithNLog()
{
Expand Down
2 changes: 1 addition & 1 deletion test/NLog.Extensions.Logging.Tests/CustomBeginScopeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private class ActionLogScope : IReadOnlyList<KeyValuePair<string, object>>

public ActionLogScope(string world)
{
if (world == null)
if (world is null)
{
throw new ArgumentNullException(nameof(world));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ public void AddNLog_ReplaceLoggerFactory()
Assert.Equal(typeof(NLogLoggerProvider), loggerProvider.GetType());
}

[Fact]
public void AddNLog_ArgumentNullException()
{
ILoggingBuilder loggingBuilder = null;
var argNulLException = Assert.Throws<ArgumentNullException>(() => loggingBuilder.AddNLog());
Assert.Equal("builder", argNulLException.ParamName);
}

[Fact]
public void AddNLog_WithConfig_ReplaceLoggerFactory()
{
Expand Down
4 changes: 2 additions & 2 deletions test/NLog.Extensions.Logging.Tests/NLogTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public abstract class NLogTestBase

protected NLogLoggerProvider ConfigureLoggerProvider(NLogProviderOptions options = null, Action<ServiceCollection> configureServices = null)
{
if (_serviceProvider == null)
if (_serviceProvider is null)
{
var logFactory = new LogFactory();
LoggerProvider = new NLogLoggerProvider(options ?? new NLogProviderOptions { CaptureMessageTemplates = true, CaptureMessageProperties = true }, logFactory);
Expand All @@ -28,7 +28,7 @@ protected NLogLoggerProvider ConfigureLoggerProvider(NLogProviderOptions options

protected IServiceProvider ConfigureTransientService<T>(Action<ServiceCollection> configureServices = null, NLogProviderOptions options = null) where T : class
{
if (_serviceProvider == null)
if (_serviceProvider is null)
ConfigureLoggerProvider(options, s => { s.AddTransient<T>(); configureServices?.Invoke(s); });
return _serviceProvider;
}
Expand Down

0 comments on commit 8778b7d

Please sign in to comment.