Skip to content
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-spacing never not firing on object.property[ index ]; #2941

Closed
vvo opened this issue Jul 7, 2015 · 24 comments

Comments

Projects
None yet
6 participants
@vvo
Copy link

commented Jul 7, 2015

When using "array-bracket-spacing": [2, "never"], writing object.property[ 1 ] is allowed. I would expect it to fail like trying to array[ 2 ]

seems like a bug?

@gyandeeps gyandeeps added the triage label Jul 7, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

  • What version of eslint are you using?
  • what kind of parse are you using?
  • Can you share the code and the actual output?

I ran this code and I got the errors (using online demo)

/* eslint "array-bracket-spacing": [2, "never"] */
var foo = object.property[ 1 ];

report:

2:10 - "object" is not defined.
2:25 - There should be no space after '['
2:29 - There should be no space before ']'
@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Try this in online demo:

/* eslint "array-bracket-spacing": [2, "never"] */
var obj = {hello: {mom: [1, 2]}};

obj.hello.mom[ 2 ] = "3";
@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Even this fails to report the error:

/* eslint "array-bracket-spacing": [2, "never"] */
var obj = {hello: {mom: [1, 2]}};

obj.hello.mom[ 2 ] = "3";

obj.hello[ 0 ] = "ueaj";
@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Wonder if the local eslint config is taken into account, but reading the docs, array-bracket-spacing should be never by default

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

I can still see the errors come on online demo for your above example

/* eslint "array-bracket-spacing": [2, "never"] */
var obj = {hello: {mom: [1, 2]}};

obj.hello.mom[ 2 ] = "3";

obj.hello[ 0 ] = "ueaj";
1:50 - Expected linebreaks to be 'LF' but found 'CRLF'.
4:13 - There should be no space after '['
4:17 - There should be no space before ']'
6:9 - There should be no space after '['
6:13 - There should be no space before ']'
@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Are you using the livedemo or creating a script and running eslint on it? I do not have any error using the livedemo, duh?

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

2015-07-07-160707_1228x317_scrot

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

I am using the live demo. Can you refresh your page and start again maybe.

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Same, no issue whatsoever. Amazing. Could it be the linefeeds or smg messing up here?

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

> cat weird.js 
/* eslint "array-bracket-spacing": [2, "never"] */
var obj = {hello: {mom: [1, 2]}};

obj.hello.mom[ 2 ] = "3";

obj.hello[ 0 ] = "ueaj";

obj[ 2 ] = "weee";

var hello = [ 2 ];

> eslint -v
v0.24.0

> eslint weird.js

weird.js
  10:12  error  There should be no space after '['                                     array-bracket-spacing
  10:16  error  There should be no space before ']'                                    array-bracket-spacing
  11:0   error  Expected an assignment or function call and instead saw an expression  no-unused-expressions
  11:8   error  Missing semicolon                                                      semi

✖ 4 problems (4 errors, 0 warnings)

> node -v
v0.12.5
@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

only one error shown when using var

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

property access is not warned

@gyandeeps

This comment has been minimized.

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

How come I cannot make it error in the online demo nor in nodejs

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

Using your example:

2015-07-07-162943_1296x320_scrot

@btmills

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

@gyandeeps I'm seeing the same thing @vvo is seeing - array-bracket-spacing doesn't check computed member expressions. The tests you linked to only test that computed member expressions without spacing are valid, but they would still pass linting if there were space between those brackets. We don't have invalid tests for member expressions there at all.

Since we're deprecating space-in-brackets, I'm inclined to say this is an omission - though from what I'm not sure. Do we need a third rule to cover computed member expressions, or should they be part of array-bracket-spacing (despite the array name)? @xjamundx thoughts?

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

hmmm... why is it giving errors on my machine on online demo. I am using windows 7 and chrome 43 if that makes a difference.

@btmills

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

@gyandeeps perhaps you have demo rules configured differently from the defaults? Try an incognito tab.

@xjamundx

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

I got it, space-in-brackets was on for me so these errors are coming from that rule. Sorry about that.

@btmills

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

@xjamundx thank you! I'm thinking this would be good to point out in the docs, and a PR to close this would:

  • Add computed-property-spacing to the deprecation notice and related rules for space-in-brackets.
  • Add computed-property-spacing to the related rules for array-bracket-spacing.
  • Add array-bracket-spacing to the related rules for computed-property-spacing.

Does that sound good?

@xjamundx

This comment has been minimized.

Copy link
Contributor

commented Jul 7, 2015

Yeah I'll throw something together by this evening.

@btmills btmills added rule documentation and removed triage labels Jul 7, 2015

@xjamundx xjamundx self-assigned this Jul 7, 2015

@ilyavolodin ilyavolodin added the accepted label Jul 7, 2015

@nzakas

This comment has been minimized.

Copy link
Member

commented Jul 7, 2015

Just to clarify for everyone: array-bracket-spacing works only on array literals, so the reported behavior is correct. What you're looking for here is computed-property-spacing.

@vvo

This comment has been minimized.

Copy link
Author

commented Jul 7, 2015

thanks all

ilyavolodin added a commit that referenced this issue Jul 16, 2015

Merge pull request #3025 from xjamundx/bracket-docs
docs: additional computed-property-spacing documentation (fixes #2941)

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.