Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Guard.ThrowIfNull to throw ArgumentNullException, better than NRE #646

Merged
merged 1 commit into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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