Pimp the docs linter (and assorted changes) (#2553)

### Pimp the docs linter (and assorted changes) #### `scripts/lint-docs.js` - Add an optional cache for faster runs - Add a final report - Don't return anything from `exec()` - Cover more files #### `scripts/_command.js` - Look for a "--cache" option #### `package.json` scripts - Added `watch:lint-docs` - Added `cleanup:lint` to remove the eslint and lint-docs cache files - Changed `lint:docs` to use the `--cache` option - Added `test:js` so that we can run the test suite without the linter - Changed `test` to defer to `test:js` #### Actual lint fixes: - Bad link in a migration guide - The unicode dashes in the "https://en.wikipedia.org/wiki/Subject–verb–object" are not escaped by marked ### Some more lint-docs pimping #### `scripts/lint-docs.js` - some code reorg and cleanup (take a hint from the local coding conventions) - fix misc bugs - pass a User-Agent header to the requests - even nicer reporting #### `package.json` - bump the @babel/parser dep to the latest #### Docs - tweaks based on lints missed due to previous bugs ### Docs: use the github page for velocity.js, the home page has too many errors. Co-Authored-By: Isiah Meadows <contact@isiahmeadows.com>
2019-12-19 23:40:52 +01:00 · 2019-12-19 23:40:52 +01:00 · 4a3a486d80
commit 4a3a486d80
parent d257025253
11 changed files with 182 additions and 62 deletions
--- a/.gitignore
+++ b/.gitignore
@ -5,6 +5,7 @@
 /.vscode
 /.DS_Store
 /.eslintcache
+/.lint-docs-cache

 # These are artifacts from various scripts
 /dist
--- a/docs/animation.md
+++ b/docs/animation.md
@ -9,7 +9,7 @@

 ### Technology choices

-Animations are often used to make applications come alive. Nowadays, browsers have good support for CSS animations, and there are [various](https://greensock.com/gsap) [libraries](https://velocityjs.org/) that provide fast JavaScript-based animations. There's also an upcoming [Web API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API) and a [polyfill](https://github.com/web-animations/web-animations-js) if you like living on the bleeding edge.
+Animations are often used to make applications come alive. Nowadays, browsers have good support for CSS animations, and there are [various](https://greensock.com/gsap) [libraries](https://github.com/julianshapiro/velocity) that provide fast JavaScript-based animations. There's also an upcoming [Web API](https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API) and a [polyfill](https://github.com/web-animations/web-animations-js) if you like living on the bleeding edge.

 Mithril does not provide any animation APIs per se, since these other options are more than sufficient to achieve rich, complex animations. Mithril does, however, offer hooks to make life easier in some specific cases where it's traditionally difficult to make animations work.

--- a/docs/installation.md
+++ b/docs/installation.md
@ -51,9 +51,9 @@ $ npm install webpack webpack-cli --save-dev
 ```

 3. Add a "start" entry to the scripts section in `package.json`.
-```javascript
+```json
 {
-	// ...
+	"...": "...",
 	"scripts": {
 		"start": "webpack src/index.js --output bin/app.js -d --watch"
 	}
--- a/docs/keys.md
+++ b/docs/keys.md
@ -503,7 +503,7 @@ If you absolutely must and you have no control over this, use a prefix denoting

 ```javascript
 things.map(function(thing) {
-	return m(".thing"
+	return m(".thing",
 		{key: (typeof thing.id) + ":" + thing.id},
 		// ...
 	)
--- a/docs/migration-v02x.md
+++ b/docs/migration-v02x.md
@ -834,7 +834,7 @@ The first of the two headers, `Content-Type`, will trigger a CORS prefetch as it

 ## `m.deferred` removed

-v0.2.x used its own custom asynchronous contract object, exposed as `m.deferred`, which was used as the basis for `m.request`. v2.x uses Promises instead, and implements a [polyfill](promises.md) in non-supporting environments. In situations where you would have used `m.deferred`, you should use Promises instead.
+v0.2.x used its own custom asynchronous contract object, exposed as `m.deferred`, which was used as the basis for `m.request`. v2.x uses Promises instead, and implements a [polyfill](promise.md) in non-supporting environments. In situations where you would have used `m.deferred`, you should use Promises instead.

 ### v0.2.x

--- a/docs/migration-v1x.md
+++ b/docs/migration-v1x.md
@ -32,7 +32,7 @@ var Counter = {
    oninit: function(vnode) { vnode.state = 0 },
    view: function(vnode) {
        return m(".counter", [
-            m("button", {onclick: function() { vnode.state-- }}, "-")
+            m("button", {onclick: function() { vnode.state-- }}, "-"),
            vnode.state,
            m("button", {onclick: function() { vnode.state++ }}, "+")
        ])
@ -47,7 +47,7 @@ var Counter = {
    oninit: function(vnode) { vnode.state.count = 0 },
    view: function(vnode) {
        return m(".counter", [
-            m("button", {onclick: function() { vnode.state.count-- }}, "-")
+            m("button", {onclick: function() { vnode.state.count-- }}, "-"),
            vnode.state.count,
            m("button", {onclick: function() { vnode.state.count++ }}, "+")
        ])
--- a/ospec/README.md
+++ b/ospec/README.md
@ -10,7 +10,7 @@ Noiseless testing framework
 - ~360 LOC including the CLI runner
 - terser and faster test code than with mocha, jasmine or tape
 - test code reads like bullet points
- assertion code follows [SVO](https://en.wikipedia.org/wiki/Subject–verb–object) structure in present tense for terseness and readability
+- assertion code follows [SVO](https://en.wikipedia.org/wiki/Subject%E2%80%93verb%E2%80%93object) structure in present tense for terseness and readability
 - supports:
 	- test grouping
 	- assertions
@ -98,10 +98,10 @@ o.spec("math", function() {
 The `o.spy()` method can be used to create a stub function that keeps track of its call count and received parameters

 ```javascript
-//code to be tested
+// code to be tested
 function call(cb, arg) {cb(arg)}

-//test suite
+// test suite
 var o = require("ospec")

 o.spec("call()", function() {
@ -119,13 +119,13 @@ o.spec("call()", function() {
 A spy can also wrap other functions, like a decorator:

 ```javascript
-//code to be tested
+// code to be tested
 var count = 0
 function inc() {
 	count++
 }

-//test suite
+// test suite
 var o = require("ospec")

 o.spec("call()", function() {
@ -194,7 +194,7 @@ This can also be changed on a per-test basis using the `o.timeout(delay)` functi

 ```javascript
 o("setTimeout calls callback", function(done, timeout) {
-	o.timeout(500) //wait 500ms before bailing out of the test
+	o.timeout(500) // wait 500ms before bailing out of the test

 	setTimeout(done, 300)
 })
@ -258,7 +258,7 @@ o.spec("math", function() {
 		})
 	})

-	//tests only run after async hooks complete
+	// tests only run after async hooks complete
 	o("addition", function() {
 		acc += 1

@ -303,12 +303,12 @@ o.spec("math", function() {
 ### Running the test suite

 ```javascript
-//define a test
+// define a test
 o("addition", function() {
 	o(1 + 1).equals(2)
 })

-//run the suite
+// run the suite
 o.run()
 ```

@ -433,7 +433,7 @@ Starts an assertion. There are six types of assertion: `equals`, `notEquals`, `d

 Assertions have this form:

-```
+```javascript
 o(actualValue).equals(expectedValue)
 ```

@ -441,13 +441,13 @@ As a matter of convention, the actual value should be the first argument and the

 Assertions can also accept an optional description curried parameter:

-```
+```javascript
 o(actualValue).equals(expectedValue)("this is a description for this assertion")
 ```

 Assertion descriptions can be simplified using ES6 tagged template string syntax:

-```
+```javascript
 o(actualValue).equals(expectedValue) `this is a description for this assertion`
 ```

--- a/package-lock.json
+++ b/package-lock.json
@ -31,9 +31,9 @@
      }
    },
    "@babel/parser": {
-      "version": "7.6.2",
-      "resolved": "https://registry.npmjs.org/@babel/parser/-/parser-7.6.2.tgz",
-      "integrity": "sha512-mdFqWrSPCmikBoaBYMuBulzTIKuXVPtEISFbRRVNwMWpCms/hmE2kRq0bblUHaNRKrjRlmVbx1sDHmjmRgD2Xg==",
+      "version": "7.7.5",
+      "resolved": "https://registry.npmjs.org/@babel/parser/-/parser-7.7.5.tgz",
+      "integrity": "sha512-KNlOe9+/nk4i29g0VXgl8PEXIRms5xKLJeuZ6UptN0fHv+jDiriG+y94X6qAgWTR0h3KaoM1wK5G5h7MHFRSig==",
      "dev": true
    },
    "@nodelib/fs.scandir": {
--- a/package.json
+++ b/package.json
@ -11,25 +11,27 @@
    "watch": "run-p watch:**",
    "watch:js": "node scripts/bundler browser.js -output mithril.js -watch",
    "watch:docs": "node scripts/generate-docs --watch",
+    "watch:docs-lint": "node scripts/lint-docs --watch",
    "build": "run-p build:browser build:min build:stream-min",
    "build:browser": "node scripts/bundler browser.js -output mithril.js",
    "build:docs": "node scripts/generate-docs",
    "build:min": "node scripts/bundler browser.js -output mithril.min.js -minify -save",
    "build:stream-min": "node scripts/minify-stream",
+    "cleanup:lint": "rimraf .eslintcache .lint-docs-cache",
    "lint": "run-s -cn lint:**",
    "lint:js": "eslint . --cache",
-    "lint:docs": "node scripts/lint-docs",
+    "lint:docs": "node scripts/lint-docs --cache",
    "perf": "node performance/test-perf.js",
    "pretest": "npm run lint",
-    "test": "node ospec/bin/ospec",
-    "posttest": "npm run lint",
+    "test": "run-s test:js",
+    "test:js": "node ospec/bin/ospec",
    "cover": "istanbul cover --print both ospec/bin/ospec",
    "release": "npm version -m 'v%s'",
    "version": "npm run build && git add mithril.js mithril.min.js stream.js stream.min.js README.md"
  },
  "devDependencies": {
    "@alrra/travis-scripts": "^3.0.1",
-    "@babel/parser": "^7.6.2",
+    "@babel/parser": "^7.7.5",
    "benchmark": "^2.1.4",
    "chokidar": "^3.2.1",
    "escape-string-regexp": "^2.0.0",
--- a/scripts/_command.js
+++ b/scripts/_command.js
@ -18,6 +18,7 @@ process.on("unhandledRejection", function (e) {

 module.exports = ({exec, watch}) => {
 	const index = process.argv.indexOf("--watch")
+	const useCache = process.argv.indexOf("--cache") >= 0
 	if (index >= 0) {
 		process.argv.splice(index, 1)

@ -29,7 +30,7 @@ module.exports = ({exec, watch}) => {

 		watch()
 	} else {
-		Promise.resolve(exec()).then((code) => {
+		Promise.resolve(exec({useCache})).then((code) => {
 			if (code != null) process.exitCode = code
 		})
 	}
--- a/scripts/lint-docs.js
+++ b/scripts/lint-docs.js
@ -14,24 +14,22 @@ const request = require("request-promise-native")
 class LintRenderer extends marked.Renderer {
 	constructor(file) {
 		super()
-		this._file = file
 		this._dir = path.dirname(file)
 		this._context = undefined
 		this._code = undefined
 		this._lang = undefined
 		this._error = undefined
 		this._awaiting = []
+		this._warnings = []
+		this._errors = []
 	}

-	_emitTolerate(...data) {
-		let str = data.join("\n")
-		if (str.endsWith("\n")) str = str.slice(0, -1)
-		console.log(`${this._file} - ${str}\n${"-".repeat(60)}`)
+	_addWarning(...data) {
+		this._warnings.push(formatMessage(...data))
 	}

-	_emit(...data) {
-		this._emitTolerate(...data)
-		process.exitCode = 1
+	_addError(...data) {
+		this._errors.push(formatMessage(...data))
 	}

 	_block() {
@ -42,31 +40,48 @@ class LintRenderer extends marked.Renderer {
 		// Don't fail if something byzantine shows up - it's the freaking
 		// internet. Just log it and move on.
 		const httpError = (e) =>
-			this._emitTolerate(`http error for ${href}`, e.message)
+			this._addWarning(`http error for ${href}`, e.message)

 		// Prefer https: > http: where possible, but allow http: when https: is
 		// inaccessible.
 		if ((/^https?:\/\//).test(href)) {
 			const url = href.replace(/#.*$/, "")
-			this._awaiting.push(request.head(url).then(() => {
-				const isHTTPS = href.startsWith("https:")
+			const isHTTPS = href.startsWith("https:")
+			// pass along realistic headers, some sites (i.e. the IETF) return a 403 otherwise.
+			const headers = {
+				"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:71.0) Gecko/20100101 Firefox/71.0",
+			}
+			// some more headers if more were ever needed (from my local Firefox)
+
+			// "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",
+			// "Accept-Language": "en-US,en;q=0.5",
+			// "Accept-Encoding": "gzip, deflate, br",
+			// "DNT": "1",
+			// "Connection": "keep-alive",
+			// "Upgrade-Insecure-Requests": "1",
+			// "Pragma": "no-cache",
+			// "Cache-Control": "no-cache"
+
+			this._awaiting.push(request.head(url, {headers}).then(() => {
 				if (!isHTTPS) {
-					return request.head(`https:${url.slice(7)}`).then(
-						() => this._emit("change http: to https:"),
+					return request.head(`https:${url.slice(7)}`, {headers}).then(
+						() => this._addError("change http: to https:"),
 						() => { /* ignore inner errors */ }
 					)
 				}
 			}, (e) => {
 				if (e.statusCode === 404) {
-					this._emit(`broken external link: ${href}`)
+					this._addError(`broken external link: ${href}`)
 				}
 				else {
 					if (
-						e.error.code === "ERR_TLS_CERT_ALTNAME_INVALID" &&
-						href.startsWith("https://")
+						isHTTPS && (
+							e.error.code === "ERR_TLS_CERT_ALTNAME_INVALID" ||
+							(/ssl/i).test(e.message)
+						)
 					) {
-						return request.head(`http:${url.slice(6)}`).then(
-							() => this._emit(`change ${href} to use http:`),
+						return request.head(`http:${url.slice(6)}`, {headers}).then(
+							() => this._addError(`change ${href} to use http:`),
 							// ignore inner errors
 							() => httpError(e)
 						)
@ -80,7 +95,7 @@ class LintRenderer extends marked.Renderer {
 			if (exec != null) {
 				const resolved = path.resolve(this._dir, exec[1])
 				this._awaiting.push(fs.access(resolved).catch(() => {
-					this._emit(`broken internal link: ${href}`)
+					this._addError(`broken internal link: ${href}`)
 				}))
 			}
 		}
@ -89,6 +104,8 @@ class LintRenderer extends marked.Renderer {
 	code(code, lang) {
 		this._code = code
 		this._lang = lang
+		this._error = null
+
 		if (!lang || lang === "js" || lang === "javascript") {
 			try {
 				// Could be within any production.
@ -115,7 +132,7 @@ class LintRenderer extends marked.Renderer {
 		if (!this._lang) {
 			// TODO: ensure all code blocks have tags, and check this in CI.
 			if (this._error == null) {
-				this._emit(
+				this._addError(
 					"Code block possibly missing `javascript` language tag",
 					this._block(),
 				)
@ -123,7 +140,7 @@ class LintRenderer extends marked.Renderer {

 			try {
 				JSON.parse(this._code)
-				this._emit(
+				this._addError(
 					"Code block possibly missing `json` language tag",
 					this._block(),
 				)
@ -135,9 +152,9 @@ class LintRenderer extends marked.Renderer {
 	}

 	_ensureCodeIsSyntaticallyValid() {
-		if (!this.lang || !(/^js$|^javascript$/).test(this._lang)) return
+		if (!this._lang || !(/^js$|^javascript$/).test(this._lang)) return
 		if (this._error != null) {
-			this._emit(
+			this._addError(
 				"JS code block has invalid syntax", this._error.message,
 				this._block()
 			)
@ -145,28 +162,124 @@ class LintRenderer extends marked.Renderer {
 	}

 	_ensureCommentStyle() {
-		if (!this.lang || !(/^js$|^javascript$/).test(this._lang)) return
+		if (!this._lang || !(/^js$|^javascript$/).test(this._lang)) return
 		if ((/(^|\s)\/\/[\S]/).test(this._code)) {
-			this._emit("Comment is missing a preceding space", this._block())
+			this._addError("Comment is missing a preceding space", this._block())
 		}
 	}
 }

+async function getFileInfo(file) {
+	const {size, mtime} = await fs.stat(file)
+	const timestamp = Number(mtime)
+	return {size, timestamp}
+}
+
+function report(file, data, totals, nextCache) {
+	const {_warnings, _errors} = data;
+	if (_warnings.length + _errors.length > 0) {
+		console.log("- ".repeat(file.length/2 + 1))
+		console.log(file)
+		console.log("- ".repeat(file.length/2 + 1) + "\n")
+		if (_errors.length > 0) {
+			process.exitCode = 1
+			const s = _errors.length > 1 ? "s " : " -"
+			console.log(`-- ${_errors.length} Error${s}----------`)
+			_errors.forEach((msg) => console.log(`\n${msg}`))
+			console.log("\n")
+		}
+		if (_warnings.length > 0) {
+			const s = _warnings.length > 1 ? "s " : " -"
+			console.log(`-- ${_warnings.length} Warning${s}--------`)
+			_warnings.forEach((msg) => console.log(`\n${msg}`))
+			console.log("\n")
+		}
+		if (totals != null) {
+			totals.errors += _errors.length
+			totals.warnings += _warnings.length
+		}
+	}
+	if (nextCache != null) nextCache[file] = data
+}
+
+function formatMessage(...data) {
+	let str = data.join("\n")
+	if (str.endsWith("\n")) str = str.slice(0, -1)
+	return str
+}
+
 exports.lintOne = lintOne
-async function lintOne(file) {
+// `cache` and `nextCache` are only passed from `lintAll()`, not when watching
+async function lintOne(file, totals, cache, nextCache) {
 	const contents = await fs.readFile(file, "utf-8")
+	// check for nextCache, because cache will be undefined the first time the linter runs
+	const {size, timestamp} = (nextCache != null) ? await getFileInfo(file) : {}
+	if (cache != null && cache[file] != null) {
+		const cached = cache[file]
+		if (
+			size === cached.size &&
+			timestamp === cached.timestamp &&
+			cached._errors.length + cached._warnings.length === 0
+		) {
+			report(file, cached, totals, nextCache)
+			return
+		}
+	}
 	const renderer = new LintRenderer(file)
 	marked(contents, {renderer})
-	return Promise.all(renderer._awaiting)
+	return Promise.all(renderer._awaiting).then(() => {
+		const {_warnings, _errors} = renderer
+		report(file, {_warnings, _errors, size, timestamp}, totals, nextCache)
+	})
+}
+
+
+const cachePath = path.join(process.cwd(), ".lint-docs-cache")
+
+async function loadCache() {
+	try {
+		const source = await fs.readFile(cachePath, "utf-8")
+		try {
+			return JSON.parse(source)
+		} catch (e) {
+			console.error(e)
+			return
+		}
+	} catch (e) {
+		return
+	}
+}
+
+function saveCache(nextCache) {
+	return fs.writeFile(cachePath, JSON.stringify(nextCache), "utf-8")
+}
+
+function finalReport(totals) {
+	const buffer = []
+	if (totals.errors > 0) {
+		buffer.push(`${totals.errors} error${totals.errors > 1 ? "s" : ""}`)
+	}
+	if (totals.warnings > 0) {
+		buffer.push(`${totals.warnings} warning${totals.warnings > 1 ? "s" : ""}`)
+	}
+	if (buffer.length > 0) console.log(`\n${buffer.join(", ")} found in the docs\n`)
+	else console.log("The docs are in good shape!\n")
 }

 exports.lintAll = lintAll
-function lintAll() {
-	return new Promise((resolve, reject) => {
-		const glob = new Glob(path.resolve(__dirname, "../docs/**/*.md"), {
+async function lintAll({useCache}) {
+	const cache = useCache ? await loadCache() : null
+	const totals = {
+		errors: 0,
+		warnings: 0,
+	}
+	// always populate the cache, even if we don't read from it
+	const nextCache = {}
+	await new Promise((resolve, reject) => {
+		const glob = new Glob(path.resolve(__dirname, "../**/*.md"), {
 			ignore: [
 				"**/change-log.md",
-				"**/migration-*.md",
+				"**/migration-v02x.md",
 				"**/node_modules/**",
 			],
 			nodir: true,
@ -174,12 +287,15 @@ function lintAll() {
 		const awaiting = []

 		glob.on("match", (file) => {
-			awaiting.push(lintOne(file))
+			awaiting.push(lintOne(file, totals, cache, nextCache))
 		})

 		glob.on("error", reject)
 		glob.on("end", () => resolve(Promise.all(awaiting)))
 	})
+	finalReport(totals)
+	await saveCache(nextCache)
+	// don't return anything so that _command.js picks up the errorCode.
 }

 /* eslint-disable global-require */
@ -188,10 +304,10 @@ if (require.main === module) {
 		exec: lintAll,
 		watch() {
 			require("chokidar")
-				.watch(path.resolve(__dirname, "../docs/**/*.md"), {
-					ignore: [
+				.watch(path.resolve(__dirname, "../**/*.md"), {
+					ignored: [
 						"**/change-log.md",
-						"**/migration-*.md",
+						"**/migration-v02x.md",
 						"**/node_modules/**",
 					],
 				})