feat(lint/noRestrictedImports): add rule

[anonrig]

Adds no-restricted-imports rule to Biome.

[netlify]

✅ Deploy Preview for biomejs ready!

Name	Link
🔨 Latest commit	38d464c
🔍 Latest deploy log	https://app.netlify.com/sites/biomejs/deploys/65c24d3c55056e00087c0ea5
😎 Deploy Preview	https://deploy-preview-1723--biomejs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 99 (🟢 up 4 from production) Accessibility: 97 (no change from production) Best Practices: 100 (no change from production) SEO: 93 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codspeed-hq]

CodSpeed Performance Report

Merging #1723 will not alter performance

_{Comparing anonrig:add-no-restricted-imports (38d464c) with main (7823cee)}

Summary

✅ 93 untouched benchmarks

[arendjr]

I like this rule, but would it be possible to merge this with useImportRestrictions? It means the logic for the rule should also be put behind a config option, but given the naming I think it would be too confusing to have them both. I think I would call the option for this rule something like bannedModules if they were combined.

I’m not sure how to put the config inside the rule documentation unfortunately…

[arendjr]

Btw, we’ve had some discussion in the past about where we want to go with the useImportRestrictions rule, which can still be found over here: rome/tools#4678

Unfortunately most of the ideas got blocked on needing project-level analysis (on the roadmap for 2024 now), so only the package private logic ended up being implemented. And there’s no config yet, since we only have one option implemented :)

So if you’re okay with adding bannedModules to the configuration for useImportRestrictions, then we can put the existing logic behind the packagePrivate option we discussed there. It’s fine to make ”none” the default, and setting it to ”all” would enable the existing logic in that rule.

[Conaclos]

Unfortunately, we don't support setting options in example. We could just document the option. Any idea @ematipico ?

[ematipico]

It could be an excellent addition and maybe a good first issue. As for now, we only document the options.

[ematipico]

This rule, without options, does nothing. This means that the options are mandatory for this rule, and it is in contrast with all the rules we have.

Which means that if I enabled this rule like this:

{
	"noRestrictedImports": "warn"
}

Biome is happy, but the rule is never triggered. I have mixed feelings about creating a rule that only works if options are passed. And our infrastructure/deserialization wasn't designed for this case.

I like @arendjr suggestion. We could merge the options/logic of this rule to useImportRestrictions, although the options of this rules have a different objective compared to useImportRestrictions. The latter prevents the use of sub-paths.

So here my proposal:

1. Merge this rule
2. Update useImportRestrictions to accept two options:
   • bannedModules: the options of this rule
   • allowPrivateImports: allows to turn off the main logic of useImportRestrictions
3. Add the logic of this rule to useImportRestrictions
4. Delete noRestrictedImports

@anonrig @arendjr @Conaclos, how does that sound?

@anonrig if you like the idea, would like to take charge of the proposal?

[arendjr]

@ematipico Sounds like a plan! 👍

We may just need to bikeshed a bit about the naming for the allowPrivateImports option :) I do think it should at least remain an enum, so we can add a third option to support package private annotations when we have the infra to analyze annotations from imports.

[ematipico]

Always open to bikeshed names! I'm bad at that

[anonrig]

I personally think we should have 1-to-1 correlation when we are migrating existing eslint rules but I'm ok with merging it with another rule as well.

[ematipico]

Looks good, just a change in the documentation and a nit about the code

[Conaclos]

This rule, without options, does nothing. This means that the options are mandatory for this rule, and it is in contrast with all the rules we have.

Which means that if I enabled this rule like this:

{
	"noRestrictedImports": "warn"
}

Biome is happy, but the rule is never triggered. I have mixed feelings about creating a rule that only works if options are passed. And our infrastructure/deserialization wasn't designed for this case.

I like @arendjr suggestion. We could merge the options/logic of this rule to useImportRestrictions, although the options of this rules have a different objective compared to useImportRestrictions. The latter prevents the use of sub-paths.

So here my proposal:

1. Merge this rule

2. Update `useImportRestrictions` to accept two options:
   
   * `bannedModules`: the options of **this** rule
   * `allowPrivateImports`: allows to turn **off** the main logic of `useImportRestrictions`

3. Add the logic of **this** rule to `useImportRestrictions`

4. Delete `noRestrictedImports`

@anonrig @arendjr @Conaclos, how does that sound?

@anonrig if you like the idea, would like to take charge of the proposal?

I have mixed feeling about merging the two rules together. Maybe the name useImportRetrictions is not enough informative? I don't remember the rationals behind useImportRetrictions. I need more time to think about that. In the meantime we could merge this PR. The rule is a nursery rule, we could still change it / merge it with useImportRetrictions before the 1.6 release or in a future release.

[anonrig]

I think this PR is ready to merge. Regarding merging with another rule, we can make that decision later since this is a Nursery linting rule

[yann-combarnous]

Nice, any plan to also support the patterns option from https://eslint.org/docs/latest/rules/no-restricted-imports?

[Conaclos]

@yann-combarnous

Nice, any plan to also support the patterns option from https://eslint.org/docs/latest/rules/no-restricted-imports?

I plan to add this in a future version.