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.

Sign up for GitHub

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

Jump to bottom

Fix bug in rustdoc -Whelp #98799

Merged
merged 1 commit into from
Jul 4, 2022
Merged

Fix bug in rustdoc -Whelp #98799

merged 1 commit into from
Jul 4, 2022

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 2, 2022 •
edited
Loading

Previously, this printed the debugging options, not the lint options,
and only handled -Whelp, not -A/-D/-F.

This also fixes a few other misc issues:

  • Fix // check-stdout for UI tests; previously it only worked for run-fail and compile-fail tests
  • Add lint headers for tool lints, not just builtin lints

#98533 (comment)

r? @GuillaumeGomez

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Jul 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2022
jyn514
jyn514 commented Jul 2, 2022
View reviewed changes
src/tools/compiletest/src/runtest.rs
@@ -3245,7 +3246,7 @@ impl<'test> TestCx<'test> {

if !self.props.error_patterns.is_empty() {
// "// error-pattern" comments
self.check_error_patterns(&proc_res.stderr, &proc_res, pm);
self.check_error_patterns(&output_to_check, &proc_res, pm);
Copy link
Member Author

Choose a reason for hiding this comment

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

before, // check-stdout was broken and did nothing for UI tests

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jyn514 jyn514 force-pushed the rustdoc-lint-help branch 2 times, most recently from 564518d to fa39bd4 Compare July 2, 2022 06:23
jyn514
jyn514 commented Jul 2, 2022
View reviewed changes
src/librustdoc/lib.rs
Comment on lines +777 to +786
let mut lint_store = rustc_lint::new_lint_store(
sess.opts.debugging_opts.no_interleave_lints,
sess.unstable_options(),
);
let registered_lints = if let Some(register_lints) = compiler.register_lints() {
register_lints(sess, &mut lint_store);
true
} else {
false
};
Copy link
Member Author

@jyn514 jyn514 Jul 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

this can't just be register_plugins() because that ends up trying to parse the source file, which fails because it's an empty string. this code is taken from

rust/compiler/rustc_driver/src/lib.rs

Lines 249 to 263 in e1b6f16

if sopts.describe_lints {
let mut lint_store = rustc_lint::new_lint_store(
sopts.debugging_opts.no_interleave_lints,
compiler.session().unstable_options(),
);
let registered_lints =
if let Some(register_lints) = compiler.register_lints() {
register_lints(compiler.session(), &mut lint_store);
true
} else {
false
};
describe_lints(compiler.session(), &lint_store, registered_lints);
return;
}

jyn514
jyn514 commented Jul 2, 2022
View reviewed changes
src/test/rustdoc-ui/issue-83883-describe-lints.stdout

Lint checks provided by plugins loaded by this crate:

$NAMES $LEVELS $MEANINGS
Copy link
Member Author

Choose a reason for hiding this comment

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

this ended up fixing my original complaint in #98533 (comment) :)

@jyn514
Fix bug in rustdoc -Whelp
17da4e0 
Previously, this printed the debugging options, not the lint options,
and only handled `-Whelp`, not `-A/-D/-F`.

This also fixes a few other misc issues:
- Fix `// check-stdout` for UI tests; previously it only worked for run-fail and compile-fail tests
- Add lint headers for tool lints, not just builtin lints
- Remove duplicate run-make test
@GuillaumeGomez
Copy link
Member

Just to be sure: there are other UI tests that check the output you removed?

@jyn514
Copy link
Member Author

jyn514 commented Jul 2, 2022

Yes, you can see them in the diff: src/test/rustdoc-ui/issue-83883-describe-lints.rs
The run-make test did the same thing and couldn't be blessed, so it would have to be manually updated on each new lint.

@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 3, 2022

📌 Commit 17da4e0 has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#98799 - jyn514:rustdoc-lint-help, r=Guillau…
c1f9a14 
…meGomez

Fix bug in `rustdoc -Whelp`

Previously, this printed the debugging options, not the lint options,
and only handled `-Whelp`, not `-A/-D/-F`.

This also fixes a few other misc issues:
- Fix `// check-stdout` for UI tests; previously it only worked for run-fail and compile-fail tests
- Add lint headers for tool lints, not just builtin lints

rust-lang#98533 (comment)

r? `@GuillaumeGomez`
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 3, 2022
Closed
RalfJung added a commit to RalfJung/rust that referenced this pull request Jul 3, 2022
@RalfJung
Rollup merge of rust-lang#98799 - jyn514:rustdoc-lint-help, r=Guillau…
1db568d 
…meGomez

Fix bug in `rustdoc -Whelp`

Previously, this printed the debugging options, not the lint options,
and only handled `-Whelp`, not `-A/-D/-F`.

This also fixes a few other misc issues:
- Fix `// check-stdout` for UI tests; previously it only worked for run-fail and compile-fail tests
- Add lint headers for tool lints, not just builtin lints

rust-lang#98533 (comment)

r? ``@GuillaumeGomez``
This was referenced Jul 3, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 3, 2022
@bors
Auto merge of rust-lang#98864 - RalfJung:rollup-ptzklyc, r=RalfJung
2557603 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#94831 (Link to stabilization section in std-dev-guide for library tracking issue template)
 - rust-lang#98764 (add Miri to the nightly docs)
 - rust-lang#98773 (rustdoc: use <details> tag for the source code sidebar)
 - rust-lang#98799 (Fix bug in `rustdoc -Whelp`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ce76d73 into rust-lang:master Jul 4, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 4, 2022
@jyn514 jyn514 deleted the rustdoc-lint-help branch February 25, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.
6 participants
@jyn514 @rust-log-analyzer @GuillaumeGomez @bors @rust-highfive @rustbot