fix(no-duplicate): fix no-duplicate false positive on side-effect + type imports

[todor-a]

Fixes #3130.

[ljharb]

the way this is written, it is literally always true, because nodes.length is > 1, and everything in it is in one of the three groups (since the "value" condition is an "else"). is that intentional?

[todor-a]

(since the "value" condition is an "else")

not sure i got this. my thinking was - if there are duplicates and one of them is a type import, while the other is a side effect, do not err

[ljharb]

ahhhhh you're right, this is === 0. ok, so it seems like maybe this would work as well?

const kinds = Object.groupBy(nodes, (node) => node.importKind === 'type' ? 'type' : node.specifiers.length === 0 ? 'sideEffect' : 'other');
if (kinds.type.length > 0 && kinds.sideEffect.length > 0 && kinds.other.length === 0) {
  continue;
}

(ofc Object.groupBy would need to use https://npmjs.com/object.groupby instead, but that's still slightly more efficient)

and an even simpler approach:

var hasType = false;
var hasSideEffect = false;
var hasOther = false;
for (var i = 0; !hasOther && i < nodes.length; i += 1) {
  var node = nodes[i];
  if (node.importKind === 'type') {
    hasType = true;
  } else if (node.specifiers.length === 0) {
    hasSideEffect = true;
  } else {
    hasOther = true;
  }
}
if (!hasOther && hasType && hasSideEffect) {
  continue;
}

that way uses no deps, breaks as soon as it finds an invalidating condition, and avoids creating three arrays.

[ljharb]

let's also add a similar invalid test, with output?

[todor-a]

do you mean this:

import { A } from 'a';
import 'a';

?

[ljharb]

i think more like

import type { A } from 'a';
import B from 'a';

with prefer-inline set to true?

[todor-a]

ah, got it, will add it

[todor-a]

This fails but I think for the wrong reasons and is a different issue. It currently suggests:

import B, type { A } from 'a';

which is syntactically wrong

[ljharb]

I agree, I think that's part of #3195 perhaps? It'd be cool to fix that too here :-)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.61%. Comparing base (01c9eb0) to head (4045eed).

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #3194       +/-   ##
===========================================
+ Coverage   82.25%   95.61%   +13.36%     
===========================================
  Files          94       83       -11     
  Lines        4283     3696      -587     
  Branches     1478     1336      -142     
===========================================
+ Hits         3523     3534       +11     
+ Misses        760      162      -598     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.