Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Switch from tslint to eslint #1205

Merged
merged 4 commits into from Feb 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .eslintignore
@@ -0,0 +1,2 @@
source/danger.d.ts
source/runner/_tests/fixtures/*.ts
66 changes: 66 additions & 0 deletions .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",
},
}
1 change: 1 addition & 0 deletions .vscode/extensions.json
Expand Up @@ -2,6 +2,7 @@
"recommendations": [
"Orta.vscode-jest",
"esbenp.prettier-vscode",
"dbaeumer.vscode-eslint",
"christian-kohler.path-intellisense",
"wayou.vscode-todo-highlight"
]
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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 -->

Expand Down
2 changes: 1 addition & 1 deletion 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:
Expand Down
16 changes: 10 additions & 6 deletions package.json
Expand Up @@ -34,7 +34,7 @@
"git add"
],
"*.@(ts|tsx)": [
"tslint --fix",
"yarn lint:fix",
"yarn prettier --write",
"git add"
]
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion scripts/create-danger-dts.ts
Expand Up @@ -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")
Expand Down
20 changes: 8 additions & 12 deletions 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 = `//
Expand All @@ -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()
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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

Expand Down
10 changes: 5 additions & 5 deletions source/api/fetch.ts
Expand Up @@ -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)
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 2 additions & 1 deletion source/ci_source/get_ci_source.ts
Expand Up @@ -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))
}
Expand Down
1 change: 1 addition & 0 deletions source/ci_source/providers/_tests/_bamboo.test.ts
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions source/ci_source/providers/_tests/_buddyWorks.test.ts
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion source/ci_source/providers/_tests/_codebuild.test.ts
Expand Up @@ -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()

Expand Down
6 changes: 4 additions & 2 deletions source/ci_source/providers/_tests/_gitHubActions.test.ts
Expand Up @@ -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()
})
Expand All @@ -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()
})
Expand Down
1 change: 1 addition & 0 deletions source/commands/ci/_tests/runner.test.ts
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions source/commands/ci/runner.ts
Expand Up @@ -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"

Expand Down Expand Up @@ -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}`)

Expand Down
2 changes: 0 additions & 2 deletions source/commands/danger-runner.ts
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion source/commands/init/default-dangerfile.ts
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions source/commands/init/interfaces.ts
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions source/commands/init/state-setup.ts
Expand Up @@ -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)),
Expand Down