Skip to content

Conversation

@granddaifuku
Copy link
Contributor

@granddaifuku granddaifuku commented Apr 12, 2025

This PR fixes a part of #11306.

Note that disallowed_names still doesn't warn about using disallowed struct (and its field) /module/crate names. I'm planning to address this in a separate PR (if it makes sense to you).

The changes are a bit large, but they primarily involve two updates:

  • Enhancing the disallowed_names lint to emit warnings for functions with disallowed names.
  • Updating tests that include such functions to explicitly allow the lint.

changelog: [disallowed_names]: Warn on usage of disallowed function names

@rustbot
Copy link
Collaborator

rustbot commented Apr 12, 2025

r? @y21

rustbot has assigned @y21.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 12, 2025
@granddaifuku granddaifuku marked this pull request as draft April 12, 2025 18:42
@rustbot

This comment has been minimized.

@granddaifuku granddaifuku force-pushed the disallowed-names-lint-function-names branch from 103b261 to 992f226 Compare May 10, 2025 12:04
@granddaifuku granddaifuku changed the title Lint disallowed function names Emit disallowed_names lint for functions May 10, 2025
@granddaifuku
Copy link
Contributor Author

Hi @y21, this PR is ready for review now - could I kindly ask you to take a look when you have a moment? The changes are a bit large, but they primarily involve two updates:

  1. Enhancing the disallowed_names lint to emit warnings for functions with disallowed names.
  2. Updating tests that include such functions to explicitly allow the lint.

Let me know if you have any questions. Thanks in advance!

@granddaifuku granddaifuku marked this pull request as ready for review May 10, 2025 15:11
@rustbot

This comment has been minimized.

@granddaifuku granddaifuku force-pushed the disallowed-names-lint-function-names branch from 1c726a6 to 0bda6b2 Compare May 18, 2025 11:41
@rustbot

This comment has been minimized.

@granddaifuku granddaifuku force-pushed the disallowed-names-lint-function-names branch from dc779cd to 76af243 Compare June 1, 2025 13:47
@granddaifuku
Copy link
Contributor Author

Hello @y21, I updated the PR to include the up-to-date master branch.
Would you mind reviewing this PR when you have a moment?
Many thanks!

@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2025

☔ The latest upstream changes (possibly f8a3929) made this pull request unmergeable. Please resolve the merge conflicts.

@y21
Copy link
Member

y21 commented Jun 7, 2025

Sorry for the long wait. This change makes sense to me, although given the impact on the tests I created a thread on Zulip to see if there's concerns with this: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/disallowed_names.20for.20function.20names

Going to leave that for a few days and if nothing comes up then I'd say we're good

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

This should probably ignore functions in testing modules.

View changes since this review

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