Permalink
Browse files

Fix: Allow irregular whitespace in comments (fixes #5368)

The `no-irregular-whitespace` rule is unduly limiting "irregular"
whitespace to string literals alone (despite a comment at the top
of the file indicating otherwise).  This prevents non-English
speakers from commenting their code in their own language.  For
instance, French language mandates (half-width, but hey) unbreakable
whitespace before two-part punctuation marks (in effect: `;`, `:`,
`?` and `!`), and this rule prevents them from doing so.

This commit fixes that.  Technically, it required choosing a different
parsing path as the existing use cases: apparently the rule
infrastructure only triggers `'Program`' entities after any potential
top-of-file comment block (or perhaps Esprima does this), and because
of the way the `no-irregular-whitespace` rule is originally written,
getting an initial `LineComment` or `BlockComment` node before the
main `Program` node prevents us from then stripping these errors
from the report.  So we accrue comment nodes until `Program:exit`,
then use them for error stripping at that final moment, right before
contributing to the report.

A `skipComments` option is introduced, boolean, defaulting to `true`,
that gives users control over whether comments are, indeed, not
checked for irregular whitespace.  This was suggested by @nzakas
when reviewing #5368.

This also updates the docs and tests (all tests passing, checked).
  • Loading branch information...
tdd committed Feb 22, 2016
1 parent 7cfc1be commit 25a5b2c3e96d43670b9ecc97c370c80dfc0570c1
@@ -16,7 +16,7 @@ Known issues these spaces cause:
This rule is aimed at catching invalid whitespace that is not a normal tab and space. Some of these characters may cause issues in modern browsers and others will be a debugging issue to spot.
With this rule enabled the following characters will cause warnings outside of strings:
With this rule enabled the following characters will cause warnings outside of strings and comments:
\u000B - Line Tabulation (\v) - <VT>
\u000C - Form Feed (\f) - <FF>
@@ -89,6 +89,24 @@ function thing() {
function thing() {
return 'th <NBSP>ing';
}
// Description<NBSP>: some descriptive text
/*
Description<NBSP>: some descriptive text
*/
```
## Options
The `no-irregular-whitespace` rule has no required option and has one optional one that needs to be passed in a single options object:
* **skipComments** *(default: `false`)*: whether to ignore irregular whitespace within comments (`true`) or whether to check for them in there, too (`false`).
For example, to specify that you want to skip checking for irregular whitespace within comments, use the following configuration:
```json
"no-irregular-whitespace": [2, {"skipComments": true}]
```
## When Not To Use It
@@ -1,6 +1,7 @@
/**
* @fileoverview Rule to disalow whitespace that is not a tab or space, whitespace inside strings and comments are allowed
* @author Jonathan Kingston
* @author Christophe Porteneuve
* @copyright 2014 Jonathan Kingston. All rights reserved.
*/
@@ -18,13 +19,20 @@ module.exports = function(context) {
// Module store of errors that we have found
var errors = [];
// Comment nodes. We accumulate these as we go, so we can be sure to trigger them after the whole `Program` entity is parsed, even for top-of-file comments.
var commentNodes = [];
// Lookup the `skipComments` option, which defaults to `false`.
var options = context.options[0] || {};
var skipComments = !!options.skipComments;
/**
* Removes errors that occur inside a string node
* @param {ASTNode} node to check for matching errors.
* @returns {void}
* @private
*/
function removeStringError(node) {
function removeWhitespaceError(node) {
var locStart = node.loc.start;
var locEnd = node.loc.end;
@@ -40,20 +48,32 @@ module.exports = function(context) {
}
/**
* Checks nodes for errors that we are choosing to ignore and calls the relevant methods to remove the errors
* Checks identifier or literal nodes for errors that we are choosing to ignore and calls the relevant methods to remove the errors
* @param {ASTNode} node to check for matching errors.
* @returns {void}
* @private
*/
function removeInvalidNodeErrors(node) {
function removeInvalidNodeErrorsInIdentifierOrLiteral(node) {
if (typeof node.value === "string") {
// If we have irregular characters remove them from the errors list
if (node.raw.match(irregularWhitespace) || node.raw.match(irregularLineTerminators)) {
removeStringError(node);
removeWhitespaceError(node);
}
}
}
/**
* Checks comment nodes for errors that we are choosing to ignore and calls the relevant methods to remove the errors
* @param {ASTNode} node to check for matching errors.
* @returns {void}
* @private
*/
function removeInvalidNodeErrorsInComment(node) {
if (node.value.match(irregularWhitespace) || node.value.match(irregularLineTerminators)) {
removeWhitespaceError(node);
}
}
/**
* Checks the program source for irregular whitespace
* @param {ASTNode} node The program node
@@ -107,6 +127,23 @@ module.exports = function(context) {
}
}
/**
* Stores a comment node (`LineComment` or `BlockComment`) for later stripping of errors within; a necessary deferring of processing to deal with top-of-file comments.
* @param {ASTNode} node The comment node
* @returns {void}
* @private
*/
function rememberCommentNode(node) {
commentNodes.push(node);
}
/**
* A no-op function to act as placeholder for comment accumulation when the `skipComments` option is `false`.
* @returns {void}
* @private
*/
function noop() {}
return {
"Program": function(node) {
/**
@@ -120,10 +157,17 @@ module.exports = function(context) {
checkForIrregularLineTerminators(node);
},
"Identifier": removeInvalidNodeErrors,
"Literal": removeInvalidNodeErrors,
"Identifier": removeInvalidNodeErrorsInIdentifierOrLiteral,
"Literal": removeInvalidNodeErrorsInIdentifierOrLiteral,
"LineComment": skipComments ? rememberCommentNode : noop,
"BlockComment": skipComments ? rememberCommentNode : noop,
"Program:exit": function() {
if (skipComments) {
// First strip errors occurring in comment nodes. We have to do this post-`Program` to deal with top-of-file comments.
commentNodes.forEach(removeInvalidNodeErrorsInComment);
}
// If we have any errors remaining report on them
errors.forEach(function(error) {
context.report.apply(context, error);
@@ -132,4 +176,14 @@ module.exports = function(context) {
};
};
module.exports.schema = [];
module.exports.schema = [
{
"type": "object",
"properties": {
"skipComments": {
"type": "boolean"
}
},
"additionalProperties": false
}
];
Oops, something went wrong.

0 comments on commit 25a5b2c

Please sign in to comment.