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
Escape characters are handled incorrectly in inline config comments #9366
Comments
I think this is happening because backslash escapes are handled differently in config comments. I noticed this in #9309 but didn't end up having time to investigate. |
If this helps, here's another repro, in the web interface. |
Oh, and this actually works as intended. (Note: those two repros use a simpler regexp, but it works to the same effect.) |
I guess it's because there's no need to escape in source code. |
(fixes eslint#9366) levn parsing string differently than JSON.parse, e.g. JSON.parse( '{ "foo": "\\n" }') // => { foo: '\n' } levn.parse('Object', '{ "foo": "\\n" }') // => { foo: '\\n' }
I was the one who suggested using levn for inline comment handling, reasoning that it would be nice to have the same syntax in inline comments as on the command line (for options like @eslint/eslint-team Should we consider removing levn from inline comment handling, given this issue? Or can we just update the inline comment docs to point to levn? Maybe we could reasonably fix this in levn? |
To clarify, do you mean that we would keep levn's semantics (aside from this case) without using the library? Or do you mean that we would require inline comments to actually be JSON? I would be fine with the former (provided it isn't too much work), but I don't think we should do the latter -- it would be a big breaking change, and I like not needing to quote everything in inline comments.
This doesn't seem ideal to me -- I think it's unexpected that a comment which appears to be JSON would not get parsed as JSON.
👍, although it seems unmaintained. |
Line 2421 in ee99876
so this test should be failing -- not a valid regex in JSON, it passed just because of the levn issue? |
Yes, that is the correct understanding. |
I found another example of this while trying to implement #10197. In the max-len docs, we specify a rule config inline and it crashes ESLint when trying to initialize the rule: /*eslint max-len: ["error", { "ignorePattern": "^\\s*var\\s.+=\\s*require\\s*\\(" }]*/ I'll try to write a report in levn in the next few days. |
I've opened gkz/levn#2 to try to get some discussion going. (That said, there hasn't been any activity on levn in nearly three years, so I might need to take it a step further and write a PR.) |
How do we want to proceed with this? It unfortunately doesn't look like there has been any movement on the |
Thoughts on using the JSON5 parser for this? |
I think the only big difference is that string values must be quoted (though it can be single or double quotes). |
It seems like that might be a significant breaking change, since it would make Would it work to parse the strings as YAML instead? I know YAML is a superset of JSON and allows unquoted strings, although I'm not sure how it handles some edge cases. |
That's correct, parsing as JSON5 would be a breaking change. I made a quick PoC to help evaluate this.
That's an interesting thought! Seems worth exploring. |
The levn project has just issued version 0.4.1 which allows quoted string literals to use the same escaping rules as JavaScript itself, so upgrading to latest levn may resolve this issue. |
Tell us about your environment
What parser (default, Babel-ESLint, etc.) are you using?
Default
Please show your full configuration:
Configuration
What did you do? Please include the actual source code causing the issue.
What did you expect to happen? It to pass (as it does without the comment)
What actually happened? Please include the actual, raw output from ESLint.
The text was updated successfully, but these errors were encountered: