Skip to content

add allow_unused config to missing_docs_in_private_items #14453

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

Conversation

quangcito
Copy link
Contributor

@quangcito quangcito commented Mar 23, 2025

fixes #14413
add allow_unused config to missing_docs_in_private_items for underscored field

changelog: [missing_docs_in_private_items]: add allow_unused config to missing_docs_in_private_items for underscored field

Explaination (quoted from the issue's author): When writing structures that must adhere to a specific format (such as memory mapped I/O structures) there are inevitably fields that are "reserved" by specifications and thus need no documentation. In cases where private docs are preferred but reserved fields need no explanation, having to #[allow/expect] this lint removes the ability to check for otherwise used fields' documentation.

@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2025

r? @Jarcho

rustbot has assigned @Jarcho.
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 Mar 23, 2025
@quangcito quangcito marked this pull request as ready for review March 23, 2025 09:13
Comment on lines 267 to 272
// Skip checking if the field starts with underscore and allow_unused is enabled
if self.allow_unused && sf.ident.as_str().starts_with('_') {
self.prev_span = Some(sf.span);
return;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You can group if conditions and keep only one exit point, this should be clearer since self.prev_span must be adjusted in all paths before leaving:

    fn check_field_def(&mut self, cx: &LateContext<'tcx>, sf: &'tcx hir::FieldDef<'_>) {
        if !sf.is_positional()
            && !(self.allow_unused && sf.ident.as_str().starts_with('_'))
            && !is_from_proc_macro(cx, sf)
        {
            let attrs = cx.tcx.hir_attrs(sf.hir_id);
            self.check_missing_docs_attrs(cx, sf.def_id, attrs, sf.span, "a", "struct field");
        }
        self.prev_span = Some(sf.span);
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for reviewing this PR and the guidance!

@@ -723,6 +723,16 @@ Minimum chars an ident can have, anything below or equal to this will be linted.
* [`min_ident_chars`](https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars)


## `missing-docs-allow-unused`
Whether to allow fields starting with underscore to skip documentation requirements
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: shouldn't it be "with underscores" or "with an underscore"? (repeated in other places)

_field1: i32, // This should not trigger a warning
_field2: i32, // This should not trigger a warning
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also add the following test to ensure that a documented unused field does not trigger a warning, and that the documentation is not applied to the next field (which should be documented) either?

struct Test3 {
    //~^ missing_docs_in_private_items
    /// This field is documented although this is not mandatory
    _unused: i32, // This should not trigger a warning because it starts with underscore
    field3: i32,  //~ missing_docs_in_private_items
}

@samueltardieu
Copy link
Contributor

Taking this one since I've reviewed it already.
r? @samueltardieu

@rustbot rustbot assigned samueltardieu and unassigned Jarcho Apr 6, 2025
@samueltardieu
Copy link
Contributor

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 7, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot

This comment has been minimized.

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label May 3, 2025
@quangcito quangcito force-pushed the 14413/missing_docs_in_private_items-config-suggestion-allow_unused branch from 95b6d00 to 64880ca Compare May 3, 2025 22:17
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) has-merge-commits PR has merge commits, merge with caution. labels May 3, 2025
@quangcito
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 3, 2025
@quangcito quangcito requested a review from samueltardieu May 3, 2025 23:35
Copy link
Contributor

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

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

Thanks.

@samueltardieu samueltardieu added this pull request to the merge queue May 6, 2025
Merged via the queue into rust-lang:master with commit 73dd05c May 6, 2025
11 checks passed
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.

missing_docs_in_private_items config suggestion allow_unused
4 participants