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

Use banned-api for deprecation checks in general #1507

Open
not-my-profile opened this issue Dec 31, 2022 · 1 comment
Open

Use banned-api for deprecation checks in general #1507

not-my-profile opened this issue Dec 31, 2022 · 1 comment
Labels
rule Implementing or modifying a lint rule

Comments

@not-my-profile
Copy link
Contributor

not-my-profile commented Dec 31, 2022

The checks for uses of deprecated standard library features are currently split up across different rules, for example:

  • UP019: typing.Text is deprecated, use str
  • PGH002: warn is deprecated in favor of warning

Since there are many more deprecated things in the standard library (e.g. the 24 superseded modules), I think it would make more sense to define these checks via the recently introduced banned-api mechanism rather than introducing separate checks for every single thing that is deprecated in the standard library. And I think we could reuse the flexible configuration mechanism we already have for rules.

In particular I suggest the following to be a default config:

[tool.ruff.banned-api.select]
"typing.Text" = {msg = "deprecated", autofix-to = "str"}
"logging.warn" = {msg = "deprecated", autofix-to = "logging.warning"}
"optparse" = {msg = "deprecated, use `argparse` instead"}
# etc ... other 23 deprecated stdlib modules

And allow users to either:

  • disable these checks completely via tool.ruff.banned-api.select = {}
  • ignore specific errors via e.g. tool.ruff.banned-api.ignore = ["optparse"]
  • ban additional API via tool.ruff.banned-api.extend-select

Since this would make banned-api more of a core feature I think it would make sense to move the setting out of the flake8-tidy-imports namespace (and change the check code from TID251 to some RUF code).

The steps I have in mind are:

  1. convert TID251 to RUF???, moving the config section and enabling banned-api to be configured via select, ignore, extend-select & extend-ignore
  2. ban the 24 deprecated stdlib modules by default (along with typing.Text and logging.warn as shown above)
  3. make UP019 and PGH002 transform banned-api, e.g. ruff.extend-ignore = [ "UP019" ] would be internally converted to ruff.banned-api.extend-ignore = ["typing.Text"]
  4. Add the possibility to add import and respect the import sorting. #835
  5. Implement autofix-to for banned-api.
  6. Introduce a params setting for banned-api, so that e.g. UP021 could be defined as
    banned-api.select."subprocess.run" = {params = {universal_newlines = {autofix-to="text"}}

What do you think of this?

@not-my-profile not-my-profile changed the title Allow banned-api to be configured via the same mechanism we use for rules Use banned-api for deprecation checks in general Dec 31, 2022
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Dec 31, 2022
@flying-sheep
Copy link
Contributor

This also intersects with UP035 / deprecated-import

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants