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

Update: no-restricted-imports custom message for patterns (fixes #11843) #14580

Merged

Conversation

lexholden
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 autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

As per the spec agreed upon in #11843 - the rule no-restricted-imports now allows multiple patterns to be grouped together with a message that is logged if any match fails

For example, { patterns: [{ group: ["foo/*", "bar/*"], message: "'foo' and 'bar' are deprecated, please use 'baz' instead." }] } would log error 'foo/some/subimport' import is restricted from being used by a pattern. 'foo' and 'bar' are deprecated, please use 'baz' instead.

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

Wrote this for a personal project, but happy to tweak anything required to shepherd it through any part of the community process that I've missed from the contributor guidelines or add some more robust test edge cases if proposed.

@eslint-github-bot eslint-github-bot bot added the triage An ESLint team member will look at this issue soon label May 8, 2021
@eslint-github-bot
Copy link

Hi @lexholden!, 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 must be one of the following:

    The Tag is one of the following:

    • Fix - for a bug fix.
    • Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
    • New - implements a new feature.
    • Breaking - for a backwards-incompatible enhancement or feature.
    • Docs - changes to documentation only.
    • Build - changes to build process only.
    • Upgrade - for a dependency upgrade.
    • Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).

    You can use the labels of the issue you are working on to determine the best tag.

  • There should be a space following the initial tag and colon, for example 'New: Message'.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@eslint-github-bot
Copy link

Hi @lexholden!, 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 issue reference must be formatted as follows:

    If the pull request addresses an issue, then the issue number should be mentioned at the end. If the commit doesn't completely fix the issue, then use (refs #1234) instead of (fixes #1234).

    Here are some good commit message summary examples:

    Build: Update Travis to only test Node 0.10 (refs #734)
    Fix: Semi rule incorrectly flagging extra semicolon (fixes #840)
    Upgrade: Esprima to 1.2, switch to using comment attachment (fixes #730)
    

Read more about contributing to ESLint here

@lexholden lexholden force-pushed the issue11843-no-restricted-import-messages branch from 57af49c to 147ad2e Compare May 8, 2021 20:01
@eslint-github-bot
Copy link

Hi @lexholden!, 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

Read more about contributing to ESLint here

@lexholden lexholden force-pushed the issue11843-no-restricted-import-messages branch from 147ad2e to 2a56777 Compare May 8, 2021 20:01
@aladdin-add aladdin-add added accepted There is consensus among the team that this change meets the criteria for inclusion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels May 10, 2021
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.

@lexholden thanks for the PR! This looks very good, but the schema needs some modifications. I also left some suggestions about the docs and tests.

docs/rules/no-restricted-imports.md Outdated Show resolved Hide resolved
docs/rules/no-restricted-imports.md Outdated Show resolved Hide resolved
docs/rules/no-restricted-imports.md Outdated Show resolved Hide resolved
lib/rules/no-restricted-imports.js Show resolved Hide resolved
lib/rules/no-restricted-imports.js Outdated Show resolved Hide resolved
tests/lib/rules/no-restricted-imports.js Show resolved Hide resolved
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@eslint-github-bot
Copy link

Hi @lexholden!, thanks for the Pull Request

The pull request title 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 must be one of the following:

    The Tag is one of the following:

    • Fix - for a bug fix.
    • Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
    • New - implements a new feature.
    • Breaking - for a backwards-incompatible enhancement or feature.
    • Docs - changes to documentation only.
    • Build - changes to build process only.
    • Upgrade - for a dependency upgrade.
    • Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).

    You can use the labels of the issue you are working on to determine the best tag.

  • There should be a space following the initial tag and colon, for example 'New: Message'.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@eslint-github-bot
Copy link

Hi @lexholden!, thanks for the Pull Request

The pull request title 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 must be one of the following:

    The Tag is one of the following:

    • Fix - for a bug fix.
    • Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
    • New - implements a new feature.
    • Breaking - for a backwards-incompatible enhancement or feature.
    • Docs - changes to documentation only.
    • Build - changes to build process only.
    • Upgrade - for a dependency upgrade.
    • Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).

    You can use the labels of the issue you are working on to determine the best tag.

  • There should be a space following the initial tag and colon, for example 'New: Message'.

  • The length of the commit message must be less than or equal to 72

Read more about contributing to ESLint here

@lexholden lexholden changed the title Updated no-restricted-imports rule to allow custom messages for patterns per spec in #11843 Update: no-restricted-imports rule with custom messages (fixes #11843) May 19, 2021
@lexholden
Copy link
Contributor Author

@mdjermanovic thanks for the excellent suggestions, agree with all of them and pushed the changed. Let me know if you think of anything else.

@mdjermanovic mdjermanovic linked an issue May 20, 2021 that may be closed by this pull request
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.

This looks great, I just missed in the first review that we should also add to docs some examples of incorrect/correct code with the new configuration.

It can be something like this

incorrect:

/*eslint no-restricted-imports: ["error", { patterns: [{
    group: ["lodash/*"],
    message: "Please use the default import from 'lodash' instead."
}]}]*/

import pick from 'lodash/pick';

correct:

/*eslint no-restricted-imports: ["error", { patterns: [{
    group: ["lodash/*"],
    message: "Please use the default import from 'lodash' instead."
}]}]*/

import lodash from 'lodash';

@lexholden
Copy link
Contributor Author

@mdjermanovic good point. Done.

@mdjermanovic mdjermanovic changed the title Update: no-restricted-imports rule with custom messages (fixes #11843) Update: no-restricted-imports custom message for patterns (fixes #11843) May 20, 2021
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!

@mdjermanovic mdjermanovic merged commit 52655dd into eslint:master May 21, 2021
@mdjermanovic
Copy link
Member

Thanks for contributing!

@maxisam
Copy link

maxisam commented May 21, 2021

Thank you @lexholden This is a great feature!

topheman added a commit to topheman/nextjs-github-browser that referenced this pull request May 24, 2021
Update needed to get the fix for "no-restricted-imports custom message for patterns"

See PR : eslint/eslint#14580

Now we can add a custom message for */generated/graphql patterns.
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Nov 18, 2021
@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 Nov 18, 2021
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 rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for "message" with "patterns" in no-restricted-imports
4 participants