-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Cleanup rustdoc lint listing #80455
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
Cleanup rustdoc lint listing #80455
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,7 @@ use rustc_span::DUMMY_SP; | |
|
||
use std::cell::{Cell, RefCell}; | ||
use std::mem; | ||
use std::path::PathBuf; | ||
use std::rc::Rc; | ||
|
||
use crate::clean; | ||
|
@@ -273,6 +274,58 @@ where | |
(lint_opts, lint_caps) | ||
} | ||
|
||
crate fn get_rustdoc_lints( | ||
cfg: FxHashSet<(String, Option<String>)>, | ||
opts: config::Options, | ||
) -> Vec<String> { | ||
let config = interface::Config { | ||
opts, | ||
crate_cfg: cfg, | ||
input: Input::File(PathBuf::new()), | ||
input_path: None, | ||
output_file: None, | ||
output_dir: None, | ||
file_loader: None, | ||
diagnostic_output: DiagnosticOutput::Default, | ||
stderr: None, | ||
lint_caps: Default::default(), | ||
register_lints: None, | ||
override_queries: None, | ||
make_codegen_backend: None, | ||
registry: rustc_driver::diagnostics_registry(), | ||
}; | ||
let missing_docs_lints = rustc_lint::builtin::MISSING_DOCS.name; | ||
let renamed_and_removed_lints = rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name; | ||
let unknown_lints = rustc_lint::builtin::UNKNOWN_LINTS.name; | ||
|
||
// Rustdoc silences all lints by default, only warning on lints in the `rustdoc` group. | ||
// These lints aren't in the lint group, but we want to warn on them anyway. | ||
let mut rustdoc_lints = vec![ | ||
missing_docs_lints.to_owned(), | ||
renamed_and_removed_lints.to_owned(), | ||
unknown_lints.to_owned(), | ||
]; | ||
|
||
interface::run_compiler(config, |compiler| { | ||
let sopts = &compiler.session().opts; | ||
let lint_store = rustc_lint::new_lint_store( | ||
sopts.debugging_opts.no_interleave_lints, | ||
compiler.session().unstable_options(), | ||
); | ||
if let Some((_, lints, _)) = | ||
lint_store.get_lint_groups().iter().find(|(name, _, _)| *name == "rustdoc") | ||
{ | ||
for lint in lints { | ||
// Funny thing about this: the lints in the compiler are capitalized but the lints | ||
// you get through the `LintStore` are not, which means you have to capitalize them. | ||
rustdoc_lints.push(lint.to_string().to_uppercase()); | ||
GuillaumeGomez marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
}); | ||
|
||
rustdoc_lints | ||
} | ||
|
||
/// Parse, resolve, and typecheck the given crate. | ||
crate fn create_config( | ||
RustdocOptions { | ||
|
@@ -301,51 +354,14 @@ crate fn create_config( | |
let cpath = Some(input.clone()); | ||
let input = Input::File(input); | ||
|
||
let broken_intra_doc_links = lint::builtin::BROKEN_INTRA_DOC_LINKS.name; | ||
let private_intra_doc_links = lint::builtin::PRIVATE_INTRA_DOC_LINKS.name; | ||
let missing_docs = rustc_lint::builtin::MISSING_DOCS.name; | ||
let missing_doc_example = rustc_lint::builtin::MISSING_DOC_CODE_EXAMPLES.name; | ||
let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; | ||
let no_crate_level_docs = rustc_lint::builtin::MISSING_CRATE_LEVEL_DOCS.name; | ||
let invalid_codeblock_attributes_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTES.name; | ||
let invalid_html_tags = rustc_lint::builtin::INVALID_HTML_TAGS.name; | ||
let renamed_and_removed_lints = rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name; | ||
let non_autolinks = rustc_lint::builtin::NON_AUTOLINKS.name; | ||
let unknown_lints = rustc_lint::builtin::UNKNOWN_LINTS.name; | ||
|
||
// In addition to those specific lints, we also need to allow those given through | ||
// command line, otherwise they'll get ignored and we don't want that. | ||
let lints_to_show = vec![ | ||
broken_intra_doc_links.to_owned(), | ||
private_intra_doc_links.to_owned(), | ||
missing_docs.to_owned(), | ||
missing_doc_example.to_owned(), | ||
private_doc_tests.to_owned(), | ||
no_crate_level_docs.to_owned(), | ||
invalid_codeblock_attributes_name.to_owned(), | ||
invalid_html_tags.to_owned(), | ||
renamed_and_removed_lints.to_owned(), | ||
unknown_lints.to_owned(), | ||
non_autolinks.to_owned(), | ||
]; | ||
|
||
let (lint_opts, lint_caps) = init_lints(lints_to_show, lint_opts, |lint| { | ||
// FIXME: why is this necessary? | ||
if lint.name == broken_intra_doc_links || lint.name == invalid_codeblock_attributes_name { | ||
None | ||
} else { | ||
Some((lint.name_lower(), lint::Allow)) | ||
} | ||
}); | ||
|
||
let crate_types = | ||
if proc_macro_crate { vec![CrateType::ProcMacro] } else { vec![CrateType::Rlib] }; | ||
// plays with error output here! | ||
let sessopts = config::Options { | ||
let mut sessopts = config::Options { | ||
maybe_sysroot, | ||
search_paths: libs, | ||
crate_types, | ||
lint_opts: if !display_warnings { lint_opts } else { vec![] }, | ||
lint_opts: vec![], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a change in behavior (the same one from #73314). I think it's a good change, but it shouldn't be mixed in with a cleanup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No it's not. Look just below: I set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be exact, it's done here: if !display_warnings {
sessopts.lint_opts = lint_opts;
} |
||
lint_cap, | ||
cg: codegen_options, | ||
externs, | ||
|
@@ -360,9 +376,27 @@ crate fn create_config( | |
..Options::default() | ||
}; | ||
|
||
let crate_cfg = interface::parse_cfgspecs(cfgs); | ||
|
||
let invalid_codeblock_attributes_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTES.name; | ||
let broken_intra_doc_links = lint::builtin::BROKEN_INTRA_DOC_LINKS.name; | ||
|
||
let lints_to_show = get_rustdoc_lints(crate_cfg.clone(), sessopts.clone()); | ||
let (lint_opts, lint_caps) = init_lints(lints_to_show, lint_opts, |lint| { | ||
// FIXME: why is this necessary? | ||
if lint.name == broken_intra_doc_links || lint.name == invalid_codeblock_attributes_name { | ||
None | ||
Comment on lines
+386
to
+388
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you remove this, does it still warn on those lints? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to update it in this PR to avoid mixing too many things together. We can take a look when this is merged if you want. |
||
} else { | ||
Some((lint.name_lower(), lint::Allow)) | ||
} | ||
}); | ||
if !display_warnings { | ||
sessopts.lint_opts = lint_opts; | ||
} | ||
|
||
interface::Config { | ||
opts: sessopts, | ||
crate_cfg: interface::parse_cfgspecs(cfgs), | ||
crate_cfg, | ||
input, | ||
input_path: cpath, | ||
output_file: None, | ||
|
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.
This should not be mixed in with the cleanup; it's the same sort of change as #77364, which I'm still not convinced is the right change.
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.
It's only emitted by rustdoc, so the context is a bit different there. This whole PR wouldn't be possible without this change because otherwise I'll need to still handle this lint differently than the others. However, because it was a bit out of the scope of this PR, I have put this change in a commit on its own (it's the first commit, you can check ;) ).