Skip to content

Conversation

@axunonb
Copy link
Collaborator

@axunonb axunonb commented May 12, 2025

  • Add package Microsoft.CodeAnalysis.NetAnalyzers (for build only)
  • Add dotnet_diagnostic.CA1305.severity = warning to .editorconfig
  • Fix CA1305 warning or disable with pragma where appropriate
  • Add unit test as proposed in issue

Resolves #792

@codecov
Copy link

codecov bot commented May 12, 2025

Codecov Report

Attention: Patch coverage is 90.62500% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
....Net/Serialization/DataTypes/DurationSerializer.cs 83% 1 Missing ⚠️
...l.Net/Serialization/DataTypes/IntegerSerializer.cs 67% 1 Missing ⚠️
...et/Serialization/DataTypes/StatusCodeSerializer.cs 0% 1 Missing ⚠️

❌ Your project status has failed because the head coverage (67%) is below the target coverage (80%). You can increase the head coverage or adjust the target coverage.

Impacted file tree graph

@@         Coverage Diff         @@
##           main   #794   +/-   ##
===================================
  Coverage    67%    67%           
===================================
  Files       105    105           
  Lines      4414   4419    +5     
  Branches   1060   1060           
===================================
+ Hits       2936   2940    +4     
- Misses     1044   1045    +1     
  Partials    434    434           
Files with missing lines Coverage Δ
Ical.Net/DataTypes/GeographicLocation.cs 50% <100%> (ø)
Ical.Net/DataTypes/UTCOffset.cs 64% <100%> (+5%) ⬆️
....Net/Serialization/DataTypes/DateTimeSerializer.cs 85% <100%> (ø)
...alization/DataTypes/RecurrencePatternSerializer.cs 93% <100%> (ø)
...Net/Serialization/DataTypes/UtcOffsetSerializer.cs 65% <100%> (+2%) ⬆️
...l.Net/Serialization/DataTypes/WeekDaySerializer.cs 61% <100%> (ø)
....Net/Serialization/DataTypes/DurationSerializer.cs 86% <83%> (ø)
...l.Net/Serialization/DataTypes/IntegerSerializer.cs 57% <67%> (ø)
...et/Serialization/DataTypes/StatusCodeSerializer.cs 46% <0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@axunonb axunonb force-pushed the wip/axunonb/pr/ca1305-warnings-fixed branch 2 times, most recently from b6c1018 to a636a61 Compare May 12, 2025 14:39
@axunonb axunonb requested a review from minichma May 12, 2025 14:41
* Add package Microsoft.CodeAnalysis.NetAnalyzers (for build only)
* Add `dotnet_diagnostic.CA1305.severity = warning` to .editorconfig
* Fix `CA1305` warning or disable where appropriate
@axunonb axunonb force-pushed the wip/axunonb/pr/ca1305-warnings-fixed branch from a636a61 to 8e868f5 Compare May 12, 2025 17:04
@axunonb axunonb marked this pull request as ready for review May 12, 2025 17:06
minichma
minichma previously approved these changes May 12, 2025
Copy link
Collaborator

@minichma minichma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was fast. Just a note on string interpolation. In a work project we have some utility method like the following:

        public static string Invariant(IFormattable v)
            => (v == null) ? null : v.ToString(null, CultureInfo.InvariantCulture);

This can be used to make string interpolation culture invariant like so:

Invariant($"{...}")

Maybe an option instead of the pragmas.

@axunonb axunonb force-pushed the wip/axunonb/pr/ca1305-warnings-fixed branch from 6ee7fd9 to 11d3840 Compare May 12, 2025 20:17
@sonarqubecloud
Copy link

@axunonb
Copy link
Collaborator Author

axunonb commented May 12, 2025

public static string Invariant(IFormattable v)
=> (v == null) ? null : v.ToString(null, CultureInfo.InvariantCulture);

Thanks for the hint.

We can also use the built-in FormattableString.Invariant e.g.

sb.Append(FormattableString.Invariant($"{sign * (ts.Hours)}H"));

directly. I was not aware of that.

Here the compiler automatically creates a FormattableString object instead of a string under the hood

FormattableString fs = $"{sign * (ts.Hours ?? 0)}H";
sb.Append(FormattableString.Invariant(fs));

From NET6.0 the right way seems to be (according to SonarCloud)
string.Create(CultureInfo.InvariantCulture, $"{sign * (ts.Hours)}H"); with less memory allocation
which unfortunately is not supported in netstandard2.x

@axunonb axunonb requested a review from minichma May 12, 2025 20:27
@minichma
Copy link
Collaborator

Very nice and good to know, haven't been aware of that either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parsing fails in certain cultures

3 participants