New: Add fixer for object-shorthand (fixes #6412)

[NickHeiner]

I've tested this manually by linking it into an existing project of mine and seeing that it cleans up ~50 detected rule violations. I've manually spot-checked the fixes to verify that they are what I'd expect.

Some of this would be cleaner if I could return multiple fixes instead of just one fixer call.

Thanks for the review!

[eslintbot]

LGTM

[mention-bot]

By analyzing the blame information on this pull request, we identified @nzakas, @lemonmade and @vitorbal to be potential reviewers

[platinumazure]

Noting for other reviewers that the star spacing might be wrong here-- this will be fixed by another pass in auto-fix. This would also be another example of something that could go into a shared repository settings config. (@NickHeiner you can ignore this)

[NickHeiner]

Yep, I was thinking about things like that! I'm glad to hear that it's not an issue for this PR.

[nzakas]

Can you add some tests that have new lines and comments in them? Like:

var x = {
    a: "B",
    c: c,
    // foo
};

[NickHeiner]

Yeah, I'm happy to add that. And do you really want all those other test cases to be on the same line? I was trying to keep with the existing style, but it does get a bit unwieldy.

Also, this is obviously beyond the scope of this PR, but it feels a bit brittle to be outputting a fix as a string, instead of an AST -> AST transformation. If the fixer were just outputting an AST, then issues like what @platinumazure called out above would not occur, because the fixer could produce the necessary structure without also having an opinion about how that structure gets rendered as a string.

[eslintbot]

LGTM

[NickHeiner]

@nzakas Additional test for comments and newlines added. Good call.

[nzakas]

@NickHeiner we know what we have is a bit brittle, but switching to something else is a massive amount of work and AST -> AST transforms would leave out whitespace fixes (which is what our fixing system was designed for primarily). You can read more about it here: #5329.

[nzakas]

@NickHeiner for the tests, we really need a few invalid cases with newlines and comments that are being autofixed so we know we aren't missing up newlines or comments.

[NickHeiner]

we know what we have is a bit brittle, but switching to something else is a massive amount of work

Makes sense! 😄 And if this system is designed for whitespace fixes, then yes this is a nice, simple way to address that issue.

[NickHeiner]

@nzakas I added the following test case:

        { code: "var x = {y: z,\n x: x,\n a: b\n // comment \n}", output: "var x = {y: z,\n x,\n a: b\n // comment \n}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected property shorthand.", type: "Property" }] },

It seemed to be that this would adequately cover comments and newlines. But I'm happy to add a few more examples.

[eslintbot]

LGTM

[NickHeiner]

6 additional test cases with comments and newlines added.

[ilyavolodin]

LGTM. @NickHeiner Thanks for working with us on this!

[eslintbot]

LGTM

[nzakas]

Thanks @NickHeiner !

[NickHeiner]

You're welcome! Thanks for the review.