-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
[array-bracket-newline] doesn't handle inline comments properly #12416
[array-bracket-newline] doesn't handle inline comments properly #12416
Comments
This is related to the /*eslint array-bracket-newline: ["error", "consistent"]*/
const asd = [ // comment
123,
123
];
This looks like a bug to me. The above example is auto-fixed to: /*eslint array-bracket-newline: ["error", "consistent"]*/
const asd = [ // comment
123,
123]; where the rule still reports the first error. This could be manually fixed to not report the error like this: /*eslint array-bracket-newline: ["error", "consistent"]*/
const asd = [123, // comment
123]; or like this: /*eslint array-bracket-newline: ["error", "consistent"]*/
const asd = [
// comment
123,
123
]; I don't think that the rule intends to enforce neither of those fixes, given how the other options work. In particular, /*eslint array-bracket-newline: ["error", "always"]*/
const asd = [ // comment
123,
123
]; It seems inconsistent that The code for the Couldn't find anything specific about this in #9136 and #9206 where the option is implemented and there are no test cases with the I think that this should be accepted as a bug. Bug fix would be to check for a linebreak between the |
Yeah I didn't even know that. Btw this might be a good idea for discovering new bugs, if something that passes an "always" or "never" rule doesn't pass a "consistent" rule there's probably an issue. |
The proposed fix is to change the logic that applies only when the option is Instead of checking for a linebreak between I can't make an example where this change would produce new (potentially unwanted) warnings on an array that doesn't have any warnings at the moment. It seems impossible. So, this looks like a bug and the fix looks safe. But, I'll leave this open for some time before accepting, maybe other team members have a different opinion. |
Agreed that this seems like a bug. |
I'm working on this. |
Tell us about your environment
What parser (default, Babel-ESLint, etc.) are you using?
@typescript-eslint/parser
Please show your full configuration:
Here it is, just uncomment the "array-bracket-newline" rule.
What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.
npx eslint src --ext '.ts, .tsx'
What did you expect to happen?
No error detected.
What actually happened? Please include the actual, raw output from ESLint.
Adding an inline comment like that trips this rule, the array is actually consistently written as far as I'm concerned but the rule is kind of considering the inline comment as an item of the array.
Are you willing to submit a pull request to fix this bug?
I'm not too familiar with the AST, but with some pointers perhaps I can help.
The text was updated successfully, but these errors were encountered: