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

New rule in flake8-tidy-imports for banning specific function calls #16204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kujenga
Copy link

@kujenga kujenga commented Feb 17, 2025

Summary

This commit introduces a new lint rule (TID254) that flags forbidden function calls, such as os.system and os.popen, in alignment with a project’s banned functions configuration. Highlights include:

  • Creating the BannedFunction rule to detect and report banned function calls.
  • Updating error code mappings to reference TID254.
  • Adding new configuration options (banned_functions) within flake8_tidy_imports.
  • Providing a new test fixture (TID254.py) and snapshot to verify detection.
  • Ensuring seamless integration with expression analysis in the AST checker.

This was requested here: #9236 and I have encountered some use cases for this as well.

While this is not technically part of the flake8-tidy-imports module it seemed like the closest fit from the current set of them.

n.b. I don't have a huge amount of rust experience, and this was implemented with significant assistance from llms so please let me know if anything looks off!

Test Plan

Tests were added modeled off of TID251 for banned APIs to cover a variety of cases as outlined in the test file.

I also verified this against another codebase with a release build of ruff and it seems to be working as expected.

@kujenga kujenga force-pushed the at/banned-function-rule branch from 31ef9e9 to c541dea Compare February 17, 2025 06:05
…calls

This commit introduces a new lint rule (TID254) that flags forbidden function
calls, such as `os.system` and `os.popen`, in alignment with a project’s
banned functions configuration. Highlights include:

- Creating the `BannedFunction` rule to detect and report banned function calls.
- Updating error code mappings to reference TID254.
- Adding new configuration options (`banned_functions`) within `flake8_tidy_imports`.
- Providing a new test fixture (`TID254.py`) and snapshot to verify detection.
- Ensuring seamless integration with expression analysis in the AST checker.
@kujenga kujenga force-pushed the at/banned-function-rule branch from c541dea to f520766 Compare February 17, 2025 06:15
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer preview Related to preview mode features labels Feb 17, 2025
@MichaReiser
Copy link
Member

We'll have to look into how this rule plays with the banned API rule and what happens if there are conflicting configurations or if it woudl be better to extend the existing banned api rule instead.

@kujenga
Copy link
Author

kujenga commented Feb 17, 2025

I can definitely see that being a cleaner design! Trying to do this sort of thing with that rule, and being unable to do so, is what lead me down this path. I wasn't sure how backward compatibility might factor in with changing that one, but happy to re-work this if that seems like a better end state here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants