Rule Proposal: func-name-matching

[mysticatea]

From requireMatchingFunctionName.

This rule will enforce function expression names with the variable name (or property name) which is the destination of the assignment.

Options

{
    "func-name-matching": ["error", {"module.exports": "ignore"}]
}

• module.exports - The setting for module.exports = function foo() {};
  • "ignore" (default) - The function is ignored.
  • "propertyName" - The function's name should be exports. Original JSCS rule has this option.
  • (in my head, in future) "fileName" - The function's name should match camelcase of the file name.

Examples:

/*eslint func-name-matching: "error"*/

// ✔ GOOD ---------------------------------------------------------------------
let foo = function foo() {};
foo = function foo() {};
obj.foo = function foo() {};
let obj = {foo: function foo() {}};

// ignore anonymous function expressions.
// it's warned by `func-names` rule.
let foo = function() {};
foo = function() {};
obj.foo = function() {};
let obj = {foo: function() {}};

// ignore it if it's not assigned
foo(function bar() {});

// ignore 'module.exports' by default.
module.exports = function foo() {};


// ✘ BAD ----------------------------------------------------------------------
let foo = function bar() {};
foo = function bar() {};
obj.foo = function bar() {};
let obj = {foo: function bar() {}};

// Should warn simple computed properties from original JSCS rule.
obj["foo"] = function bar() {};
let obj = {["foo"]: function bar() {}};

[alberto]

Should this be added as an option for func-names instead?

Also there is an additional option to also check module exports

[mysticatea]

In the first proposal, this had been merged to func-names rule: #6059
But I got aware of those 2 rules are orthogonal. This rule checks names if function expression names exist, otherwise does nothing.

I thought the name exports has no mean.
But original JSCS rule has the option, so we can add the option.

[alberto]

That's why I proposed an option. Values could be always, never and matching.

[mysticatea]

I think 4 options are needed in that case: always, never, matching-if-exist, matching-always
So, I think the separation is proper. 2 x 2.

[alberto]

Oh, now I see. It could also be the 3 options mentioned and a refinement for always. I was proposing adding them to func-names to avoid conflicts between the rules, but it would be more of a misconfiguration to enable both this rule and func-names: ["error", "never"].

[mysticatea]

Separated version:

func-names	func-name-matching
off	off	allows anything
off	on	requires matching if name exists
always	off	requires any names
always	on	requires matched names (in callback, any names)
never	off	disallow names
never	on	disallow names (func-name-matching has no effect)

And an option -- includeModuleExports in func-name-matching?

---

Merged version, maybe:

func-names
off	allows anything
matching	requires matching if name exists
always	requires any names
always-matching	requires matched names (in callback, any names)
never	disallow names

And an option -- matchingIncludeModuleExports?

[mysticatea]

@eslint/eslint-team Any insights about combining?

[alberto]

Now that I read this again I am not sure it is a big problem if the rules are separated.
I was thinking there would be a conflict between func-names: never and func-name-matching: on

Given this code:

let foo = function bar() {};

We would get two errors on bar, one saying "Unexpected named function expression bar" and the other "Function name does not match property name". But it's less of a conflict than I tought at first, so I'm ok with separate rules.

[mikesherov]

This seems like two rules to me, like @alberto said.

[alberto]

heh, it was actually @mysticatea who was proposing that. I just backed out 😅

[nzakas]

I also think having two rules makes the most sense.

[mysticatea]

OK, I saw consensus.
I updated an option of this rule to fit to original JSCS rule.

In future, I hope the option to match the containing file's name for module.exports and export default. Of course, I'm happy if it's now.

[alberto]

@eslint/eslint-team opinions?

I can champion this one

[nzakas]

Per TSC meeting (7/21/16), marking as accepted.

[annie]

i can work on this!

[alberto]

Go ahead @azhang496!

[kaicataldo]

Thanks for all the recent contributions to ESLint, @azhang496!

[vitorbal]

Yeah, thanks, @azhang496! Great job :)

[annie]

thanks everyone for being so welcoming! :)

[annie]

should there be an option for other reserved/ignored words in addition to module.exports?

[nzakas]

I think we can just special case module exports for now and leave it open in case other special cases pop up later.

I would suggest, though a slightly different format for the option, as I think the original is unclear:

{
    "func-name-matching": ["error", { includeCommonJSExports: false  }]
}

Where includeCommonJSExports is false by default and true to enable. I think that's a lot clearer and also disambiguates between CommonJS modules and ES modules.

Sorry @mysticatea , I think the filename match doesn't belong in this rule. :)

[mysticatea]

Sorry @mysticatea , I think the filename match doesn't belong in this rule. :)

I'm OK.
Just one point, includeCommonJSExports sounds that it will change the behavior of both module.exports and exports.foo. What about includeCommonJSDefaultExports or includeCommonJSModuleExports?