chore: make pkglint check peerDependencies (#1116)

pkglint will ensure that concrete dependency versions (in
'dependencies') must match peerDependency versions. This prevents us
from giving conflicting version ranges to NPM which can never be
satisfied.

tools/pkglint/lib/packagejson.ts

@@ -64,6 +64,18 @@ export class PackageJson {
     this.reports.push(report);
   }
 
+  public get dependencies(): {[key: string]: string} {
+    return this.json.dependencies || {};
+  }
+
+  public get devDependencies(): {[key: string]: string} {
+    return this.json.devDependencies || {};
+  }
+
+  public get peerDependencies(): {[key: string]: string} {
+    return this.json.peerDependencies || {};
+  }
+
   public applyFixes() {
     const fixable = this.reports.filter(r => r.fix);
     const nonFixable = this.reports.filter(r => !r.fix);
@@ -174,6 +186,14 @@ export class PackageJson {
     }
   }
 
+  public addPeerDependency(module: string, version: string) {
+    if (!('peerDependencies' in this.json)) {
+      this.json.peerDependencies = {};
+    }
+
+    this.peerDependencies[module] = version;
+  }
+
   /**
    * Whether the package-level file contains the given line
    */

tools/pkglint/lib/rules.ts

@@ -1,6 +1,7 @@
 import caseUtils = require('case');
 import fs = require('fs');
 import path = require('path');
+import semver = require('semver');
 import { LICENSE, NOTICE } from './licensing';
 import { PackageJson, ValidationRule } from './packagejson';
 import { deepGet, deepSet, expectDevDependency, expectJSON, fileShouldBe, fileShouldContain, monoRepoVersion } from './util';
@@ -310,6 +311,39 @@ export class MustUseCDKBuild extends ValidationRule {
   }
 }
 
+/**
+ * Dependencies in both regular and peerDependencies must agree in semver
+ *
+ * In particular, verify that depVersion satisfies peerVersion. This prevents
+ * us from instructing NPM to construct impossible closures, where we say:
+ *
+ *    peerDependency: A@1.0.0
+ *    dependency: A@2.0.0
+ *
+ * There is no version of A that would satisfy this.
+ *
+ * The other way around is not necessary--the depVersion can be bumped without
+ * bumping the peerVersion (if the API didn't change this may be perfectly
+ * valid). This prevents us from restricting a user's potential combinations of
+ * libraries unnecessarily.
+ */
+export class RegularDependenciesMustSatisfyPeerDependencies extends ValidationRule {
+  public validate(pkg: PackageJson): void {
+    for (const [depName, peerVersion] of Object.entries(pkg.peerDependencies)) {
+      const depVersion = pkg.dependencies[depName];
+      if (depVersion === undefined) { continue; }
+
+      // Make sure that depVersion satisfies peerVersion.
+      if (!semver.intersects(depVersion, peerVersion)) {
+        pkg.report({
+          message: `dependency ${depName}: concrete version ${depVersion} does not match peer version '${peerVersion}'`,
+          fix: () => pkg.addPeerDependency(depName, depVersion)
+        });
+      }
+    }
+  }
+}
+
 export class MustIgnoreSNK extends ValidationRule {
   public validate(pkg: PackageJson): void {
     fileShouldContain(pkg, '.npmignore', '*.snk');

tools/pkglint/package.json

@@ -36,8 +36,10 @@
     "@types/yargs": "^8.0.3"
   },
   "dependencies": {
+    "@types/semver": "^5.5.0",
     "case": "^1.5.5",
     "fs-extra": "^4.0.2",
+    "semver": "^5.6.0",
     "yargs": "^9.0.1"
   }
 }