From 016a28ac08a60498dd1f36dfdc550df8e6f8118c Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 8 Jan 2018 23:16:27 +0530 Subject: [PATCH] tools: non-Ascii linter for /lib only Non-ASCII characters in /lib get compiled into the node binary, and may bloat the binary size unnecessarily. A linter rule may help prevent this. PR-URL: https://github.com/nodejs/node/pull/18043 Backport-PR-URL: https://github.com/nodejs/node/pull/19499 Fixes: https://github.com/nodejs/node/issues/11209 Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater Reviewed-By: Teddy Katz --- lib/.eslintrc.yaml | 1 + lib/console.js | 4 +- lib/internal/http2/core.js | 2 +- lib/internal/test/unicode.js | 2 + lib/stream.js | 2 +- lib/timers.js | 2 + lib/zlib.js | 2 +- tools/eslint-rules/non-ascii-character.js | 61 +++++++++++++++++++++++ 8 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 tools/eslint-rules/non-ascii-character.js diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 437aa575645ad6..0b00638e2a638c 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -6,3 +6,4 @@ rules: buffer-constructor: error no-let-in-for-declaration: error lowercase-name-for-primitive: error + non-ascii-character: error diff --git a/lib/console.js b/lib/console.js index d0f7e61fd5a709..4495074231a2eb 100644 --- a/lib/console.js +++ b/lib/console.js @@ -81,7 +81,7 @@ function createWriteErrorHandler(stream) { // If there was an error, it will be emitted on `stream` as // an `error` event. Adding a `once` listener will keep that error // from becoming an uncaught exception, but since the handler is - // removed after the event, non-console.* writes won’t be affected. + // removed after the event, non-console.* writes won't be affected. // we are only adding noop if there is no one else listening for 'error' if (stream.listenerCount('error') === 0) { stream.on('error', noop); @@ -114,7 +114,7 @@ function write(ignoreErrors, stream, string, errorhandler, groupIndent) { // even in edge cases such as low stack space. if (e.message === 'Maximum call stack size exceeded') throw e; - // Sorry, there’s no proper way to pass along the error here. + // Sorry, there's no proper way to pass along the error here. } finally { stream.removeListener('error', noop); } diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index d83897c20b7902..cce54ef377fe0c 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1537,7 +1537,7 @@ function processRespondWithFD(fd, headers, offset = 0, length = -1, return; } // exact length of the file doesn't matter here, since the - // stream is closing anyway — just use 1 to signify that + // stream is closing anyway - just use 1 to signify that // a write does exist trackWriteState(this, 1); } diff --git a/lib/internal/test/unicode.js b/lib/internal/test/unicode.js index 1445276d9ae891..7172a43ec20a8a 100644 --- a/lib/internal/test/unicode.js +++ b/lib/internal/test/unicode.js @@ -3,4 +3,6 @@ // This module exists entirely for regression testing purposes. // See `test/parallel/test-internal-unicode.js`. +/* eslint-disable non-ascii-character */ module.exports = '✓'; +/* eslint-enable non-ascii-character */ diff --git a/lib/stream.js b/lib/stream.js index edc5f231b83411..9a816600a05e5a 100644 --- a/lib/stream.js +++ b/lib/stream.js @@ -45,7 +45,7 @@ try { try { Stream._isUint8Array = process.binding('util').isUint8Array; } catch (e) { - // This throws for Node < 4.2.0 because there’s no util binding and + // This throws for Node < 4.2.0 because there's no util binding and // returns undefined for Node < 7.4.0. } } diff --git a/lib/timers.js b/lib/timers.js index 0e6ae45950c5c1..f3c3c6308433eb 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -89,6 +89,7 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // +/* eslint-disable non-ascii-character */ // // ╔════ > Object Map // ║ @@ -110,6 +111,7 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1 // ║ // ╚════ > Linked List // +/* eslint-enable non-ascii-character */ // // With this, virtually constant-time insertion (append), removal, and timeout // is possible in the JavaScript layer. Any one list of timers is able to be diff --git a/lib/zlib.js b/lib/zlib.js index 7f41200f86be19..bbe89043248459 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -339,7 +339,7 @@ Zlib.prototype.flush = function flush(kind, callback) { this._scheduledFlushFlag = maxFlush(kind, this._scheduledFlushFlag); // If a callback was passed, always register a new `drain` + flush handler, - // mostly because that’s simpler and flush callbacks piling up is a rare + // mostly because that's simpler and flush callbacks piling up is a rare // thing anyway. if (!alreadyHadFlushScheduled || callback) { const drainHandler = () => this.flush(this._scheduledFlushFlag, callback); diff --git a/tools/eslint-rules/non-ascii-character.js b/tools/eslint-rules/non-ascii-character.js new file mode 100644 index 00000000000000..e67aac7cd91e82 --- /dev/null +++ b/tools/eslint-rules/non-ascii-character.js @@ -0,0 +1,61 @@ +/** + * @fileOverview Any non-ASCII characters in lib/ will increase the size + * of the compiled node binary. This linter rule ensures that + * any such character is reported. + * @author Sarat Addepalli + */ + +'use strict'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const nonAsciiRegexPattern = /[^\r\n\x20-\x7e]/; +const suggestions = { + '’': '\'', + '‛': '\'', + '‘': '\'', + '“': '"', + '‟': '"', + '”': '"', + '«': '"', + '»': '"', + '—': '-' +}; + +module.exports = (context) => { + + const reportIfError = (node, sourceCode) => { + + const matches = sourceCode.text.match(nonAsciiRegexPattern); + + if (!matches) return; + + const offendingCharacter = matches[0]; + const offendingCharacterPosition = matches.index; + const suggestion = suggestions[offendingCharacter]; + + let message = `Non-ASCII character '${offendingCharacter}' detected.`; + + message = suggestion ? + `${message} Consider replacing with: ${suggestion}` : + message; + + context.report({ + node, + message, + loc: sourceCode.getLocFromIndex(offendingCharacterPosition), + fix: (fixer) => { + return fixer.replaceText( + node, + suggestion ? `${suggestion}` : '' + ); + } + }); + }; + + return { + Program: (node) => reportIfError(node, context.getSourceCode()) + }; +};