feat: no-restricted-imports support casing

[gfyoung]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What rule do you want to change?

no-restricted-imports

Does this change cause the rule to produce more or fewer warnings?

By default, the amount of warnings is unchanged. Otherwise, it should be fewer warnings.

How will the change be implemented? (New option, new default behavior, etc.)?

A new option is added, with a default value that maintains backwards-compatibility.

Please provide some example code that this change will affect:

import 'food';
import 'fooBar';

"no-restricted-imports": ["error", {
    "patterns": [{
      "group": ["foo[A-Z]*"]
    }]
}]

What does the rule currently do for this code?

It will issue warnings for both imports.

What will the rule do after it's changed?

If we add the new option:

"no-restricted-imports": ["error", {
    "patterns": [{
      "group": ["foo[A-Z]*"],
      "caseSensitive": true
    }]
}]

The rule will issue a warning ONLY for the second import.

What changes did you make? (Give an overview)

Enhances no-restricted-imports to support more granular import restrictions via case-sensitive pattern matching. By default, this rule remains case-insensitive to maintain backwards compatibility with earlier versions. Only by explicitly specifying it in a custom pattern object (see the added docs) can it be triggered.

Path matching in .gitignore can be case-sensitive, and [email protected] supports this as well as maintains support for relative paths, whose support was dropped after 4.0.6 but restored in 5.2.0 (this was a blocker for past upgrades).

Is there anything you'd like reviewers to focus on?

N/A

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ gfyoung (dc3b219)

[eslint-github-bot]

Hi @gfyoung!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

• The commit message tag wasn't recognized. Did you mean "docs", "fix", or "feat"?
• There should be a space following the initial tag and colon, for example 'feat: Message'.
• The first letter of the tag should be in lowercase

To Fix: You can fix this problem by running git commit --amend, editing your commit message, and then running git push -f to update this pull request.

Read more about contributing to ESLint here

[mdjermanovic]

The option makes sense to me 👍, and it would be nice to update the ignore dependency.

Waiting for more opinions from team members, since this is an enhancement.

[gfyoung]

@mdjermanovic: PR has been updated with the requested changes, with tests still passing. 👍

[btmills]

This seems reasonable, but before adding new API, I find it helpful to understand the motivation. @gfyoung can you elaborate a bit on what use case you're trying to accomplish with case-sensitive path patterns?

[gfyoung]

@btmills: I believe the use case described in the PR description illustrates the motivation.

If not, let me know what further information or detail is required.

[btmills]

@gfyoung the PR description covered a lot of the how but not much of the why. What are you hoping to do with ESLint that you can't today?

[gfyoung]

What are you hoping to do with ESLint that you can't today?

I want to be able to do more granular import restrictions using case-sensitive regular expressions.

[btmills]

Okay, and what are you hoping to use case-sensitive import restrictions for?

[gfyoung]

The example provided in the PR description illustrates a case where I would want to use case-sensitive import restrictions.

[nzakas]

@gfyoung the problem is that your description looks like a theoretical problem and not a real-life use case. What @btmills is asking for is something more concrete. What situation did you run into where this option would be helpful?

[gfyoung]

the problem is that your description looks like a theoretical problem and not a real-life use case.

@nzakas: First off, expanding the API for the sake of a theoretical problem would be a waste of time for all of us. I have been waiting to submit this PR for quite some time, as I had to get commits to ignore merged and released first.

Without sharing actual code, I thought the example using foo would be minimal and sufficient to explain the situation where this option would be helpful. However, if you would like to see more realistic import names, here is an example, without sharing actual code, that should help to provide something more concrete:

import * as bots from bots;
import * as botUtils from botUtils;   // TODO: all imports should be from `bots`

In this case, we want all developers to importing from the bots file and not any of its constituent bot[A-Z]* files, which are in the process of being refactored and moved, but this will take quite some time to complete.

[nzakas]

@gfyoung please understand, we review a lot of PRs, and not all of them are well thought out. We need to ask followup questions and for more examples in order to figure out which ones are worth pursuing and which don’t provide much value to the project.

Answering our questions helps a lot, and if the PR description was enough, we wouldn’t ask for more info.

[nzakas]

Looks good, just a couple of suggestions.

[gfyoung]

@nzakas: I understand the need to request clarification. As a maintainer myself, I can relate to the experience of working through PRs that have not been as well thought out as we would like. That said, suggesting that someone's PR description is a "theoretical problem" before knowing more (e.g., not wanting to divulge actual code) is where some umbrage was taken. IMO prefer to give contributors benefit of the doubt to prove themselves before coming to a conclusion about the PR.

Anyhow, I'll leave it at that. Appreciate the feedback so far. 👍

I was able to address one of your feedback points but ran into issues with the other. See my comments in the conversations.

[mdjermanovic]

Path matching in .gitignore can be case-sensitive, and [email protected] supports this as well as maintains support for relative paths, whose support was dropped after 4.0.6 but restored in 5.2.0 (this was a blocker for past upgrades)

Question: The ignorecase option we need for this PR was already available in 4.0.6, but we must upgrade to 5.2.0 because of the caching bug that was fixed by kaelzhang/node-ignore@f2f67cb? (I noticed that the invalid test fails with 4.0.6, but works when the valid test is removed).

[gfyoung]

Yes, that is in part why (it's one of several patches I submitted to the library to make this upgrade here possible).

[mdjermanovic]

LGTM if accepted, thanks!

[nzakas]

@gfyoung Im sorry that I didn’t explain well enough. We get a lot of people suggesting rules or changes to rules that are, in fact, theoretical or unlikely enough that it doesn’t make sense to accept. I did not mean to imply that your use case was theoretical, only that we needed more justification. Maintaining all of our rules, and ensuring things like rules don’t contradict each other, takes a lot work, so while we always assume positive intent from contributors (as we did here), we can’t assume that every case should be merged, which is why we asked for more details.

[nzakas]

@btmills do you have the info you need here?

[aladdin-add]

LGTM, thanks! 💯

[btmills]

LGTM! Thanks for the additional explanation.

[gfyoung]

Fantastic! Thank you for all the feedback @btmills @mdjermanovic @nzakas 🙏