Skip to content

Commit

Permalink
Improve code consistency from SonarCloud hints (#365)
Browse files Browse the repository at this point in the history
* Fix SonarCloud code consistency hints
* Add timeout to RegEx
* Remove an unused variable
  • Loading branch information
axunonb authored Jan 7, 2024
1 parent a995ac4 commit a233d38
Show file tree
Hide file tree
Showing 14 changed files with 39 additions and 46 deletions.
4 changes: 2 additions & 2 deletions src/SmartFormat.Extensions.Time/TimeFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private TimeTextInfo GetTimeTextInfo(IFormattingInfo formattingInfo, bool v2Comp

if (timeTextInfoFromCulture != null) return timeTextInfoFromCulture;

if(timeTextInfoFromCulture is null && FallbackLanguage == string.Empty)
if(FallbackLanguage == string.Empty)
throw new FormattingException(formattingInfo.Placeholder, $"{nameof(TimeTextInfo)} could not be found for the given culture argument '{formattingInfo.FormatterOptions}'.", 0);

if(FallbackLanguage != string.Empty)
Expand Down Expand Up @@ -232,4 +232,4 @@ private static CultureInfo GetCultureInfo(IFormattingInfo formattingInfo, bool v

return cultureInfo;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Copyright SmartFormat Project maintainers and contributors.
// Licensed under the MIT license.

using System;
using System.Collections.Generic;
using System.Text.RegularExpressions;

Expand All @@ -10,9 +11,8 @@ namespace SmartFormat.Extensions.Time.Utilities;
internal static class TimeSpanFormatOptionsConverter
{
private static readonly Regex parser =
new Regex(
@"\b(w|week|weeks|d|day|days|h|hour|hours|m|minute|minutes|s|second|seconds|ms|millisecond|milliseconds|auto|short|fill|full|abbr|noabbr|less|noless)\b",
RegexOptions.Compiled | RegexOptions.IgnoreCase);
new(@"\b(w|week|weeks|d|day|days|h|hour|hours|m|minute|minutes|s|second|seconds|ms|millisecond|milliseconds|auto|short|fill|full|abbr|noabbr|less|noless)\b",
RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromMilliseconds(500));

public static TimeSpanFormatOptions Merge(this TimeSpanFormatOptions left, TimeSpanFormatOptions right)
{
Expand Down Expand Up @@ -117,4 +117,4 @@ public static TimeSpanFormatOptions Parse(string formatString)

return t;
}
}
}
30 changes: 14 additions & 16 deletions src/SmartFormat.Extensions.Xml/XmlSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,19 @@ public class XmlSource : Source
/// <inheritdoc />
public override bool TryEvaluateSelector(ISelectorInfo selectorInfo)
{
if (selectorInfo.CurrentValue is XElement element)
{
var selector = selectorInfo.SelectorText;
// Find elements that match a selector
var selectorMatchedElements =
element.Elements()
.Where(x => x.Name.LocalName == selector)
.ToList();
if (selectorMatchedElements.Any())
{
selectorInfo.Result = selectorMatchedElements;
return true;
}
}
if (selectorInfo.CurrentValue is not XElement element) return false;

var selector = selectorInfo.SelectorText;
// Find elements that match a selector
var selectorMatchedElements =
element.Elements()
.Where(x => x.Name.LocalName == selector)
.ToList();

if (selectorMatchedElements.Count == 0) return false;

selectorInfo.Result = selectorMatchedElements;
return true;

return false;
}
}
}
2 changes: 0 additions & 2 deletions src/SmartFormat.Tests/Core/FormatterExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public void Add_Known_FormatterExtensions_In_Recommended_Order()

#region: Default Extensions :

#pragma warning disable CA1861 // Not called repeatedly, but called with different arguments
[TestCase("{0:cond:zero|one|two}", 0, "zero")]
[TestCase("{0:cond:zero|one|two}", 1, "one")]
[TestCase("{0:cond:zero|one|two}", 2, "two")]
Expand All @@ -97,7 +96,6 @@ public void Invoke_extensions_by_name(string format, object arg0, string expecte
var actualResult = smart.Format(new CultureInfo("en-US"), format, arg0); // must be culture with decimal point
Assert.That(actualResult, Is.EqualTo(expectedResult));
}
#pragma warning restore CA1861

