From 5f0ca356ac92e4a7c7a9f286811bb957228c5423 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 11 May 2020 11:07:42 -0700 Subject: [PATCH] Add danger for showing dts-critic suggestions (#44430) * First attempt * cwd churn * cwd churn 2 * cwd churn 3 * cwd churn 4 * cwd churn 5 * cwd churn 6 * dump ls * Add a TS dangerfile * Faff with the CI * try to read suggestions * add bogus aframe change * use correct dir * handle no suggestions * delete more of aframe to get dts-critic to notice * now try yauzl * move danger to travis for now * Revert "delete more of aframe to get dts-critic to notice" This reverts commit 65ea09ff423500eb4320658dbd23adab05a6832b. * Revert "add bogus aframe change" This reverts commit a6546bd79f7cae2784031e0409dba731e7f3a1e5. * first try at formatting with markdown * maybe it's a line-oriented format * rebreak aframe * Better formatting Still not best * better wording, maybe better formatting * maybe I need a space to get autonumbering * even better formatting * special-case for >5 properties * Use a summary discolosure * Adds links to unpkg and the dts * Fix the unpkg link * Better unpkg/file linking 1. only show files if more than one was edited. 2. put top-level unpkg on top-level package heading * fix formatting and single-file skip * url needs trailing / * Revert "now try yauzl" This reverts commit 08ab2388194155d008f5ed68620a788ef9b8ae57. * Revert "rebreak aframe" This reverts commit 6ceed76f8abcbf83db55b8afc46d612378e64bd8. Co-authored-by: Orta Therox --- .travis.yml | 3 +++ azure-pipelines.yml | 10 +++++---- dangerfile.ts | 55 +++++++++++++++++++++++++++++++++++++++++++++ package.json | 1 + 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 dangerfile.ts diff --git a/.travis.yml b/.travis.yml index d3e32d854e..b58c290895 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,3 +8,6 @@ notifications: script: - npm run test - if [[ $TRAVIS_EVENT_TYPE == "cron" ]]; then npm run update-codeowners || travis_terminate 1; fi + - TOKEN='7469b4e94ce21b43e3ab7a' + - TOKEN+='79960c12a1e067f2ec' + - DANGER_GITHUB_API_TOKEN=$TOKEN yarn danger ci diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1d4152525f..4a478c5ca6 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -12,13 +12,15 @@ jobs: displayName: 'npm install' inputs: verbose: false - + - script: | if [[ $BUILD_REASON == "Schedule" ]]; then git config --global user.email "types@microsoft.com" && git config --global user.name "TypeScript Bot" && npm run update-codeowners; fi - git checkout -- . && npm run test + git checkout -- . + + npm run test + displayName: 'npm run test' - + trigger: - master - \ No newline at end of file diff --git a/dangerfile.ts b/dangerfile.ts new file mode 100644 index 0000000000..d1b84a0c80 --- /dev/null +++ b/dangerfile.ts @@ -0,0 +1,55 @@ +import fs = require('fs') +import os = require('os') +import { markdown, danger } from "danger" +const suggestionsDir = [os.homedir(), ".dts", "suggestions"].join('/') +const lines: string[] = [] +const missingProperty = /module exports a property named '(.+?)', which is missing/ + +if (fs.existsSync(suggestionsDir)) { + lines.push('Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.') + lines.push('The check for missing properties isn\'t always right, so take this list as advice, not a requirement.') + for (const suggestionFile of fs.readdirSync(suggestionsDir)) { + const path = [suggestionsDir, suggestionFile].join('/') + const suggestions = fs.readFileSync(path, "utf8").split('\n').map(x => JSON.parse(x)) as Array<{ fileName: string, ruleName: string, message: string }> + const packageName = suggestionFile.slice(0, suggestionFile.indexOf('.txt')) + const missingProperties: { [s: string]: string[] } = {} + for (const s of suggestions) { + const fileName = s.fileName.slice(s.fileName.indexOf("types/" + packageName + "/") + ("types/" + packageName + "/").length) + const match = s.message.match(missingProperty) + const identifier = match ? match[1] : s.message + if (fileName in missingProperties) { + missingProperties[fileName].push(identifier) + } + else { + missingProperties[fileName] = [identifier] + } + } + + const topUnpkgURL = `https://unpkg.com/browse/${packageName}@latest/`; + lines.push("## " + packageName + ` ([unpkg](${topUnpkgURL}))`) + for (const fileName in missingProperties) { + if (Object.keys(missingProperties).length > 1) { + const originalJS = fileName.replace(".d.ts", ".js") + const unpkgURL = `https://unpkg.com/browse/${packageName}@latest/${originalJS}` + const dtsName = packageName.replace("@", "").replace("/", "__") + const dtsURL = `https://github.com/DefinitelyTyped/DefinitelyTyped/blob/${danger.github.pr.head.sha}/types/${dtsName}/${fileName}` + + lines.push(`### ${fileName} ([unpkg](${unpkgURL}), [d.ts](${dtsURL}))`); + } + const properties = missingProperties[fileName] + lines.push(`was missing the following properties: +1. ` + properties.slice(0,5).join('\n1. ')) + if (properties.length > 5) { + const extras = properties.slice(5) + lines.push(` +
+as well as these ${extras.length} other properties... +

${extras.join(", ")}

+
+`) + } + } + } + markdown(lines.join('\n')) +} + diff --git a/package.json b/package.json index 864a4495cd..9dd91d8f48 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "prettier": "prettier" }, "devDependencies": { + "danger": "^10.1.1", "dtslint": "latest", "prettier": "^2.0.2", "types-publisher": "github:Microsoft/types-publisher#production"