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: nodejs#18043
Fixes: nodejs#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 MayaLekova committed May 8, 2018
1 parent da1e81d commit ee8bdf3
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 wont 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, theres 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
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ try {
try {
Stream._isUint8Array = process.binding('util').isUint8Array;
} catch (e) {
// This throws for Node < 4.2.0 because theres 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
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ║
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/zlib.js
Original file line number Diff line number Diff line change
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 thats 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
Original file line number Diff line number Diff line change
@@ -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 ee8bdf3

Please sign in to comment.