FreeAndNil commented on code in PR #131:
URL: https://github.com/apache/logging-log4net/pull/131#discussion_r1540190416
##########
src/log4net.Tests/Util/SystemInfoTest.cs:
##########
@@ -55,9 +55,9 @@ public void
TestAssemblyLocationInfoDoesNotThrowNotSupportedExceptionForDynamicA
private static Func<string> GetAssemblyLocationInfoMethodCall()
{
- var method =
typeof(SystemInfoTest).GetMethod("TestAssemblyLocationInfoMethod", new Type[0]);
- var methodCall = Expression.Call(null, method, new Expression[0]);
- return Expression.Lambda<Func<string>>(methodCall, new
ParameterExpression[0]).Compile();
+ var method =
typeof(SystemInfoTest).GetMethod("TestAssemblyLocationInfoMethod",
Array.Empty<Type>());
Review Comment:
```suggestion
var method =
typeof(SystemInfoTest).GetMethod(nameof(TestAssemblyLocationInfoMethod),
Array.Empty<Type>());
```
##########
src/log4net.Tests/Util/PatternConverterTest.cs:
##########
@@ -111,53 +113,56 @@ public void PatternConverterProperties()
</log4net>");
ILoggerRepository rep =
LogManager.CreateRepository(Guid.NewGuid().ToString());
- XmlConfigurator.Configure(rep, log4netConfig["log4net"]);
+ XmlConfigurator.Configure(rep, log4netConfig["log4net"]!);
ILog log = LogManager.GetLogger(rep.Name, "PatternConverterProperties");
log.Debug("Message");
- PropertyKeyCountPatternConverter converter =
+ PropertyKeyCountPatternConverter? converter =
PropertyKeyCountPatternConverter.MostRecentInstance;
- Assert.AreEqual(2, converter.Properties.Count);
+ Assert.IsNotNull(converter);
+ Assert.IsNotNull(converter!.Properties);
+ Assert.AreEqual(2, converter!.Properties!.Count);
Assert.AreEqual("4", converter.Properties["two-plus-two"]);
PatternStringAppender appender =
(PatternStringAppender)LogManager.GetRepository(rep.Name).GetAppenders()[0];
- Assert.AreEqual("2", appender.Setting.Format());
+ Assert.IsNotNull(appender.Setting);
+ Assert.AreEqual("2", appender.Setting!.Format());
}
}
public class PropertyKeyCountPatternLayoutConverter : PatternLayoutConverter
{
- private static PropertyKeyCountPatternLayoutConverter mostRecentInstance;
+ private static PropertyKeyCountPatternLayoutConverter? mostRecentInstance;
public PropertyKeyCountPatternLayoutConverter() => mostRecentInstance =
this;
- protected override void Convert(TextWriter writer, LoggingEvent
loggingEvent) => writer.Write(Properties.GetKeys().Length);
+ protected override void Convert(TextWriter writer, LoggingEvent
loggingEvent) => writer.Write(Properties!.GetKeys().Length);
- public static PropertyKeyCountPatternLayoutConverter MostRecentInstance =>
mostRecentInstance;
+ public static PropertyKeyCountPatternLayoutConverter? MostRecentInstance
=> mostRecentInstance;
}
public class PropertyKeyCountPatternConverter : PatternConverter
{
- private static PropertyKeyCountPatternConverter mostRecentInstance;
+ private static PropertyKeyCountPatternConverter? mostRecentInstance;
public PropertyKeyCountPatternConverter() => mostRecentInstance = this;
- public override void Convert(TextWriter writer, object state)
- => writer.Write(Properties.GetKeys().Length);
+ public override void Convert(TextWriter writer, object? state)
+ => writer.Write(Properties!.GetKeys().Length);
- public static PropertyKeyCountPatternConverter MostRecentInstance =>
mostRecentInstance;
+ public static PropertyKeyCountPatternConverter? MostRecentInstance =>
mostRecentInstance;
}
public class PatternStringAppender : StringAppender
{
- private static PatternStringAppender mostRecentInstace;
+ private static PatternStringAppender? mostRecentInstace;
public PatternStringAppender() => mostRecentInstace = this;
- public PatternString Setting { get; set; }
+ public PatternString? Setting { get; set; }
- public static PatternStringAppender MostRecentInstace => mostRecentInstace;
+ public static PatternStringAppender? MostRecentInstace =>
mostRecentInstace;
Review Comment:
Please use an auto property.
##########
src/log4net/ILog.cs:
##########
@@ -216,7 +216,7 @@ public interface ILog : ILoggerWrapper
/// </remarks>
/// <seealso cref="M:Debug(object)"/>
/// <seealso cref="IsDebugEnabled"/>
- void DebugFormat(IFormatProvider provider, string format, params object[]
args);
+ void DebugFormat(IFormatProvider? provider, string format, params object[]
args);
Review Comment:
```suggestion
void DebugFormat(IFormatProvider? provider, string format, params
object?[]? args);
```
same here
##########
src/log4net/Layout/XmlLayoutSchemaLog4j.cs:
##########
@@ -203,33 +208,38 @@ protected override void FormatXml(XmlWriter writer,
LoggingEvent loggingEvent)
writer.WriteAttributeString("name", entry.Key);
// Use an ObjectRenderer to convert the object to a string
- string valueStr =
loggingEvent.Repository.RendererMap.FindAndRender(entry.Value);
- writer.WriteAttributeString("value", valueStr);
+ string? valueStr =
loggingEvent.Repository?.RendererMap.FindAndRender(entry.Value);
+ if (!string.IsNullOrEmpty(valueStr))
+ {
+ writer.WriteAttributeString("value", valueStr);
+ }
writer.WriteEndElement();
}
writer.WriteEndElement();
}
- string exceptionStr = loggingEvent.GetExceptionString();
- if (exceptionStr != null && exceptionStr.Length > 0)
+ string? exceptionStr = loggingEvent.GetExceptionString();
+ if (!string.IsNullOrEmpty(exceptionStr))
{
// Append the stack trace line
writer.WriteStartElement("log4j:throwable", "log4j", "data",
"log4net");
- Transform.WriteEscapedXmlString(writer, exceptionStr,
InvalidCharReplacement);
+ Transform.WriteEscapedXmlString(writer, exceptionStr!,
InvalidCharReplacement);
writer.WriteEndElement();
}
if (LocationInfo)
{
LocationInfo? locationInfo = loggingEvent.LocationInformation;
-
- writer.WriteStartElement("log4j:locationInfo", "log4j",
"locationInfo", "log4net");
- writer.WriteAttributeString("class", locationInfo.ClassName);
- writer.WriteAttributeString("method", locationInfo.MethodName);
- writer.WriteAttributeString("file", locationInfo.FileName);
- writer.WriteAttributeString("line", locationInfo.LineNumber);
- writer.WriteEndElement();
+ if (locationInfo is not null)
Review Comment:
pattern matching
##########
src/log4net/MDC.cs:
##########
@@ -84,14 +65,10 @@ private MDC()
/// </para>
/// </remarks>
/*[Obsolete("MDC has been replaced by ThreadContext.Properties")]*/
- public static string Get(string key)
+ public static string? Get(string key)
{
- object obj = ThreadContext.Properties[key];
- if (obj == null)
- {
- return null;
- }
- return obj.ToString();
+ object? obj = ThreadContext.Properties[key];
Review Comment:
inline local
##########
src/log4net/Util/PatternParser.cs:
##########
@@ -320,27 +277,28 @@ private void ProcessLiteral(string text)
/// <param name="converterName">the name of the converter</param>
/// <param name="option">the optional option for the converter</param>
/// <param name="formattingInfo">the formatting info for the
converter</param>
- private void ProcessConverter(string converterName, string option,
FormattingInfo formattingInfo)
+ private void ProcessConverter(string converterName, string? option,
FormattingInfo formattingInfo)
{
- LogLog.Debug(declaringType, "Converter [" + converterName + "] Option ["
+ option + "] Format [min=" + formattingInfo.Min + ",max=" + formattingInfo.Max
+ ",leftAlign=" + formattingInfo.LeftAlign + "]");
+ LogLog.Debug(declaringType, $"Converter [{converterName}] Option
[{option}] Format
[min={formattingInfo.Min},max={formattingInfo.Max},leftAlign={formattingInfo.LeftAlign}]");
// Lookup the converter type
- ConverterInfo converterInfo =
(ConverterInfo)m_patternConverters[converterName];
- if (converterInfo == null)
+ ConverterInfo? converterInfo =
(ConverterInfo)PatternConverters[converterName];
Review Comment:
use pattern matching
##########
src/log4net/Util/TypeConverters/IPAddressConverter.cs:
##########
@@ -72,25 +70,22 @@ public bool CanConvertFrom(Type sourceType)
/// </exception>
public object ConvertFrom(object source)
{
- string str = source as string;
- if (str != null && str.Length > 0)
+ if (source is string str && str.Length > 0)
{
try
{
// Try an explicit parse of string representation of an IPAddress
(v4 or v6)
- IPAddress result;
- if (IPAddress.TryParse(str, out result))
+ if (IPAddress.TryParse(str, out IPAddress result))
{
return result;
}
// Try to resolve via DNS. This is a blocking call.
// GetHostEntry works with either an IPAddress string or a host name
- IPHostEntry host = Dns.GetHostEntry(str);
- if (host != null &&
- host.AddressList != null &&
+ IPHostEntry? host = Dns.GetHostEntry(str);
+ if (host?.AddressList is not null &&
Review Comment:
```suggestion
if (host?.AddressList?.FirstOrDefault() is IPAddress address)
```
##########
src/log4net/Util/ThreadContextStacks.cs:
##########
@@ -24,54 +24,30 @@ namespace log4net.Util
/// <summary>
/// Implementation of Stacks collection for the <see
cref="log4net.ThreadContext"/>
/// </summary>
- /// <remarks>
- /// <para>
- /// Implementation of Stacks collection for the <see
cref="log4net.ThreadContext"/>
- /// </para>
- /// </remarks>
/// <author>Nicko Cadell</author>
public sealed class ThreadContextStacks
{
private readonly ContextPropertiesBase m_properties;
- #region Public Instance Constructors
-
/// <summary>
- /// Internal constructor
- /// </summary>
- /// <remarks>
- /// <para>
/// Initializes a new instance of the <see cref="ThreadContextStacks" />
class.
- /// </para>
- /// </remarks>
+ /// </summary>
internal ThreadContextStacks(ContextPropertiesBase properties)
{
m_properties = properties;
}
- #endregion Public Instance Constructors
-
- #region Public Instance Properties
-
/// <summary>
- /// Gets the named thread context stack
+ /// Gets the named thread context stack.
/// </summary>
- /// <value>
- /// The named stack
- /// </value>
- /// <remarks>
- /// <para>
- /// Gets the named thread context stack
- /// </para>
- /// </remarks>
public ThreadContextStack this[string key]
{
get
{
- ThreadContextStack stack = null;
+ ThreadContextStack? stack;
- object propertyValue = m_properties[key];
- if (propertyValue == null)
+ object? propertyValue = m_properties[key];
+ if (propertyValue is null)
Review Comment:
use pattern matching
##########
src/log4net/Appender/TextWriterAppender.cs:
##########
@@ -371,7 +371,7 @@ protected virtual void WriteHeader()
{
if (Layout is not null && QuietWriter is not null && !QuietWriter.Closed)
{
- string h = Layout.Header;
+ string? h = Layout.Header;
Review Comment:
use pattern matching
##########
src/log4net/Appender/SmtpPickupDirAppender.cs:
##########
@@ -187,6 +187,11 @@ protected override void SendBuffer(LoggingEvent[] events)
/// </remarks>
public override void ActivateOptions()
{
+ if (PickupDir is null)
+ {
+ throw new ArgumentException($"{nameof(PickupDir)} must be specified");
Review Comment:
```suggestion
throw new ArgumentException($"{nameof(PickupDir)} must be
specified", nameof(PickupDir));
```
##########
src/log4net/Appender/UdpAppender.cs:
##########
@@ -315,8 +315,9 @@ public override void ActivateOptions()
if (RemoteAddress is null)
{
- throw new ArgumentNullException("The required property 'Address' was
not specified.");
+ throw new ArgumentNullException($"The required property
'{nameof(RemoteAddress)}' was not specified.");
Review Comment:
```suggestion
throw new ArgumentNullException(nameof(RemoteAddress), $"The
required property '{nameof(RemoteAddress)}' was not specified.");
```
##########
src/log4net/Core/LogImpl.cs:
##########
@@ -326,7 +329,7 @@ public virtual void DebugFormat(string format, object arg0,
object arg1, object
/// methods instead.
/// </para>
/// </remarks>
- public virtual void DebugFormat(IFormatProvider provider, string format,
params object[] args)
+ public virtual void DebugFormat(IFormatProvider? provider, string format,
params object[] args)
Review Comment:
```suggestion
public virtual void DebugFormat(IFormatProvider? provider, string
format, params object?[]? args)
```
The same should apply to the other ...Format-Methods.
##########
src/log4net/Appender/EventLogAppender.cs:
##########
@@ -344,7 +344,7 @@ protected override void Append(LoggingEvent loggingEvent)
int eventID = m_eventId;
// Look for the EventID property
- object eventIDPropertyObj = loggingEvent.LookupProperty("EventID");
+ object? eventIDPropertyObj = loggingEvent.LookupProperty("EventID");
Review Comment:
use pattern matching
##########
src/log4net/Appender/BufferingAppenderSkeleton.cs:
##########
@@ -437,7 +437,7 @@ protected override void Append(LoggingEvent loggingEvent)
loggingEvent.Fix = Fix;
// Add to the buffer, returns the event discarded from the buffer if
there is no space remaining after the append
- LoggingEvent discardedLoggingEvent = m_cb.Append(loggingEvent);
+ LoggingEvent? discardedLoggingEvent = m_cb.Append(loggingEvent);
Review Comment:
```suggestion
if (m_cb.Append(loggingEvent) is LoggingEvent discardedLoggingEvent)
```
##########
src/log4net/Layout/XmlLayout.cs:
##########
@@ -266,14 +276,16 @@ protected override void FormatXml(XmlWriter writer,
LoggingEvent loggingEvent)
if (LocationInfo)
{
- LocationInfo locationInfo = loggingEvent.LocationInformation;
-
- writer.WriteStartElement(m_elmLocation, Prefix, ELM_LOCATION, Prefix);
- writer.WriteAttributeString(ATTR_CLASS, locationInfo.ClassName);
- writer.WriteAttributeString(ATTR_METHOD, locationInfo.MethodName);
- writer.WriteAttributeString(ATTR_FILE, locationInfo.FileName);
- writer.WriteAttributeString(ATTR_LINE, locationInfo.LineNumber);
- writer.WriteEndElement();
+ LocationInfo? locationInfo = loggingEvent.LocationInformation;
Review Comment:
Pattern matching
##########
src/log4net/Layout/XmlLayoutSchemaLog4j.cs:
##########
@@ -170,24 +170,29 @@ protected override void FormatXml(XmlWriter writer,
LoggingEvent loggingEvent)
TimeSpan timeSince1970 = loggingEvent.TimeStampUtc - s_date1970;
writer.WriteAttributeString("timestamp",
XmlConvert.ToString((long)timeSince1970.TotalMilliseconds));
- writer.WriteAttributeString("level", loggingEvent.Level.DisplayName);
+ if (loggingEvent.Level is not null)
+ {
+ writer.WriteAttributeString("level", loggingEvent.Level.DisplayName);
+ }
writer.WriteAttributeString("thread", loggingEvent.ThreadName);
// Append the message text
- writer.WriteStartElement("log4j:message", "log4j", "message", "log4net");
- Transform.WriteEscapedXmlString(writer, loggingEvent.RenderedMessage,
InvalidCharReplacement);
- writer.WriteEndElement();
+ if (loggingEvent.RenderedMessage is not null)
+ {
+ writer.WriteStartElement("log4j:message", "log4j", "message",
"log4net");
+ Transform.WriteEscapedXmlString(writer, loggingEvent.RenderedMessage,
InvalidCharReplacement);
+ writer.WriteEndElement();
+ }
- object ndcObj = loggingEvent.LookupProperty("NDC");
+ object? ndcObj = loggingEvent.LookupProperty("NDC");
Review Comment:
Pattern matching
##########
src/log4net/Layout/PatternLayout.cs:
##########
@@ -1127,24 +1075,25 @@ public override void Format(TextWriter writer,
LoggingEvent loggingEvent)
/// </remarks>
public void AddConverter(ConverterInfo converterInfo)
{
- if (converterInfo == null) throw new
ArgumentNullException("converterInfo");
-
+ if (converterInfo is null)
+ {
+ throw new ArgumentNullException(nameof(converterInfo));
+ }
if (!typeof(PatternConverter).IsAssignableFrom(converterInfo.Type))
{
- throw new ArgumentException("The converter type specified [" +
converterInfo.Type + "] must be a subclass of log4net.Util.PatternConverter",
"converterInfo");
+ throw new ArgumentException($"The converter type specified
[{converterInfo.Type}] must be a subclass of log4net.Util.PatternConverter",
"converterInfo");
Review Comment:
```suggestion
throw new ArgumentException($"The converter type specified
[{converterInfo.Type}] must be a subclass of log4net.Util.PatternConverter",
nameof(converterInfo));
```
##########
src/log4net/LogManager.cs:
##########
@@ -745,9 +745,9 @@ public static bool Flush(int millisecondsTimeout)
/// </summary>
/// <param name="loggers">The loggers to get the wrappers for.</param>
/// <returns>The wrapper objects for the loggers specified.</returns>
- private static ILog[] WrapLoggers(ILogger[] loggers)
+ private static ILog?[] WrapLoggers(ILogger[] loggers)
Review Comment:
```suggestion
private static ILog[] WrapLoggers(ILogger[] loggers)
```
WrapLogger returns not null, when ILog is not null.
We need to have
https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.notnullifnotnullattribute
```
namespace System.Diagnostics.CodeAnalysis;
/// <summary>
/// Specifies that the output will be non-null if the named parameter is
non-null.
/// </summary>
[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property |
AttributeTargets.ReturnValue,
AllowMultiple = true, Inherited = false)]
internal sealed class NotNullIfNotNullAttribute : Attribute
{
/// <summary>
/// Initializes the attribute with the associated parameter name.
/// </summary>
/// <param name="parameterName">
/// The associated parameter name.
/// The output will be non-null if the argument to the parameter specified
is non-null.
/// </param>
public NotNullIfNotNullAttribute(string parameterName)
=> ParameterName = parameterName;
/// <summary>
/// Gets the associated parameter name.
/// </summary>
public string ParameterName { get; }
}
```
##########
src/log4net/ObjectRenderer/DefaultRenderer.cs:
##########
@@ -194,25 +171,22 @@ public void RenderObject(RendererMap rendererMap, object
obj, TextWriter writer)
return;
}
- IEnumerator objEnumerator = obj as IEnumerator;
- if (objEnumerator != null)
+ if (obj is IEnumerator objEnumerator)
{
RenderEnumerator(rendererMap, objEnumerator, writer);
return;
}
- if (obj is DictionaryEntry)
+ if (obj is DictionaryEntry entry)
{
- RenderDictionaryEntry(rendererMap, (DictionaryEntry)obj, writer);
+ RenderDictionaryEntry(rendererMap, entry, writer);
return;
}
- string str = obj.ToString();
- writer.Write((str == null) ? SystemInfo.NullText : str);
+ string? str = obj.ToString();
Review Comment:
inline
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -311,7 +310,7 @@ public void Configure(XmlElement? element)
{
LogLog.Debug(declaringType, $"Attaching appender named
[{refName}] to appender named [{appender.Name}].");
- IAppender referencedAppender =
FindAppenderByReference(currentElement);
+ IAppender? referencedAppender =
FindAppenderByReference(currentElement);
Review Comment:
Pattern matching
##########
src/log4net/Repository/Hierarchy/XmlHierarchyConfigurator.cs:
##########
@@ -423,7 +422,7 @@ protected void ParseChildrenOfLoggerElement(XmlElement
catElement, Logger log, b
if (currentElement.LocalName == APPENDER_REF_TAG)
{
- IAppender appender = FindAppenderByReference(currentElement);
+ IAppender? appender = FindAppenderByReference(currentElement);
Review Comment:
Pattern matching
##########
src/log4net/Util/PatternStringConverters/IdentityPatternConverter.cs:
##########
@@ -43,15 +43,14 @@ internal sealed class IdentityPatternConverter :
PatternConverter
/// Writes the current thread identity to the output <paramref
name="writer"/>.
/// </para>
/// </remarks>
- public override void Convert(TextWriter writer, object state)
+ public override void Convert(TextWriter writer, object? state)
{
try
{
- if (System.Threading.Thread.CurrentPrincipal != null &&
- System.Threading.Thread.CurrentPrincipal.Identity != null &&
- System.Threading.Thread.CurrentPrincipal.Identity.Name != null)
+ string? name =
System.Threading.Thread.CurrentPrincipal?.Identity?.Name;
+ if (name is not null)
Review Comment:
use pattern matching
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
