Skip to content

Commit

Permalink
tools: non-Ascii linter for /lib only
Browse files Browse the repository at this point in the history
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: #18043
Fixes: #11209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
  • Loading branch information
SirR4T authored and MylesBorins committed Feb 21, 2018
1 parent be5c293 commit e00bb16
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 5 deletions.
1 change: 1 addition & 0 deletions lib/.eslintrc.yaml
Expand Up @@ -6,3 +6,4 @@ rules:
buffer-constructor: error
no-let-in-for-declaration: error
lowercase-name-for-primitive: error
non-ascii-character: error
4 changes: 2 additions & 2 deletions lib/console.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/http2/core.js
Expand Up @@ -1876,7 +1876,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);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/internal/test/unicode.js
Expand Up @@ -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 */
2 changes: 1 addition & 1 deletion lib/stream.js
Expand Up @@ -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.
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/timers.js
Expand Up @@ -90,6 +90,7 @@ const TIMEOUT_MAX = 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
// β•‘
Expand All @@ -111,6 +112,7 @@ const TIMEOUT_MAX = 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
Expand Down
2 changes: 1 addition & 1 deletion lib/zlib.js
Expand Up @@ -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);
Expand Down
61 changes: 61 additions & 0 deletions 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 <sarat.addepalli@gmail.com>
*/

'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())
};
};

0 comments on commit e00bb16

Please sign in to comment.