More granular config for 'object-shorthand'

[pablobirukov]

Duplicating the rationale from palantir/tslint#3778

The object-shorthand rule currently does not offer granularity for enforcing/disallowing shorthand separately for property assignment vs method declarations.

I would prefer to allow (but not enforce!) shorthand for method declarations, but disallow shorthand for property assignments.

Here's an example of how I would like to configure the object-shorthand rule:

const someValue = 20;

const myObj = { 
    // error: shorthand property assignment not allowed
    someValue,
    // valid
    anotherValue: someValue,
    // valid
    someMethod(): number {
        return 42;
    },
    // also valid
    anotherMethod: (): number => {
       return 42;
    }
};

The motivation for this is that shorthand method declaration is very convenient and improves readability, while shorthand property assignment creates a coupling between the object's property name and a local variable name that can lead to bugs when refactoring code later.

What rule do you want to change?

object-shorthand

Does this change cause the rule to produce more or fewer warnings?

Number of warnings stays the same

How will the change be implemented? (New option, new default behavior, etc.)?

More granular config options similar to TSLint version

Are you willing to submit a pull request to implement this change?

Yes

[kaicataldo]

I'm generally supportive of this! I just took a look at the current options and they're really not very intuitive. I think it would make sense to deprecate "methods" and "properties" and allow for the first config option to be an object that has the following options (in addition to the current string options):

{
    "object-shorthand": ["error", 
        "always" | 
        "never" |
        "consistent" |
        "consistent-as-needed" |
        {
            "property": "always" | "never",
            "method": "always" | "never"
        }
    ]
}

I don't think it makes sense to allow setting property or method to consistent or consistent-as-needed.

[pablobirukov]

@kaicataldo thanks for the reply.

I quite like the proposed syntax. Can't be sure about consistent or consistent-as-needed though, as I don't see a use for them in the first place.

Do we need someone else's approval to make sure my PR will be approved?

[kaicataldo]

We recommend waiting until consensus is reached before working on it so that people don't spend time working on a change that the team feels isn't a good fit for the project. Consensus is reached when a member of the team champions the change and three other team members support it (with a 👍 reaction). You can read more details about the process here.

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

[kaicataldo]

I'll champion this.

[pablobirukov]

I could have pushed a PR by now. All I want is to know it won't be rejected

[kaicataldo]

@pablobirukov Please see #12351 (comment). I'll try to drum up support for this.

[pablobirukov]

@katerberg thanks, I remember that, and that's what I'm waiting for 👍

[jsphstls]

I am also interested in a new config for this rule. My case is likely the opposite of the originally posted example. I would like to disallow the methods shorthand only, but there is no current configuration that does this. The current documentation did not make that configuration seem impossible, so I only found out after time with failed attempts.

My (personal) reasoning for preventing the method shorthand is that it encourages method creation inside object brackets which leads to more activity at a nested level. With a combination of other rules the fat arrow => is the primary visual identifier for functions. Method shorthand does not permit the fat arrow.

Less better:

const something = {
  add (a, b) {
     return a + b
  },
  otherStuff
}

More better:

const add = (a,b) => a + b

const something = {
  add,
  otherStuff
}

[Nautigsam]

I think more granular config is also needed for enforcing arrow functions. For now arrow functions with body are considered invalid with avoidExplicitReturnArrows = true while short arrow functions are not enforced at all. It would be great to configure which level we want. For example, I would need to totally forbid arrow functions:

const o = {
  // considered valid
  accepted() {
    return {};
  },
  // enforced by object-shortand with avoidExplicitReturnArrows option
  invalid: () => {
    return {};
  },
  // not enforced by object-shortand
  alsoInvalid: () => ({})
}

[kaicataldo]

The team has made the difficult decision to freeze stylistic rules (please see https://eslint.org/blog/2020/05/changes-to-rules-policies for more details), and we unfortunately won't be able to accept this proposal.

Thanks for contributing to ESLint and for being willing to implement your proposal.