New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrectly detecting unnecessary escapement #494

Closed
Tatsh opened this Issue Mar 24, 2012 · 6 comments

Comments

Projects
None yet
5 participants
@Tatsh
Copy link

Tatsh commented Mar 24, 2012

fUTF8Test.js: line 60, col 37, Unnecessary escapement.
fUTF8Test.js: line 60, col 43, Unnecessary escapement.
fUTF8Test.js: line 76, col 37, Unnecessary escapement.
fUTF8Test.js: line 76, col 43, Unnecessary escapement.
fUTF8Test.js: line 92, col 37, Unnecessary escapement.
fUTF8Test.js: line 92, col 43, Unnecessary escapement.

Lines:

[' Iñtërnâtiônàlizætiøn   ', " \x6b..\x6e", 'Iñtërnâtiônàlizætiø']
[' Iñtërnâtiônàlizætiøn   ', " \x49..\x6e", 'ñtërnâtiônàlizætiøn   ']
[' Iñtërnâtiônàlizætiøn   ', " \x6b..\x6eI",  ' Iñtërnâtiônàlizætiø']

The file in question:
https://github.com/tatsh/flourish-js/blob/master/tests/fUTF8Test.js

@RaymondCrandall

This comment has been minimized.

Copy link

RaymondCrandall commented Mar 25, 2012

This too may be related...

https://twitter.com/#!/oscargodson/status/183805286824427521

Going to sleep, I will see if I can figure something out tomorrow?

@Tatsh

This comment has been minimized.

Copy link

Tatsh commented Mar 25, 2012

Don't think that's related. His issue is a missing \ since his regex is in quotes and wants to use a special character within it (hence needing \\). (https://developer.mozilla.org/en/JavaScript/Guide/Regular_Expressions#section_9 see the part where it says 'In this example, you could replace the line:...')

I think the issue here is that JSHint is not recognising the use of \x notation for strings is all. It thinks I want a literal x and then the hexadecimal (hence 'unnecessary' escapement). \\x would be incorrect because that would mean print \ and x and then the hexadecimal number as it appears (6b).

\u notation is not affected (both single and double quoted).

I see that what you are essentially doing is having JSHint check for unnecessary usage of \x notation now after seeing the source. In my case 0x6b is k so yes, technically it is 'unnecessary' to write it that way as I can simply write k. Because of this, I think the best thing for this would be an option. /*jshint slashx:true */?

I did change your esc() function but all this essentially does is remove any possibility of having an unnecessary escape warning. It is the same as returning immediately really. This is why I think there should just be an option.

                function esc(n) {
                    var isXNotation = s.substr(j, 1) === 'x';
                    var i = parseInt(s.substr(j + 1, n), 16);
                    j += n;
                    if (i >= 32 && i <= 126 &&
                            i !== 34 && i !== 92 && i !== 39 && !isXNotation) {
                        warningAt("Unnecessary escapement.", line, character);
                    }
                    character += n;
                    c = String.fromCharCode(i);
                }
@piuccio

This comment has been minimized.

Copy link

piuccio commented Sep 20, 2012

I get the same issue for escaped unicode symbols, like for instance on this file

I could write those strings directly in Unicode but escaping is better because it avoids encoding issues that might happen in case the file is converted into ISO Latin 1 or even ANSI

@JamesMGreene

This comment has been minimized.

Copy link

JamesMGreene commented Nov 25, 2012

I am seeing similar issues. Pertinent gist and tweet.

@valueof

This comment has been minimized.

Copy link
Member

valueof commented Nov 26, 2012

I'll make sure it's fixed before the next release. 8 months is long enough. :-)

@JamesMGreene

This comment has been minimized.

Copy link

JamesMGreene commented Nov 26, 2012

@antonkovalyov Thanks, you rock!

To clarify (w/o looking at links): although @Tatsh stated that \u notation characters were not affected, both @piuccio and I are seeing these errors for \u chars, too!

@valueof valueof closed this in d8f5cf4 Dec 22, 2012

jugglinmike added a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014

Don't display unnecessary escapement for \u and \x strings.
Because sometimes escapement is necessary[1] and if they aren't,
they don't break anything.

[1] - For example, for sanitizing:

	var toRawIssueContentReplacements = {
		'"': '&quot;',
		'<': '\u003c',
		'>': '\u003e'
	};

Closes jshintGH-494.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment