GeezerVison

Minor nit: maybe slashes would look a tad better for the corners instead of a pipe?

I think it would be okay to make an exception for this file and just add an eslint-disable line for the rule.

(It would be good to have this file as a test that we can use UTF-8 in sources, even if we prefer not to.)

@addaleax I don't really think it is a way to test this. It wasn't possible to use UTF-8 in sources since #5458 and before #11129 yet all worked well since it doesn't actually matter whether comments are encoded correctly while these are just comments.

Unrelated whitespace change :-)

Either the comment is misleading, or it works not the way you planned it to. This will report the first non-ASCII character per token, not per line.

This line makes the values in the tokens (couldn't think of a better name) array look like:

{ type: 'Punctuator',
  value: ';',
  start: 22327,
  end: 22328,
  loc:
   SourceLocation {
     start: Position { line: 767, column: 1 },
     end: Position { line: 767, column: 2 } },
  range: [ 22327, 22328 ] }
{ type: 'Line',
  value: ' Copyright Joyent, Inc. and other Node contributors.',
  start: 0,
  end: 54,
  range: [ 0, 54 ],
  loc:
   { start: Position { line: 1, column: 0 },
     end: Position { line: 1, column: 54 } } }

In the case of type Line we only report the the first occurrence of a non-ASCII character. In my latest iteration I took the comment out since it didn't really help clarify.

Why not do this in one pass without the extra errors array? You can make this a named function and call it instead of errors.push().

IMO, it would be better to flip the condition and return early (if (!matches) return;) reducing the indentation level for the rest of the function.

This will fail to match non-ascii whitespace that could appear between tokens, so it's not quite disallowing all non-ascii characters in files. This could be fixed by matching the regex against source.text rather than against each token.

That said, I think the no-irregular-whitespace rule will cover non-ascii whitespace.

If the latter is true, it would be good to have a comment explaining that.

The comment alone would not be sufficient, it is important that the linter ensures there are no irregular Unicode whitespace characters since they cannot be seen during code review.

@aqrln by:

If the latter is true

I mean that if:

I think the no-irregular-whitespace rule will cover non-ascii whitespace.

@not-an-aardvark's theory is correct, and the non-ASCII whitespace characters are already covered in a separate rule, then we could just use that for whitespace, and add a comment in here to explain that we don't need to worry about whitespace as it's covered in another rule.

@gibfahn ah, I see, sorry. I didn't pay enough attention to that "if the latter" part so I didn't understand you right.

It might be clearer to use a regex literal here instead of the RegExp constructor. Right now, \x00 and \x7F are interpreted as part of the string, so the resulting regex pattern actually contains a null character. This still works fine, but it could be confusing for debugging (e.g. if the regex is printed, it will be difficult to tell that it contains a null character).

This could result in an invalid report location if the offending character is in a block comment. For example:

/* foo
■ */

The rule reports an error for this comment at line 1, column 7, but that location doesn't actually exist.

I'd prefer see eslint-disable-line or eslint-disable-next-line to target the places where the control characters are needed.