#endregion

Expand Down
4 changes: 0 additions & 4 deletions src/SmartFormat.Tests/Core/SettingsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ public void TryingToAddDisallowedSelectorCharacters_Should_Throw()
public void ExistingSelectorCharacter_Should_Not_Be_Added()
{
var settings = new SmartSettings();
#pragma warning disable CA1861 // Not called repeatedly
settings.Parser.AddCustomSelectorChars(new[] {'A', ' '});
settings.Parser.AddCustomSelectorChars(new[] {' '});
#pragma warning restore CA1861
Assert.Multiple(() =>
{
Assert.That(settings.Parser.CustomSelectorChars().Count(c => c == 'A'), Is.EqualTo(0));
Expand All @@ -42,10 +40,8 @@ public void TryingToAddDisallowedOperatorCharacters_Should_Throw()
public void ExistingOperatorCharacter_Should_Not_Be_Added()
{
var settings = new SmartSettings();
#pragma warning disable CA1861 // Not called repeatedly
settings.Parser.AddCustomOperatorChars(new[] {settings.Parser.OperatorChars()[0], '°'});
settings.Parser.AddCustomOperatorChars(new[] {'°'});
#pragma warning restore CA1861

Assert.Multiple(() =>
{
Expand Down
2 changes: 0 additions & 2 deletions src/SmartFormat.Tests/Extensions/ListFormatterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,11 @@ public void NestedArraysTest(string format, string expected)
[Test, Description("Format a list of lists")]
public void List_Of_Lists_With_Element_Format()
{
#pragma warning disable CA1861 // Not called repeatedly, but called with different arguments
var data = new List<int[]> {
new[] { 1, 2, 3 },
new[] { 4, 5, 6 },
new[] { 7, 8, 9 }
};
#pragma warning restore CA1861

var formatter = new SmartFormatter()
.AddExtensions(new ListFormatter(), new DefaultSource())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ public void NamedFormatter_should_use_specific_language(string format, object ar
Assert.That(actualResult, Is.EqualTo(expectedResult));
}

#pragma warning disable CA1861 // Not called repeatedly, but called with different arguments
[TestCase("{0:plural:zero|one|many}", new string[0], "zero")]
[TestCase("{0:plural:zero|one|many}", new[] { "alice" }, "one")]
[TestCase("{0:plural:zero|one|many}", new[] { "alice", "bob" }, "many")]
Expand All @@ -281,7 +280,6 @@ public void Test_should_allow_ienumerable_parameter(string format, object arg0,
var actualResult = smart.Format(culture, format, arg0);
Assert.That(actualResult, Is.EqualTo(expectedResult));
}
#pragma warning restore CA1861

[Test]
public void Test_With_CustomPluralRuleProvider()
Expand Down
4 changes: 3 additions & 1 deletion src/SmartFormat.Tests/SmartFormat.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
</ItemGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
<NoWarn>$(NoWarn);CA1861</NoWarn>
<WarningLevel>4</WarningLevel>
<DefineConstants>DEBUG;TRACE</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|AnyCPU'">
<WarningLevel>3</WarningLevel>
<NoWarn>$(NoWarn);CA1861</NoWarn>
<WarningLevel>4</WarningLevel>
<DefineConstants>RELEASE</DefineConstants>
</PropertyGroup>

Expand Down
8 changes: 6 additions & 2 deletions src/SmartFormat.Tests/TestUtils/ReflectionTools.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,16 @@ IEnumerable<Type> GetAllAscendants(Type t)
}
}

#if NET6_0_OR_GREATER
ArgumentNullException.ThrowIfNull(assembly, nameof(assembly));
ArgumentNullException.ThrowIfNull(genericTypeDefinition, nameof(genericTypeDefinition));
#else
if (assembly == null)
throw new ArgumentNullException(nameof(assembly));

if (genericTypeDefinition == null)
throw new ArgumentNullException(nameof(genericTypeDefinition));

#endif
if (!genericTypeDefinition.IsGenericTypeDefinition)
throw new ArgumentException(
@"Specified type is not a valid generic type definition.",
Expand All @@ -61,4 +65,4 @@ public static bool IsSubclassOf(Type typeToTest, Assembly assembly, Type generic
{
return GetSubclassesOf(assembly, genericTypeDefinition).Any(t => t == typeToTest);
}
}
}
6 changes: 3 additions & 3 deletions src/SmartFormat/Extensions/ConditionalFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class ConditionalFormatter : IFormatter
private static readonly Regex _complexConditionPattern
= new(@"^ (?: ([&/]?) ([<>=!]=?) ([0-9.-]+) )+ \?",
// Description: and/or comparator value
RegexOptions.IgnorePatternWhitespace | RegexOptions.Compiled);
RegexOptions.IgnorePatternWhitespace | RegexOptions.Compiled, TimeSpan.FromMilliseconds(500));

private char _splitChar = '|';

Expand Down Expand Up @@ -127,7 +127,7 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
case DateTime dateTimeArg when dateTimeArg.ToUniversalTime() <= SystemTime.Now().ToUniversalTime():
paramIndex = 0;
break;
case DateTime dateTimeArg:
case DateTime:
paramIndex = paramCount - 1;
break;
// Date: Past|Present|Future or Past/Present|Future
Expand Down Expand Up @@ -239,4 +239,4 @@ private static bool TryEvaluateCondition(Format parameter, decimal value, out bo
outputItem = parameter.Substring(newStartIndex);
return true;
}
}
}
4 changes: 2 additions & 2 deletions src/SmartFormat/Extensions/IsMatchFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public bool TryEvaluateFormat(IFormattingInfo formattingInfo)
$"Formatter named '{formattingInfo.Placeholder?.FormatterName}' requires at least 2 format options.");
}

