Skip to content

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@YuliiaKovalova
YuliiaKovalova committed Aug 25, 2023
1 parent fce84f4 commit cede57e
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 30 deletions.
19 changes: 8 additions & 11 deletions src/MSBuildLocator.Tests/SemanticVersionParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,12 @@ namespace Microsoft.Build.Locator.Tests
{
public class SemanticVersionParserTests
{
private readonly SemanticVersionParser _testedInstance;
public SemanticVersionParserTests() => _testedInstance = new SemanticVersionParser();

[Fact]
public void TryParseTest_ReleaseVersion()
{
var version = "7.0.333";

var isParsed = _testedInstance.TryParse(version, out var parsedVerion);
var isParsed = SemanticVersionParser.TryParse(version, out var parsedVerion);

parsedVerion.ShouldNotBeNull();
isParsed.ShouldBeTrue();
Expand All @@ -34,7 +31,7 @@ public void TryParseTest_PreviewVersion()
{
var version = "8.0.0-preview.6.23329.7";

var isParsed = _testedInstance.TryParse(version, out var parsedVerion);
var isParsed = SemanticVersionParser.TryParse(version, out var parsedVerion);

parsedVerion.ShouldNotBeNull();
isParsed.ShouldBeTrue();
Expand All @@ -49,7 +46,7 @@ public void TryParseTest_InvalidInput_LeadingZero()
{
var version = "0.0-preview.6";

var isParsed = _testedInstance.TryParse(version, out var parsedVerion);
var isParsed = SemanticVersionParser.TryParse(version, out var parsedVerion);

Assert.Null(parsedVerion);
isParsed.ShouldBeFalse();
Expand All @@ -60,7 +57,7 @@ public void TryParseTest_InvalidInput_FourPartsVersion()
{
var version = "5.0.3.4";

var isParsed = _testedInstance.TryParse(version, out var parsedVerion);
var isParsed = SemanticVersionParser.TryParse(version, out var parsedVerion);

Assert.Null(parsedVerion);
isParsed.ShouldBeFalse();
Expand All @@ -71,7 +68,7 @@ public void VersionSortingTest_WithPreview()
{
var versions = new[] { "7.0.7", "8.0.0-preview.6.23329.7", "8.0.0-preview.3.23174.8" };

var maxVersion = versions.Select(v => _testedInstance.TryParse(v, out var parsedVerion) ? parsedVerion : null).Max();
var maxVersion = versions.Select(v => SemanticVersionParser.TryParse(v, out var parsedVerion) ? parsedVerion : null).Max();

maxVersion.OriginalValue.ShouldBe("8.0.0-preview.6.23329.7");
}
Expand All @@ -81,7 +78,7 @@ public void VersionSortingTest_ReleaseOnly()
{
var versions = new[] { "7.0.7", "3.7.2", "10.0.0" };

var maxVersion = versions.Select(v => _testedInstance.TryParse(v, out var parsedVerion) ? parsedVerion : null).Max();
var maxVersion = versions.Max(v => SemanticVersionParser.TryParse(v, out var parsedVerion) ? parsedVerion : null);

maxVersion.OriginalValue.ShouldBe("10.0.0");
}
Expand All @@ -91,7 +88,7 @@ public void VersionSortingTest_WithInvalidFolderNames()
{
var versions = new[] { "7.0.7", "3.7.2", "dummy", "5.7.8.9" };

var maxVersion = versions.Select(v => _testedInstance.TryParse(v, out var parsedVerion) ? parsedVerion : null).Max();
var maxVersion = versions.Max(v => SemanticVersionParser.TryParse(v, out var parsedVerion) ? parsedVerion : null);

maxVersion.OriginalValue.ShouldBe("7.0.7");
}
Expand All @@ -101,7 +98,7 @@ public void VersionSortingTest_WithAllInvalidFolderNames()
{
var versions = new[] { "dummy", "5.7.8.9" };

var maxVersion = versions.Select(v => _testedInstance.TryParse(v, out var parsedVerion) ? parsedVerion : null).Max();
var maxVersion = versions.Max(v => SemanticVersionParser.TryParse(v, out var parsedVerion) ? parsedVerion : null);

maxVersion.ShouldBeNull();
}
Expand Down
7 changes: 2 additions & 5 deletions src/MSBuildLocator/DotNetSdkLocationHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,14 @@ private static IntPtr HostFxrResolver(Assembly assembly, string libraryName)
var hostFxrRoot = Path.Combine(DotnetPath.Value, "host", "fxr");
if (Directory.Exists(hostFxrRoot))
{
var versionParser = new SemanticVersionParser();
// Load hostfxr from the highest version, because it should be backward-compatible
var hostFxrAssemblyDirectory = Directory.GetDirectories(hostFxrRoot)
.Select(str => versionParser.TryParse(str, out var version) ? version : null)
.Max();
.Max(str => SemanticVersionParser.TryParse(str, out var version) ? version : null);

if (hostFxrAssemblyDirectory != null && !string.IsNullOrEmpty(hostFxrAssemblyDirectory.OriginalValue))
{
var hostfxrAssembly = Directory.GetFiles(hostFxrAssemblyDirectory.OriginalValue)
.Where(filePath => filePath.Equals(Path.Combine(hostFxrLibName, libExtention)))
.FirstOrDefault();
.FirstOrDefault(filePath => filePath.Equals(Path.Combine(hostFxrLibName, libExtention)));

if (hostfxrAssembly != null)
{
Expand Down
5 changes: 4 additions & 1 deletion src/MSBuildLocator/Utils/SemanticVersion.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using System.Linq;

Expand Down
23 changes: 15 additions & 8 deletions src/MSBuildLocator/Utils/SemanticVersionParser.cs
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
#if NETCOREAPP
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

#if NETCOREAPP

using System;
using System.Linq;

namespace Microsoft.Build.Locator
{
internal class SemanticVersionParser
/// <summary>
/// Converts string in its semantic version.
/// The basic parser logic is taken from https://github.com/NuGetArchive/NuGet.Versioning/releases/tag/rc-preview1.
/// </summary>
internal static class SemanticVersionParser
{
/// <summary>
/// Parse a version string
/// </summary>
/// <returns>false if the version wasn't parsed</returns>
public bool TryParse(string value, out SemanticVersion version)
public static bool TryParse(string value, out SemanticVersion version)
{
version = null;

Expand Down Expand Up @@ -56,17 +63,17 @@ public bool TryParse(string value, out SemanticVersion version)
return false;
}

private bool IsLetterOrDigitOrDash(char c)
private static bool IsLetterOrDigitOrDash(char c)
{
int x = (int)c;

// "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-"
return (x >= 48 && x <= 57) || (x >= 65 && x <= 90) || (x >= 97 && x <= 122) || x == 45;
}

private bool IsValidPart(string s, bool allowLeadingZeros) => IsValidPart(s.ToCharArray(), allowLeadingZeros);
private static bool IsValidPart(string s, bool allowLeadingZeros) => IsValidPart(s.ToCharArray(), allowLeadingZeros);

private bool IsValidPart(char[] chars, bool allowLeadingZeros)
private static bool IsValidPart(char[] chars, bool allowLeadingZeros)
{
bool result = true;

Expand Down Expand Up @@ -94,7 +101,7 @@ private bool IsValidPart(char[] chars, bool allowLeadingZeros)
/// <summary>
/// Parse the version string into version/release
/// </summary>
private (string Version, string[] ReleaseLabels) ParseSections(string value)
private static (string Version, string[] ReleaseLabels) ParseSections(string value)
{
string versionString = null;
string[] releaseLabels = null;
Expand Down Expand Up @@ -142,7 +149,7 @@ private bool IsValidPart(char[] chars, bool allowLeadingZeros)
return (versionString, releaseLabels);
}

private Version NormalizeVersionValue(Version version)
private static Version NormalizeVersionValue(Version version)
{
Version normalized = version;

Expand Down
10 changes: 5 additions & 5 deletions src/MSBuildLocator/Utils/VersionComparer.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
using System;
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;

namespace Microsoft.Build.Locator
Expand All @@ -8,10 +11,7 @@ internal sealed class VersionComparer
/// <summary>
/// Determines if both versions are equal.
/// </summary>
public bool Equals(SemanticVersion x, SemanticVersion y)
{
return Compare(x, y) == 0;
}
public bool Equals(SemanticVersion x, SemanticVersion y) => Compare(x, y) == 0;

/// <summary>
/// Compare versions.
Expand Down

0 comments on commit cede57e

Please sign in to comment.