Skip to content

Commit cf452f3

Browse files
committed
Collect and use #![doc(test(attr(..)))] at module level too
1 parent 9026066 commit cf452f3

12 files changed

+199
-25
lines changed

src/librustdoc/doctest.rs

+5-13
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@ pub(crate) struct GlobalTestOptions {
4545
/// Whether inserting extra indent spaces in code block,
4646
/// default is `false`, only `true` for generating code link of Rust playground
4747
pub(crate) insert_indent_space: bool,
48-
/// Additional crate-level attributes to add to doctests.
49-
pub(crate) attrs: Vec<String>,
5048
/// Path to file containing arguments for the invocation of rustc.
5149
pub(crate) args_file: PathBuf,
5250
}
@@ -396,12 +394,9 @@ fn scrape_test_config(
396394
attrs: &[hir::Attribute],
397395
args_file: PathBuf,
398396
) -> GlobalTestOptions {
399-
use rustc_ast_pretty::pprust;
400-
401397
let mut opts = GlobalTestOptions {
402398
crate_name,
403399
no_crate_inject: false,
404-
attrs: Vec::new(),
405400
insert_indent_space: false,
406401
args_file,
407402
};
@@ -418,13 +413,7 @@ fn scrape_test_config(
418413
if attr.has_name(sym::no_crate_inject) {
419414
opts.no_crate_inject = true;
420415
}
421-
if attr.has_name(sym::attr)
422-
&& let Some(l) = attr.meta_item_list()
423-
{
424-
for item in l {
425-
opts.attrs.push(pprust::meta_list_item_to_string(item));
426-
}
427-
}
416+
// NOTE: `test(attr(..))` is handled when discovering the individual tests
428417
}
429418

430419
opts
@@ -872,6 +861,7 @@ pub(crate) struct ScrapedDocTest {
872861
langstr: LangString,
873862
text: String,
874863
name: String,
864+
global_crate_attrs: Vec<String>,
875865
}
876866

877867
impl ScrapedDocTest {
@@ -881,6 +871,7 @@ impl ScrapedDocTest {
881871
logical_path: Vec<String>,
882872
langstr: LangString,
883873
text: String,
874+
global_crate_attrs: Vec<String>,
884875
) -> Self {
885876
let mut item_path = logical_path.join("::");
886877
item_path.retain(|c| c != ' ');
@@ -890,7 +881,7 @@ impl ScrapedDocTest {
890881
let name =
891882
format!("{} - {item_path}(line {line})", filename.prefer_remapped_unconditionaly());
892883

893-
Self { filename, line, langstr, text, name }
884+
Self { filename, line, langstr, text, name, global_crate_attrs }
894885
}
895886
fn edition(&self, opts: &RustdocOptions) -> Edition {
896887
self.langstr.edition.unwrap_or(opts.edition)
@@ -974,6 +965,7 @@ impl CreateRunnableDocTests {
974965
&scraped_test.text,
975966
Some(&self.opts.crate_name),
976967
edition,
968+
scraped_test.global_crate_attrs.clone(),
977969
self.can_merge_doctests,
978970
Some(test_id),
979971
Some(&scraped_test.langstr),

src/librustdoc/doctest/extracted.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@ impl ExtractedDocTests {
3535
) {
3636
let edition = scraped_test.edition(options);
3737

38-
let ScrapedDocTest { filename, line, langstr, text, name } = scraped_test;
38+
let ScrapedDocTest { filename, line, langstr, text, name, global_crate_attrs } =
39+
scraped_test;
3940

4041
let doctest = DocTestBuilder::new(
4142
&text,
4243
Some(&opts.crate_name),
4344
edition,
45+
global_crate_attrs,
4446
false,
4547
None,
4648
Some(&langstr),

src/librustdoc/doctest/make.rs

+13-3
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ pub(crate) struct DocTestBuilder {
4141
pub(crate) supports_color: bool,
4242
pub(crate) already_has_extern_crate: bool,
4343
pub(crate) has_main_fn: bool,
44+
pub(crate) global_crate_attrs: Vec<String>,
4445
pub(crate) crate_attrs: String,
4546
/// If this is a merged doctest, it will be put into `everything_else`, otherwise it will
4647
/// put into `crate_attrs`.
@@ -57,6 +58,7 @@ impl DocTestBuilder {
5758
source: &str,
5859
crate_name: Option<&str>,
5960
edition: Edition,
61+
global_crate_attrs: Vec<String>,
6062
can_merge_doctests: bool,
6163
// If `test_id` is `None`, it means we're generating code for a code example "run" link.
6264
test_id: Option<String>,
@@ -88,6 +90,7 @@ impl DocTestBuilder {
8890
// If the AST returned an error, we don't want this doctest to be merged with the
8991
// others.
9092
return Self::invalid(
93+
Vec::new(),
9194
String::new(),
9295
String::new(),
9396
String::new(),
@@ -104,12 +107,16 @@ impl DocTestBuilder {
104107
let can_be_merged = can_merge_doctests
105108
&& !has_global_allocator
106109
&& crate_attrs.is_empty()
110+
// FIXME: We can probably merge tests which have the same global crate attrs,
111+
// like we already do for the edition
112+
&& global_crate_attrs.is_empty()
107113
// If this is a merged doctest and a defined macro uses `$crate`, then the path will
108114
// not work, so better not put it into merged doctests.
109115
&& !(has_macro_def && everything_else.contains("$crate"));
110116
Self {
111117
supports_color,
112118
has_main_fn,
119+
global_crate_attrs,
113120
crate_attrs,
114121
maybe_crate_attrs,
115122
crates,
@@ -122,6 +129,7 @@ impl DocTestBuilder {
122129
}
123130

124131
fn invalid(
132+
global_crate_attrs: Vec<String>,
125133
crate_attrs: String,
126134
maybe_crate_attrs: String,
127135
crates: String,
@@ -131,6 +139,7 @@ impl DocTestBuilder {
131139
Self {
132140
supports_color: false,
133141
has_main_fn: false,
142+
global_crate_attrs,
134143
crate_attrs,
135144
maybe_crate_attrs,
136145
crates,
@@ -160,7 +169,8 @@ impl DocTestBuilder {
160169
let mut line_offset = 0;
161170
let mut prog = String::new();
162171
let everything_else = self.everything_else.trim();
163-
if opts.attrs.is_empty() {
172+
173+
if self.global_crate_attrs.is_empty() {
164174
// If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
165175
// lints that are commonly triggered in doctests. The crate-level test attributes are
166176
// commonly used to make tests fail in case they trigger warnings, so having this there in
@@ -169,8 +179,8 @@ impl DocTestBuilder {
169179
line_offset += 1;
170180
}
171181

172-
// Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
173-
for attr in &opts.attrs {
182+
// Next, any attributes that came from #![doc(test(attr(...)))].
183+
for attr in &self.global_crate_attrs {
174184
prog.push_str(&format!("#![{attr}]\n"));
175185
line_offset += 1;
176186
}

src/librustdoc/doctest/markdown.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,14 @@ impl DocTestVisitor for MdCollector {
2424
let filename = self.filename.clone();
2525
// First line of Markdown is line 1.
2626
let line = 1 + rel_line.offset();
27-
self.tests.push(ScrapedDocTest::new(filename, line, self.cur_path.clone(), config, test));
27+
self.tests.push(ScrapedDocTest::new(
28+
filename,
29+
line,
30+
self.cur_path.clone(),
31+
config,
32+
test,
33+
Vec::new(),
34+
));
2835
}
2936

3037
fn visit_header(&mut self, name: &str, level: u32) {
@@ -89,7 +96,6 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
8996
crate_name,
9097
no_crate_inject: true,
9198
insert_indent_space: false,
92-
attrs: vec![],
9399
args_file,
94100
};
95101

src/librustdoc/doctest/runner.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::html::markdown::{Ignore, LangString};
1212
/// Convenient type to merge compatible doctests into one.
1313
pub(crate) struct DocTestRunner {
1414
crate_attrs: FxIndexSet<String>,
15+
global_crate_attrs: FxIndexSet<String>,
1516
ids: String,
1617
output: String,
1718
output_merged_tests: String,
@@ -23,6 +24,7 @@ impl DocTestRunner {
2324
pub(crate) fn new() -> Self {
2425
Self {
2526
crate_attrs: FxIndexSet::default(),
27+
global_crate_attrs: FxIndexSet::default(),
2628
ids: String::new(),
2729
output: String::new(),
2830
output_merged_tests: String::new(),
@@ -46,6 +48,9 @@ impl DocTestRunner {
4648
for line in doctest.crate_attrs.split('\n') {
4749
self.crate_attrs.insert(line.to_string());
4850
}
51+
for line in &doctest.global_crate_attrs {
52+
self.global_crate_attrs.insert(line.to_string());
53+
}
4954
}
5055
self.ids.push_str(&format!(
5156
"tests.push({}::TEST);\n",
@@ -85,16 +90,16 @@ impl DocTestRunner {
8590
code_prefix.push('\n');
8691
}
8792

88-
if opts.attrs.is_empty() {
93+
if self.global_crate_attrs.is_empty() {
8994
// If there aren't any attributes supplied by #![doc(test(attr(...)))], then allow some
9095
// lints that are commonly triggered in doctests. The crate-level test attributes are
9196
// commonly used to make tests fail in case they trigger warnings, so having this there in
9297
// that case may cause some tests to pass when they shouldn't have.
9398
code_prefix.push_str("#![allow(unused)]\n");
9499
}
95100

96-
// Next, any attributes that came from the crate root via #![doc(test(attr(...)))].
97-
for attr in &opts.attrs {
101+
// Next, any attributes that came from #![doc(test(attr(...)))].
102+
for attr in &self.global_crate_attrs {
98103
code_prefix.push_str(&format!("#![{attr}]\n"));
99104
}
100105

src/librustdoc/doctest/rust.rs

+45-1
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@
33
use std::env;
44
use std::sync::Arc;
55

6+
use rustc_ast_pretty::pprust;
67
use rustc_data_structures::fx::FxHashSet;
78
use rustc_hir::def_id::{CRATE_DEF_ID, LocalDefId};
89
use rustc_hir::{self as hir, CRATE_HIR_ID, intravisit};
910
use rustc_middle::hir::nested_filter;
1011
use rustc_middle::ty::TyCtxt;
1112
use rustc_resolve::rustdoc::span_of_fragments;
1213
use rustc_span::source_map::SourceMap;
13-
use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span};
14+
use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span, sym};
1415

1516
use super::{DocTestVisitor, ScrapedDocTest};
1617
use crate::clean::{Attributes, extract_cfg_from_attrs};
@@ -21,6 +22,7 @@ struct RustCollector {
2122
tests: Vec<ScrapedDocTest>,
2223
cur_path: Vec<String>,
2324
position: Span,
25+
global_crate_attrs: Vec<String>,
2426
}
2527

2628
impl RustCollector {
@@ -54,6 +56,7 @@ impl DocTestVisitor for RustCollector {
5456
self.cur_path.clone(),
5557
config,
5658
test,
59+
self.global_crate_attrs.clone(),
5760
));
5861
}
5962

@@ -73,6 +76,7 @@ impl<'tcx> HirCollector<'tcx> {
7376
cur_path: vec![],
7477
position: DUMMY_SP,
7578
tests: vec![],
79+
global_crate_attrs: Vec::new(),
7680
};
7781
Self { codes, tcx, collector }
7882
}
@@ -149,6 +153,46 @@ impl<'tcx> intravisit::Visitor<'tcx> for HirCollector<'tcx> {
149153
self.tcx
150154
}
151155

156+
fn visit_mod(&mut self, m: &'tcx hir::Mod<'tcx>, _s: Span, hir_id: hir::HirId) {
157+
let attrs = self.tcx.hir_attrs(hir_id);
158+
159+
if !attrs.is_empty() {
160+
// Try collecting `#![doc(test(attr(...)))]` from the attribute module
161+
//
162+
// We do it in 2 steps since the first `meta_item_list` returns owned
163+
// `MetaItemInner` instead of reference to them.
164+
let test_attrs: Vec<_> = attrs
165+
.iter()
166+
.filter(|a| a.has_name(sym::doc))
167+
.flat_map(|a| a.meta_item_list().unwrap_or_default())
168+
.filter(|a| a.has_name(sym::test))
169+
.collect();
170+
let additional_global_crate_attrs: Vec<_> = test_attrs
171+
.iter()
172+
.flat_map(|a| a.meta_item_list().unwrap_or_default())
173+
.filter(|a| a.has_name(sym::attr))
174+
.flat_map(|a| a.meta_item_list().unwrap_or_default())
175+
.map(|i| pprust::meta_list_item_to_string(i))
176+
.collect();
177+
178+
if additional_global_crate_attrs.is_empty() {
179+
intravisit::walk_mod(self, m)
180+
} else {
181+
// Add the additional attributes to the global_crate_attrs vector
182+
let old_len = self.collector.global_crate_attrs.len();
183+
self.collector.global_crate_attrs.extend(additional_global_crate_attrs);
184+
185+
let r = intravisit::walk_mod(self, m);
186+
187+
// Restore global_crate_attrs to it's previous size/content
188+
self.collector.global_crate_attrs.truncate(old_len);
189+
r
190+
}
191+
} else {
192+
intravisit::walk_mod(self, m)
193+
}
194+
}
195+
152196
fn visit_item(&mut self, item: &'tcx hir::Item<'_>) {
153197
let name = match &item.kind {
154198
hir::ItemKind::Impl(impl_) => {

src/librustdoc/html/markdown.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,10 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
300300
crate_name: krate.map(String::from).unwrap_or_default(),
301301
no_crate_inject: false,
302302
insert_indent_space: true,
303-
attrs: vec![],
304303
args_file: PathBuf::new(),
305304
};
306-
let doctest = doctest::DocTestBuilder::new(&test, krate, edition, false, None, None);
305+
let doctest =
306+
doctest::DocTestBuilder::new(&test, krate, edition, Vec::new(), false, None, None);
307307
let (test, _) = doctest.generate_unique_doctest(&test, false, &opts, krate);
308308
let channel = if test.contains("#![feature(") { "&amp;version=nightly" } else { "" };
309309

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Same test as dead-code-module but with 2 doc(test(attr())) at different levels.
2+
3+
//@ edition: 2024
4+
//@ compile-flags:--test
5+
//@ normalize-stdout: "tests/rustdoc-ui/doctest" -> "$$DIR"
6+
//@ normalize-stdout: "finished in \d+\.\d+s" -> "finished in $$TIME"
7+
//@ failure-status: 101
8+
9+
#![doc(test(attr(allow(unused_variables))))]
10+
11+
mod my_mod {
12+
#![doc(test(attr(deny(warnings))))]
13+
14+
/// Example
15+
///
16+
/// ```rust,no_run
17+
/// trait T { fn f(); }
18+
/// ```
19+
pub fn f() {}
20+
}
21+
22+
/// Example
23+
///
24+
/// ```rust,no_run
25+
/// trait OnlyWarning { fn no_deny_warnings(); }
26+
/// ```
27+
pub fn g() {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
2+
running 2 tests
3+
test $DIR/dead-code-module-2.rs - my_mod::f (line 16) - compile ... FAILED
4+
test $DIR/dead-code-module-2.rs - g (line 24) - compile ... ok
5+
6+
failures:
7+
8+
---- $DIR/dead-code-module-2.rs - my_mod::f (line 16) stdout ----
9+
error: trait `T` is never used
10+
--> $DIR/dead-code-module-2.rs:17:7
11+
|
12+
LL | trait T { fn f(); }
13+
| ^
14+
|
15+
note: the lint level is defined here
16+
--> $DIR/dead-code-module-2.rs:15:9
17+
|
18+
LL | #![deny(warnings)]
19+
| ^^^^^^^^
20+
= note: `#[deny(dead_code)]` implied by `#[deny(warnings)]`
21+
22+
error: aborting due to 1 previous error
23+
24+
Couldn't compile the test.
25+
26+
failures:
27+
$DIR/dead-code-module-2.rs - my_mod::f (line 16)
28+
29+
test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in $TIME
30+

0 commit comments

Comments
 (0)