Trim leading v from TargetFrameworkVersion #7544
Conversation
| @@ -285,8 +285,7 @@ private static bool PlatformAnalysisAllowed(AnalyzerOptions options, Compilation | |||
| string tfmVersion = options.GetMSBuildPropertyValue(MSBuildPropertyOptionNames.TargetFrameworkVersion, compilation) ?? ""; | |||
|
|
|||
| if (tfmIdentifier.Equals(NetCoreAppIdentifier, StringComparison.OrdinalIgnoreCase) && | |||
| tfmVersion.StartsWith("v", StringComparison.OrdinalIgnoreCase) && | |||
| Version.TryParse(tfmVersion[1..], out var version) && | |||
| Version.TryParse(tfmVersion.TrimStart(['v', 'V']), out var version) && | |||
There was a problem hiding this comment.
An interesting "quirk" of this is that technically vvvvvvvv5.0 is also valid (and the SDK appears to "support" it as well since they also just TrimStart("vV")); but I don't think that's worth adding an explicit test around 😄
| @@ -285,8 +285,7 @@ private static bool PlatformAnalysisAllowed(AnalyzerOptions options, Compilation | |||
| string tfmVersion = options.GetMSBuildPropertyValue(MSBuildPropertyOptionNames.TargetFrameworkVersion, compilation) ?? ""; | |||
|
|
|||
| if (tfmIdentifier.Equals(NetCoreAppIdentifier, StringComparison.OrdinalIgnoreCase) && | |||
| tfmVersion.StartsWith("v", StringComparison.OrdinalIgnoreCase) && | |||
| Version.TryParse(tfmVersion[1..], out var version) && | |||
| Version.TryParse(tfmVersion.TrimStart(['v', 'V']), out var version) && | |||
There was a problem hiding this comment.
Worth noting that the reason this is TrimStart(['v', 'V']) and not TrimStart("vV") is because analyzers are .NET Standard 2.0 and the latter API returns a ROSpan<char>.
Since we're .NET Standard 2.0, there is no Version.TryParse(ROSpan<char>, out Version) API and we have to convert back to string anways. So I went with what I thought was the shorter/simpler option here.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7544 +/- ##
==========================================
- Coverage 96.50% 96.50% -0.01%
==========================================
Files 1452 1452
Lines 347574 347575 +1
Branches 11418 11418
==========================================
- Hits 335424 335423 -1
- Misses 9254 9256 +2
Partials 2896 2896 |
|
-- I don't actually have permission to merge, so if someone else could that'd be great; thanks! |
This is follow up to #7542 which ensures that our checks of
build_property.TargetFrameworkVersionsupport no leadingv, the same as the SDK allows.