Allow git+***:// dependencies (#3735)

* Move getExoticResolver function to resolvers/index.js

* Add tests for getExoticResolver

* Use Git Resolver for any `git+***://` dependencies
Fix #3677

Author: Volune

__tests__/resolvers/index.js

@@ -0,0 +1,82 @@
+/* @flow */
+
+import {getExoticResolver} from '../../src/resolvers/index.js';
+// exotic resolvers
+import GitResolver from '../../src/resolvers/exotics/git-resolver.js';
+import GistResolver from '../../src/resolvers/exotics/gist-resolver.js';
+import RegistryResolver from '../../src/resolvers/exotics/registry-resolver.js';
+import TarballResolver from '../../src/resolvers/exotics/tarball-resolver.js';
+// registry resolvers
+import NpmRegistryResolver from '../../src/resolvers/registries/npm-resolver.js';
+import YarnRegistryResolver from '../../src/resolvers/registries/yarn-resolver.js';
+
+test('getExoticResolver does not return a Resolver for 0.0.0 pattern', () => {
+  const pattern = '0.0.0';
+  expect(getExoticResolver(pattern)).toEqual(null);
+});
+
+test('getExoticResolver returns NPM Registry Resolver for npm:0.0.0 pattern', () => {
+  const pattern = 'npm:0.0.0';
+  const resolver = getExoticResolver(pattern);
+  expect(resolver).toBeDefined();
+  expect(resolver && resolver.prototype).toBeInstanceOf(RegistryResolver);
+  expect(resolver && resolver.factory).toEqual(NpmRegistryResolver);
+});
+
+test('getExoticResolver returns Yarn Registry Resolver for yarn:0.0.0 pattern', () => {
+  const pattern = 'yarn:0.0.0';
+  const resolver = getExoticResolver(pattern);
+  expect(resolver).toBeDefined();
+  expect(resolver && resolver.prototype).toBeInstanceOf(RegistryResolver);
+  expect(resolver && resolver.factory).toEqual(YarnRegistryResolver);
+});
+
+test('getExoticResolver returns Git Resolver for git+ssh://... pattern', () => {
+  const pattern = 'git+ssh://user@hostname/path/repo#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for git+https://... pattern', () => {
+  const pattern = 'git+https://hostname/path/repo#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for git+file:///... pattern', () => {
+  const pattern = 'git+file:///path/repo#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for git://... pattern', () => {
+  const pattern = 'git://user@hostname/path/repo#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for ssh://... pattern', () => {
+  const pattern = 'ssh://user@hostname/path/repo#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for http://... pattern with .git extension', () => {
+  const pattern = 'http://hostname/path/repo.git#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Git Resolver for https://... pattern with .git extension', () => {
+  const pattern = 'https://hostname/path/repo.git#master';
+  expect(getExoticResolver(pattern)).toEqual(GitResolver);
+});
+
+test('getExoticResolver returns Tarball Resolver for http://... pattern', () => {
+  const pattern = 'http://hostname/path/file.tgz';
+  expect(getExoticResolver(pattern)).toEqual(TarballResolver);
+});
+
+test('getExoticResolver returns Tarball Resolver for https://... pattern', () => {
+  const pattern = 'https://hostname/path/file.tgz';
+  expect(getExoticResolver(pattern)).toEqual(TarballResolver);
+});
+
+test('getExoticResolver returns Gist Resolver for gist://... pattern', () => {
+  const pattern = 'gist:id#hash';
+  expect(getExoticResolver(pattern)).toEqual(GistResolver);
+});

src/cli/commands/add.js

@@ -7,6 +7,7 @@ import type Config from '../../config.js';
 import type {ListOptions} from './list.js';
 import Lockfile from '../../lockfile/wrapper.js';
 import PackageRequest from '../../package-request.js';
+import {getExoticResolver} from '../../resolvers/index.js';
 import {buildTree} from './list.js';
 import {wrapLifecycle, Install} from './install.js';
 import {MessageError} from '../../errors.js';
@@ -56,7 +57,7 @@ export class Add extends Install {
     const {exact, tilde} = this.flags;
     const parts = PackageRequest.normalizePattern(pattern);
     let version;
-    if (PackageRequest.getExoticResolver(pattern)) {
+    if (getExoticResolver(pattern)) {
       // wasn't a name/range tuple so this is just a raw exotic pattern
       version = pattern;
     } else if (parts.hasVersion && parts.range) {

src/cli/commands/import.js

@@ -7,6 +7,7 @@ import type Config from '../../config.js';
 import {Install} from './install.js';
 import {verifyTreeCheck} from './check.js';
 import {MessageError} from '../../errors.js';
+import {getExoticResolver} from '../../resolvers/index.js';
 import BaseResolver from '../../resolvers/base-resolver.js';
 import HostedGitResolver, {explodeHostedGitFragment} from '../../resolvers/exotics/hosted-git-resolver.js';
 import GistResolver, {explodeGistFragment} from '../../resolvers/exotics/gist-resolver.js';
@@ -40,7 +41,7 @@ class ImportResolver extends BaseResolver {
     return this.config.cwd;
   }
 
-  resolveHostedGit(info: Manifest, Resolver: typeof HostedGitResolver): Manifest {
+  resolveHostedGit(info: Manifest, Resolver: Class<HostedGitResolver>): Manifest {
     const {range} = PackageRequest.normalizePattern(this.pattern);
     const exploded = explodeHostedGitFragment(range, this.reporter);
     const hash = (info: any).gitHead;
@@ -127,7 +128,7 @@ class ImportResolver extends BaseResolver {
 
   resolveImport(info: Manifest): Manifest {
     const {range} = PackageRequest.normalizePattern(this.pattern);
-    const Resolver = PackageRequest.getExoticResolver(range);
+    const Resolver = getExoticResolver(range);
     if (Resolver && Resolver.prototype instanceof HostedGitResolver) {
       return this.resolveHostedGit(info, Resolver);
     } else if (Resolver && Resolver === GistResolver) {

src/cli/commands/install.js

@@ -18,6 +18,7 @@ import PackageResolver from '../../package-resolver.js';
 import PackageLinker from '../../package-linker.js';
 import PackageRequest from '../../package-request.js';
 import {registries} from '../../registries/index.js';
+import {getExoticResolver} from '../../resolvers/index.js';
 import {clean} from './clean.js';
 import * as constants from '../../constants.js';
 import * as fs from '../../util/fs.js';
@@ -209,7 +210,7 @@ export class Install {
     const excludeNames = [];
     for (const pattern of excludePatterns) {
       // can't extract a package name from this
-      if (PackageRequest.getExoticResolver(pattern)) {
+      if (getExoticResolver(pattern)) {
         continue;
       }
 

src/cli/commands/upgrade.js

@@ -4,7 +4,7 @@ import type {Reporter} from '../../reporters/index.js';
 import type Config from '../../config.js';
 import {Add} from './add.js';
 import Lockfile from '../../lockfile/wrapper.js';
-import PackageRequest from '../../package-request.js';
+import {getExoticResolver} from '../../resolvers/index.js';
 import {MessageError} from '../../errors.js';
 
 export function setFlags(commander: Object) {
@@ -69,7 +69,7 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
 function getDependency(allDependencies, dependency): string {
   const remoteSource = allDependencies[dependency];
 
-  if (remoteSource && PackageRequest.getExoticResolver(remoteSource)) {
+  if (remoteSource && getExoticResolver(remoteSource)) {
     return remoteSource;
   }
 

src/package-request.js

@@ -10,11 +10,10 @@ import Lockfile from './lockfile/wrapper.js';
 import PackageReference from './package-reference.js';
 import {registries as registryResolvers} from './resolvers/index.js';
 import {MessageError} from './errors.js';
-import {entries} from './util/misc.js';
 import * as constants from './constants.js';
 import * as versionUtil from './util/version.js';
 import WorkspaceResolver from './resolvers/contextual/workspace-resolver.js';
-import * as resolvers from './resolvers/index.js';
+import {getExoticResolver} from './resolvers/index.js';
 import * as fs from './util/fs.js';
 
 const path = require('path');
@@ -38,16 +37,6 @@ export default class PackageRequest {
     resolver.usedRegistries.add(req.registry);
   }
 
-  static getExoticResolver(pattern: string): ?Function {
-    // TODO make this type more refined
-    for (const [, Resolver] of entries(resolvers.exotics)) {
-      if (Resolver.isVersion(pattern)) {
-        return Resolver;
-      }
-    }
-    return null;
-  }
-
   parentRequest: ?PackageRequest;
   lockfile: Lockfile;
   reporter: Reporter;
@@ -108,8 +97,7 @@ export default class PackageRequest {
   async findVersionOnRegistry(pattern: string): Promise<Manifest> {
     const {range, name} = await this.normalize(pattern);
 
-    const exoticResolver = PackageRequest.getExoticResolver(range);
-
+    const exoticResolver = getExoticResolver(range);
     if (exoticResolver) {
       let data = await this.findExoticVersionInfo(exoticResolver, range);
 
@@ -145,7 +133,7 @@ export default class PackageRequest {
   }
 
   async normalizeRange(pattern: string): Promise<string> {
-    if (pattern.includes(':') || pattern.includes('@') || PackageRequest.getExoticResolver(pattern)) {
+    if (pattern.includes(':') || pattern.includes('@') || getExoticResolver(pattern)) {
       return Promise.resolve(pattern);
     }
 
@@ -220,8 +208,7 @@ export default class PackageRequest {
    */
 
   findVersionInfo(): Promise<Manifest> {
-    const exoticResolver = PackageRequest.getExoticResolver(this.pattern);
-
+    const exoticResolver = getExoticResolver(this.pattern);
     if (exoticResolver) {
       return this.findExoticVersionInfo(exoticResolver, this.pattern);
     } else if (WorkspaceResolver.isWorkspace(this.pattern, this.resolver.workspaceLayout)) {
@@ -409,7 +396,7 @@ export default class PackageRequest {
 
         const normalized = PackageRequest.normalizePattern(pattern);
 
-        if (PackageRequest.getExoticResolver(pattern) || PackageRequest.getExoticResolver(normalized.range)) {
+        if (getExoticResolver(pattern) || getExoticResolver(normalized.range)) {
           latest = wanted = 'exotic';
           url = normalized.range;
         } else {

src/package-resolver.js

@@ -4,6 +4,7 @@ import type {Manifest, DependencyRequestPatterns, DependencyRequestPattern} from
 import type {RegistryNames} from './registries/index.js';
 import type PackageReference from './package-reference.js';
 import type {Reporter} from './reporters/index.js';
+import {getExoticResolver} from './resolvers/index.js';
 import type Config from './config.js';
 import PackageRequest from './package-request.js';
 import RequestManager from './util/request-manager.js';
@@ -443,7 +444,7 @@ export default class PackageResolver {
         semver.validRange(range) &&
         semver.valid(lockfileEntry.version) &&
         !semver.satisfies(lockfileEntry.version, range) &&
-        !PackageRequest.getExoticResolver(range) &&
+        !getExoticResolver(range) &&
         hasVersion
       ) {
         this.reporter.warn(this.reporter.lang('incorrectLockfileEntry', req.pattern));

src/resolvers/base-resolver.js

@@ -18,6 +18,7 @@ export default class BaseResolver {
     this.config = request.config;
   }
 
+  static +isVersion: string => boolean;
   resolver: PackageResolver;
   reporter: Reporter;
   fragment: string;
@@ -26,13 +27,13 @@ export default class BaseResolver {
   config: Config;
   registry: RegistryNames;
 
-  fork(Resolver: Function, resolveArg: any, ...args: Array<string>): Promise<Manifest> {
+  fork(Resolver: Class<BaseResolver>, resolveArg: any, ...args: Array<string>): Promise<Manifest> {
     const resolver = new Resolver(this.request, ...args);
     resolver.registry = this.registry;
     return resolver.resolve(resolveArg);
   }
 
-  resolve(): Promise<Manifest> {
+  resolve(resolveArg?: any): Promise<Manifest> {
     throw new Error('Not implemented');
   }
 }

src/resolvers/exotics/git-resolver.js

@@ -12,8 +12,10 @@ import Git from '../../util/git.js';
 
 const urlParse = require('url').parse;
 
+const GIT_PROTOCOL_PATTERN = /git\+.+:/;
+
 // we purposefully omit https and http as those are only valid if they end in the .git extension
-const GIT_PROTOCOLS = ['git:', 'git+ssh:', 'git+https:', 'ssh:'];
+const GIT_PROTOCOLS = ['git:', 'ssh:'];
 
 const GIT_HOSTS = ['github.com', 'gitlab.com', 'bitbucket.com', 'bitbucket.org'];
 
@@ -44,6 +46,10 @@ export default class GitResolver extends ExoticResolver {
       return true;
     }
 
+    if (GIT_PROTOCOL_PATTERN.test(parts.protocol)) {
+      return true;
+    }
+
     if (GIT_PROTOCOLS.indexOf(parts.protocol) >= 0) {
       return true;
     }

src/resolvers/index.js

@@ -1,5 +1,7 @@
 /* @flow */
 
+import BaseResolver from './base-resolver.js';
+
 import RegistryNpm from './registries/npm-resolver.js';
 import RegistryYarn from './registries/yarn-resolver.js';
 
@@ -19,16 +21,25 @@ import ExoticGitLab from './exotics/gitlab-resolver.js';
 import ExoticGist from './exotics/gist-resolver.js';
 import ExoticBitbucket from './exotics/bitbucket-resolver.js';
 
-export const exotics = {
-  git: ExoticGit,
-  tarball: ExoticTarball,
-  github: ExoticGitHub,
-  file: ExoticFile,
-  link: ExoticLink,
-  gitlab: ExoticGitLab,
-  gist: ExoticGist,
-  bitbucket: ExoticBitbucket,
-};
+const exotics: Set<Class<$Subtype<BaseResolver>>> = new Set([
+  ExoticGit,
+  ExoticTarball,
+  ExoticGitHub,
+  ExoticFile,
+  ExoticLink,
+  ExoticGitLab,
+  ExoticGist,
+  ExoticBitbucket,
+]);
+
+export function getExoticResolver(pattern: string): ?Class<$Subtype<BaseResolver>> {
+  for (const Resolver of exotics) {
+    if (Resolver.isVersion(pattern)) {
+      return Resolver;
+    }
+  }
+  return null;
+}
 
 //
 
@@ -59,8 +70,10 @@ import ExoticRegistryResolver from './exotics/registry-resolver.js';
 for (const key in registries) {
   const RegistryResolver = registries[key];
 
-  exotics[key] = class extends ExoticRegistryResolver {
-    static protocol = key;
-    static factory = RegistryResolver;
-  };
+  exotics.add(
+    class extends ExoticRegistryResolver {
+      static protocol = key;
+      static factory = RegistryResolver;
+    },
+  );
 }