no-self-assign failing for property assignment

[Zzzen]

What version of ESLint are you using?
3.1.1
What parser (default, Babel-ESLint, etc.) are you using?
Default
Please show your full configuration:
{ "env": { "browser": true }, "parserOptions": { "ecmaVersion": 6 }, "rules": { "no-self-assign": "error" } }
What did you do? Please include the actual source code causing the issue.

var a = 123;
a = a;

var b = { a };
b.a = b.a; 

What did you expect to happen?
I would expect b.a = b.a to be a error;
What actually happened? Please include the actual, raw output from ESLint.
Only a.a is an error.

2:5 error 'a' is assigned to itself no-self-assign

✖ 1 problem (1 error, 0 warnings)

[platinumazure]

Hmm... Seems like MemberExpressions just aren't being checked, even though a lot of other kinds of self-assignments are.

@mysticatea Is there a reason MemberExpressions aren't checked in this rule, or was it maybe just an oversight?

[mysticatea]

Thank you for this issue.
I think simply I had forgotten MemberExpressions.

Granted, property accesses can trigger side-effects, but I think that a use of self-assignments in order to trigger side-effects is a corner case.

[nzakas]

Are we sure that this should be a default behavior? As mentioned, properties can have getters/setters, so we need to be certain that this makes sense as a default.

And since this is flagging something that wasn't flagged before, this should be an enhancement "Update".

[mysticatea]

I removed accepted label by this is an enhancement. I will move forward my PR after this issue is accepted.

I think it's good for default behavior since it does not expect that self-assignments have side-effects, normally.

[nzakas]

I think it's good for default behavior since it does not expect that self-assignments have side-effects, normally.

My question is, can we assume that? I'm not sure we can.

[platinumazure]

Can we put it behind a rule option so users can make that call?

On Jul 21, 2016 11:12 AM, "Nicholas C. Zakas" notifications@github.com
wrote:

I think it's good for default behavior since it does not expect that
self-assignments have side-effects, normally.

My question is, can we assume that? I'm not sure we can.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6718 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AARWemfF03LpMmHU-ynqSqglzbW_cmdtks5qX5phgaJpZM4JQXbi
.

[ilyavolodin]

I think I would prefer this behind an option as well. Just to be on a safe side. Maybe the option can be on by default, but at least that way users will have a way to turn it off.

[nzakas]

For best backwards compatibility, I'd suggest having the option off by default and allow people to opt-in.

[platinumazure]

👍 to @nzakas' proposal.

[mysticatea]

Hmm, OK.
I agreed we need an option (maybe {props: true}), false by default.

[nzakas]

That sounds good. Since this is an enhancement now, we need one more 👍

[ilyavolodin]

👍