From b176762189eebe1d89bb9bc67f1272c802208d18 Mon Sep 17 00:00:00 2001 From: iSazonov Date: Fri, 6 Oct 2017 06:18:47 +0300 Subject: [PATCH 1/8] Make SemanticVersion compatible with SymVer 2.0 --- .../DotNetTypes_format_ps1xml.cs | 48 +++ .../engine/PSVersionInfo.cs | 317 +++++++++++++----- .../engine/Types_Ps1Xml.cs | 16 + .../engine/Basic/SemanticVersion.Tests.ps1 | 216 ++++++------ 4 files changed, 424 insertions(+), 173 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs index b1dd1c8374d..6ae20552979 100644 --- a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs +++ b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs @@ -43,6 +43,14 @@ internal static IEnumerable GetFormatData() "System.Version", ViewsOf_System_Version()); + yield return new ExtendedTypeDefinition( + "System.Version#IncludeLabel", + ViewsOf_System_Version_With_Label()); + + yield return new ExtendedTypeDefinition( + "System.Management.Automation.SemanticVersion", + ViewsOf_Semantic_Version_With_Label()); + yield return new ExtendedTypeDefinition( "System.Drawing.Printing.PrintDocument", ViewsOf_System_Drawing_Printing_PrintDocument()); @@ -478,6 +486,46 @@ private static IEnumerable ViewsOf_System_Version() .EndTable()); } + private static IEnumerable ViewsOf_System_Version_With_Label() + { + yield return new FormatViewDefinition("System.Version", + TableControl.Create() + .AddHeader(width: 6) + .AddHeader(width: 6) + .AddHeader(width: 6) + .AddHeader(width: 8) + .AddHeader(width: 26) + .AddHeader(width: 27) + .StartRowDefinition() + .AddPropertyColumn("Major") + .AddPropertyColumn("Minor") + .AddPropertyColumn("Build") + .AddPropertyColumn("Revision") + .AddPropertyColumn("PSSemanticVersionPreLabel") + .AddPropertyColumn("PSSemanticVersionBuildLabel") + .EndRowDefinition() + .EndTable()); + } + + private static IEnumerable ViewsOf_Semantic_Version_With_Label() + { + yield return new FormatViewDefinition("System.Management.Automation.SemanticVersion", + TableControl.Create() + .AddHeader(width: 6) + .AddHeader(width: 6) + .AddHeader(width: 6) + .AddHeader(width: 9) + .AddHeader(width: 11) + .StartRowDefinition() + .AddPropertyColumn("Major") + .AddPropertyColumn("Minor") + .AddPropertyColumn("Patch") + .AddPropertyColumn("PreLabel") + .AddPropertyColumn("BuildLabel") + .EndRowDefinition() + .EndTable()); + } + private static IEnumerable ViewsOf_System_Drawing_Printing_PrintDocument() { yield return new FormatViewDefinition("System.Drawing.Printing.PrintDocument", diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index 7b420f15870..cd57939372b 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -7,6 +7,8 @@ using System.Collections; using System.Globalization; using System.Management.Automation.Internal; +using System.Text; +using System.Text.RegularExpressions; using Microsoft.Win32; namespace System.Management.Automation @@ -384,12 +386,18 @@ IEnumerator IEnumerable.GetEnumerator() /// public sealed class SemanticVersion : IComparable, IComparable, IEquatable { + private static readonly string s_VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; + private static readonly string s_LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; + private static readonly string s_LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; + private const string PreLabelPropertyName = "PSSemanticVersionPreLabel"; + private const string BuildLabelPropertyName = "PSSemanticVersionBuildLabel"; + private const string TypeNameForVersionWithLabel = "System.Version#IncludeLabel"; + private string versionString; + /// /// Construct a SemanticVersion from a string. /// /// The version to parse - /// - /// /// /// public SemanticVersion(string version) @@ -398,8 +406,47 @@ public SemanticVersion(string version) Major = v.Major; Minor = v.Minor; - Patch = v.Patch; - Label = v.Label; + Patch = v.Patch < 0 ? 0 : v.Patch; + PreLabel = v.PreLabel; + BuildLabel = v.BuildLabel; + } + + /// + /// Construct a SemanticVersion. + /// + /// The major version + /// The minor version + /// The minor version + /// The preLabel for the version + /// The buildLabel for the version + /// + /// If starts with '-' ot contains '+'. + /// If contains '+'. + /// + public SemanticVersion(int major, int minor, int patch, string preLabel, string buildLabel) + : this(major, minor, patch) + { + if (!string.IsNullOrEmpty(preLabel)) + { + if (preLabel.StartsWith('-') || preLabel.Contains("+")) throw new FormatException(nameof(preLabel)); + + PreLabel = preLabel; + } + else + { + PreLabel = null; + } + + if (!string.IsNullOrEmpty(buildLabel)) + { + if (buildLabel.StartsWith('-') || buildLabel.Contains("+")) throw new FormatException(nameof(buildLabel)); + + BuildLabel = buildLabel; + } + else + { + BuildLabel = null; + } } /// @@ -410,17 +457,27 @@ public SemanticVersion(string version) /// The minor version /// The label for the version /// - /// If , , or is less than 0. - /// - /// - /// If is null or an empty string. + /// + /// If starts with '-'. /// public SemanticVersion(int major, int minor, int patch, string label) : this(major, minor, patch) { - if (string.IsNullOrEmpty(label)) throw PSTraceSource.NewArgumentNullException(nameof(label)); + if (!string.IsNullOrEmpty(label)) + { + if ((label.StartsWith('-') || label.StartsWith('.'))) throw new FormatException(nameof(label)); + + var match = Regex.Match(label, s_LabelRegEx); + if (!match.Success) throw new FormatException(nameof(label)); - Label = label; + PreLabel = match.Groups["preLabel"].Value; + BuildLabel = match.Groups["buildLabel"].Value; + } + else + { + PreLabel = null; + BuildLabel = null; + } } /// @@ -441,7 +498,8 @@ public SemanticVersion(int major, int minor, int patch) Major = major; Minor = minor; Patch = patch; - Label = null; + PreLabel = null; + BuildLabel = null; } /// @@ -463,43 +521,66 @@ public SemanticVersion(int major, int minor) : this(major, minor, 0) {} /// public SemanticVersion(int major) : this(major, 0, 0) {} - private const string LabelPropertyName = "PSSemanticVersionLabel"; - /// /// Construct a from a , /// copying the NoteProperty storing the label if the expected property exists. /// /// The version. + /// + /// If is null. + /// + /// + /// If is more than 0. + /// public SemanticVersion(Version version) { + if (version == null) throw PSTraceSource.NewArgumentNullException(nameof(version)); if (version.Revision > 0) throw PSTraceSource.NewArgumentException(nameof(version)); Major = version.Major; Minor = version.Minor; Patch = version.Build == -1 ? 0 : version.Build; var psobj = new PSObject(version); - var labelNote = psobj.Properties[LabelPropertyName]; - if (labelNote != null) + var preLabelNote = psobj.Properties[PreLabelPropertyName]; + if (preLabelNote != null) { - Label = labelNote.Value as string; + PreLabel = preLabelNote.Value as string; + } + var buildLabelNote = psobj.Properties[BuildLabelPropertyName]; + if (buildLabelNote != null) + { + BuildLabel = buildLabelNote.Value as string; } } /// /// Convert a to a . - /// If there is a , it is added as a NoteProperty to the - /// result so that you can round trip back to a - /// without losing the label. + /// If there is a or/and a , + /// it is added as a NoteProperty to the result so that you can round trip + /// back to a without losing the label. /// /// public static implicit operator Version(SemanticVersion semver) { + PSObject psobj; + var result = new Version(semver.Major, semver.Minor, semver.Patch); - if (!string.IsNullOrEmpty(semver.Label)) + if (!string.IsNullOrEmpty(semver.PreLabel) || !string.IsNullOrEmpty(semver.BuildLabel)) { - var psobj = new PSObject(result); - psobj.Properties.Add(new PSNoteProperty(LabelPropertyName, semver.Label)); + psobj = new PSObject(result); + + if (!string.IsNullOrEmpty(semver.PreLabel)) + { + psobj.Properties.Add(new PSNoteProperty(PreLabelPropertyName, semver.PreLabel)); + } + + if (!string.IsNullOrEmpty(semver.BuildLabel)) + { + psobj.Properties.Add(new PSNoteProperty(BuildLabelPropertyName, semver.BuildLabel)); + } + + psobj.TypeNames.Insert(0, TypeNameForVersionWithLabel); } return result; @@ -521,9 +602,16 @@ public static implicit operator Version(SemanticVersion semver) public int Patch { get; } /// - /// The last component in a SemanticVersion - may be null if not specified. + /// /// - public string Label { get; } + //public string Label { get; } + + public string PreLabel { get; } + + /// + /// + /// + public string BuildLabel { get; } /// /// Parse and return the result if it is a valid , otherwise throws an exception. @@ -531,12 +619,12 @@ public static implicit operator Version(SemanticVersion semver) /// The string to parse /// /// - /// /// /// public static SemanticVersion Parse(string version) { if (version == null) throw PSTraceSource.NewArgumentNullException(nameof(version)); + if (version == String.Empty) throw new FormatException(nameof(version)); var r = new VersionResult(); r.Init(true); @@ -571,77 +659,127 @@ public static bool TryParse(string version, out SemanticVersion result) private static bool TryParseVersion(string version, ref VersionResult result) { + if (version.EndsWith('-') || version.EndsWith('+') || version.EndsWith('.')) + { + result.SetFailure(ParseFailureKind.FormatException); + return false; + } + + string versionSansLabel = null; + var major=0; + var minor=0; + var patch=0; + string preLabel = null; + string buildLabel = null; + var dashIndex = version.IndexOf('-'); + var plusIndex = version.IndexOf('+'); - // Empty label? - if (dashIndex == version.Length - 1) + if (dashIndex > plusIndex) { - result.SetFailure(ParseFailureKind.ArgumentException); - return false; + if (plusIndex == -1) + { + // No buildLabel - buildLabel == null; + preLabel = version.Substring(dashIndex+1); + versionSansLabel = version.Substring(0, dashIndex); + } + else + { + // No preLabel - preLabel == null; + buildLabel = version.Substring(plusIndex+1); + versionSansLabel = version.Substring(0, plusIndex); + dashIndex = -1; + } + } + else + { + if (dashIndex == -1) + { + // Here dashIndex == plusIndex == -1 + // No preLabel - preLabel == null; + // No buildLabel - buildLabel == null; + versionSansLabel = version; + } + else + { + preLabel = version.Substring(dashIndex+1, plusIndex-dashIndex-1); + buildLabel = version.Substring(plusIndex+1); + versionSansLabel = version.Substring(0, dashIndex); + } } - var versionSansLabel = (dashIndex < 0) ? version : version.Substring(0, dashIndex); - string[] parsedComponents = versionSansLabel.Split(Utils.Separators.Dot); - if (parsedComponents.Length > 3) + if ((dashIndex != - 1 && String.IsNullOrEmpty(preLabel)) || + (plusIndex != - 1 && String.IsNullOrEmpty(buildLabel)) || + String.IsNullOrEmpty(versionSansLabel)) { - result.SetFailure(ParseFailureKind.ArgumentException); + // We have dash and no preLabel or + // we have plus and no buildLabel or + // we have no main version part (versionSansLabel==null) + result.SetFailure(ParseFailureKind.FormatException); return false; } - int major = 0, minor = 0, patch = 0; - if (!TryParseComponent(parsedComponents[0], "major", ref result, out major)) + var match = Regex.Match(versionSansLabel, s_VersionSansRegEx); + if (!match.Success) { + result.SetFailure(ParseFailureKind.FormatException); return false; } - if (parsedComponents.Length >= 2 && !TryParseComponent(parsedComponents[1], "minor", ref result, out minor)) + if (!int.TryParse(match.Groups["major"].Value, out major)) { + result.SetFailure(ParseFailureKind.FormatException); return false; } - if (parsedComponents.Length == 3 && !TryParseComponent(parsedComponents[2], "patch", ref result, out patch)) + if (match.Groups["minor"].Success && !int.TryParse(match.Groups["minor"].Value, out minor)) { + result.SetFailure(ParseFailureKind.FormatException); return false; } - result._parsedVersion = dashIndex < 0 - ? new SemanticVersion(major, minor, patch) - : new SemanticVersion(major, minor, patch, version.Substring(dashIndex + 1)); - return true; - } - - private static bool TryParseComponent(string component, string componentName, ref VersionResult result, out int parsedComponent) - { - if (!Int32.TryParse(component, NumberStyles.Integer, CultureInfo.InvariantCulture, out parsedComponent)) + if (match.Groups["patch"].Success && !int.TryParse(match.Groups["patch"].Value, out patch)) { - result.SetFailure(ParseFailureKind.FormatException, component); + result.SetFailure(ParseFailureKind.FormatException); return false; } - if (parsedComponent < 0) + if (preLabel != null && !Regex.IsMatch(preLabel, s_LabelUnitRegEx) || + (buildLabel != null && !Regex.IsMatch(buildLabel, s_LabelUnitRegEx))) { - result.SetFailure(ParseFailureKind.ArgumentOutOfRangeException, componentName); + result.SetFailure(ParseFailureKind.FormatException); return false; } + result._parsedVersion = new SemanticVersion(major, minor, patch, preLabel, buildLabel); return true; } /// - /// ToString + /// Implement ToString() /// public override string ToString() { - if (Patch < 0) + if (versionString == null) { - return string.IsNullOrEmpty(Label) - ? StringUtil.Format("{0}.{1}", Major, Minor) - : StringUtil.Format("{0}.{1}-{2}", Major, Minor, Label); + StringBuilder result = new StringBuilder(); + + result.Append(Major).Append(Utils.Separators.Dot).Append(Minor).Append(Utils.Separators.Dot).Append(Patch); + + if (!string.IsNullOrEmpty(PreLabel)) + { + result.Append("-").Append(PreLabel); + } + + if (!string.IsNullOrEmpty(BuildLabel)) + { + result.Append("+").Append(BuildLabel); + } + + versionString = result.ToString(); } - return string.IsNullOrEmpty(Label) - ? StringUtil.Format("{0}.{1}.{2}", Major, Minor, Patch) - : StringUtil.Format("{0}.{1}.{2}-{3}", Major, Minor, Patch, Label); + return versionString; } /// @@ -664,7 +802,8 @@ public int CompareTo(object version) } /// - /// Implement + /// Implement . + /// Meets SymVer 2.0 p.11 http://semver.org/ /// public int CompareTo(SemanticVersion value) { @@ -680,16 +819,14 @@ public int CompareTo(SemanticVersion value) if (Patch != value.Patch) return Patch > value.Patch ? 1 : -1; - if (Label == null) - return value.Label == null ? 0 : 1; + if (PreLabel == null) + return value.PreLabel == null ? 0 : 1; - if (value.Label == null) + if (value.PreLabel == null) return -1; - if (!string.Equals(Label, value.Label, StringComparison.Ordinal)) - return string.Compare(Label, value.Label, StringComparison.Ordinal); - - return 0; + // SymVer 2.0 standard requires to ignore 'BuildLabel' (Build metadata). + return ComparePreLabel(this.PreLabel, value.PreLabel); } /// @@ -705,9 +842,10 @@ public override bool Equals(object obj) /// public bool Equals(SemanticVersion other) { + // SymVer 2.0 standard requires to ignore 'BuildLabel' (Build metadata). return other != null && (Major == other.Major) && (Minor == other.Minor) && (Patch == other.Patch) && - string.Equals(Label, other.Label, StringComparison.Ordinal); + string.Equals(PreLabel, other.PreLabel, StringComparison.Ordinal); } /// @@ -715,11 +853,7 @@ public bool Equals(SemanticVersion other) /// public override int GetHashCode() { - return Utils.CombineHashCodes( - Major.GetHashCode(), - Minor.GetHashCode(), - Patch.GetHashCode(), - Label == null ? 0 : Label.GetHashCode()); + return versionString.GetHashCode(); } /// @@ -748,7 +882,6 @@ public override int GetHashCode() /// public static bool operator <(SemanticVersion v1, SemanticVersion v2) { - if ((object)v1 == null) throw PSTraceSource.NewArgumentException(nameof(v1)); return (v1.CompareTo(v2) < 0); } @@ -757,7 +890,6 @@ public override int GetHashCode() /// public static bool operator <=(SemanticVersion v1, SemanticVersion v2) { - if ((object)v1 == null) throw PSTraceSource.NewArgumentException(nameof(v1)); return (v1.CompareTo(v2) <= 0); } @@ -766,7 +898,7 @@ public override int GetHashCode() /// public static bool operator >(SemanticVersion v1, SemanticVersion v2) { - return (v2 < v1); + return (v1.CompareTo(v2) > 0); } /// @@ -774,7 +906,42 @@ public override int GetHashCode() /// public static bool operator >=(SemanticVersion v1, SemanticVersion v2) { - return (v2 <= v1); + return (v1.CompareTo(v2) >= 0); + } + + private static int ComparePreLabel(string preLabel1, string preLabel2) + { + if (String.IsNullOrEmpty(preLabel1)) { return String.IsNullOrEmpty(preLabel2) ? 0 : 1; } + if (String.IsNullOrEmpty(preLabel2)) { return -1; } + + var units1 = preLabel1.Split('.'); + var units2 = preLabel2.Split('.'); + + var minLength = units1.Length < units2.Length ? units1.Length : units2.Length; + + for (int i = 0; i < minLength; i++) + { + var ac = units1[i]; + var bc = units2[i]; + int number1, number2; + var isNumber1 = Int32.TryParse(ac, out number1); + var isNumber2 = Int32.TryParse(bc, out number2); + + if (isNumber1 && isNumber2) + { + if (number1 != number2) { return number1 < number2 ? -1 : 1; } + } + else + { + if (isNumber1) { return -1; } + if (isNumber2) { return 1; } + + int result = String.CompareOrdinal(ac, bc); + if (result != 0) { return result; } + } + } + + return units1.Length.CompareTo(units2.Length); } internal enum ParseFailureKind diff --git a/src/System.Management.Automation/engine/Types_Ps1Xml.cs b/src/System.Management.Automation/engine/Types_Ps1Xml.cs index a81f019701f..e4f17289383 100644 --- a/src/System.Management.Automation/engine/Types_Ps1Xml.cs +++ b/src/System.Management.Automation/engine/Types_Ps1Xml.cs @@ -1982,6 +1982,22 @@ public static IEnumerable Get() var td251 = new TypeData(@"Deserialized.System.Management.Automation.DebuggerCommandResults", true); td251.TargetTypeForDeserialization = typeof(Microsoft.PowerShell.DeserializingTypeConverter); yield return td251; + + var td252 = new TypeData(@"System.Version#IncludeLabel", true); + td252.Members.Add("ToString", + new ScriptMethodData(@"ToString", GetScriptBlock(@" + $suffix = """" + if (![String]::IsNullOrEmpty($this.PSSemanticVersionPreLabel)) + { + $suffix = ""-""+$this.PSSemanticVersionPreLabel + } + if (![String]::IsNullOrEmpty($this.PSSemanticVersionBuildLabel)) + { + $suffix += ""+""+$this.PSSemanticVersionBuildLabel + } + ""$($this.Major).$($this.Minor).$($this.Build)""+$suffix + "))); + yield return td252; } } } diff --git a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 index bbfcb37b7fb..5f69f43180e 100644 --- a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 +++ b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 @@ -2,46 +2,56 @@ using namespace System.Management.Automation using namespace System.Management.Automation.Language Describe "SemanticVersion api tests" -Tags 'CI' { - Context "constructing valid versions" { - It "string argument constructor" { - $v = [SemanticVersion]::new("1.2.3-alpha") + Context "Constructing valid versions" { + It "String argument constructor" { + $v = [SemanticVersion]::new("1.2.3-Alpha-super.3+BLD.a1-xxx.03") $v.Major | Should Be 1 $v.Minor | Should Be 2 $v.Patch | Should Be 3 - $v.Label | Should Be "alpha" - $v.ToString() | Should Be "1.2.3-alpha" + $v.PreLabel | Should Be "Alpha-super.3" + $v.BuildLabel | Should Be "BLD.a1-xxx.03" + $v.ToString() | Should Be "1.2.3-Alpha-super.3+BLD.a1-xxx.03" $v = [SemanticVersion]::new("1.0.0") $v.Major | Should Be 1 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.Label | Should BeNullOrEmpty + $v.PreLabel | Should BeNullOrEmpty + $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "1.0.0" $v = [SemanticVersion]::new("3.0") $v.Major | Should Be 3 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.Label | Should BeNullOrEmpty + $v.PreLabel | Should BeNullOrEmpty + $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "3.0.0" $v = [SemanticVersion]::new("2") $v.Major | Should Be 2 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.Label | Should BeNullOrEmpty + $v.PreLabel | Should BeNullOrEmpty + $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "2.0.0" } - # After the above test, we trust the properties and rely on ToString for validation + # After the above test, we trust the properties and rely on ToString for validation - It "int args constructor" { + It "Int args constructor" { $v = [SemanticVersion]::new(1, 0, 0) $v.ToString() | Should Be "1.0.0" $v = [SemanticVersion]::new(3, 2, 0, "beta.1") $v.ToString() | Should Be "3.2.0-beta.1" + $v = [SemanticVersion]::new(3, 2, 0, "beta.1+meta") + $v.ToString() | Should Be "3.2.0-beta.1+meta" + + $v = [SemanticVersion]::new(3, 2, 0, "beta.1", "meta") + $v.ToString() | Should Be "3.2.0-beta.1+meta" + $v = [SemanticVersion]::new(3, 1) $v.ToString() | Should Be "3.1.0" @@ -49,7 +59,7 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.ToString() | Should Be "3.0.0" } - It "version arg constructor" { + It "Version arg constructor" { $v = [SemanticVersion]::new([Version]::new(1, 2)) $v.ToString() | Should Be '1.2.0' @@ -57,37 +67,63 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.ToString() | Should Be '1.2.3' } - It "semantic version can round trip through version" { - $v1 = [SemanticVersion]::new(3, 2, 1, "prerelease") + It "Can covert to 'Version' type" { + $v1 = [SemanticVersion]::new(3, 2, 1, "prerelease", "meta") + $v2 = [Version]$v1 + $v2.GetType() | Should Be "Version" + $v2.PSobject.TypeNames[0] | Should Be "System.Version#IncludeLabel" + $v2.Major | Should Be 3 + $v2.Minor | Should Be 2 + $v2.Build | Should Be 1 + $v2.PSSemanticVersionPreLabel | Should Be "prerelease" + $v2.PSSemanticVersionBuildLabel | Should Be "meta" + $v2.ToString() | Should Be "3.2.1-prerelease+meta" + } + + It "Semantic version can round trip through version" { + $v1 = [SemanticVersion]::new(3, 2, 1, "prerelease", "meta") $v2 = [SemanticVersion]::new([Version]$v1) - $v2.ToString() | Should Be "3.2.1-prerelease" + $v2.ToString() | Should Be "3.2.1-prerelease+meta" } } Context "Comparisons" { - $v1_0_0 = [SemanticVersion]::new(1, 0, 0) - $v1_1_0 = [SemanticVersion]::new(1, 1, 0) - $v1_1_1 = [SemanticVersion]::new(1, 1, 1) - $v2_1_0 = [SemanticVersion]::new(2, 1, 0) - $v1_0_0_alpha = [SemanticVersion]::new(1, 0, 0, "alpha") - $v1_0_0_beta = [SemanticVersion]::new(1, 0, 0, "beta") + BeforeAll { + $v1_0_0 = [SemanticVersion]::new(1, 0, 0) + $v1_1_0 = [SemanticVersion]::new(1, 1, 0) + $v1_1_1 = [SemanticVersion]::new(1, 1, 1) + $v2_1_0 = [SemanticVersion]::new(2, 1, 0) + $v1_0_0_alpha = [SemanticVersion]::new(1, 0, 0, "alpha.1.1") + $v1_0_0_alpha2 = [SemanticVersion]::new(1, 0, 0, "alpha.1.2") + $v1_0_0_beta = [SemanticVersion]::new(1, 0, 0, "beta") + $v1_0_0_betaBuild = [SemanticVersion]::new(1, 0, 0, "beta", "BUILD") + + $testCases = @( + @{ lhs = $v1_0_0; rhs = $v1_1_0 } + @{ lhs = $v1_0_0; rhs = $v1_1_1 } + @{ lhs = $v1_1_0; rhs = $v1_1_1 } + @{ lhs = $v1_0_0; rhs = $v2_1_0 } + @{ lhs = $v1_0_0_alpha; rhs = $v1_0_0_beta } + @{ lhs = $v1_0_0_alpha; rhs = $v1_0_0_alpha2 } + @{ lhs = $v1_0_0_alpha; rhs = $v1_0_0 } + @{ lhs = $v1_0_0_beta; rhs = $v1_0_0 } + @{ lhs = $v2_1_0; rhs = "3.0"} + @{ lhs = "1.5"; rhs = $v2_1_0} + ) + } + + It "Build meta should be ignored" { + $v1_0_0_beta -eq $v1_0_0_betaBuild | Should Be $true + $v1_0_0_betaBuild -lt $v1_0_0_beta | Should Be $false + $v1_0_0_beta -lt $v1_0_0_betaBuild | Should Be $false + } - $testCases = @( - @{ lhs = $v1_0_0; rhs = $v1_1_0 } - @{ lhs = $v1_0_0; rhs = $v1_1_1 } - @{ lhs = $v1_1_0; rhs = $v1_1_1 } - @{ lhs = $v1_0_0; rhs = $v2_1_0 } - @{ lhs = $v1_0_0_alpha; rhs = $v1_0_0_beta } - @{ lhs = $v1_0_0_alpha; rhs = $v1_0_0 } - @{ lhs = $v1_0_0_beta; rhs = $v1_0_0 } - @{ lhs = $v2_1_0; rhs = "3.0"} - @{ lhs = "1.5"; rhs = $v2_1_0} - ) It " less than " -TestCases $testCases { param($lhs, $rhs) $lhs -lt $rhs | Should Be $true $rhs -lt $lhs | Should Be $false } + It " less than or equal " -TestCases $testCases { param($lhs, $rhs) $lhs -le $rhs | Should Be $true @@ -95,11 +131,13 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $lhs -le $lhs | Should Be $true $rhs -le $rhs | Should Be $true } + It " greater than " -TestCases $testCases { param($lhs, $rhs) $lhs -gt $rhs | Should Be $false $rhs -gt $lhs | Should Be $true } + It " greater than or equal " -TestCases $testCases { param($lhs, $rhs) $lhs -ge $rhs | Should Be $false @@ -108,11 +146,10 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $rhs -ge $rhs | Should Be $true } - $testCases = @( + It "Equality " -TestCases @( @{ operand = $v1_0_0 } @{ operand = $v1_0_0_alpha } - ) - It "Equality " -TestCases $testCases { + ) { param($operand) $operand -eq $operand | Should Be $true $operand -ne $operand | Should Be $false @@ -134,84 +171,67 @@ Describe "SemanticVersion api tests" -Tags 'CI' { } } - Context "error handling" { - - # The specific errors aren't too useful here, but noted in comments - # so when we pick up a version of Pester that will let us check FullyQualifiedErrorId, - # it's easier to tweak the tests - - $testCases = @( - @{ expectedResult = $false; version = $null } - @{ expectedResult = $false; version = [NullString]::Value } - @{ expectedResult = $false; version = "" } - @{ expectedResult = $false; version = "1.0.0-" } - @{ expectedResult = $false; version = "-" } - @{ expectedResult = $false; version = "." } - @{ expectedResult = $false; version = "-alpha" } - @{ expectedResult = $false; version = "1..0" } - @{ expectedResult = $false; version = "1.0.-alpha" } - @{ expectedResult = $false; version = "1.0." } - @{ expectedResult = $false; version = ".0.0" } - ) - - It "parts of version missing" -TestCases $testCases { - param($version, $expectedResult) - { [SemanticVersion]::new($version) } | Should Throw # PSArgumentException - { [SemanticVersion]::Parse($version) } | Should Throw # PSArgumentException - $semVer = $null - [SemanticVersion]::TryParse($_, [ref]$semVer) | Should Be $expectedResult - $semVer | Should Be $null - } - - $testCases = @( - @{ expectedResult = $false; version = "-1.0.0" } - @{ expectedResult = $false; version = "1.-1.0" } - @{ expectedResult = $false; version = "1.0.-1" } - ) - - It "range check of versions: " -TestCases $testCases { - param($version, $expectedResult) - { [SemanticVersion]::new($version) } | Should Throw # PSArgumentException - { [SemanticVersion]::Parse($version) } | Should Throw # PSArgumentException - $semVer = $null - [SemanticVersion]::TryParse($_, [ref]$semVer) | Should Be $expectedResult - $semVer | Should Be $null - } - - $testCases = @( - @{ expectedResult = $false; version = "aa.0.0" } - @{ expectedResult = $false; version = "1.bb.0" } - @{ expectedResult = $false; version = "1.0.cc" } - ) - - It "format errors: " -TestCases $testCases { - param($version, $expectedResult) - { [SemanticVersion]::new($version) } | Should Throw # PSArgumentException - { [SemanticVersion]::Parse($version) } | Should Throw # PSArgumentException + Context "Error handling" { + + It ": ''" -TestCases @( + @{ name = "Missing parts: 'null'"; errorId = "PSArgumentNullException";expectedResult = $false; version = $null } + @{ name = "Missing parts: 'NullString'"; errorId = "PSArgumentNullException";expectedResult = $false; version = [NullString]::Value } + @{ name = "Missing parts: 'EmptyString'";errorId = "FormatException"; expectedResult = $false; version = "" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "-" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "." } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "+" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "-alpha" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1..0" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.-alpha" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.+alpha" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0-alpha+" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0-+" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0+-" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0+" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0-" } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.0." } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0." } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = "1.0.." } + @{ name = "Missing parts"; errorId = "FormatException"; expectedResult = $false; version = ".0.0" } + @{ name = "Range check of versions"; errorId = "FormatException"; expectedResult = $false; version = "-1.0.0" } + @{ name = "Range check of versions"; errorId = "FormatException"; expectedResult = $false; version = "1.-1.0" } + @{ name = "Range check of versions"; errorId = "FormatException"; expectedResult = $false; version = "1.0.-1" } + @{ name = "Format errors"; errorId = "FormatException"; expectedResult = $false; version = "aa.0.0" } + @{ name = "Format errors"; errorId = "FormatException"; expectedResult = $false; version = "1.bb.0" } + @{ name = "Format errors"; errorId = "FormatException"; expectedResult = $false; version = "1.0.cc" } + ) { + param($version, $expectedResult, $errorId) + { [SemanticVersion]::new($version) } | ShouldBeErrorId $errorId + if ($version -eq $null) { + # PowerShell convert $null to Empty string + { [SemanticVersion]::Parse($version) } | ShouldBeErrorId "FormatException" + } else { + { [SemanticVersion]::Parse($version) } | ShouldBeErrorId $errorId + } $semVer = $null [SemanticVersion]::TryParse($_, [ref]$semVer) | Should Be $expectedResult $semVer | Should Be $null } It "Negative version arguments" { - { [SemanticVersion]::new(-1, 0) } | Should Throw # PSArgumentException - { [SemanticVersion]::new(1, -1) } | Should Throw # PSArgumentException - { [SemanticVersion]::new(1, 1, -1) } | Should Throw # PSArgumentException + { [SemanticVersion]::new(-1, 0) } | ShouldBeErrorId "PSArgumentException" + { [SemanticVersion]::new(1, -1) } | ShouldBeErrorId "PSArgumentException" + { [SemanticVersion]::new(1, 1, -1) } | ShouldBeErrorId "PSArgumentException" } - It "Incompatible version throws" { + It "Incompatible 'Version' throws" { # Revision isn't supported - { [SemanticVersion]::new([Version]::new(0, 0, 0, 4)) } | Should Throw # PSArgumentException - { [SemanticVersion]::new([Version]::new("1.2.3.4")) } | Should Throw # PSArgumentException + { [SemanticVersion]::new([Version]::new(0, 0, 0, 4)) } | ShouldBeErrorId "PSArgumentException" + { [SemanticVersion]::new([Version]::new("1.2.3.4")) } | ShouldBeErrorId "PSArgumentException" } } Context "Serialization" { $testCases = @( - @{ expectedResult = "1.0.0"; semver = [SemanticVersion]::new(1, 0, 0) } - @{ expectedResult = "1.0.1"; semver = [SemanticVersion]::new(1, 0, 1) } - @{ expectedResult = "1.0.0-alpha"; semver = [SemanticVersion]::new(1, 0, 0, "alpha") } - @{ expectedResult = "1.0.0-beta"; semver = [SemanticVersion]::new(1, 0, 0, "beta") } + @{ errorId = "PSArgumentException"; expectedResult = "1.0.0"; semver = [SemanticVersion]::new(1, 0, 0) } + @{ errorId = "PSArgumentException"; expectedResult = "1.0.1"; semver = [SemanticVersion]::new(1, 0, 1) } + @{ errorId = "PSArgumentException"; expectedResult = "1.0.0-alpha"; semver = [SemanticVersion]::new(1, 0, 0, "alpha") } + @{ errorId = "PSArgumentException"; expectedResult = "1.0.0-Alpha-super.3+BLD.a1-xxx.03"; semver = [SemanticVersion]::new(1, 0, 0, "Alpha-super.3+BLD.a1-xxx.03") } ) It "Can round trip: " -TestCases $testCases { param($semver, $expectedResult) From 777628f415a241964b0c03898fdeac0d0f7d303b Mon Sep 17 00:00:00 2001 From: iSazonov Date: Thu, 2 Nov 2017 11:54:03 +0300 Subject: [PATCH 2/8] Address feedback Replace static strings with const. Add strong checks in constructors. Add comments. Add static Compare() and fix comparisions operators. --- .../engine/PSVersionInfo.cs | 67 ++++++++++--------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index cd57939372b..fdc712d58e9 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -386,9 +386,9 @@ IEnumerator IEnumerable.GetEnumerator() /// public sealed class SemanticVersion : IComparable, IComparable, IEquatable { - private static readonly string s_VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; - private static readonly string s_LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; - private static readonly string s_LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; + private const string s_VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; + private const string s_LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; + private const string s_LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; private const string PreLabelPropertyName = "PSSemanticVersionPreLabel"; private const string BuildLabelPropertyName = "PSSemanticVersionBuildLabel"; private const string TypeNameForVersionWithLabel = "System.Version#IncludeLabel"; @@ -416,7 +416,7 @@ public SemanticVersion(string version) /// /// The major version /// The minor version - /// The minor version + /// The patch version /// The preLabel for the version /// The buildLabel for the version /// @@ -428,25 +428,17 @@ public SemanticVersion(int major, int minor, int patch, string preLabel, string { if (!string.IsNullOrEmpty(preLabel)) { - if (preLabel.StartsWith('-') || preLabel.Contains("+")) throw new FormatException(nameof(preLabel)); + if (!Regex.IsMatch(preLabel, s_LabelUnitRegEx)) throw new FormatException(nameof(preLabel)); PreLabel = preLabel; } - else - { - PreLabel = null; - } if (!string.IsNullOrEmpty(buildLabel)) { - if (buildLabel.StartsWith('-') || buildLabel.Contains("+")) throw new FormatException(nameof(buildLabel)); + if (!Regex.IsMatch(buildLabel, s_LabelUnitRegEx)) throw new FormatException(nameof(buildLabel)); BuildLabel = buildLabel; } - else - { - BuildLabel = null; - } } /// @@ -463,21 +455,17 @@ public SemanticVersion(int major, int minor, int patch, string preLabel, string public SemanticVersion(int major, int minor, int patch, string label) : this(major, minor, patch) { + // We presume the SymVer : + // 1) major.minor.patch-label + // 2) 'label' starts with letter or digit. if (!string.IsNullOrEmpty(label)) { - if ((label.StartsWith('-') || label.StartsWith('.'))) throw new FormatException(nameof(label)); - var match = Regex.Match(label, s_LabelRegEx); if (!match.Success) throw new FormatException(nameof(label)); PreLabel = match.Groups["preLabel"].Value; BuildLabel = match.Groups["buildLabel"].Value; } - else - { - PreLabel = null; - BuildLabel = null; - } } /// @@ -498,8 +486,9 @@ public SemanticVersion(int major, int minor, int patch) Major = major; Minor = minor; Patch = patch; - PreLabel = null; - BuildLabel = null; + // We presume: + // PreLabel = null; + // BuildLabel = null; } /// @@ -602,14 +591,13 @@ public static implicit operator Version(SemanticVersion semver) public int Patch { get; } /// - /// + /// PreLabel position in the SymVer string 'major.minor.patch-PreLabel+BuildLabel'. /// - //public string Label { get; } public string PreLabel { get; } /// - /// + /// BuildLabel position in the SymVer string 'major.minor.patch-PreLabel+BuildLabel'. /// public string BuildLabel { get; } @@ -672,20 +660,24 @@ private static bool TryParseVersion(string version, ref VersionResult result) string preLabel = null; string buildLabel = null; + // We parse the SymVer 'version' string 'major.minor.patch-PreLabel+BuildLabel'. var dashIndex = version.IndexOf('-'); var plusIndex = version.IndexOf('+'); if (dashIndex > plusIndex) { + // 'preLabel' can contains dashes. if (plusIndex == -1) { - // No buildLabel - buildLabel == null; + // No buildLabel: buildLabel == null + // Format is 'major.minor.patch-PreLabel' preLabel = version.Substring(dashIndex+1); versionSansLabel = version.Substring(0, dashIndex); } else { - // No preLabel - preLabel == null; + // No preLabel: preLabel == null + // Format is 'major.minor.patch+BuildLabel' buildLabel = version.Substring(plusIndex+1); versionSansLabel = version.Substring(0, plusIndex); dashIndex = -1; @@ -698,10 +690,12 @@ private static bool TryParseVersion(string version, ref VersionResult result) // Here dashIndex == plusIndex == -1 // No preLabel - preLabel == null; // No buildLabel - buildLabel == null; + // Format is 'major.minor.patch' versionSansLabel = version; } else { + // Format is 'major.minor.patch-PreLabel+BuildLabel' preLabel = version.Substring(dashIndex+1, plusIndex-dashIndex-1); buildLabel = version.Substring(plusIndex+1); versionSansLabel = version.Substring(0, dashIndex); @@ -782,6 +776,15 @@ public override string ToString() return versionString; } + /// + /// Implement Compare. + /// + public static int Compare(SemanticVersion versionA, SemanticVersion versionB) + { + if (versionA == null) return -1; + return versionA.CompareTo(versionB); + } + /// /// Implement /// @@ -882,7 +885,7 @@ public override int GetHashCode() /// public static bool operator <(SemanticVersion v1, SemanticVersion v2) { - return (v1.CompareTo(v2) < 0); + return (Compare(v1, v2) < 0); } /// @@ -890,7 +893,7 @@ public override int GetHashCode() /// public static bool operator <=(SemanticVersion v1, SemanticVersion v2) { - return (v1.CompareTo(v2) <= 0); + return (Compare(v1, v2) <= 0); } /// @@ -898,7 +901,7 @@ public override int GetHashCode() /// public static bool operator >(SemanticVersion v1, SemanticVersion v2) { - return (v1.CompareTo(v2) > 0); + return (Compare(v1, v2) > 0); } /// @@ -906,7 +909,7 @@ public override int GetHashCode() /// public static bool operator >=(SemanticVersion v1, SemanticVersion v2) { - return (v1.CompareTo(v2) >= 0); + return (Compare(v1, v2) >= 0); } private static int ComparePreLabel(string preLabel1, string preLabel2) From 9571e0643b5c6078baa20bae9ddd8e0bcdc27745 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 5 Nov 2017 16:07:30 +0500 Subject: [PATCH 3/8] Remove duplicate checks and add comment. --- src/System.Management.Automation/engine/PSVersionInfo.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index fdc712d58e9..512bca01316 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -822,12 +822,6 @@ public int CompareTo(SemanticVersion value) if (Patch != value.Patch) return Patch > value.Patch ? 1 : -1; - if (PreLabel == null) - return value.PreLabel == null ? 0 : 1; - - if (value.PreLabel == null) - return -1; - // SymVer 2.0 standard requires to ignore 'BuildLabel' (Build metadata). return ComparePreLabel(this.PreLabel, value.PreLabel); } @@ -914,6 +908,7 @@ public override int GetHashCode() private static int ComparePreLabel(string preLabel1, string preLabel2) { + // Symver 2.0 standard p.9 - Pre-release versions have a lower precedence than the associated normal version. if (String.IsNullOrEmpty(preLabel1)) { return String.IsNullOrEmpty(preLabel2) ? 0 : 1; } if (String.IsNullOrEmpty(preLabel2)) { return -1; } From 3654b0fab0000cac422b6e67e71f6822abe67d35 Mon Sep 17 00:00:00 2001 From: iSazonov Date: Tue, 7 Nov 2017 07:48:32 +0300 Subject: [PATCH 4/8] Remove "s_' prefix from constant names --- .../engine/PSVersionInfo.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index 512bca01316..497724a985f 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -386,9 +386,9 @@ IEnumerator IEnumerable.GetEnumerator() /// public sealed class SemanticVersion : IComparable, IComparable, IEquatable { - private const string s_VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; - private const string s_LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; - private const string s_LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; + private const string VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; + private const string LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; + private const string LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; private const string PreLabelPropertyName = "PSSemanticVersionPreLabel"; private const string BuildLabelPropertyName = "PSSemanticVersionBuildLabel"; private const string TypeNameForVersionWithLabel = "System.Version#IncludeLabel"; @@ -428,14 +428,14 @@ public SemanticVersion(int major, int minor, int patch, string preLabel, string { if (!string.IsNullOrEmpty(preLabel)) { - if (!Regex.IsMatch(preLabel, s_LabelUnitRegEx)) throw new FormatException(nameof(preLabel)); + if (!Regex.IsMatch(preLabel, LabelUnitRegEx)) throw new FormatException(nameof(preLabel)); PreLabel = preLabel; } if (!string.IsNullOrEmpty(buildLabel)) { - if (!Regex.IsMatch(buildLabel, s_LabelUnitRegEx)) throw new FormatException(nameof(buildLabel)); + if (!Regex.IsMatch(buildLabel, LabelUnitRegEx)) throw new FormatException(nameof(buildLabel)); BuildLabel = buildLabel; } @@ -460,7 +460,7 @@ public SemanticVersion(int major, int minor, int patch, string label) // 2) 'label' starts with letter or digit. if (!string.IsNullOrEmpty(label)) { - var match = Regex.Match(label, s_LabelRegEx); + var match = Regex.Match(label, LabelRegEx); if (!match.Success) throw new FormatException(nameof(label)); PreLabel = match.Groups["preLabel"].Value; @@ -713,7 +713,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) return false; } - var match = Regex.Match(versionSansLabel, s_VersionSansRegEx); + var match = Regex.Match(versionSansLabel, VersionSansRegEx); if (!match.Success) { result.SetFailure(ParseFailureKind.FormatException); @@ -738,8 +738,8 @@ private static bool TryParseVersion(string version, ref VersionResult result) return false; } - if (preLabel != null && !Regex.IsMatch(preLabel, s_LabelUnitRegEx) || - (buildLabel != null && !Regex.IsMatch(buildLabel, s_LabelUnitRegEx))) + if (preLabel != null && !Regex.IsMatch(preLabel, LabelUnitRegEx) || + (buildLabel != null && !Regex.IsMatch(buildLabel, LabelUnitRegEx))) { result.SetFailure(ParseFailureKind.FormatException); return false; From 2a7adce1c98e2b3b117f82a0fafbd5c5b41a4ada Mon Sep 17 00:00:00 2001 From: iSazonov Date: Tue, 7 Nov 2017 08:06:07 +0300 Subject: [PATCH 5/8] Fix comments and names. --- .../engine/PSVersionInfo.cs | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index 497724a985f..76a0da4bd7e 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -388,7 +388,7 @@ public sealed class SemanticVersion : IComparable, IComparable, { private const string VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; private const string LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; - private const string LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*?$"; + private const string LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*$"; private const string PreLabelPropertyName = "PSSemanticVersionPreLabel"; private const string BuildLabelPropertyName = "PSSemanticVersionBuildLabel"; private const string TypeNameForVersionWithLabel = "System.Version#IncludeLabel"; @@ -407,7 +407,7 @@ public SemanticVersion(string version) Major = v.Major; Minor = v.Minor; Patch = v.Patch < 0 ? 0 : v.Patch; - PreLabel = v.PreLabel; + PreReleaseLabel = v.PreReleaseLabel; BuildLabel = v.BuildLabel; } @@ -420,8 +420,8 @@ public SemanticVersion(string version) /// The preLabel for the version /// The buildLabel for the version /// - /// If starts with '-' ot contains '+'. - /// If contains '+'. + /// If don't match 'LabelUnitRegEx'. + /// If don't match 'LabelUnitRegEx'. /// public SemanticVersion(int major, int minor, int patch, string preLabel, string buildLabel) : this(major, minor, patch) @@ -430,7 +430,7 @@ public SemanticVersion(int major, int minor, int patch, string preLabel, string { if (!Regex.IsMatch(preLabel, LabelUnitRegEx)) throw new FormatException(nameof(preLabel)); - PreLabel = preLabel; + PreReleaseLabel = preLabel; } if (!string.IsNullOrEmpty(buildLabel)) @@ -450,7 +450,7 @@ public SemanticVersion(int major, int minor, int patch, string preLabel, string /// The label for the version /// /// - /// If starts with '-'. + /// If don't match 'LabelRegEx'. /// public SemanticVersion(int major, int minor, int patch, string label) : this(major, minor, patch) @@ -463,7 +463,7 @@ public SemanticVersion(int major, int minor, int patch, string label) var match = Regex.Match(label, LabelRegEx); if (!match.Success) throw new FormatException(nameof(label)); - PreLabel = match.Groups["preLabel"].Value; + PreReleaseLabel = match.Groups["preLabel"].Value; BuildLabel = match.Groups["buildLabel"].Value; } } @@ -487,7 +487,7 @@ public SemanticVersion(int major, int minor, int patch) Minor = minor; Patch = patch; // We presume: - // PreLabel = null; + // PreReleaseLabel = null; // BuildLabel = null; } @@ -533,7 +533,7 @@ public SemanticVersion(Version version) var preLabelNote = psobj.Properties[PreLabelPropertyName]; if (preLabelNote != null) { - PreLabel = preLabelNote.Value as string; + PreReleaseLabel = preLabelNote.Value as string; } var buildLabelNote = psobj.Properties[BuildLabelPropertyName]; if (buildLabelNote != null) @@ -544,7 +544,7 @@ public SemanticVersion(Version version) /// /// Convert a to a . - /// If there is a or/and a , + /// If there is a or/and a , /// it is added as a NoteProperty to the result so that you can round trip /// back to a without losing the label. /// @@ -555,13 +555,13 @@ public static implicit operator Version(SemanticVersion semver) var result = new Version(semver.Major, semver.Minor, semver.Patch); - if (!string.IsNullOrEmpty(semver.PreLabel) || !string.IsNullOrEmpty(semver.BuildLabel)) + if (!string.IsNullOrEmpty(semver.PreReleaseLabel) || !string.IsNullOrEmpty(semver.BuildLabel)) { psobj = new PSObject(result); - if (!string.IsNullOrEmpty(semver.PreLabel)) + if (!string.IsNullOrEmpty(semver.PreReleaseLabel)) { - psobj.Properties.Add(new PSNoteProperty(PreLabelPropertyName, semver.PreLabel)); + psobj.Properties.Add(new PSNoteProperty(PreLabelPropertyName, semver.PreReleaseLabel)); } if (!string.IsNullOrEmpty(semver.BuildLabel)) @@ -591,13 +591,12 @@ public static implicit operator Version(SemanticVersion semver) public int Patch { get; } /// - /// PreLabel position in the SymVer string 'major.minor.patch-PreLabel+BuildLabel'. + /// PreReleaseLabel position in the SymVer string 'major.minor.patch-PreReleaseLabel+BuildLabel'. /// - - public string PreLabel { get; } + public string PreReleaseLabel { get; } /// - /// BuildLabel position in the SymVer string 'major.minor.patch-PreLabel+BuildLabel'. + /// BuildLabel position in the SymVer string 'major.minor.patch-PreReleaseLabel+BuildLabel'. /// public string BuildLabel { get; } @@ -660,7 +659,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) string preLabel = null; string buildLabel = null; - // We parse the SymVer 'version' string 'major.minor.patch-PreLabel+BuildLabel'. + // We parse the SymVer 'version' string 'major.minor.patch-PreReleaseLabel+BuildLabel'. var dashIndex = version.IndexOf('-'); var plusIndex = version.IndexOf('+'); @@ -670,7 +669,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) if (plusIndex == -1) { // No buildLabel: buildLabel == null - // Format is 'major.minor.patch-PreLabel' + // Format is 'major.minor.patch-PreReleaseLabel' preLabel = version.Substring(dashIndex+1); versionSansLabel = version.Substring(0, dashIndex); } @@ -695,7 +694,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) } else { - // Format is 'major.minor.patch-PreLabel+BuildLabel' + // Format is 'major.minor.patch-PreReleaseLabel+BuildLabel' preLabel = version.Substring(dashIndex+1, plusIndex-dashIndex-1); buildLabel = version.Substring(plusIndex+1); versionSansLabel = version.Substring(0, dashIndex); @@ -760,9 +759,9 @@ public override string ToString() result.Append(Major).Append(Utils.Separators.Dot).Append(Minor).Append(Utils.Separators.Dot).Append(Patch); - if (!string.IsNullOrEmpty(PreLabel)) + if (!string.IsNullOrEmpty(PreReleaseLabel)) { - result.Append("-").Append(PreLabel); + result.Append("-").Append(PreReleaseLabel); } if (!string.IsNullOrEmpty(BuildLabel)) @@ -823,7 +822,7 @@ public int CompareTo(SemanticVersion value) return Patch > value.Patch ? 1 : -1; // SymVer 2.0 standard requires to ignore 'BuildLabel' (Build metadata). - return ComparePreLabel(this.PreLabel, value.PreLabel); + return ComparePreLabel(this.PreReleaseLabel, value.PreReleaseLabel); } /// @@ -842,7 +841,7 @@ public bool Equals(SemanticVersion other) // SymVer 2.0 standard requires to ignore 'BuildLabel' (Build metadata). return other != null && (Major == other.Major) && (Minor == other.Minor) && (Patch == other.Patch) && - string.Equals(PreLabel, other.PreLabel, StringComparison.Ordinal); + string.Equals(PreReleaseLabel, other.PreReleaseLabel, StringComparison.Ordinal); } /// From 519350161c008c50e621dae1d341d3bceb45669d Mon Sep 17 00:00:00 2001 From: iSazonov Date: Tue, 7 Nov 2017 08:57:53 +0300 Subject: [PATCH 6/8] Fix Compare(), GetHashCode() and add comments. --- .../engine/PSVersionInfo.cs | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index 76a0da4bd7e..dd80b710a60 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -780,8 +780,20 @@ public override string ToString() /// public static int Compare(SemanticVersion versionA, SemanticVersion versionB) { - if (versionA == null) return -1; - return versionA.CompareTo(versionB); + if (versionA != null) { + if (versionB != null) + { + return versionA.CompareTo(versionB); + } + return 1; + } + + if (versionB != null) + { + return -1; + } + + return 0; } /// @@ -849,7 +861,7 @@ public bool Equals(SemanticVersion other) /// public override int GetHashCode() { - return versionString.GetHashCode(); + return this.ToString().GetHashCode(); } /// @@ -907,7 +919,15 @@ public override int GetHashCode() private static int ComparePreLabel(string preLabel1, string preLabel2) { - // Symver 2.0 standard p.9 - Pre-release versions have a lower precedence than the associated normal version. + // Symver 2.0 standard p.9 + // Pre-release versions have a lower precedence than the associated normal version. + // Comparing each dot separated identifier from left to right + // until a difference is found as follows: + // identifiers consisting of only digits are compared numerically + // and identifiers with letters or hyphens are compared lexically in ASCII sort order. + // Numeric identifiers always have lower precedence than non-numeric identifiers. + // A larger set of pre-release fields has a higher precedence than a smaller set, + // if all of the preceding identifiers are equal. if (String.IsNullOrEmpty(preLabel1)) { return String.IsNullOrEmpty(preLabel2) ? 0 : 1; } if (String.IsNullOrEmpty(preLabel2)) { return -1; } From 69e03550c5bf2387477ac62e7d4b8780cb904b22 Mon Sep 17 00:00:00 2001 From: iSazonov Date: Tue, 7 Nov 2017 09:39:30 +0300 Subject: [PATCH 7/8] Fix tests --- test/powershell/engine/Basic/SemanticVersion.Tests.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 index 5f69f43180e..173788a7325 100644 --- a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 +++ b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 @@ -8,7 +8,7 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.Major | Should Be 1 $v.Minor | Should Be 2 $v.Patch | Should Be 3 - $v.PreLabel | Should Be "Alpha-super.3" + $v.PreReleaseLabel | Should Be "Alpha-super.3" $v.BuildLabel | Should Be "BLD.a1-xxx.03" $v.ToString() | Should Be "1.2.3-Alpha-super.3+BLD.a1-xxx.03" @@ -16,7 +16,7 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.Major | Should Be 1 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.PreLabel | Should BeNullOrEmpty + $v.PreReleaseLabel | Should BeNullOrEmpty $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "1.0.0" @@ -24,7 +24,7 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.Major | Should Be 3 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.PreLabel | Should BeNullOrEmpty + $v.PreReleaseLabel | Should BeNullOrEmpty $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "3.0.0" @@ -32,7 +32,7 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v.Major | Should Be 2 $v.Minor | Should Be 0 $v.Patch | Should Be 0 - $v.PreLabel | Should BeNullOrEmpty + $v.PreReleaseLabel | Should BeNullOrEmpty $v.BuildLabel | Should BeNullOrEmpty $v.ToString() | Should Be "2.0.0" } From 2836c8b883e70de88eecdbef0e2c59505af96c08 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 8 Nov 2017 13:23:22 -0800 Subject: [PATCH 8/8] [Feature] Address comment and fix immature replacement --- .../DotNetTypes_format_ps1xml.cs | 6 ++-- .../engine/PSVersionInfo.cs | 33 +++++++++---------- .../engine/Types_Ps1Xml.cs | 8 ++--- .../engine/Basic/SemanticVersion.Tests.ps1 | 4 +-- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs index 6ae20552979..6e7d3d7eebb 100644 --- a/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs +++ b/src/System.Management.Automation/FormatAndOutput/DefaultFormatters/DotNetTypes_format_ps1xml.cs @@ -501,8 +501,8 @@ private static IEnumerable ViewsOf_System_Version_With_Lab .AddPropertyColumn("Minor") .AddPropertyColumn("Build") .AddPropertyColumn("Revision") - .AddPropertyColumn("PSSemanticVersionPreLabel") - .AddPropertyColumn("PSSemanticVersionBuildLabel") + .AddPropertyColumn("PSSemVerPreReleaseLabel") + .AddPropertyColumn("PSSemVerBuildLabel") .EndRowDefinition() .EndTable()); } @@ -520,7 +520,7 @@ private static IEnumerable ViewsOf_Semantic_Version_With_L .AddPropertyColumn("Major") .AddPropertyColumn("Minor") .AddPropertyColumn("Patch") - .AddPropertyColumn("PreLabel") + .AddPropertyColumn("PreReleaseLabel") .AddPropertyColumn("BuildLabel") .EndRowDefinition() .EndTable()); diff --git a/src/System.Management.Automation/engine/PSVersionInfo.cs b/src/System.Management.Automation/engine/PSVersionInfo.cs index dd80b710a60..55e30b350e4 100644 --- a/src/System.Management.Automation/engine/PSVersionInfo.cs +++ b/src/System.Management.Automation/engine/PSVersionInfo.cs @@ -389,8 +389,8 @@ public sealed class SemanticVersion : IComparable, IComparable, private const string VersionSansRegEx = @"^(?\d+)(\.(?\d+))?(\.(?\d+))?$"; private const string LabelRegEx = @"^((?[0-9A-Za-z][0-9A-Za-z\-\.]*))?(\+(?[0-9A-Za-z][0-9A-Za-z\-\.]*))?$"; private const string LabelUnitRegEx = @"^[0-9A-Za-z][0-9A-Za-z\-\.]*$"; - private const string PreLabelPropertyName = "PSSemanticVersionPreLabel"; - private const string BuildLabelPropertyName = "PSSemanticVersionBuildLabel"; + private const string PreLabelPropertyName = "PSSemVerPreReleaseLabel"; + private const string BuildLabelPropertyName = "PSSemVerBuildLabel"; private const string TypeNameForVersionWithLabel = "System.Version#IncludeLabel"; private string versionString; @@ -417,20 +417,20 @@ public SemanticVersion(string version) /// The major version /// The minor version /// The patch version - /// The preLabel for the version - /// The buildLabel for the version + /// The pre-release label for the version + /// The build metadata for the version /// - /// If don't match 'LabelUnitRegEx'. + /// If don't match 'LabelUnitRegEx'. /// If don't match 'LabelUnitRegEx'. /// - public SemanticVersion(int major, int minor, int patch, string preLabel, string buildLabel) + public SemanticVersion(int major, int minor, int patch, string preReleaseLabel, string buildLabel) : this(major, minor, patch) { - if (!string.IsNullOrEmpty(preLabel)) + if (!string.IsNullOrEmpty(preReleaseLabel)) { - if (!Regex.IsMatch(preLabel, LabelUnitRegEx)) throw new FormatException(nameof(preLabel)); + if (!Regex.IsMatch(preReleaseLabel, LabelUnitRegEx)) throw new FormatException(nameof(preReleaseLabel)); - PreReleaseLabel = preLabel; + PreReleaseLabel = preReleaseLabel; } if (!string.IsNullOrEmpty(buildLabel)) @@ -665,7 +665,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) if (dashIndex > plusIndex) { - // 'preLabel' can contains dashes. + // 'PreReleaseLabel' can contains dashes. if (plusIndex == -1) { // No buildLabel: buildLabel == null @@ -675,7 +675,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) } else { - // No preLabel: preLabel == null + // No PreReleaseLabel: preLabel == null // Format is 'major.minor.patch+BuildLabel' buildLabel = version.Substring(plusIndex+1); versionSansLabel = version.Substring(0, plusIndex); @@ -705,7 +705,7 @@ private static bool TryParseVersion(string version, ref VersionResult result) (plusIndex != - 1 && String.IsNullOrEmpty(buildLabel)) || String.IsNullOrEmpty(versionSansLabel)) { - // We have dash and no preLabel or + // We have dash and no preReleaseLabel or // we have plus and no buildLabel or // we have no main version part (versionSansLabel==null) result.SetFailure(ParseFailureKind.FormatException); @@ -780,12 +780,9 @@ public override string ToString() /// public static int Compare(SemanticVersion versionA, SemanticVersion versionB) { - if (versionA != null) { - if (versionB != null) - { - return versionA.CompareTo(versionB); - } - return 1; + if (versionA != null) + { + return versionA.CompareTo(versionB); } if (versionB != null) diff --git a/src/System.Management.Automation/engine/Types_Ps1Xml.cs b/src/System.Management.Automation/engine/Types_Ps1Xml.cs index e4f17289383..15b32d1d312 100644 --- a/src/System.Management.Automation/engine/Types_Ps1Xml.cs +++ b/src/System.Management.Automation/engine/Types_Ps1Xml.cs @@ -1987,13 +1987,13 @@ public static IEnumerable Get() td252.Members.Add("ToString", new ScriptMethodData(@"ToString", GetScriptBlock(@" $suffix = """" - if (![String]::IsNullOrEmpty($this.PSSemanticVersionPreLabel)) + if (![String]::IsNullOrEmpty($this.PSSemVerPreReleaseLabel)) { - $suffix = ""-""+$this.PSSemanticVersionPreLabel + $suffix = ""-""+$this.PSSemVerPreReleaseLabel } - if (![String]::IsNullOrEmpty($this.PSSemanticVersionBuildLabel)) + if (![String]::IsNullOrEmpty($this.PSSemVerBuildLabel)) { - $suffix += ""+""+$this.PSSemanticVersionBuildLabel + $suffix += ""+""+$this.PSSemVerBuildLabel } ""$($this.Major).$($this.Minor).$($this.Build)""+$suffix "))); diff --git a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 index 173788a7325..981b4ffceb2 100644 --- a/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 +++ b/test/powershell/engine/Basic/SemanticVersion.Tests.ps1 @@ -75,8 +75,8 @@ Describe "SemanticVersion api tests" -Tags 'CI' { $v2.Major | Should Be 3 $v2.Minor | Should Be 2 $v2.Build | Should Be 1 - $v2.PSSemanticVersionPreLabel | Should Be "prerelease" - $v2.PSSemanticVersionBuildLabel | Should Be "meta" + $v2.PSSemVerPreReleaseLabel | Should Be "prerelease" + $v2.PSSemVerBuildLabel | Should Be "meta" $v2.ToString() | Should Be "3.2.1-prerelease+meta" }