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

PatField and ExprField produce no warning for #[doc(...)] attribute #115462

Closed
gurry opened this issue Sep 2, 2023 · 4 comments · Fixed by #115478
Closed

PatField and ExprField produce no warning for #[doc(...)] attribute #115462

gurry opened this issue Sep 2, 2023 · 4 comments · Fixed by #115478
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@gurry
Copy link
Contributor

gurry commented Sep 2, 2023

Code

struct X {
    foo: i32,
}

#[allow(unused_variables)] // Not necessary for reproducing. Just to suppress an unrelated warning
fn main() {
    let X {
        #[doc(alias = "PatFieldAlias")]
        foo
    } = X {
        foo: 123
    };

    let _ = X {
        #[doc(alias = "ExprFieldAlias")]
        foo: 123,
    };
}

Current output

No output (compilation succeeds without warning)

Desired output

warning: unused doc comment
   |
14 |       let X {
15 |           #[doc(alias = "PatFieldAlias")]
   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
16 | /         foo
17 | |     } = X {
18 | |         foo: 123
19 | |     };
   | |______- rustdoc does not generate documentation for pattern fields
   |
   = help: use `//` for a plain comment
   = note: `#[warn(unused_doc_comments)]` on by default

warning: unused doc comment
   |
22 |       let _ = X {
23 |           #[doc(alias = "ExprFieldAlias")]
   |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
24 | /         foo: 123,
25 | |     };
   | |______- rustdoc does not generate documentation for expression fields
   |
   = help: use `//` for a plain comment
   = note: `#[warn(unused_doc_comments)]` on by default

Rationale and extra context

The same warning is produced at other places like statements, expressions, functions etc. For completeness and consistency it should be produced for pattern and expression fields too.

Other cases

No response

Anything else?

rustc 1.74.0-nightly (35e4163 2023-09-01)
binary: rustc
commit-hash: 35e4163
commit-date: 2023-09-01
host: x86_64-pc-windows-msvc
release: 1.74.0-nightly
LLVM version: 17.0.0

rustc version older than 1.74.0 may ICE on this code. The ICE was fixed in 1.74.0

@gurry gurry added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 2, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 2, 2023
@gurry
Copy link
Contributor Author

gurry commented Sep 2, 2023

The reason these warnings are not produced is because the lint infrastructure currently exposes no hooks for PatField and ExprField, i.e. there are no check_pat_field and check_expr_field methods on EarlyLintPass (or LateLintPass for that matter).

To fix the issue, these methods may have to be added here:

macro_rules! early_lint_methods {

and then invoked from here:
fn visit_pat_field(&mut self, field: &'a ast::PatField) {
self.with_lint_attrs(field.id, &field.attrs, |cx| {
ast_visit::walk_pat_field(cx, field);
});
}

and here:
fn visit_expr_field(&mut self, f: &'a ast::ExprField) {
self.with_lint_attrs(f.id, &f.attrs, |cx| {
ast_visit::walk_expr_field(cx, f);
})
}

Alternatively, we can emit the warning from the already existing check_pat and check_expr methods of EarlyLintPass and not bother with the above changes at all.

@compiler-errors
Copy link
Member

Alternatively, we can emit the warning from the already existing check_pat and check_expr methods of EarlyLintPass and not bother with the above changes at all.

Yeah, I would expect this is the best way of doing it.

@gurry
Copy link
Contributor Author

gurry commented Sep 2, 2023

@rustbot claim

@gurry
Copy link
Contributor Author

gurry commented Sep 2, 2023

@rustbot label -needs-triage

@rustbot rustbot removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 2, 2023
@bors bors closed this as completed in 3db7fc1 Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants