Node: Linter rule: use ASCII quotes not Unicode ones

[gibfahn]

• Subsystem: eslint

See: https://github.com/nodejs/node/pull/11129#issuecomment-277796833

I added a second commit that replaces a few U+2019 quotes with ASCII quotes so that their files can be stored as one-byte strings.
Probably one for another PR, but this sounds like a good candidate for a lint rule.

Basically we need a lint rule that checks for U+2019 quotes: ’, and suggests that they be replaced with ASCII quotes: '.

cc/ @bnoordhuis @richardlau

[addaleax]

Two things:

• This should apply to lib/ and only to lib/, because that’s where we need it.
• This isn’t really about quotation marks; ideally, any non-ASCII character would be forbidden (sigh)…

[silverwind]

We could do something like this (match is fed to the RegExp constructor):

````yaml

• match: '’'
  replacement: '''
• match: '[—–]'
  replacement: '-'
• match: '[^\x00-\x7F]'
  replacement: ''
  ````

The last one might be a bit extreme :)

[aqrln]

@silverwind I'd also add replacements for “” and «» to ASCII double quotes.

[silverwind]

You can always contribute improvements later, above is just about the general rule config layout.

[gibfahn]

I'd also add replacements for “” and «» to ASCII double quotes.

This is an open issue looking for someone to raise a PR. If whoever raises it wants to include those then that sounds good to me.

[not-an-aardvark]

If the goal is to allow files to be stored as one-byte strings, would it be better for the rule to just disallow all non-ascii characters in files?

edit: Never mind, I see @addaleax also mentioned this above.

[hkal]

@gibfahn I'll give this a go if no one else is assigned.

[gibfahn]

@hkal go for it!

[hkal]

Do we still need this after https://github.com/nodejs/node/pull/11129?

cc @bnoordhuis

[bnoordhuis]

Would be good to enforce it because Unicode files take up twice as much space in the binary as plain ASCII files.

[Trott]

This is for quotation marks inside of strings only, right?

I think we should be able to use ESLint's no-restricted-syntax to find these things rather than write a custom rule.

/cc @not-an-aardvark

[not-an-aardvark]

Yes, that should be possible to do. A selector like 'Literal[value=/\u2019/]' will match all string literals containing \u2019.

[vsemozhetbyt]

If we ban all the non-ASCII chars, this could beLiteral[value=/[^\n\x20-\x7e]/]. But this does not affect comments if I get this right.

Currently, we have non-ASCII chars in 4 *.js files in lib folder:

1. lib/console.js (two ’ in comments)
2. lib/stream.js (one ’ in comments)
3. lib/internal/test/unicode.js (one ✓ in code)
4. lib/timers.js (many pseudographics chars in comments)

[SirR4T]

Hi. First time contributing here, so please bear with some doubts. I was able to follow @Trott and @not-an-aardvark 's comments, and added two new linter rules. Now make -j4 lint-js fails with the following errors:

/Users/sarat/Play/Github/node/test/addons-napi/test_string/test.js
  45:14  error  use an ASCII double quote (`"`) instead of a Unicode double quote (`“`, `”`, `«`, `»`)  no-restricted-syntax

/Users/sarat/Play/Github/node/test/parallel/test-buffer-ascii.js
  31:15  error  use an ASCII single quote (`'`) instead of a Unicode quote (`’`)  no-restricted-syntax
  32:15  error  use an ASCII single quote (`'`) instead of a Unicode quote (`’`)  no-restricted-syntax
...

Doubts:

1. Is this the correct place to add the eslint rules?
2. I see that /* eslint-disable no-restricted-syntax */ can be used to bypass the rules. Since the linter is failing with this commit, should I be adding this to bypass current strings? I notice that all the failing strings are in tests/ folder, so I'm presuming they are necessary and need not be fixed.