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

disallowed_methods: Support functions in addition to methods #6674

Merged
merged 1 commit into from
Feb 7, 2021

Conversation

phlip9
Copy link
Contributor

@phlip9 phlip9 commented Feb 4, 2021

Context:

Hey all! I have a particular use case where I'd like to ban certain functions in a code base I work on. For example, I want to ban Instant::now() (among others) as we have a time service for mocking time in deterministic simulation tests. Obviously, it doesn't make sense to include a lint like this for all clippy users. Imagine my excitement when I spotted the new disallowed_methods lint in clippy--perfect! Unfortunately, after playing around with it for a bit, I was frustrated to realize that it didn't support functions like Instant::now(), so I've added support for them in this PR.

It might also make sense to rename the lint from disallowed_methods -> disallowed_functions, though I've held off from including that rename in this change, since I'm unsure of clippy's breaking change policy.

Change

Support functions in addition to methods. In other words, support:

disallowed_methods = ["alloc::vec::Vec::new"] (a function) in addition to
disallowed_methods = ["alloc::vec::Vec::leak"] (a method).

Improve the documentation to clarify that users must specify the full qualified path for each disallowed function, which can be confusing for reexports. Include an example clippy.toml.

Simplify the actual lint pass so we can reuse utils::fn_def_id.

changelog: disallowed_method: Now supports functions in addition to methods

In other words, support:

`disallowed_methods = ["alloc::vec::Vec::new"]` (a free function) in
addition to
`disallowed_methods = ["alloc::vec::Vec::leak"]` (a method).

Improve the documentation to clarify that users must specify the full
qualified path for each disallowed function, which can be confusing for
reexports. Include an example `clippy.toml`.

Simplify the actual lint pass so we can reuse `utils::fn_def_id`.
@rust-highfive
Copy link

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 4, 2021
@phlip9
Copy link
Contributor Author

phlip9 commented Feb 4, 2021

cc @ilknarf If you want to comment, since you created the lint : )

@llogiq
Copy link
Contributor

llogiq commented Feb 7, 2021

Thank you! @bors r+

@bors
Copy link
Contributor

bors commented Feb 7, 2021

📌 Commit 7b7e3ca has been approved by llogiq

@bors
Copy link
Contributor

bors commented Feb 7, 2021

⌛ Testing commit 7b7e3ca with merge 83b7b16...

@bors
Copy link
Contributor

bors commented Feb 7, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 83b7b16 to master...

@bors bors merged commit 83b7b16 into rust-lang:master Feb 7, 2021
@phlip9 phlip9 deleted the disallowed_functions branch February 11, 2021 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants