-
Notifications
You must be signed in to change notification settings - Fork 1.8k
docs(clippy_utils): Add documentation for ast_utils #15588
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the initiative.
However, the comments could be more useful for the user. Also, don't use the "fixes …" in the PR description, it doesn't fix it, it is just a small part of what needs to be done, the issue should not be auto-closed.
r? samueltardieu
@rustbot author
left.len() == right.len() && left.iter().all(|l| right.iter().any(|r| eq_fn(l, r))) | ||
} | ||
|
||
/// Returns `true` if the two given `[Ident]` nodes are structurally equivalent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use "Checks if …" instead of "return true
if …" (and in other places too). Here, you can be more precise:
/// Returns `true` if the two given `[Ident]` nodes are structurally equivalent. | |
/// Checks if `l` and `r` have the same name. |
&& over(&l.attrs, &r.attrs, eq_attr) | ||
} | ||
|
||
/// Returns `true` if the two given optional `[Label]` nodes are structurally equivalent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those comments are not really useful. It would be more useful to indicate what happens when both labels are defined with the same identifier name, and when both of them are None
. It is not clear here if two None
are "structurally equivalent" just from reading the comment.
Reminder, once the PR becomes ready for a review, use |
☔ The latest upstream changes (possibly f3c020c) made this pull request unmergeable. Please resolve the merge conflicts. |
Fixes #15569
This PR resolves the
missing_docs
warnings for theclippy_utils::ast_utils
module.#![warn(missing_docs)]
lint on the module.ident_iter
submodule.This makes the utility functions easier to understand and helps maintain documentation coverage for the crate.