Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add importNames support for patterns in no-restricted-imports #16059

Merged
merged 7 commits into from Jul 2, 2022
Merged

feat: add importNames support for patterns in no-restricted-imports #16059

merged 7 commits into from Jul 2, 2022

Conversation

brandongregoryscott
Copy link
Contributor

Prerequisites checklist

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:

Resolves #14274

What changes did you make? (Give an overview)

This change expands the functionality of no-restricted-imports to allow restricting only specific named imports from an import pattern ban. For example, if I wanted to ban importing Foo from '../../../my/relative/module', but importing Bar from the matched pattern is OK, I can now do that:

"no-restricted-imports": ["error", {            
    "patterns": [{
        "group": ["**/my/relative-module"],
        "importNames": ["Foo"],
        "message": "Don't import Foo from my/relative-module. Instead import from @repo/foo"
    }]
}]
import { Foo } from '../../../my/relative-module' // error
import { Foo, Bar } from '../../../my/relative-module' // error
import { Bar } from '../../../my/relative-module' // ok

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

Nope - this is my first time contributing here, so please forgive me if I've missed anything - happy to update as needed!

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label Jun 26, 2022
@eslint-github-bot
Copy link

Hi @brandongregoryscott!, 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'.

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 26, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: brandongregoryscott / name: Brandon Scott (ba96189)

@netlify
Copy link

netlify bot commented Jun 26, 2022

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit fdd0dc1
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/62bdd6bdd99aee000837df59

@eslint-github-bot
Copy link

Hi @brandongregoryscott!, 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 length of the commit message must be less than or equal to 72

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

@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Jun 26, 2022
@brandongregoryscott brandongregoryscott changed the title feat(no-restricted-imports): add importNames functionality for patterns feat: add importNames support for restricted import patterns Jun 26, 2022
@mdjermanovic mdjermanovic added rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jun 27, 2022
@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jun 28, 2022
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I left two comments.

Also, this new feature should be documented in docs/src/rules/no-restricted-imports.md

lib/rules/no-restricted-imports.js Show resolved Hide resolved
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
@mdjermanovic mdjermanovic changed the title feat: add importNames support for restricted import patterns feat: add importNames support for patterns in no-restricted-imports Jun 29, 2022
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
docs/src/rules/no-restricted-imports.md Show resolved Hide resolved
@brandongregoryscott
Copy link
Contributor Author

I pushed up that change @mdjermanovic, feel free to take a look when you have time! I am noticing there's some duplicated logic in here, but I'm not sure if it's a concern right now.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small suggestions and then LGTM.

lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
tests/lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
tests/lib/rules/no-restricted-imports.js Show resolved Hide resolved
@mdjermanovic
Copy link
Member

I am noticing there's some duplicated logic in here, but I'm not sure if it's a concern right now.

Yes, the code that handles patterns now looks very similar to the code that handles paths, but that's okay right now. At some point later we could consider refactoring.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

I'll leave this open if someone else wants to review it before merging.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks, and congrats on your first ESLint contribution, @brandongregoryscott! This will be going out in the release shortly.

