Fix: key-spacing shouldn't report object pattern properties

[mdjermanovic]

What is the purpose of this pull request? (put an "X" next to item)

[X] Bug fix

Tell us about your environment

• ESLint Version: 6.1.0
• Node Version: 10.16.0
• npm Version: 6.9.0

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

default

Please show your full configuration:

Configuration

module.exports = {
  parserOptions: {
    ecmaVersion: 6,
  },
};

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

/*eslint key-spacing: ["error", { "beforeColon": false }]*/
({ a : foo } = bar)

/*eslint key-spacing: ["error", { "align": "value" }]*/
({
    foo: a,
    bar:  b
} = baz)

What did you expect to happen?

No warnings. By the documentation for this rule it targets object literals, these are object patterns.
Also, there are no test cases with patterns.

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

1 warning, for beforeColon.

No warnings for align.

What changes did you make? (Give an overview)

Skip the check if the Property is not a property of an ObjectExpression.

Is there anything you'd like reviewers to focus on?

I guess this should be treated as a bug rather than as an undocumented behavior because it doesn't work well - some options report warnings, some not.

[kaicataldo]

I don't think we currently have a way to enforce this style, and I do think it makes sense to use this rule to do so (given our other rules that check curly brackets/object styling also check things like object expressions and import/export declarations).

If this is something we want to enforce (which I think makes sense for the sake of completeness), it seems like we could either land this current PR as a bug fix (since it's currently broken) and then make a new enhancement proposal to add the above behavior as an enhancement, or fixing up the behavior for object expressions to match objects as a semver-minor bug fix.

[kaicataldo]

How do we want to proceed with this? Is it a WIP?

[mdjermanovic]

How do we want to proceed with this? Is it a WIP?

This was supposed to be a finished bug fix, but now I'm not sure if it should be merged.

While the current behavior of this rule in regard to object patterns is probably an unintentional oversight, it's most likely working well (when it works at all, it depends on the configuration) and might be a desired and useful behavior for some users at the moment.

I'm closing this for now, until we finish the analysis in #12048