GeezerVison

oh? then @hkal 's approach of parsing all tokens would be preferred? I had assumed the eslint selector would select the source code too, i see where i might have gone wrong 😅

Also, doubts:

• the regexp /[^\n\x20-\x7e]/ doesn't capture \r, which I have seen some instances of, while running the linter. I should be adding that in the regexp, right?
• I have seen instances of \u0000 and \u000c etc. in code. I should be disabling this rule for these, right?

@SirR4T I’m not sure how to do it, but yes, you want all tokens and even comments.

I have seen instances of \u0000 and \u000c etc. in code. I should be disabling this rule for these, right?

It’s fine to have the escape sequences spelled out as escape sequences in the source code. What matters is whether the files themselves contain characters that don’t fit into the ASCII range, because that’s what the script that bakes those files into the node binary looks for.

I hope that’s helpful. :)

now I'm running into a very peculiar error, while doing make lint-js:

I'm unable to disable the below error, via the regular methods (//eslint-disable-line, //eslint-disable-next-line, etc.)

$ make lint-js
Running JS linter...

/Users/sarat/Play/Github/node/lib/internal/test/unicode.js
  1:1  error  Non-ASCII character '✓' detected  non-ascii-character

✖ 1 problem (1 error, 0 warnings)
  1 error, 0 warnings potentially fixable with the `--fix` option.

make: *** [lint-js] Error 1

@addaleax : review latest commit? This was the approach followed in the (now closed) pull #11371, but I think that this is catching much lesser violations.

I'm a bit confused about why non-ascii characters would bloat the binary size. Aren't the files encoded as UTF-8?

@not-an-aardvark that is what I understood, from #11129 (comment) :

I would have to assume it is because the external string API does not directly support UTF8. That is, if you look at ExternalStringResource, it assumes a utf16_t* buffer while ExternalOneByteStringResource assumes const char*. If the strings were stored as UTF8 we would incur an additional cost at startup that is not necessary.

Is that not correct?

@SirR4T Yes, that is correct. Files that don’t fit into ASCII are currently saved as UTF-16 instead:

Yay! 🎉

Could catch the lib/timers.js too, using source.text. But am still unable to bypass / skip the rule, for it ☹️

Any ideas? @Trott @not-an-aardvark

cc: @Trott @not-an-aardvark @addaleax

can someone help me out here? I'm unable to skip the lint checks, inside lib/ folder. Need to skip the rules for lib/timers.js, and the check mark in lib/internal/test/unicode.js.

I think there are a few issues:

• You're using a comment like // eslint-disable non-ascii-character, but I think it should be a block comment (/* eslint-disable non-ascii-character */)
• When the rule reports a problem, the location of the problem is currently always the top of the file, because you're passing the Program node to context.report. Instead, it would be better if the report location were the point in the file that contains the non-ascii character. To do this, you could find the index of the non-ascii character in sourceCode.text, and then pass { ... loc: sourceCode.getLocFromIndex(theIndex) } to context.report.

If you make both of those changes, I think the disable comments will work as expected.

Thanks @not-an-aardvark ! worked like a charm! pushing the latest changes, once the build is done.

CI https://ci.nodejs.org/job/node-test-pull-request/12908/

@not-an-aardvark PTAL

Landed in c45afe8

Should this be backported to v8.x-staging or v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.