Merge pull request #1246 from Microsoft/octogonz/rush-pnpm3-fix

[rush] Fix an error that was reported when parsing certain version strings from pnpm-lock.yaml

apps/rush-lib/assets/rush-init/[dot]gitattributes

@@ -11,4 +11,4 @@ yarn.lock                    merge=binary
 #
 # For more information, see this issue: https://github.com/Microsoft/web-build-tools/issues/1088
 #
-*.json            linguist-language=JSON5
+*.json                       linguist-language=JSON-with-Comments

apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts

@@ -68,40 +68,58 @@ interface IPnpmShrinkwrapYaml {
   specifiers: { [dependency: string]: string };
 }
 
+/**
+ * Given an encoded "dependency path" from the PNPM shrinkwrap file, this extracts the version component.
+ * @returns a SemVer string, or undefined if the version specifier cannot be parsed
+ */
 export function extractVersionFromPnpmVersionSpecifier(version: string): string | undefined {
-  let extractedVersion: string | undefined = undefined;
-
   if (!version) {
     return undefined;
   }
 
+  // Does the string contain any slashes?
   const versionParts: string[] = version.split('/');
 
-  // it had no slashes, so we know it is a version like "0.0.5"
   if (versionParts.length === 1) {
-    extractedVersion = version; // e.g. "0.0.5"
-  } else {
-    const isScoped: boolean = versionParts[1].indexOf('@') === 0;
-
-    // e.g. "/gulp-karma/0.0.5/[email protected]"
-    // if it has 4 parts, then it should be unscoped
-    if (versionParts.length === 4 && !isScoped) {
-      extractedVersion = versionParts[2]; // e.g. "0.0.5"
+    // No slashes
+
+    // Does it contain the V5 underscore delimiter?
+    const underscoreIndex: number = version.indexOf('_');
+    if (underscoreIndex >= 0) {
+      // This form was introduced in PNPM 3 (lockfile version 5):
+      //
+      // Example: "[email protected]"
+      // Example: "[email protected]"
+      // Example: "1.0.3_@[email protected]"
+      return version.substr(0, underscoreIndex); // e.g. "23.6.0"
+    } else {
+      // It is a simple version.
+      //
+      // Example: "0.0.5"
+      return version;
     }
+  }
 
-    // e.g. "/@ms/sp-client-utilities/3.1.1/[email protected]"
-    // if it has 5 parts, it should be scoped
-    if (versionParts.length === 5 && isScoped) {
-      extractedVersion = versionParts[3]; // e.g. "3.1.1"
-    }
+  // Does it contain an NPM scope?
+  const isScoped: boolean = versionParts[1].indexOf('@') === 0;
 
-    // e.g. "path.pkgs.visualstudio.com/@scope/depame/1.4.0"
-    if (!extractedVersion && semver.valid(versionParts[versionParts.length - 1]) !== null) {
-      extractedVersion = versionParts[versionParts.length - 1];
-    }
+  if (versionParts.length === 4 && !isScoped) {
+    // Example: "/gulp-karma/0.0.5/[email protected]"
+    // Example: "/sinon-chai/2.8.0/[email protected][email protected]")
+    return versionParts[2]; // e.g. "0.0.5"
+  }
+
+  if (versionParts.length === 5 && isScoped) {
+    // Example: "/@ms/sp-client-utilities/3.1.1/[email protected]"
+    return versionParts[3]; // e.g. "3.1.1"
+  }
+
+  if (semver.valid(versionParts[versionParts.length - 1]) !== null) {
+    // Example: "path.pkgs.visualstudio.com/@scope/depame/1.4.0"
+    return versionParts[versionParts.length - 1];  // e.g. "1.4.0"
   }
 
-  return extractedVersion;
+  return undefined;
 }
 
 export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
@@ -198,7 +216,7 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
   }
 
   protected checkValidVersionRange(dependencyVersion: string, versionRange: string): boolean { // override
-    // dependencyVersion could be a relattive or absolute path, for those cases we
+    // dependencyVersion could be a relative or absolute path, for those cases we
     // need to extract the version from the end of the path.
     return super.checkValidVersionRange(dependencyVersion.split('/').pop()!, versionRange);
   }
@@ -261,18 +279,6 @@ export class PnpmShrinkwrapFile extends BaseShrinkwrapFile {
   }
 
   private _normalizeDependencyVersion(dependencyName: string, version: string): string | undefined {
-    // version will be either:
-    // A - the version (e.g. "0.0.5")
-    // B - a peer dep version (e.g. "/gulp-karma/0.0.5/[email protected]"
-    //                           or "/@ms/sp-client-utilities/3.1.1/[email protected]"
-    //                           or "/sinon-chai/2.8.0/[email protected][email protected]")
-    // C -The dependency path is relative or absolute (e.g., /foo/1.0.0)
-
-    // check to see if this is the special style of specifiers
-    // e.g.:  "/gulp-karma/0.0.5/[email protected]" or
-    //     or "/@ms/sp-client-utilities/3.1.1/[email protected]"
-    // split it by forward slashes, then grab the second group (or the 3rd, if the package name is scoped)
-    // if the second group doesn't exist, return the version directly
     if (version) {
       const extractedVersion: string | undefined = extractVersionFromPnpmVersionSpecifier(version);
 

apps/rush-lib/src/logic/test/ShrinkwrapFile.test.ts

@@ -77,6 +77,12 @@ describe('extractVersionFromPnpmVersionSpecifier', () => {
     expect(extractVersionFromPnpmVersionSpecifier('example.pkgs.visualstudio.com/@scope/testDep/1.2.3-beta.3'))
       .toEqual('1.2.3-beta.3');
   });
+  it('extracts a V5 version without a scope', () => {
+    expect(extractVersionFromPnpmVersionSpecifier('[email protected]')).toEqual('23.6.0');
+  });
+  it('extracts a V5 peer dependency with a scope', () => {
+    expect(extractVersionFromPnpmVersionSpecifier('1.0.3_@[email protected]')).toEqual('1.0.3');
+  });
   it('handles bad cases', () => {
     expect(extractVersionFromPnpmVersionSpecifier('/foo/gulp-karma/0.0.5/[email protected]')).toEqual(undefined);
     expect(extractVersionFromPnpmVersionSpecifier('/@ms/3.1.1/[email protected]')).toEqual(undefined);

common/changes/@microsoft/rush/octogonz-rush-pnpm3-fix_2019-04-24-06-01.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Fix an issue where Rush sometimes failed to parse versions from PNPM 3.x's pnpm-lock.yaml",
+      "packageName": "@microsoft/rush",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "[email protected]"
+}

common/changes/@microsoft/rush/octogonz-rush-pnpm3-fix_2019-04-24-06-02.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Update the .gitattributes file written by \"rush init\" to use a better syntax highlighter for JSON files",
+      "packageName": "@microsoft/rush",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "[email protected]"
+}

common/config/rush/version-policies.json

@@ -82,7 +82,7 @@
     "policyName": "rush",
     "definitionName": "lockStepVersion",
     "version": "5.7.0",
-    "nextBump": "minor",
+    "nextBump": "patch",
     "mainProject": "@microsoft/rush"
   }
 ]