@btmills btmills merged commit 7023628 into eslint:main Jul 2, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Jul 4, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.18.0` -> `8.19.0`](https://renovatebot.com/diffs/npm/eslint/8.18.0/8.19.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.19.0`](https://github.com/eslint/eslint/releases/tag/v8.19.0)

[Compare Source](eslint/eslint@v8.18.0...v8.19.0)

#### Features

-   [`7023628`](eslint/eslint@7023628) feat: add importNames support for patterns in no-restricted-imports ([#&#8203;16059](eslint/eslint#16059)) (Brandon Scott)
-   [`472c368`](eslint/eslint@472c368) feat: fix handling of blockless `with` statements in indent rule ([#&#8203;16068](eslint/eslint#16068)) (Milos Djermanovic)

#### Bug Fixes

-   [`fc81848`](eslint/eslint@fc81848) fix: throw helpful exception when rule has wrong return type ([#&#8203;16075](eslint/eslint#16075)) (Bryan Mishkin)

#### Documentation

-   [`3ae0574`](eslint/eslint@3ae0574) docs: Remove duplicate rule descriptions ([#&#8203;16052](eslint/eslint#16052)) (Amaresh  S M)
-   [`f50cf43`](eslint/eslint@f50cf43) docs: Add base href to each page to fix relative URLs ([#&#8203;16046](eslint/eslint#16046)) (Nicholas C. Zakas)
-   [`ae4b449`](eslint/eslint@ae4b449) docs: make logo link clickable on small width screens ([#&#8203;16058](eslint/eslint#16058)) (Milos Djermanovic)
-   [`280f898`](eslint/eslint@280f898) docs: use only fenced code blocks ([#&#8203;16044](eslint/eslint#16044)) (Milos Djermanovic)
-   [`f5d63b9`](eslint/eslint@f5d63b9) docs: add listener only if element exists ([#&#8203;16045](eslint/eslint#16045)) (Amaresh  S M)
-   [`8b639cc`](eslint/eslint@8b639cc) docs: add missing migrating-to-8.0.0 in the user guide ([#&#8203;16048](eslint/eslint#16048)) (唯然)
-   [`b8e68c1`](eslint/eslint@b8e68c1) docs: Update release process ([#&#8203;16036](eslint/eslint#16036)) (Nicholas C. Zakas)
-   [`6d0cb11`](eslint/eslint@6d0cb11) docs: remove table of contents from markdown text ([#&#8203;15999](eslint/eslint#15999)) (Nitin Kumar)

#### Chores

-   [`e884933`](eslint/eslint@e884933) chore: use `github-slugger` for markdown anchors ([#&#8203;16067](eslint/eslint#16067)) (Strek)
-   [`02e9cb0`](eslint/eslint@02e9cb0) chore: revamp carbon ad style ([#&#8203;16078](eslint/eslint#16078)) (Amaresh  S M)
-   [`b6aee95`](eslint/eslint@b6aee95) chore: remove unwanted comments from rules markdown ([#&#8203;16054](eslint/eslint#16054)) (Strek)
-   [`6840940`](eslint/eslint@6840940) chore: correctly use .markdownlintignore in Makefile ([#&#8203;16060](eslint/eslint#16060)) (Bryan Mishkin)
-   [`48904fb`](eslint/eslint@48904fb) chore: add missing images ([#&#8203;16017](eslint/eslint#16017)) (Amaresh  S M)
-   [`910f741`](eslint/eslint@910f741) chore: add architecture to nav ([#&#8203;16039](eslint/eslint#16039)) (Strek)
-   [`9bb24c1`](eslint/eslint@9bb24c1) chore: add correct incorrect in all rules doc ([#&#8203;16021](eslint/eslint#16021)) (Deepshika S)
-   [`5a96af8`](eslint/eslint@5a96af8) chore: prepare versions data file ([#&#8203;16035](eslint/eslint#16035)) (Nicholas C. Zakas)
-   [`50afe6f`](eslint/eslint@50afe6f) chore: Included githubactions in the dependabot config ([#&#8203;15985](eslint/eslint#15985)) (Naveen)
-   [`473411e`](eslint/eslint@473411e) chore: add deploy workflow for playground ([#&#8203;16034](eslint/eslint#16034)) (Milos Djermanovic)
-   [`a30b66c`](eslint/eslint@a30b66c) chore: fix print style ([#&#8203;16025](eslint/eslint#16025)) (Amaresh  S M)
-   [`f4dad59`](eslint/eslint@f4dad59) chore: add noindex meta tag ([#&#8203;16016](eslint/eslint#16016)) (Milos Djermanovic)
-   [`db387a8`](eslint/eslint@db387a8) chore: fix sitemap ([#&#8203;16026](eslint/eslint#16026)) (Milos Djermanovic)
-   [`285fbc5`](eslint/eslint@285fbc5) chore: remove TOC from printable ([#&#8203;16020](eslint/eslint#16020)) (Strek)
-   [`8e84c21`](eslint/eslint@8e84c21) chore: remove ligatures from fonts ([#&#8203;16019](eslint/eslint#16019)) (Strek)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1444
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Dec 30, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

no-restricted-imports should support importNames on patterns exclusions
3 participants