Support running via eval/require as well as vm2

.travis.yml

@@ -7,19 +7,23 @@ cache:
 
 matrix:
   include:
+  # Some normal CI test runs
+  - node_js: '8'
+  - node_js: '6'
+
+  # Does the real danger run
   - node_js: node
-    before_install:
-    - yarn global add greenkeeper-lockfile@1
-    before_script:
-    - greenkeeper-lockfile-update
-    after_script:
+    script:
+    - yarn add jest
+    - yarn jest --outputFile test-results.json --json
     - yarn run link
+    - echo "This is only for running the real danger on this repo"
     - danger run --verbose
-    - greenkeeper-lockfile-upload
-  - node_js: '6'
 
+  # Create some fake projects at runtime
   - node_js: '7'
     script:
+      - echo "This is only for Integration tests on two blank projects"
       - yarn build
       - mkdir danger_blank_test
       - cd danger_blank_test
@@ -41,7 +45,6 @@ matrix:
       - cd ..
       - rm -rf danger_babel_test
 
-  - node_js: '8'
 
 script:
 - yarn lint

changelog.md

@@ -4,6 +4,15 @@
 
 ### Master
 
+- Moves away from vm2 to a require-based Dangerfile runner. This removes the sandboxing aspect of
+  the Dangerfile completely, but the sandboxing was mainly for Peril and I have a plan for that.
+
+  https://github.com/danger/peril/issues/159
+
+  I would like to move the main parts of Danger JS to also work like `danger process`, so I'll
+  be continuing to work as a alpha for a bit more. One interesting side-effect of this could be that
+  I can remove `schedule` from the DSL. I've not tested it yet though. Turns out this change is _real_
+  hard to write tests for. I've made #394 for that.
 
 ### 2.0.0-alpha.18 - 19
 

dangerfile.ts

@@ -2,29 +2,27 @@
 // This means we can re-use the type infra from the app, without having to
 // fake the import.
 
+// console.log(global)
+// console.log(require)
+// console.log(require.extensions)
+
 import { DangerDSL } from "./source/dsl/DangerDSL"
 declare var danger: DangerDSL
-declare var results: any
+// declare var results: any
 declare function warn(params: string): void
 declare function fail(params: string): void
-declare function message(params: string): void
-declare function markdown(params: string): void
+// declare function message(params: string): void
+// declare function markdown(params: string): void
+declare function schedule(promise: Promise<any | void>): void
 declare function schedule(promise: () => Promise<any | void>): void
-declare function schedule(callback: (resolve) => void): void
+declare function schedule(callback: (resolve: any) => void): void
 
 import * as fs from "fs"
-import * as child_process from "child_process"
-import { distanceInWords } from "date-fns"
-
-// For some reason we're getting type errors on this includes module?
-// Wonder if we could move to the includes function in ES2015?
-import * as includes from "lodash.includes"
-const sentence = danger.utils.sentence
 
 schedule(async () => {
   // Request a CHANGELOG entry if not declared #trivial
-  const hasChangelog = includes(danger.git.modified_files, "changelog.md")
-  const isTrivial = includes(danger.github.pr.body + danger.github.pr.title, "#trivial")
+  const hasChangelog = danger.git.modified_files.includes("changelog.md")
+  const isTrivial = (danger.github.pr.body + danger.github.pr.title).includes("#trivial")
   const isGreenkeeper = danger.github.pr.user.login === "greenkeeper"
 
   if (!hasChangelog && !isTrivial && !isGreenkeeper) {
@@ -33,7 +31,7 @@ schedule(async () => {
     // Politely ask for their name on the entry too
     const changelogDiff = await danger.git.diffForFile("changelog.md")
     const contributorName = danger.github.pr.user.login
-    if (changelogDiff && !includes(changelogDiff.diff, contributorName)) {
+    if (changelogDiff && changelogDiff.diff.includes(contributorName)) {
       warn("Please add your GitHub name to the changelog entry, so we can attribute you correctly.")
     }
   }
@@ -51,23 +49,25 @@ jest()
 // This also serves as the "one true DSL" for a Danger run against a PR
 // which tools can then work against.
 
-// import dtsGenerator from "./scripts/danger-dts"
-// const currentDTS = dtsGenerator()
-// const savedDTS = fs.readFileSync("source/danger.d.ts").toString()
-// if (currentDTS !== savedDTS) {
-//   const message = "There are changes to the Danger DSL which are not reflected in the current danger.d.ts."
-//   const idea = "Please run <code>yarn declarations</code> and update this PR."
-//   fail(`${message}<br/><i>${idea}</i>`)
-// }
+// debugger
+
+import dtsGenerator from "./scripts/danger-dts"
+const currentDTS = dtsGenerator()
+const savedDTS = fs.readFileSync("source/danger.d.ts").toString()
+if (currentDTS !== savedDTS) {
+  const message = "There are changes to the Danger DSL which are not reflected in the current danger.d.ts."
+  const idea = "Please run <code>yarn declarations</code> and update this PR."
+  fail(`${message}<br/><i>${idea}</i>`)
+}
 
 // Always ensure we name all CI providers in the README. These
 // regularly get forgotten on a PR adding a new one.
+const sentence = danger.utils.sentence
 
-// import { realProviders } from "./source/ci_source/providers"
-// import Fake from "./source/ci_source/providers/Fake"
-// const readme = fs.readFileSync("README.md").toString()
-// const names = realProviders.map(p => new p({}).name)
-// const missing = names.filter(n => !readme.includes(n))
-// if (missing.length) {
-//   warn(`These providers are missing from the README: ${sentence(missing)}`)
-// }
+import { realProviders } from "./source/ci_source/providers"
+const readme = fs.readFileSync("README.md").toString()
+const names = realProviders.map(p => new p({}).name)
+const missing = names.filter(n => !readme.includes(n))
+if (missing.length) {
+  warn(`These providers are missing from the README: ${sentence(missing)}`)
+}

package.json

@@ -31,15 +31,15 @@
   },
   "scripts": {
     "precommit": "lint-staged",
-    "prepush": "npm run build",
+    "prepush": "yarn run build",
     "test": "jest",
     "test:watch": "jest --watch",
     "lint": "tslint \"source/**/*.ts\"",
     "lint:fix": "tslint \"source/**/*.ts\" --fix",
-    "prepublishOnly": "npm run build && yarn declarations",
-    "build": "shx rm -rf ./distribution && tsc && madge ./distribution --circular",
+    "prepublishOnly": "yarn run build && yarn declarations",
+    "build": "shx rm -rf ./distribution && tsc -p tsconfig.production.json && madge ./distribution --circular",
     "build:watch": "tsc -w",
-    "link": "npm run build && chmod +x distribution/commands/danger.js && npm link",
+    "link": "yarn run build && chmod +x distribution/commands/danger.js && npm link",
     "declarations": "ts-node ./scripts/create-danger-dts.ts",
     "docs:cp_defs":
       "mkdir docs/docs_generate; cp source/danger.d.ts docs/docs_generate; cp node_modules/github/lib/index.d.ts docs/docs_generate/github.d.ts",
@@ -81,7 +81,7 @@
     "jest-json-reporter": "^1.2.2",
     "lint-staged": "^4.0.0",
     "madge": "^2.0.0",
-    "prettier": "^1.5.3",
+    "prettier": "^1.7.4",
     "shx": "^0.2.1",
     "ts-jest": "^21",
     "ts-node": "^3.2.1",
@@ -106,8 +106,9 @@
     "parse-diff": "^0.4.0",
     "parse-link-header": "^1.0.1",
     "pinpoint": "^1.1.0",
+    "require-from-string": "^2.0.1",
     "rfc6902": "^1.3.0",
-    "vm2": "patriksimek/vm2",
+    "vm2": "patriksimek/vm2#custom_files",
     "voca": "^1.2.0"
   },
   "optionalDependencies": {}

source/ambient.d.ts

@@ -15,131 +15,24 @@ declare module "pinpoint"
 
 declare module "*/package.json"
 
-declare module "vm2" {
-  /**
-   *  Require options for a VM
-   */
-  export interface VMRequire {
-    /** Array of allowed builtin modules, accepts ["*"] for all (default: none) */
-    builtin?: string[]
-    /*
-    * `host` (default) to require modules in host and proxy them to sandbox. `sandbox` to load, compile and
-    * require modules in sandbox. Builtin modules except `events` always required in host and proxied to sandbox
-    */
-    context?: "host" | "sandbox"
-    /** `true` or an array of allowed external modules (default: `false`) */
-    external?: boolean | string[]
-    /** Array of modules to be loaded into NodeVM on start. */
-    import?: string[]
-    /** Restricted path where local modules can be required (default: every path). */
-    root?: string
-    /** Collection of mock modules (both external or builtin). */
-    mock?: any
-  }
-
-  /**
-   * A custom compiler function for all of the JS that comes
-   * into the VM
-   */
-  type CompilerFunction = (code: string, filename: string) => string
-
-  /**
-   *  Options for creating a NodeVM
-   */
-  export interface VMOptions {
-    /**
-     * `javascript` (default) or `coffeescript` or custom compiler function (which receives the code, and it's filepath).
-     *  The library expects you to have coffee-script pre-installed if the compiler is set to `coffeescript`.
-     */
-    compiler?: "javascript" | "coffeescript" | CompilerFunction
-    /** VM's global object. */
-    sandbox?: any
-    /**
-     * Script timeout in milliseconds.  Timeout is only effective on code you run through `run`.
-     * Timeout is NOT effective on any method returned by VM.
-     */
-    timeout?: number
-  }
-
-  /**
-   *  Options specific o
-   */
-  export interface NodeVMOptions extends VMOptions {
-    /** `inherit` to enable console, `redirect` to redirect to events, `off` to disable console (default: `inherit`). */
-    console?: "inherit" | "redirect"
-    /** `true` or an object to enable `require` optionss (default: `false`). */
-    require?: true | VMRequire
-    /** `true` to enable VMs nesting (default: `false`). */
-    nesting?: boolean
-    /** `commonjs` (default) to wrap script into CommonJS wrapper, `none` to retrieve value returned by the script. */
-    wrapper?: "commonjs" | "none"
-  }
-
-  /**
-   * A VM with behavior more similar to running inside Node.
-   */
-  export class NodeVM {
-    constructor(options?: NodeVMOptions)
-    /** Runs the code */
-    run(js: string, path: string): any
-    /** Runs the VMScript object */
-    run(script: VMScript): any
-
-    /** Freezes the object inside VM making it read-only. Not available for primitive values. */
-    freeze(object: any, name: string): any
-    /** Protects the object inside VM making impossible to set functions as it's properties. Not available for primitive values. */
-    protect(object: any, name: string): any
-    /** Require a module in VM and return it's exports. */
-    require(module: string): any
-  }
-
-  /**
-   * VM is a simple sandbox, without `require` feature, to synchronously run an untrusted code.
-   * Only JavaScript built-in objects + Buffer are available. Scheduling functions
-   * (`setInterval`, `setTimeout` and `setImmediate`) are not available by default.
-   */
-  export class VM {
-    constructor(options?: VMOptions)
-    /** Runs the code */
-    run(js: string): any
-    /** Runs the VMScript object */
-    run(script: VMScript): any
-    /** Freezes the object inside VM making it read-only. Not available for primitive values. */
-    freeze(object: any, name: string): any
-    /** Protects the object inside VM making impossible to set functions as it's properties. Not available for primitive values */
-    protect(object: any, name: string): any
-
-    /**
-     * Create NodeVM and run code inside it.
-     *
-     * @param {String} script Javascript code.
-     * @param {String} [filename] File name (used in stack traces only).
-     * @param {Object} [options] VM options.
-     */
-    static code(script: string, filename: string, options: NodeVMOptions): NodeVM
-
-    /**
-     * Create NodeVM and run script from file inside it.
-     *
-     * @param {String} [filename] File name (used in stack traces only).
-     * @param {Object} [options] VM options.
-     */
-    static file(filename: string, options: NodeVMOptions): NodeVM
-  }
-
-  /**
-   * You can increase performance by using pre-compiled scripts.
-   * The pre-compiled VMScript can be run later multiple times. It is important to note that the code is not bound
-   * to any VM (context); rather, it is bound before each run, just for that run.
-   */
-  export class VMScript {
-    constructor(code: string, path: string)
-    /** Wraps the code */
-    wrap(prefix: string, postfix: string): VMScript
-    /** Compiles the code. If called multiple times, the code is only compiled once. */
-    compile(): any
-  }
-
-  /** Custom Error class */
-  export class VMError extends Error {}
-}
+declare module "require-from-string"
+declare module "node-eval"
+
+// declare module "require-from-string" {
+//   export interface RequireOptions {
+//     /** List of paths, that will be appended to module paths. Useful, when you want
+//      * to be able require modules from these paths. */
+//     appendPaths: string[]
+//     /**
+//      * Same as appendPath, but paths will be prepended.
+//      */
+//     prependPaths: string[]
+//   }
+//   /**
+//    * Load module from string in Node.
+//    * @param code Module code
+//    * @param filename Optional filename
+//    * @param opts
+//    */
+//   export default function(code: string, filename?: string, opts?: Partial<RequireOptions>): any
+// }

source/commands/danger-pr.ts

@@ -7,12 +7,14 @@ import { GitHub } from "../platforms/GitHub"
 import { GitHubAPI } from "../platforms/github/GitHubAPI"
 import { Executor } from "../runner/Executor"
 import { pullRequestParser } from "../platforms/github/pullRequestParser"
-import { runDangerfileEnvironment } from "../runner/DangerfileRunner"
+import { runDangerfileEnvironment } from "../runner/runners/inline"
 import { dangerfilePath } from "./utils/file-utils"
 import validateDangerfileExists from "./utils/validateDangerfileExists"
 import openRepl from "./utils/repl"
 import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs"
 
+import inlineRunner from "../runner/runners/inline"
+
 const d = debug("danger:pr")
 
 program.usage("[options] <pr_url>").description("Emulate running Danger against an existing GitHub Pull Request.")
@@ -51,12 +53,12 @@ async function runDanger(source: FakeCI, platform: GitHub, file: string) {
     verbose: app.verbose,
   }
 
-  const exec = new Executor(source, platform, config)
+  const exec = new Executor(source, platform, inlineRunner, config)
 
   const runtimeEnv = await exec.setupDanger()
   const results = await runDangerfileEnvironment(file, undefined, runtimeEnv)
   if (program.repl) {
-    openRepl(runtimeEnv.sandbox)
+    openRepl(runtimeEnv)
   } else {
     jsome(results)
   }

source/commands/danger-process.ts

@@ -7,6 +7,8 @@ import runDangerSubprocess, { prepareDangerDSL } from "./utils/runDangerSubproce
 import setSharedArgs, { SharedCLI } from "./utils/sharedDangerfileArgs"
 import getRuntimeCISource from "./utils/getRuntimeCISource"
 
+import inlineRunner from "../runner/runners/inline"
+
 // Given the nature of this command, it can be tricky to test, so I use a command like this:
 //
 // env DANGER_GITHUB_API_TOKEN='xxx' DANGER_FAKE_CI="YEP" DANGER_TEST_REPO='artsy/eigen' DANGER_TEST_PR='2408'
@@ -62,7 +64,7 @@ async function run() {
         verbose: app.verbose,
       }
 
-      const exec = new Executor(source, platform, config)
+      const exec = new Executor(source, platform, inlineRunner, config)
       const dangerDSL = await exec.dslForDanger()
       const processInput = prepareDangerDSL(dangerDSL)