--fix isn't handling "',' should be placed last comma-style" errors

[Gozala]

What version of ESLint are you using? - v3.3.1

What parser (default, Babel-ESLint, etc.) are you using? - babel-eslint

Please show your full configuration:

{
  "parser": "babel-eslint",
  "extends": "standard",
  "plugins": [
    "flowtype"
  ],
  "rules": {
      "flowtype/define-flow-type": 1,
      "flowtype/space-before-type-colon": [
          1,
          "never"
      ],
      "flowtype/use-flow-type": 1,
      "flowtype/valid-syntax": 1,
      "no-duplicate-imports": 0
  },
  "settings": {
      "flowtype": {
          "onlyFilesWithFlowAnnotation": false
      }
  }
}

Standar is defined here https://github.com/feross/eslint-config-standard/blob/master/eslintrc.json

What did you do? Please include the actual source code causing the issue.

export var obj =
  { a: 1
  , b: 2
  }

What did you expect to happen?

ESLint should fix comma style by placing at the end:

export var obj =
  { a: 1,
    b: 2
  }

What actually happened? Please include the actual, raw output from ESLint.

eslint --fix ./example.js

/Users/gozala/Projects/browser.html/example.js
  3:4  error  ',' should be placed last  comma-style

[Gozala]

I don't mind giving this a try if someone is willing to mentor me.

[platinumazure]

Let's see what the team thinks.

Personally, I'm worried about problems being caused by having to possibly re-align indentation, as well as handling possible conflicts with comma-spacing. So I'm 👎 but not strongly (and I can be persuaded).

[Gozala]

Personally, I'm worried about problems being caused by having to possibly re-align indentation, as well as handling possible conflicts with comma-spacing.

Is it not possible to re-align indentation with respect to config ?

So I'm 👎 but not strongly (and I can be persuaded).

To be honest I'm just looking for some solution that would take arbitrary code and reformat it comply with ESLint config. I was going after esformatter but was pointed out that just providing --fix for ESLint is a better way to go about it.

I don't actually know if that is out of scope for ESLint or not.

Also maybe doing it via plugin is more appropriate option ? If possible at all.

[gyandeeps]

Is this issue asking to make comma-style rules fixable?

Just to brainstorm a lil bit, what if we during the fix, replace the comma with a space (spot where we are removing the comma) and simply add the comma at the spot its needed. Anything i am missing or could go wrong with this approach?

[Gozala]

Is this issue asking to make comma-style rules fixable?

Yes

[mysticatea]

👍

Such change of comma's position does not change the meaning of code.
I think we welcome that kind of auto-fixing.

[gyandeeps]

I think i have a PR which works. I will send it out to collect some feedback on it.

[gyandeeps]

here is something #6957 i had in mind.

[gyandeeps]

@eslint/eslint-team Needs some eyes/thoughts here.

1. Should this issue be accepted?
2. I have a WIP PR, is that the right direction?

[mikesherov]

I'm 👍 on accepting this issue.

[ilyavolodin]

👍 on accepting.

[platinumazure]

I'll remove my 👎. We just need a champion now (@gyandeeps want to champion?).

[nzakas]

I'm a bit worried about this - trying to autofix across lines can be tricky given the current capabilities. For instance, what would happen here?

export var obj =
  { a: 1.     // Do somerhing
  , b: 2
  }

I like the idea, I just want to make sure it can be done safely.

[mikesherov]

Yeah, this seems like one that just needs really really clear edge case tests.

[gyandeeps]

@nzakas I was able to cover your specific use case but I dont think we can safely solve all scenarios for comments.
Worst case: fix would get rid of the comment but it will always output valid code (based on my experimentation).

[nzakas]

I dont think we can accept that worst-case scenario, comments are important. If there's a comment in there, and we can't keep it, we just should do the fix.

Also, what happens when the property value is a function? Does it still work? Or if there's a concise method? Or shorthand assignment? We really need a lot more tests to consider releasing this into the wild.

[gyandeeps]

I guess then it will be difficult to get the fix done. Can we just do the fix if their are no comments else dont do fix?

guys, feel free to suggest some cases so that i can include them in tests. Because i might miss a lot of them based on the variety.

[mysticatea]

Ah, there was the discussion about comments.
I wrote an implement idea to the wip PR: #6957 (comment)

[nzakas]

Yeah, our general approach is to do fixes only when it's 100% safe. If there are safe cases that can be fixed, we can definitely do those.