From 3d63b03afb0af82abc688856db52ba003d209d60 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 8 Jan 2018 23:16:27 +0530 Subject: [PATCH 1/4] 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. Fixes: #11209 --- lib/.eslintrc.yaml | 1 + tools/eslint-rules/non-ascii-character.js | 57 +++++++++++++++++++++++ 2 files changed, 58 insertions(+) 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/tools/eslint-rules/non-ascii-character.js b/tools/eslint-rules/non-ascii-character.js new file mode 100644 index 00000000000000..36dc29790966a0 --- /dev/null +++ b/tools/eslint-rules/non-ascii-character.js @@ -0,0 +1,57 @@ +/** + * @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 nonAsciiSelector = 'Literal[value=/[^\n\x20-\x7e]/]'; +const nonAsciiRegexPattern = new RegExp(/[^\n\x20-\x7e]/); +const suggestions = { + '’': '\'', + '‛': '\'', + '‘': '\'', + '“': '"', + '‟': '"', + '”': '"', + '«': '"', + '»': '"', + '—': '-' +}; + +module.exports = (context) => { + + const reportError = (node) => { + + const nodeValue = node.value; + const offendingCharacter = nodeValue.match(nonAsciiRegexPattern)[0]; + const suggestion = suggestions[offendingCharacter]; + + let message = `Non-ASCII character '${offendingCharacter}' detected.`; + + message = suggestion ? + `${message} Consider replacing with: ${suggestion}` : + message; + + context.report({ + node, + message, + fix: (fixer) => { + return fixer.replaceText( + node, + suggestion ? `${suggestion}` : '' + ); + } + }); + }; + + return { + [nonAsciiSelector]: (node) => reportError(node, context) + }; +}; From 1399bf5f2befe6d78f674fd49ffa20f844c9530d Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Mon, 8 Jan 2018 23:57:17 +0530 Subject: [PATCH 2/4] tools: fix non-ascii linter detection the linter should detect not just literals, but also source code and all comments too. --- tools/eslint-rules/non-ascii-character.js | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/eslint-rules/non-ascii-character.js b/tools/eslint-rules/non-ascii-character.js index 36dc29790966a0..5e78c7c87700d1 100644 --- a/tools/eslint-rules/non-ascii-character.js +++ b/tools/eslint-rules/non-ascii-character.js @@ -11,8 +11,7 @@ // Rule Definition //------------------------------------------------------------------------------ -const nonAsciiSelector = 'Literal[value=/[^\n\x20-\x7e]/]'; -const nonAsciiRegexPattern = new RegExp(/[^\n\x20-\x7e]/); +const nonAsciiRegexPattern = new RegExp(/[^\r\n\x20-\x7e]/); const suggestions = { '’': '\'', '‛': '\'', @@ -27,10 +26,14 @@ const suggestions = { module.exports = (context) => { - const reportError = (node) => { + const reportIfError = (node, token) => { - const nodeValue = node.value; - const offendingCharacter = nodeValue.match(nonAsciiRegexPattern)[0]; + const { value } = token; + const matches = value.match(nonAsciiRegexPattern); + + if (!matches) return; + + const offendingCharacter = matches[0]; const suggestion = suggestions[offendingCharacter]; let message = `Non-ASCII character '${offendingCharacter}' detected.`; @@ -52,6 +55,13 @@ module.exports = (context) => { }; return { - [nonAsciiSelector]: (node) => reportError(node, context) + Program: (node) => { + const source = context.getSourceCode(); + const sourceTokens = source.getTokens(node); + const commentTokens = source.getAllComments(); + const tokens = sourceTokens.concat(commentTokens); + + tokens.forEach((token) => reportIfError(node, token)); + } }; }; From bd452d5b3783a52cdc0c2fc19a015e3025ee30af Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Thu, 11 Jan 2018 14:56:05 +0530 Subject: [PATCH 3/4] tools: using source.text instead of tokens --- 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 | 16 ++++------------ 7 files changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/console.js b/lib/console.js index b6fb3d88b13df4..e216934a7f7464 100644 --- a/lib/console.js +++ b/lib/console.js @@ -84,7 +84,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); @@ -125,7 +125,7 @@ function write(ignoreErrors, stream, string, errorhandler, groupIndent) { // even in edge cases such as low stack space. if (e.message === MAX_STACK_MESSAGE && e.name === 'RangeError') 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 7c5792c6ce101d..948a9c4b22f283 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1885,7 +1885,7 @@ function processRespondWithFD(self, 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(self, 1); } diff --git a/lib/internal/test/unicode.js b/lib/internal/test/unicode.js index 1445276d9ae891..6432eb9d9decdf 100644 --- a/lib/internal/test/unicode.js +++ b/lib/internal/test/unicode.js @@ -3,4 +3,4 @@ // This module exists entirely for regression testing purposes. // See `test/parallel/test-internal-unicode.js`. -module.exports = '✓'; +module.exports = '✓'; // eslint-disable-line 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 6fc552fac7d444..548ac2450c782a 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -97,6 +97,7 @@ const Timeout = timerInternals.Timeout; // 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 // ║ @@ -118,6 +119,7 @@ const Timeout = timerInternals.Timeout; // ║ // ╚════ > 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 8446f3a03a0e8e..6262ddb6bc6522 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -374,7 +374,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 index 5e78c7c87700d1..b0f050a1397619 100644 --- a/tools/eslint-rules/non-ascii-character.js +++ b/tools/eslint-rules/non-ascii-character.js @@ -11,7 +11,7 @@ // Rule Definition //------------------------------------------------------------------------------ -const nonAsciiRegexPattern = new RegExp(/[^\r\n\x20-\x7e]/); +const nonAsciiRegexPattern = /[^\r\n\x20-\x7e]/; const suggestions = { '’': '\'', '‛': '\'', @@ -26,10 +26,9 @@ const suggestions = { module.exports = (context) => { - const reportIfError = (node, token) => { + const reportIfError = (node, text) => { - const { value } = token; - const matches = value.match(nonAsciiRegexPattern); + const matches = text.match(nonAsciiRegexPattern); if (!matches) return; @@ -55,13 +54,6 @@ module.exports = (context) => { }; return { - Program: (node) => { - const source = context.getSourceCode(); - const sourceTokens = source.getTokens(node); - const commentTokens = source.getAllComments(); - const tokens = sourceTokens.concat(commentTokens); - - tokens.forEach((token) => reportIfError(node, token)); - } + Program: (node) => reportIfError(node, context.getSourceCode().text) }; }; From 1e0ab236de01738c3c2677efbf8bb825ee2744a5 Mon Sep 17 00:00:00 2001 From: Sarat Addepalli Date: Fri, 19 Jan 2018 23:39:36 +0530 Subject: [PATCH 4/4] Adding loc to report --- lib/internal/test/unicode.js | 4 +++- lib/timers.js | 4 ++-- tools/eslint-rules/non-ascii-character.js | 8 +++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/internal/test/unicode.js b/lib/internal/test/unicode.js index 6432eb9d9decdf..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`. -module.exports = '✓'; // eslint-disable-line +/* eslint-disable non-ascii-character */ +module.exports = '✓'; +/* eslint-enable non-ascii-character */ diff --git a/lib/timers.js b/lib/timers.js index 548ac2450c782a..59a5d5272c38fc 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -97,7 +97,7 @@ const Timeout = timerInternals.Timeout; // TimerWrap C++ handle, which makes the call after the duration to process the // list it is attached to. // -// eslint-disable non-ascii-character +/* eslint-disable non-ascii-character */ // // ╔════ > Object Map // ║ @@ -119,7 +119,7 @@ const Timeout = timerInternals.Timeout; // ║ // ╚════ > Linked List // -// eslint-enable non-ascii-character +/* 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/tools/eslint-rules/non-ascii-character.js b/tools/eslint-rules/non-ascii-character.js index b0f050a1397619..e67aac7cd91e82 100644 --- a/tools/eslint-rules/non-ascii-character.js +++ b/tools/eslint-rules/non-ascii-character.js @@ -26,13 +26,14 @@ const suggestions = { module.exports = (context) => { - const reportIfError = (node, text) => { + const reportIfError = (node, sourceCode) => { - const matches = text.match(nonAsciiRegexPattern); + 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.`; @@ -44,6 +45,7 @@ module.exports = (context) => { context.report({ node, message, + loc: sourceCode.getLocFromIndex(offendingCharacterPosition), fix: (fixer) => { return fixer.replaceText( node, @@ -54,6 +56,6 @@ module.exports = (context) => { }; return { - Program: (node) => reportIfError(node, context.getSourceCode().text) + Program: (node) => reportIfError(node, context.getSourceCode()) }; };