var regEx = new Regex(expression, RegexOptions);
var regEx = new Regex(expression, RegexOptions, TimeSpan.FromMilliseconds(500));
var match = regEx.Match(formattingInfo.CurrentValue.ToString());

if (!match.Success)
Expand Down Expand Up @@ -157,4 +157,4 @@ public void Initialize(SmartFormatter smartFormatter)
if (smartFormatter.GetSourceExtension<KeyValuePairSource>() is null)
smartFormatter.AddExtensions(new KeyValuePairSource());
}
}
}
2 changes: 1 addition & 1 deletion src/SmartFormat/Extensions/ListFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ private static void FormatItems(ICollection items, SplitList parameters, Format
}
}

private static void WriteSpacer(IFormattingInfo formattingInfo, Format spacer, object? value)
private static void WriteSpacer(FormattingInfo formattingInfo, Format spacer, object? value)
{
if (spacer.HasNested)
formattingInfo.FormatAsChild(spacer, value);
Expand Down
4 changes: 2 additions & 2 deletions src/SmartFormat/Extensions/TemplateFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace SmartFormat.Extensions;
public class TemplateFormatter : IFormatter, IInitializer
{
private SmartFormatter? _formatter;
private IDictionary<string, Format>? _templates;
private Dictionary<string, Format>? _templates;
private readonly bool _canHandleAutoDetection = false;

/// <summary>
Expand Down Expand Up @@ -101,4 +101,4 @@ public void Initialize(SmartFormatter smartFormatter)
var stringComparer = _formatter.Settings.GetCaseSensitivityComparer();
_templates = new Dictionary<string, Format>(stringComparer);
}
}
}
5 changes: 2 additions & 3 deletions src/SmartFormat/SmartFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ public void FormatInto(IOutput output, IFormatProvider? provider, Format formatP
#region: Private methods :

private void FormatError(FormatItem errorItem, Exception innerException, int startIndex,
IFormattingInfo formattingInfo)
FormattingInfo formattingInfo)
{
OnFormattingFailure?.Invoke(this,
new FormattingErrorEventArgs(errorItem.RawText, startIndex,
Expand Down Expand Up @@ -614,8 +614,7 @@ private bool InvokeFormatterExtensions(FormattingInfo formattingInfo)
{
if (formattingInfo.Placeholder is null)
{
throw new ArgumentException(
$"{nameof(formattingInfo)}.{nameof(formattingInfo.Placeholder)} must not be null.");
throw new ArgumentException($"The property {nameof(formattingInfo)}.{nameof(formattingInfo.Placeholder)} must not be null.", nameof(formattingInfo));
}

var formatterName = formattingInfo.Placeholder.FormatterName;
Expand Down

0 comments on commit a233d38

Please sign in to comment.