New rule proposal: no-setter-return

[mdjermanovic]

Please describe what the rule should do:

no-setter-return disallows returning values from setter functions.

Reports return statement with an argument inside a setter. Targets setters in object literals, classes and property descriptors (4 well-known functions).

What category of rule is this? (place an "X" next to just one item)

[X] Warns about a potential error (problem)

Provide 2-3 code examples that this rule will warn about:

const foo = {
    set a(val) { 
        this._a = val; 
        return val; 
    }
}

class foo {
    set a(val) { 
        this._a = val * 100; 
        return this._a; 
    }
    static set b(val) {
        doSomething(val);
        return true;
    }
}

Object.defineProperty(obj, "foo", { 
    set(val) { 
        return 5 * val; 
    }
});

// also Object.defineProperties, Object.create, Reflect.defineProperty

Why should this rule be included in ESLint (instead of a plugin)?

Returning a value from a setter is either unnecessary or a possible error in logic (intention to somehow use the returned value when it's different).

As far as I know, even if there is an intention to use the value returned from a setter, it's simply impossible in ES. E.g., a.prop = foo, a.prop += foo, ++a.prop etc. do not use values returned from setters to evaluate the whole expression.

I think that just return; without value should be allowed, as it can be used for control flow.
Returning any value should be disallowed.

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

Yes.

[kaicataldo]

I support this! One thing to consider - is it worth creating a separate smaller rule, or to try to combine this with https://eslint.org/docs/rules/getter-return into a new rule (setter-getter-return)?

[mdjermanovic]

Cases like this should be also reported, so it isn't technically just return statement:

Object.defineProperty(obj, "foo", { 
    set: val => val 
});

[mdjermanovic]

I support this! One thing to consider - is it worth creating a separate smaller rule, or to try to combine this with https://eslint.org/docs/rules/getter-return into a new rule (setter-getter-return)?

That's an interesting idea! I didn't think about it. Both rules work with accessors, though require the opposite things, I'm not sure what would be better from a user's perspective.

[kaicataldo]

Just thought it was worth discussing as we're figuring it out!

Having thought about it a bit more, I think I'm leaning towards having a separate rule (i.e. using the current proposal). Smaller, focused rules tend to be easier to understand from a configuration perspective, though they might increase the overhead a little when they're first being added to the user's config (reading multiple rule doc pages, etc.).

[ilyavolodin]

@mdjermanovic Second example is also using return, it's just shortcut syntax, but it's still a return. So I think the name is fine.

[mdjermanovic]

I'm working on this, should be ready soon.