fix(lib): Diff of array should consider both sides

[ta2edchimp]

At the moment, lib/array-diff.js in my opinion return an erroneous diff:

//          "a"           , "b"
difference(['a', 'b', 'c'], ['a', 'y', 'z']) == ['b', 'c']

It should also take those elements of array b not present in array a into account:

//          "a"           , "b"
difference(['a', 'b', 'c'], ['a', 'y', 'z']) == ['b', 'c', 'y', 'z']

[codecov-io]

Current coverage is 100.00%

Merging #46 into master will not affect coverage as of 2f55840

@@            master     #46   diff @@
======================================
  Files            5       5       
  Stmts          125     132     +7
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit            125     132     +7
  Partial          0       0       
  Missed           0       0       

Review entire Coverage Diff as of 2f55840

Powered by Codecov. Updated on successful CI builds.

[kentcdodds]

LGTM

[sarbbottam]

I had considered the proposed implementation (absolute difference) but let me say why I have considered the current implementation (relative difference).

Let's suppose, my current config is ['a', 'b', 'x', 'y', 'z'] and the all available config is ['a', 'b', 'c', 'd', 'e']
so the unused rule should suggest

// diff(all-available, current) => [ only the rules present in all-available but no in current]
diff(['a', 'b', 'c', 'd', 'e'], ['a', 'b', 'x', 'y', 'z']) => [ 'c', 'd', 'e']

and not [ 'x', 'y', 'z', 'c', 'd', 'e'] as ['x', 'y', 'z'] is part of my current config (and non existent in all available rules).

If we display [ 'x', 'y', 'z', 'c', 'd', 'e'] as the list of unused rules, user might get confused why ['x', 'y', 'z'] is being displayed as unused, when it is part of the current config.

However, we might

// diff(current, all-available) => [ only the rules present in current but no in all-available]
diff(['a', 'b', 'x', 'y', 'z'], ['a', 'b', 'c', 'd', 'e']) => ['x', 'y', 'z']

and display that ['x', 'y', 'z'] is present in current config, but does not have any corresponding implementation in eslint or current eslint-plugins

---

_.difference also has the same logic.

_.difference([3, 2, 1], [4, 2]);
// → [3, 1]

Please correct me if I have misinterpreted.

[ta2edchimp]

Hmm... I now got what you were doing and considering an invokation of eslint-find-rules I totally agree. But as soon, as you're comparing two ESLint configs (using eslint-diff-rules), I'm convinced that the user expects the result of my approach.

What about this:
We leave array-diff as it is for use in context of eslint-find-rules, but extend it by an optional third parameter (type bool), that determines whether to compare in my proposal's two-way fashion?
This way, eslint-find-rules (diffing against currently available rules) stays functional as it is atm, while eslint-diff-rules (diffing two actual configurations) comprehensively compares against each other.
Do you know what I mean?

[sarbbottam]

But as soon, as you're comparing two ESLint configs (using eslint-diff-rules), I'm convinced that the user expects the result of my approach.

Yes, something like vim diff or git diff, it stroke me when using eslint-diff-rules as a user.

but extend it by an optional third parameter (type bool), that determines whether to compare in my proposal's two-way fashion

I doubt one concise list will be useful, we we cant label the difference with each file, similar to vim diff or git diff.

Why not call diff(rule-set-one, rule-set-two) followed by diff(rule-set-two, rule-set-one) and display the result accordingly.

If your are still convinced with a concise list of absolute diff, you could do

diff(rule-set-one, rule-set-two).concat(diff(rule-set-two, rule-set-one))

[ta2edchimp]

What about a totally different approach?
For #47 I changed the output of eslint-diff-rules config1 config2 --verbose to the following format:

             config1      config2
foo-rule     [a, b, c]    [a, b]
bar-rule     [a, b]       undefined
baz-rule     undefined    [a, b, c]

For the "non-verbose"/default output of eslint-diff-rules, we could do something similar, or (I'm liking that more and more while typing) a grouped approach (I think, this may be what you're thinking of?):

diff rules

in config1, but not in config2:
foo-rule, bar-rule

in config2, but not in config1:
baz-rule

(The meaning of the output between verbose and default output would differ, nonetheless. The default output would only include those rules, set in one config, but not at all in the other; while the verbose output has the imo crucial detail of showing exactly where the two configs differ.)

[sarbbottam]

Sorry I don't follow.

while the verbose output has the imo crucial detail of showing exactly where the two configs differ.

with respect to

             config1      config2
foo-rule     [a, b, c]    [a, b]
bar-rule     [a, b]       undefined
baz-rule     undefined    [a, b, c]

What is [a, b, c] [a, b] corresponding to the foo-rule?

Is it rule config?

...
//"foo-rule"  :  [a, b]
  "quotes"    :  [0, "single"]
...

Am I reading it correct?

[ta2edchimp]

Sorry, it's sometimes hard for me to be presice due to a lack of vocabulary 😜

---

What is [a, b, c] [a, b] corresponding to the foo-rule?

Is it rule config?

...
//"foo-rule"  :  [a, b]
  "quotes"    :  [0, "single"]
...

Exactly.

[sarbbottam]

No worries!! Awesome, it would be great.

Is this PR still good or #47 should take take of it?

[ta2edchimp]

Is this PR still good?

I think this one's bonkers.

I'd say, we extend #47 to include your proposal:

Why not call diff(rule-set-one, rule-set-two) followed by `diff(rule-set-two, rule-set-one) and display the result accordingly.

and eventually we'd have a consistent behavior accross the two output levels of eslint-diff-rules 👍

[ta2edchimp]

Superceded by #50 (feat(diff): compare both ways).