chore: Switch from tslint to eslint

tslint is end-of-life, and warns when you install it. Additionally, the version we were on didn't support 'import type' expressions, as well as optional-chaining & null coalescing when I tried to use those in #1204

This PR does update a bunch of tslint-disable comments, but otherwise tries to minimally change runtime source in danger-js. If we want to tighten the eslint rules, I'd be super supportive, but I didn't want to cause a ton of thrash, so mostly the rules that are enabled are the ones that don't trigger on tons of existing code.

.eslintignore

@@ -0,0 +1,2 @@
+source/danger.d.ts
+source/runner/_tests/fixtures/*.ts

.eslintrc.js

@@ -0,0 +1,66 @@
+module.exports = {
+  env: {
+    browser: true,
+    es6: true,
+    node: true,
+  },
+  extends: ["plugin:@typescript-eslint/recommended", "plugin:jest/recommended", "prettier"],
+  parser: "@typescript-eslint/parser",
+  parserOptions: {
+    project: "tsconfig.spec.json",
+    sourceType: "module",
+  },
+  plugins: ["eslint-plugin-jest", "eslint-plugin-jsdoc", "@typescript-eslint"],
+  rules: {
+    "@typescript-eslint/naming-convention": "error",
+    "@typescript-eslint/no-empty-function": "error",
+    "@typescript-eslint/no-unused-expressions": "error",
+
+    // Something is grumpy about these rules re: node imports - TODO
+    "@typescript-eslint/no-require-imports": "off",
+    "@typescript-eslint/no-var-requires": "off",
+
+    curly: "error",
+    "jsdoc/check-alignment": "error",
+    "jsdoc/check-indentation": "off",
+    "jsdoc/newline-after-description": "off",
+    "no-empty": "error",
+    // This is used intentionally in a bunch of ci_source/providers
+    "no-empty-function": "off",
+    "no-redeclare": "error",
+    "no-var": "error",
+    // There are a bunch of existing uses of 'let' where this rule would trigger
+    "prefer-const": "off",
+
+    // This project has a ton of interacting APIs, and sometimes it's helpful to be explicit, even if the type is trivial
+    "@typescript-eslint/no-inferrable-types": "off",
+
+    "no-unused-expressions": "off",
+    "@typescript-eslint/no-unused-expressions": "error",
+
+    "no-unused-vars": "off",
+    "@typescript-eslint/no-unused-vars": [
+      "warn",
+      {
+        // Allow function args to be unused
+        args: "none",
+        argsIgnorePattern: "^_",
+        varsIgnorePattern: "^_",
+        caughtErrorsIgnorePattern: "^_",
+      },
+    ],
+
+    "jest/no-disabled-tests": "warn",
+    "jest/no-focused-tests": "error",
+    "jest/no-identical-title": "error",
+    "jest/prefer-to-have-length": "off",
+    "jest/valid-expect": "error",
+    "@typescript-eslint/no-non-null-assertion": "off",
+    // Tons of violations in the codebase
+    "@typescript-eslint/naming-convention": "off",
+    // Used liberally in the codebase
+    "@typescript-eslint/no-explicit-any": "off",
+    // This has value in communicating with other Developers even if it has no functional effect.
+    "@typescript-eslint/no-empty-interface": "off",
+  },
+}

.vscode/extensions.json

@@ -2,6 +2,7 @@
   "recommendations": [
     "Orta.vscode-jest",
     "esbenp.prettier-vscode",
+    "dbaeumer.vscode-eslint",
     "christian-kohler.path-intellisense",
     "wayou.vscode-todo-highlight"
   ]

CHANGELOG.md

@@ -15,6 +15,7 @@
 
 <!-- Your comment below this -->
 
+- *Chore:* Switch from tslint to eslint (tslint is end-of-life) - [#1205](https://github.com/danger/danger-js/pull/1205) [@fbartho]
 
 <!-- Your comment above this -->
 

package.json

@@ -34,7 +34,7 @@
       "git add"
     ],
     "*.@(ts|tsx)": [
-      "tslint --fix",
+      "eslint --fix",
       "yarn prettier --write",
       "git add"
     ]
@@ -56,8 +56,8 @@
     "test:fixtures": "node ./scripts/run-fixtures.js",
     "test:update-fixtures": "yarn test:fixtures --update",
     "test:watch": "jest --watch",
-    "lint": "tslint \"source/**/*.ts\"",
-    "lint:fix": "tslint \"source/**/*.ts\" --fix",
+    "lint": "eslint \"source/*.ts\" \"source/**/*.ts\"",
+    "lint:fix": "yarn --silent lint --fix",
     "prepublishOnly": "yarn build && yarn jest && yarn declarations && yarn build:flow-types && yarn build:pretty-types",
     "build": "shx rm -rf ./distribution && tsc -p tsconfig.production.json && madge ./distribution --circular",
     "build:fast": "tsc -p tsconfig.production.json",
@@ -114,9 +114,15 @@
     "@types/prettier": "^1.16.1",
     "@types/readline-sync": "^1.4.3",
     "@types/voca": "^1.4.0",
+    "@typescript-eslint/eslint-plugin": "^5.10.1",
+    "@typescript-eslint/parser": "^5.10.1",
     "danger-plugin-jest": "^1.0.1",
     "danger-plugin-yarn": "^1.3.1",
     "date-fns": "^1.29.0",
+    "eslint": "^8.8.0",
+    "eslint-config-prettier": "^8.3.0",
+    "eslint-plugin-jest": "^26.0.0",
+    "eslint-plugin-jsdoc": "^37.7.0",
     "flow-bin": "^0.77.0",
     "husky": "^1.0.1",
     "jest": "^24.0.0",
@@ -130,8 +136,6 @@
     "shx": "^0.3.2",
     "ts-jest": "^24.0.2",
     "ts-node": "^8.0.2",
-    "tslint": "^5.11.0",
-    "tslint-config-prettier": "^1.15.0",
     "typedoc": "0.9.0",
     "typescript": "^4.5.5",
     "typescript-json-schema": "^0.32.0"

source/api/fetch.ts

@@ -63,12 +63,12 @@ export function api(
   url: string | node_fetch.Request,
   init: node_fetch.RequestInit,
   suppressErrorReporting?: boolean,
-  provessEnv: NodeJS.ProcessEnv = process.env
+  processEnv: NodeJS.ProcessEnv = process.env
 ): Promise<node_fetch.Response> {
   const isTests = typeof jest !== "undefined"
   if (isTests && !url.toString().includes("localhost")) {
     const message = `No API calls in tests please: ${url}`
-    debugger // tslint:disable-line
+    debugger
     throw new Error(message)
   }
 
@@ -79,8 +79,8 @@ export function api(
       output.push(`-X ${init.method}`)
     }
 
-    const showToken = provessEnv["DANGER_VERBOSE_SHOW_TOKEN"]
-    const token = provessEnv["DANGER_GITHUB_API_TOKEN"] || provessEnv["GITHUB_TOKEN"]
+    const showToken = processEnv["DANGER_VERBOSE_SHOW_TOKEN"]
+    const token = processEnv["DANGER_GITHUB_API_TOKEN"] || processEnv["GITHUB_TOKEN"]
 
     if (init.headers) {
       for (const prop in init.headers) {
@@ -109,7 +109,7 @@ export function api(
 
   let agent = init.agent
   const proxy =
-    provessEnv["HTTPS_PROXY"] || provessEnv["https_proxy"] || provessEnv["HTTP_PROXY"] || provessEnv["http_proxy"]
+    processEnv["HTTPS_PROXY"] || processEnv["https_proxy"] || processEnv["HTTP_PROXY"] || processEnv["http_proxy"]
 
   if (!agent && proxy) {
     let secure = url.toString().startsWith("https")

source/ci_source/get_ci_source.ts

@@ -30,7 +30,8 @@ export async function getCISourceForExternal(env: Env, modulePath: string): Prom
         console.error(`could not load CI provider at ${modulePath} due to ${error}`)
       }
       if (stat && stat.isFile()) {
-        const externalModule = require(path) //tslint:disable-line:no-require-imports
+        // eslint-disable-next-line
+        const externalModule = require(path) //  @typescript-eslint/no-var-requires @typescript-eslint/no-require-imports
         const moduleConstructor = externalModule.default || externalModule
         resolve(new moduleConstructor(env))
       }

source/ci_source/providers/_tests/_bamboo.test.ts

@@ -54,6 +54,7 @@ describe(".isPR", () => {
 
     it("needs to have a PR number", () => {
       let env = Object.assign({}, correctEnv)
+      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
       // @ts-ignore
       delete env["bamboo_repository_pr_key"]
       const pipelines = new Bamboo(env)

source/ci_source/providers/_tests/_buddyWorks.test.ts

@@ -52,6 +52,7 @@ describe(".isPR", () => {
 
   it("needs to have a PR number", () => {
     const env = Object.assign({}, correctEnv)
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
     // @ts-ignore
     delete env.BUDDY_EXECUTION_PULL_REQUEST_NO
     const buddyWorks = new BuddyWorks(env)

source/ci_source/providers/_tests/_codebuild.test.ts

@@ -9,7 +9,7 @@ const correctEnv = {
   DANGER_GITHUB_API_TOKEN: "xxx",
 }
 
-const setupCodeBuildSource = async (env: Object) => {
+const setupCodeBuildSource = async (env: Record<string, unknown>) => {
   const source = new CodeBuild(env)
   await source.setup()
 

source/ci_source/providers/_tests/_gitHubActions.test.ts

@@ -51,7 +51,8 @@ describe("pullRequestID", () => {
   it("throws an error when event.json doesn't contain issue or pull_request data", () => {
     const ci = new GitHubActions({}, {})
     expect(() => {
-      // tslint:disable-next-line:no-unused-expression
+      // eslint-disable-next-line no-unused-expressions
+      // eslint-disable-next-line @typescript-eslint/no-unused-expressions
       ci.pullRequestID
     }).toThrow()
   })
@@ -71,7 +72,8 @@ describe("repoSlug", () => {
   it("throws an error when event.json doesn't contain issue or pull_request data", () => {
     const ci = new GitHubActions({}, {})
     expect(() => {
-      // tslint:disable-next-line:no-unused-expression
+      // eslint-disable-next-line no-unused-expressions
+      // eslint-disable-next-line @typescript-eslint/no-unused-expressions
       ci.repoSlug
     }).toThrow()
   })

source/commands/ci/_tests/runner.test.ts

@@ -81,6 +81,7 @@ it("passes the strictm option from args into the executor config", async () => {
 })
 
 // TODO: This occasionally fails!
+// eslint-disable-next-line jest/no-disabled-tests
 it.skip("passes the dangerID from args into the executor config", async () => {
   const customArgs = {
     ...defaultAppArgs,

source/commands/ci/runner.ts

@@ -3,7 +3,7 @@ import { debug } from "../../debug"
 
 import { getPlatformForEnv, Platform } from "../../platforms/platform"
 import { Executor, ExecutorOptions } from "../../runner/Executor"
-import { runDangerSubprocess, addSubprocessCallAguments } from "../utils/runDangerSubprocess"
+import { runDangerSubprocess, addSubprocessCallArguments } from "../utils/runDangerSubprocess"
 import { SharedCLI } from "../utils/sharedDangerfileArgs"
 import getRuntimeCISource from "../utils/getRuntimeCISource"
 
@@ -73,7 +73,7 @@ export const runRunner = async (app: SharedCLI, config?: Partial<RunnerConfig>)
         removePreviousComments: app.removePreviousComments || false,
       }
 
-      const processName = (app.process && addSubprocessCallAguments(app.process.split(" "))) || undefined
+      const processName = (app.process && addSubprocessCallArguments(app.process.split(" "))) || undefined
       const runnerCommand = processName || dangerRunToRunnerCLI(process.argv)
       d(`Preparing to run: ${runnerCommand}`)
 

source/commands/danger-runner.ts

@@ -23,8 +23,6 @@ const d = debug("runner")
 
 // Given the nature of this command, it can be tricky to test, so I use a command like this:
 //
-// tslint:disable-next-line:max-line-length
-//
 // yarn build; cat source/_tests/fixtures/danger-js-pr-395.json | env DANGER_FAKE_CI="YEP" DANGER_TEST_REPO='danger/danger-js' DANGER_TEST_PR='395' node distribution/commands/danger-runner.js --text-only
 //
 // Which will build danger, then run just the dangerfile runner with a fixtured version of the JSON

source/commands/init/default-dangerfile.ts

@@ -39,7 +39,7 @@ export const generateDefaultDangerfile = (state: InitState) => {
 
 export const formatDangerfile = (dangerfile: string, dangerfileState: any) => {
   if (dangerfileState.hasPrettier) {
-    // tslint:disable-next-line:no-require-imports
+    // eslint-disable-next-line @typescript-eslint/no-require-imports
     const { format } = require("prettier")
     // Get package settings
     const localPrettier = fs.existsSync("package.json") && JSON.parse(fs.readFileSync("package.json", "utf8")).prettier

source/commands/init/interfaces.ts

@@ -22,10 +22,10 @@ export interface InitState {
 }
 
 export interface InitUI {
-  header: (msg: String) => void
+  header: (msg: string) => void
   command: (command: string) => void
-  say: (msg: String) => void
-  pause: (secs: number) => Promise<{}>
+  say: (msg: string) => void
+  pause: (secs: number) => Promise<unknown>
   waitForReturn: () => void
   link: (name: string, href: string) => string
   askWithAnswers: (message: string, answers: string[]) => string

source/commands/init/state-setup.ts

@@ -11,14 +11,14 @@ import { getRepoSlug } from "./get-repo-slug"
 import { InitState, InitUI } from "./interfaces"
 
 export const createUI = (state: InitState, app: any): InitUI => {
-  const say = (msg: String) => console.log(msg)
+  const say = (msg: string) => console.log(msg)
   const fancyLink = (name: string, href: string) => hyperLinker(name, href)
   const inlineLink = (_name: string, href: string) => chalk.underline(href)
   const linkToUse = state.supportsHLinks ? fancyLink : inlineLink
 
   return {
     say,
-    header: (msg: String) => say(chalk.bold("\n## " + msg + "\n")),
+    header: (msg: string) => say(chalk.bold("\n## " + msg + "\n")),
     command: (command: string) => say("> " + chalk.white.bold(command) + " \n"),
     link: (name: string, href: string) => linkToUse(name, href),
     pause: async (secs: number) => new Promise(done => setTimeout(done, secs * 1000)),

source/commands/utils/_tests/dangerRunToRunnerCLI.test.ts

@@ -1,5 +1,5 @@
 import dangerRunToRunnerCLI from "../dangerRunToRunnerCLI"
-import { addSubprocessCallAguments } from "../runDangerSubprocess"
+import { addSubprocessCallArguments } from "../runDangerSubprocess"
 
 describe("it can handle the command", () => {
   it("`danger ci`", () => {
@@ -52,7 +52,7 @@ it("`danger pr --dangerfile 'myDanger file.ts'`", () => {
 
 it("Adds correct subprocess arguments", () => {
   expect(
-    addSubprocessCallAguments(
+    addSubprocessCallArguments(
       ["danger-swift"],
       ["danger", "danger-pr", "--process", "danger-swift", "--dangerfile", "File.swift"]
     )
@@ -61,9 +61,11 @@ it("Adds correct subprocess arguments", () => {
 
 describe("npx danger-ts", () => {
   it("switches to the danger in the node_mods", () => {
+    expect.assertions(1)
     // the code uses join which gives different results on windows, and I'm too lazy to add a specific test for that
     const isWindows = process.platform === "win32"
     if (!isWindows) {
+      // eslint-disable-next-line jest/no-conditional-expect
       expect(
         dangerRunToRunnerCLI([
           "/Users/ortatherox/.nvm/versions/node/v14.5.0/bin/node",
@@ -73,6 +75,10 @@ describe("npx danger-ts", () => {
         "/Users/ortatherox/.nvm/versions/node/v14.5.0/bin/node",
         "/Users/ortatherox/.npm/_npx/23085/lib/node_modules/danger-ts/node_modules/danger/distribution/commands/danger-runner.js",
       ])
+    } else {
+      console.warn("Unimplemented test on windows")
+      // eslint-disable-next-line jest/no-conditional-expect
+      expect(true).toBe(true)
     }
   })
 })

source/commands/utils/runDangerSubprocess.ts

@@ -19,6 +19,7 @@ const messageToSendDSL = "danger://send-dsl"
 // Sanitizes the DSL so for sending via STDOUT
 export const prepareDangerDSL = (dangerDSL: DangerDSLJSONType) => {
   if (dangerDSL.github && dangerDSL.github.api) {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
     // @ts-ignore
     delete dangerDSL.github.api
   }
@@ -27,10 +28,13 @@ export const prepareDangerDSL = (dangerDSL: DangerDSLJSONType) => {
   return JSON.stringify(dangerJSONOutput, null, "  ") + "\n"
 }
 
-export const addSubprocessCallAguments = (call: string[], args: string[] = process.argv): string[] => {
+export const addSubprocessCallArguments = (call: string[], args: string[] = process.argv): string[] => {
   return call.concat(["runner"], args.slice(1, args.length))
 }
 
+// @deprecated
+export const addSubprocessCallAguments = addSubprocessCallArguments
+
 // Runs the Danger process
 export const runDangerSubprocess = (
   subprocessName: string[],

source/dsl/_tests/DangerResults.test.ts

@@ -67,7 +67,7 @@ describe("DangerInlineResults into DangerResults", () => {
 })
 
 describe("DangerResults operations", () => {
-  it("merges two results correcly", () => {
+  it("merges two results correctly", () => {
     const results = mergeResults(singleViolationSingleFileResults, multipleViolationSingleFileResults)
 
     expect(results).toMatchSnapshot()
@@ -97,7 +97,7 @@ describe("DangerResults operations", () => {
     expect(result).toEqual(true)
   })
 
-  it("find empty results", () => {
+  it("find empty results - multiple violations in a single file", () => {
     const result = isEmptyResults(multipleViolationSingleFileResults)
 
     expect(result).toEqual(false)

source/platforms/BitBucketCloud.ts

@@ -153,7 +153,7 @@ export class BitBucketCloud implements Platform {
 
     let state: "SUCCESSFUL" | "INPROGRESS" | "FAILED" = passed ? "SUCCESSFUL" : "FAILED"
     if (passed === "pending") {
-      state = "INPROGRESS" as "INPROGRESS"
+      state = "INPROGRESS" as const
     }
 
     let name = "Danger"

source/platforms/_tests/_bitbucket_cloud.test.ts

@@ -3,6 +3,7 @@ import { readFileSync } from "fs"
 const fixtures = resolve(__dirname, "fixtures")
 
 /** Returns JSON from the fixtured dir */
+// eslint-disable-next-line jest/no-export
 export const requestWithFixturedJSON = async (path: string): Promise<() => Promise<any>> => () =>
   Promise.resolve(JSON.parse(readFileSync(`${fixtures}/${path}`, {}).toString()))