Skip to content

Emit a warning if the doctest main function will not be run #140527

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex};
use std::{panic, str};

pub(crate) use make::DocTestBuilder;
pub(crate) use make::{BuildDocTestBuilder, DocTestBuilder};
pub(crate) use markdown::test as test_markdown;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
use rustc_errors::emitter::HumanReadableErrorType;
Expand All @@ -23,9 +23,9 @@ use rustc_hir::def_id::LOCAL_CRATE;
use rustc_interface::interface;
use rustc_session::config::{self, CrateType, ErrorOutputType, Input};
use rustc_session::lint;
use rustc_span::FileName;
use rustc_span::edition::Edition;
use rustc_span::symbol::sym;
use rustc_span::{FileName, Span};
use rustc_target::spec::{Target, TargetTuple};
use tempfile::{Builder as TempFileBuilder, TempDir};
use tracing::debug;
Expand Down Expand Up @@ -239,7 +239,7 @@ pub(crate) fn run(dcx: DiagCtxtHandle<'_>, input: Input, options: RustdocOptions
}
} else {
let mut collector = CreateRunnableDocTests::new(options, opts);
tests.into_iter().for_each(|t| collector.add_test(t));
tests.into_iter().for_each(|t| collector.add_test(t, Some(compiler.sess.dcx())));

Ok(Some(collector))
}
Expand Down Expand Up @@ -872,6 +872,7 @@ pub(crate) struct ScrapedDocTest {
langstr: LangString,
text: String,
name: String,
span: Span,
}

