tools: eslint rule to disallow Unicode quotes

[SirR4T]

Fixes: #11209

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes

• commit message follows commit guidelines

Affected core subsystem(s)

[apapirovski]

Hi @SirR4T, thank you for working on this. This is a good start!

Since the main reason for this rule is to allow source files to take up less space, we would ideally include a rule for all non-ASCII characters rather than just quotation marks.

Also, since this likely breaks some of our tests, those should have eslint comments added to disable this rule as appropriate.

[SirR4T]

thanks @apapirovski for the encouraging comments!

I added the eslint disabling guards, for all the tests which were failing.

After adding another rule, though, for all non-ASCII characters (using Literal[value=/[^\n\x20-\x7e]/], as suggested by @vsemozhetbyt on #11209 ), a lot many more tests started failing.

I expect that eslint disabling guards should be added to benchmarks which are failing, similar to the tests, but that I should rather be trying to fix the other errors. Would that be correct?

FYI, 1329 errors are due to the above linter rule alone. Should I be fixing them in multiple PRs? or in one PR alone?

[addaleax]

Also, since this likely breaks some of our tests, those should have eslint comments added to disable this rule as appropriate.

This should only be in effect for lib/, not the test suite. There is lib/.eslintrc.yaml, which should be the right place?

Also, like @vsemozhetbyt said in that issue, this lint rule would currently only catch forbidden characters inside of strings, as far as I can tell, although we probably want them for all source text in lib/.

[apapirovski]

This should only be in effect for lib/, not the test suite. There is lib/.eslintrc.yaml, which should be the right place?

This is somewhat complicated by the fact that no-restricted-syntax is impossible to extend, it can only be redefined. This might have to be a custom rule if it only needs to target the lib folder.

[SirR4T]

This is somewhat complicated by the fact that no-restricted-syntax is impossible to expand, it can only be redefined. This might have to be a custom rule if it only needs to target the lib folder.

Hmm. how would i go about writing a custom rule, for lib only?
Also, I'm not sure about the non-ASCII rule, but wouldn't it be a good thing for the quotes to apply for all?

[addaleax]

Also, I'm not sure about the non-ASCII rule, but wouldn't it be a good thing for the quotes to apply for all?

For files outside of lib/ (i.e. that don’t end up in the node binary itself), there is no real harm from using non-programming quotes…

Hmm. how would i go about writing a custom rule, for lib only?

I’m not sure, but all of the current custom rules for lib/ can be found in tools/eslint-rules/, maybe take a look at those 4 for inspiration?

[SirR4T]

@addaleax : just went through pull #11371 ... should I attempt to revive that? although I should admit, i might need more help with the review comments there.

[addaleax]

@SirR4T Yes, that sounds like a good approach. Don’t worry about that, I’d just try to re-implement as much of that patch as possible, get as far as you can with the review comments and open a PR. It should be pretty doable to bring it over the line from there. :)

[SirR4T]

Cool, closing this PR then. Will create a new one, basing the approach on above.