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

Created no-dangle rule. #87

Merged
merged 1 commit into from Jul 19, 2013
Merged

Created no-dangle rule. #87

merged 1 commit into from Jul 19, 2013

Conversation

iancmyers
Copy link
Contributor

Created the no-dangle rule, which warns when it encounters a trailing comma in an object literal. The following will raise warnings:

var foo = {
  bar: "baz",
};

I've accomplished trailing comma detection by doing a regex match on the ObjectExpression node source. I investigated inspecting the ObjectExpression's properties, however they don't seem to contain any comma information. This seemed like the best possible way to get this rule, but I'd love to hear if there's a better approach.

I've defaulted no-dangle to on, because trailing commas will cause errors in IE, so it seemed like a sane default.

Fixes issue #13

@michaelficarra
Copy link
Member

This regex is not going to be sufficient. You should be using the token stream and looking at the token right before the closing curly brace.

@nzakas
Copy link
Member

nzakas commented Jul 19, 2013

I agree, can you use getTokens() instead?

@iancmyers
Copy link
Contributor Author

Thanks guys! I knew there had to be a better way to do that!

I've rewritten and re-pushed the commit for the rule using the new approach.

@michaelficarra
Copy link
Member

Very nice. So much better.

@michaelficarra
Copy link
Member

Shouldn't you be able to apply this same rule to ArrayExpressions?

@iancmyers
Copy link
Contributor Author

@michaelficarra Yes! That's a good point.

Created the no-dangle rule, which warns when it encounters a trailing comma in an object literal. The following will raise warnings:

var foo = {
  bar: "baz",
}

Fixes issue eslint#13
nzakas added a commit that referenced this pull request Jul 19, 2013
@nzakas nzakas merged commit 0f57cf2 into eslint:master Jul 19, 2013
@iancmyers iancmyers deleted the rule-no-dangle branch July 19, 2013 18:11
@michaelficarra
Copy link
Member

... or we could just forget about them

@mathiasbynens
Copy link
Contributor

@nzakas ?

@iancmyers
Copy link
Contributor Author

@michaelficarra Checking ArrayExpressions for trailing commas was in the commit that was merged.

@michaelficarra
Copy link
Member

Ah, I see. I did not see that you updated the pull request already. Thanks for the clarification!

secondToLastToken = tokens[tokens.length - 2];

// The last token in an object/array literal will always be a closing
// curly, so we check the second to last token for a comma.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always be a closing curly

or square bracket

@nzakas
Copy link
Member

nzakas commented Jul 19, 2013

This matches JSHint, so I'm happy. :)

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants