Merge pull request #1205 from danger/fb/eslint

chore: Switch from tslint to eslint

.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 -->
 

appveyor.yml

@@ -1,6 +1,6 @@
 # Test against this version of Node.js
 environment:
-  nodejs_version: "10"
+  nodejs_version: "12"
 
 # Install scripts. (runs after repo cloning)
 install:

package.json

@@ -34,7 +34,7 @@
       "git add"
     ],
     "*.@(ts|tsx)": [
-      "tslint --fix",
+      "yarn lint: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",
@@ -125,13 +131,11 @@
     "madge": "^3.2.0",
     "nock": "^10.0.6",
     "pkg": "^4.4.2",
-    "prettier": "^1.14.2",
+    "prettier": "^2.5.1",
     "release-it": "^13.5.2",
     "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"

scripts/create-danger-dts.ts

@@ -24,7 +24,7 @@ if (allDiagnostics.length) {
   console.log(
     "\nIf you've added something new to the DSL, and the errors are about something missing, you may need to add an interface in `source/dsl/*`."
   )
-  allDiagnostics.forEach(diagnostic => {
+  allDiagnostics.forEach((diagnostic) => {
     if (diagnostic.file) {
       let { line, character } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start!)
       let message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n")

scripts/danger-dts.ts

@@ -1,11 +1,7 @@
 import * as fs from "fs"
 import * as prettier from "prettier"
 
-const mapLines = (s: string, func: (s: string) => string) =>
-  s
-    .split("\n")
-    .map(func)
-    .join("\n")
+const mapLines = (s: string, func: (s: string) => string) => s.split("\n").map(func).join("\n")
 
 const createDTS = () => {
   const header = `//
@@ -23,10 +19,10 @@ import { File } from "parse-diff"
 
   const dslFiles = fs
     .readdirSync("source/dsl")
-    .filter(f => !f.startsWith("_tests"))
-    .map(f => `source/dsl/${f}`)
+    .filter((f) => !f.startsWith("_tests"))
+    .map((f) => `source/dsl/${f}`)
 
-  dslFiles.forEach(file => {
+  dslFiles.forEach((file) => {
     // Sometimes they have more stuff, in those cases
     // offer a way to crop the file.
     const content = fs.readFileSync(file).toString()
@@ -72,7 +68,7 @@ import { File } from "parse-diff"
   const chainDefs = fs.readFileSync("distribution/commands/utils/chainsmoker.d.ts", "utf8")
   const chainDefsMinusDefaultExport = chainDefs
     .split("\n")
-    .filter(line => {
+    .filter((line) => {
       return !line.startsWith("export default function")
     })
     .join("\n")
@@ -82,10 +78,10 @@ import { File } from "parse-diff"
   // Remove all JS-y bits
   fileOutput = fileOutput
     .split("\n")
-    .filter(line => {
+    .filter((line) => {
       return !line.startsWith("import") && !line.includes("* @type ")
     })
-    .filter(line => {
+    .filter((line) => {
       return !line.includes("Please don't have")
     })
     .join("\n")
@@ -94,7 +90,7 @@ import { File } from "parse-diff"
   const noRedundantExports = trimmedWhitespaceLines
     .replace(/export interface/g, "interface")
     .replace(/export type/g, "type")
-  const indentedBody = mapLines(noRedundantExports, line => (line.length ? line : ""))
+  const indentedBody = mapLines(noRedundantExports, (line) => (line.length ? line : ""))
 
   const def = header + indentedBody + footer
 

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)),