impl ScrapedDocTest {
Expand All @@ -881,6 +882,7 @@ impl ScrapedDocTest {
logical_path: Vec<String>,
langstr: LangString,
text: String,
span: Span,
) -> Self {
let mut item_path = logical_path.join("::");
item_path.retain(|c| c != ' ');
Expand All @@ -890,7 +892,7 @@ impl ScrapedDocTest {
let name =
format!("{} - {item_path}(line {line})", filename.prefer_remapped_unconditionaly());

Self { filename, line, langstr, text, name }
Self { filename, line, langstr, text, name, span }
}
fn edition(&self, opts: &RustdocOptions) -> Edition {
self.langstr.edition.unwrap_or(opts.edition)
Expand Down Expand Up @@ -946,7 +948,7 @@ impl CreateRunnableDocTests {
}
}

fn add_test(&mut self, scraped_test: ScrapedDocTest) {
fn add_test(&mut self, scraped_test: ScrapedDocTest, dcx: Option<DiagCtxtHandle<'_>>) {
// For example `module/file.rs` would become `module_file_rs`
let file = scraped_test
.filename
Expand All @@ -970,14 +972,14 @@ impl CreateRunnableDocTests {
);

let edition = scraped_test.edition(&self.rustdoc_options);
let doctest = DocTestBuilder::new(
&scraped_test.text,
Some(&self.opts.crate_name),
edition,
self.can_merge_doctests,
Some(test_id),
Some(&scraped_test.langstr),
);
let doctest = BuildDocTestBuilder::new(&scraped_test.text)
.crate_name(&self.opts.crate_name)
.edition(edition)
.can_merge_doctests(self.can_merge_doctests)
.test_id(test_id)
.lang_str(&scraped_test.langstr)
.span(scraped_test.span)
.build(dcx);
let is_standalone = !doctest.can_be_merged
|| scraped_test.langstr.compile_fail
|| scraped_test.langstr.test_harness
Expand Down
17 changes: 7 additions & 10 deletions src/librustdoc/doctest/extracted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use serde::Serialize;

use super::{DocTestBuilder, ScrapedDocTest};
use super::{BuildDocTestBuilder, ScrapedDocTest};
use crate::config::Options as RustdocOptions;
use crate::html::markdown;

Expand Down Expand Up @@ -35,16 +35,13 @@ impl ExtractedDocTests {
) {
let edition = scraped_test.edition(options);

let ScrapedDocTest { filename, line, langstr, text, name } = scraped_test;
let ScrapedDocTest { filename, line, langstr, text, name, .. } = scraped_test;

let doctest = DocTestBuilder::new(
&text,
Some(&opts.crate_name),
edition,
false,
None,
Some(&langstr),
);
let doctest = BuildDocTestBuilder::new(&text)
.crate_name(&opts.crate_name)
.edition(edition)
.lang_str(&langstr)
.build(None);
let (full_test_code, size) = doctest.generate_unique_doctest(
&text,
langstr.test_harness,
Expand Down
146 changes: 112 additions & 34 deletions src/librustdoc/doctest/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ use std::sync::Arc;
use rustc_ast::token::{Delimiter, TokenKind};
use rustc_ast::tokenstream::TokenTree;
use rustc_ast::{self as ast, AttrStyle, HasAttrs, StmtKind};
use rustc_errors::ColorConfig;
use rustc_errors::emitter::stderr_destination;
use rustc_errors::{ColorConfig, DiagCtxtHandle};
use rustc_parse::new_parser_from_source_str;
use rustc_session::parse::ParseSess;
use rustc_span::edition::Edition;
use rustc_span::edition::{DEFAULT_EDITION, Edition};
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::sym;
use rustc_span::{FileName, kw};
use rustc_span::{DUMMY_SP, FileName, Span, kw};
use tracing::debug;

use super::GlobalTestOptions;
Expand All @@ -35,41 +35,86 @@ struct ParseSourceInfo {
maybe_crate_attrs: String,
}

/// This struct contains information about the doctest itself which is then used to generate
/// doctest source code appropriately.
pub(crate) struct DocTestBuilder {
pub(crate) supports_color: bool,
pub(crate) already_has_extern_crate: bool,
pub(crate) has_main_fn: bool,
pub(crate) crate_attrs: String,
/// If this is a merged doctest, it will be put into `everything_else`, otherwise it will
/// put into `crate_attrs`.
pub(crate) maybe_crate_attrs: String,
pub(crate) crates: String,
pub(crate) everything_else: String,
pub(crate) test_id: Option<String>,
pub(crate) invalid_ast: bool,
pub(crate) can_be_merged: bool,
/// Builder type for `DocTestBuilder`.
pub(crate) struct BuildDocTestBuilder<'a> {
source: &'a str,
crate_name: Option<&'a str>,
edition: Edition,
can_merge_doctests: bool,
// If `test_id` is `None`, it means we're generating code for a code example "run" link.
test_id: Option<String>,
lang_str: Option<&'a LangString>,
span: Span,
}

impl DocTestBuilder {
pub(crate) fn new(
source: &str,
crate_name: Option<&str>,
edition: Edition,
can_merge_doctests: bool,
// If `test_id` is `None`, it means we're generating code for a code example "run" link.
test_id: Option<String>,
lang_str: Option<&LangString>,
) -> Self {
impl<'a> BuildDocTestBuilder<'a> {
pub(crate) fn new(source: &'a str) -> Self {
Self {
source,
crate_name: None,
edition: DEFAULT_EDITION,
can_merge_doctests: false,
test_id: None,
lang_str: None,
span: DUMMY_SP,
}
}

#[inline]
pub(crate) fn crate_name(mut self, crate_name: &'a str) -> Self {
self.crate_name = Some(crate_name);
self
}

#[inline]
pub(crate) fn can_merge_doctests(mut self, can_merge_doctests: bool) -> Self {
self.can_merge_doctests = can_merge_doctests;
self
}

#[inline]
pub(crate) fn test_id(mut self, test_id: String) -> Self {
self.test_id = Some(test_id);
self
}

#[inline]
pub(crate) fn lang_str(mut self, lang_str: &'a LangString) -> Self {
self.lang_str = Some(lang_str);
self
}

#[inline]
pub(crate) fn span(mut self, span: Span) -> Self {
self.span = span;
self
}

#[inline]
pub(crate) fn edition(mut self, edition: Edition) -> Self {
self.edition = edition;
self
}

pub(crate) fn build(self, dcx: Option<DiagCtxtHandle<'_>>) -> DocTestBuilder {
let BuildDocTestBuilder {
source,
crate_name,
edition,
can_merge_doctests,
// If `test_id` is `None`, it means we're generating code for a code example "run" link.
test_id,
lang_str,
span,
} = self;
let can_merge_doctests = can_merge_doctests
&& lang_str.is_some_and(|lang_str| {
!lang_str.compile_fail && !lang_str.test_harness && !lang_str.standalone_crate
});

let result = rustc_driver::catch_fatal_errors(|| {
rustc_span::create_session_if_not_set_then(edition, |_| {
parse_source(source, &crate_name)
parse_source(source, &crate_name, dcx, span)
})
});

Expand All @@ -87,7 +132,7 @@ impl DocTestBuilder {
else {
// If the AST returned an error, we don't want this doctest to be merged with the
// others.
return Self::invalid(
return DocTestBuilder::invalid(
String::new(),
String::new(),
String::new(),
Expand All @@ -107,7 +152,7 @@ impl DocTestBuilder {
// If this is a merged doctest and a defined macro uses `$crate`, then the path will
// not work, so better not put it into merged doctests.
&& !(has_macro_def && everything_else.contains("$crate"));
Self {
DocTestBuilder {
supports_color,
has_main_fn,
crate_attrs,
Expand All @@ -120,7 +165,26 @@ impl DocTestBuilder {
can_be_merged,
}
}
}

/// This struct contains information about the doctest itself which is then used to generate
/// doctest source code appropriately.
pub(crate) struct DocTestBuilder {
pub(crate) supports_color: bool,
pub(crate) already_has_extern_crate: bool,
pub(crate) has_main_fn: bool,
pub(crate) crate_attrs: String,
/// If this is a merged doctest, it will be put into `everything_else`, otherwise it will
/// put into `crate_attrs`.
pub(crate) maybe_crate_attrs: String,
pub(crate) crates: String,
pub(crate) everything_else: String,
pub(crate) test_id: Option<String>,
pub(crate) invalid_ast: bool,
pub(crate) can_be_merged: bool,
}

impl DocTestBuilder {
fn invalid(
crate_attrs: String,
maybe_crate_attrs: String,
Expand Down Expand Up @@ -289,7 +353,12 @@ fn reset_error_count(psess: &ParseSess) {

const DOCTEST_CODE_WRAPPER: &str = "fn f(){";

fn parse_source(source: &str, crate_name: &Option<&str>) -> Result<ParseSourceInfo, ()> {
fn parse_source(
source: &str,
crate_name: &Option<&str>,
parent_dcx: Option<DiagCtxtHandle<'_>>,
span: Span,
) -> Result<ParseSourceInfo, ()> {
use rustc_errors::DiagCtxt;
use rustc_errors::emitter::{Emitter, HumanEmitter};
use rustc_span::source_map::FilePathMapping;
Expand Down Expand Up @@ -475,8 +544,17 @@ fn parse_source(source: &str, crate_name: &Option<&str>) -> Result<ParseSourceIn
}
}
if has_non_items {
// FIXME: if `info.has_main_fn` is `true`, emit a warning here to mention that
// this code will not be called.
if info.has_main_fn
&& let Some(dcx) = parent_dcx
&& !span.is_dummy()
{
dcx.span_warn(
span,
"the `main` function of this doctest won't be run as it contains \
expressions at the top level, meaning that the whole doctest code will be \
wrapped in a function",
);
}
info.has_main_fn = false;
}
Ok(info)
Expand Down
13 changes: 10 additions & 3 deletions src/librustdoc/doctest/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::fs::read_to_string;
use std::sync::{Arc, Mutex};

use rustc_session::config::Input;
use rustc_span::FileName;
use rustc_span::{DUMMY_SP, FileName};
use tempfile::tempdir;

use super::{
Expand All @@ -24,7 +24,14 @@ impl DocTestVisitor for MdCollector {
let filename = self.filename.clone();
// First line of Markdown is line 1.
let line = 1 + rel_line.offset();
self.tests.push(ScrapedDocTest::new(filename, line, self.cur_path.clone(), config, test));
self.tests.push(ScrapedDocTest::new(
filename,
line,
self.cur_path.clone(),
config,
test,
DUMMY_SP,
));
}

fn visit_header(&mut self, name: &str, level: u32) {
Expand Down Expand Up @@ -107,7 +114,7 @@ pub(crate) fn test(input: &Input, options: Options) -> Result<(), String> {
find_testable_code(&input_str, &mut md_collector, codes, None);

let mut collector = CreateRunnableDocTests::new(options.clone(), opts);
md_collector.tests.into_iter().for_each(|t| collector.add_test(t));
md_collector.tests.into_iter().for_each(|t| collector.add_test(t, None));
let CreateRunnableDocTests { opts, rustdoc_options, standalone_tests, mergeable_tests, .. } =
collector;
crate::doctest::run_tests(
Expand Down
Loading
Loading