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

object-curly-spacing false-positive with es6 import with trailing comma #3324

Closed
leebyron opened this issue Aug 8, 2015 · 8 comments
Closed
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion triage An ESLint team member will look at this issue soon

Comments

@leebyron
Copy link

leebyron commented Aug 8, 2015

version: 1.0.0

import {
  A,
  B,
} from 'module';

with rule:

"object-curly-spacing": [2, "always"]

reports:

3:4   error  A space is required before ','  object-curly-spacing

But I expected no error.

@eslintbot
Copy link

Thanks for the issue! We get a lot of issues, so this message is automatically posted to each one to help you check that you've included all of the information we need to help you.

Reporting a bug? Please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. The source code that caused the problem
  3. The configuration you're using (for the rule or your entire config file)
  4. The actual ESLint output complete with line numbers

Requesting a new rule? Please be sure to include:

  1. The use case for the rule - what is it trying to prevent or flag?
  2. Whether the rule is trying to prevent an error or is purely stylistic
  3. Why you believe this rule is generic enough to be included

Requesting a feature? Please be sure to include:

  1. The problem you want to solve (don't mention the solution)
  2. Your take on the correct solution to problem

Including this information in your issue helps us to triage it and get you a response as quickly as possible.

Thanks!

@gyandeeps gyandeeps added the triage An ESLint team member will look at this issue soon label Aug 8, 2015
@xjamundx
Copy link
Contributor

I'll look at this today..

@xjamundx
Copy link
Contributor

Weird I'm getting:

  1 failing

  1) object-curly-spacing import {
    A,
    B,
} from 'module';;:
     AssertionError: Should have no errors but had 1: [ { fatal: true,
    severity: 2,
    message: 'Unexpected token }',
    line: 4,
    column: 2 } ]
      at testValidTemplate (/Users/jamuferguson/dev/eslint/lib/testers/rule-tester.js:9:5044)
      at Context.<anonymous> (/Users/jamuferguson/dev/eslint/lib/testers/rule-tester.js:9:7934)
      at callFn (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runnable.js:251:21)
      at Test.Runnable.run (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runnable.js:244:7)
      at Runner.runTest (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:374:10)
      at /Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:452:12
      at next (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:299:14)
      at /Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:309:7
      at next (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:248:23)
      at Immediate._onImmediate (/Users/jamuferguson/dev/eslint/node_modules/mocha/lib/runner.js:276:5)
      at processImmediate [as _immediateCallback] (timers.js:367:17)

With espree 2.2.0 and eslint 1.1.0.

Are you using babel-eslint? There may be a bug in espree assuming trailing commas are allowed there...

Interestingly enough this seems to work alright:
require('espree').parse("var { A, B, } = require('module');", {ecmaFeatures: {modules: true, destructuring: true}}) (at least the parsing part...will get back on the issue at hand)

@xjamundx
Copy link
Contributor

  1. @leebyron I think this may be a babel-eslint issue in the way it's handing column numbers or something?
  2. The destructuring trailing-comma case works as expected with eslint (though it wasn't being tested)
  3. Espree currently does not support trailing commas in import and ... oh wait. it looks like I was supposed to fix that last month Support for trailing commas in Import/Export statements js#148

@leebyron
Copy link
Author

Yes, I'm using the babel-eslint parser. cc @hzoo

@hzoo
Copy link
Member

hzoo commented Aug 11, 2015

Ok since @xjamundx has a PR eslint/js#186 think this might be fixed eslint side?

@hzoo
Copy link
Member

hzoo commented Aug 11, 2015

Ok made a PR to fix after eslint/js#186

@xjamundx
Copy link
Contributor

Awesome!

ilyavolodin added a commit that referenced this issue Aug 13, 2015
Fix: trailing commas in object-curly-spacing for import/export (fixes #3324)
@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 triage An ESLint team member will look at this issue soon
Projects
None yet
Development

No branches or pull requests

5 participants