Peril support

[orta]

fs.readFile isn't available on Peril, so instead this would need to use danger.github.utils.fileContents to async get the file's contents.

Otherwise looks pretty ready, nice work 👍

[macklinu]

I think the only problem is that this plugin relies on reading a file from disk - the TSLint output JSON file - so I'm not sure using API.fileContents would work with Peril.

Perhaps it's time to change how this works and rely on the user running an NPM script that outputs the lint results as JSON - like tslint -f json - and then read and parse those results. I'm up for that breaking change if it means better support for Peril (and perhaps less configuration for users).

Thoughts?

[macklinu]

Commenting on my phone - for clarification, not sure API.fileContents would work either way. 🙂

[orta]

Ah, I thought this was calling the tslint JS API directly with any *.ts* file, I'll make a note to see how feasible this is.

I added a global 'peril' object to the runtime, which should make it possible to know if it's in Peril too: https://github.com/danger/peril/commit/76eb739192edd7bd0838439a244a72d646a255fa

[macklinu]

I feel like switching to ShellJS and executing a user-supplied lint script (like npm run lint) and then displaying the output if the exit code is not 0 might be the simplest approach here for supporting Peril. This was my original goal, but I tried using the TSLint API directly and failed to get something working (moved away from it in https://github.com/macklinu/danger-plugin-tslint/pull/7). Might try this again but with ShellJS for v3. 👍

[orta]

Peril doesn't have access to the filesystem of the eval context ( e.g. it's not a cloned version of the repo. ) so it wouldn't be able to use shellJS at all.

I did start taking a look last night, but didn't get too far (game of thrones happened) - bad psuedocode coming up:

  const allChangedFiles = [...danger.git.modified_files, ...danger.git.created_files]
  const allTS = allChangedFiles.filter(f => f.endsWith(".ts") || f.endsWith(".tsx"))

  // TS Config setup first
  // var config = ts.readConfigFile(configFile, ts.sys.readFile).config;
  // var parseConfigHost = {
  //     fileExists: fs.existsSync,
  //     readDirectory: ts.sys.readDirectory,
  //     readFile: function (file) { return fs.readFileSync(file, "utf8"); },
  //     useCaseSensitiveFileNames: true,
  // };

  var parsed = ts.parseJsonConfigFileContent(config, parseConfigHost, path.resolve(projectDirectory));
  var host = ts.createCompilerHost(parsed.options, true);
  var program  = [...]

  const setup = await danger.github.getFile("tslint.json")

  const linter = new Linter(setup)

  for (const file of allChangedFiles) {
    const contents = await danger.github.utils.getFile(file)
    if (contents) {
      linter.lint(file, contents, config)
    }
  }

  defaultResultHandler(linter.getResult())
}

So it'd need to pull in the settings, and any ts files to look at via the GH API, then do it all in memory.

[macklinu]

This is a really cool idea and what I was thinking this plugin should do originally, but I hadn't thought to pull file contents from GitHub - definitely the missing link. Gonna have to sit down with this and try some things out. Appreciate the pseudocode, definitely a helpful place to start. 😃