From 35135e2c3c5f912c2e84d0333fe41f44ccb31623 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 31 May 2024 16:56:04 -0700 Subject: [PATCH] rustdoc: Remove `DoctestVisitor::get_line` This was used to get the line number of the first line from the current docstring, which was then used together with an offset within the docstring. It's simpler to just pass the offset to the visitor and have it do the math because it's clearer and this calculation only needs to be done in one place (the Rust doctest visitor). --- src/librustdoc/doctest.rs | 11 ++++----- src/librustdoc/doctest/markdown.rs | 10 ++++---- src/librustdoc/doctest/rust.rs | 17 ++++++------- src/librustdoc/html/markdown.rs | 24 ++++++++++++++++++- .../passes/check_custom_code_classes.rs | 4 ++-- .../passes/check_doc_test_visibility.rs | 4 ++-- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index b6782d907a96..239dac6401b9 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -34,7 +34,7 @@ use std::sync::{Arc, Mutex}; use tempfile::{Builder as TempFileBuilder, TempDir}; use crate::config::Options as RustdocOptions; -use crate::html::markdown::{ErrorCodes, Ignore, LangString}; +use crate::html::markdown::{ErrorCodes, Ignore, LangString, MdRelLine}; use crate::lint::init_lints; use self::rust::HirCollector; @@ -962,10 +962,7 @@ struct ScrapedDoctest { } pub(crate) trait DoctestVisitor { - fn visit_test(&mut self, test: String, config: LangString, line: usize); - fn get_line(&self) -> usize { - 0 - } + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine); fn visit_header(&mut self, _name: &str, _level: u32) {} } @@ -1188,8 +1185,8 @@ fn doctest_run_fn( #[cfg(test)] // used in tests impl DoctestVisitor for Vec { - fn visit_test(&mut self, _test: String, _config: LangString, line: usize) { - self.push(line); + fn visit_test(&mut self, _test: String, _config: LangString, rel_line: MdRelLine) { + self.push(1 + rel_line.offset()); } } diff --git a/src/librustdoc/doctest/markdown.rs b/src/librustdoc/doctest/markdown.rs index a057056f04bd..3300b88474dc 100644 --- a/src/librustdoc/doctest/markdown.rs +++ b/src/librustdoc/doctest/markdown.rs @@ -9,7 +9,7 @@ use super::{ generate_args_file, CreateRunnableDoctests, DoctestVisitor, GlobalTestOptions, ScrapedDoctest, }; use crate::config::Options; -use crate::html::markdown::{find_testable_code, ErrorCodes, LangString}; +use crate::html::markdown::{find_testable_code, ErrorCodes, LangString, MdRelLine}; struct MdCollector { tests: Vec, @@ -18,8 +18,10 @@ struct MdCollector { } impl DoctestVisitor for MdCollector { - fn visit_test(&mut self, test: String, config: LangString, line: usize) { + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine) { let filename = self.filename.clone(); + // First line of Markdown is line 1. + let line = 1 + rel_line.offset(); self.tests.push(ScrapedDoctest { filename, line, @@ -29,10 +31,6 @@ impl DoctestVisitor for MdCollector { }); } - fn get_line(&self) -> usize { - 0 - } - fn visit_header(&mut self, name: &str, level: u32) { // We use these headings as test names, so it's good if // they're valid identifiers. diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs index afe294c1e208..92a2c4ecf3a9 100644 --- a/src/librustdoc/doctest/rust.rs +++ b/src/librustdoc/doctest/rust.rs @@ -15,7 +15,7 @@ use rustc_span::{BytePos, FileName, Pos, Span, DUMMY_SP}; use super::{DoctestVisitor, ScrapedDoctest}; use crate::clean::{types::AttributesExt, Attributes}; -use crate::html::markdown::{self, ErrorCodes, LangString}; +use crate::html::markdown::{self, ErrorCodes, LangString, MdRelLine}; struct RustCollector { source_map: Lrc, @@ -36,10 +36,17 @@ impl RustCollector { } filename } + + fn get_base_line(&self) -> usize { + let sp_lo = self.position.lo().to_usize(); + let loc = self.source_map.lookup_char_pos(BytePos(sp_lo as u32)); + loc.line + } } impl DoctestVisitor for RustCollector { - fn visit_test(&mut self, test: String, config: LangString, line: usize) { + fn visit_test(&mut self, test: String, config: LangString, rel_line: MdRelLine) { + let line = self.get_base_line() + rel_line.offset(); self.tests.push(ScrapedDoctest { filename: self.get_filename(), line, @@ -49,12 +56,6 @@ impl DoctestVisitor for RustCollector { }); } - fn get_line(&self) -> usize { - let line = self.position.lo().to_usize(); - let line = self.source_map.lookup_char_pos(BytePos(line as u32)).line; - if line > 0 { line - 1 } else { line } - } - fn visit_header(&mut self, _name: &str, _level: u32) {} } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 8edb5d31900e..3192ca32d21e 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -727,6 +727,28 @@ impl<'a, I: Iterator>> Iterator for Footnotes<'a, I> { } } +/// A newtype that represents a relative line number in Markdown. +/// +/// In other words, this represents an offset from the first line of Markdown +/// in a doc comment or other source. If the first Markdown line appears on line 32, +/// and the `MdRelLine` is 3, then the absolute line for this one is 35. I.e., it's +/// a zero-based offset. +pub(crate) struct MdRelLine { + offset: usize, +} + +impl MdRelLine { + /// See struct docs. + pub(crate) const fn new(offset: usize) -> Self { + Self { offset } + } + + /// See struct docs. + pub(crate) const fn offset(self) -> usize { + self.offset + } +} + pub(crate) fn find_testable_code( doc: &str, tests: &mut T, @@ -800,7 +822,7 @@ pub(crate) fn find_codes( if nb_lines != 0 && !&doc[prev_offset..offset.start].ends_with('\n') { nb_lines -= 1; } - let line = tests.get_line() + nb_lines + 1; + let line = MdRelLine::new(nb_lines); tests.visit_test(text, block_info, line); prev_offset = offset.start; } diff --git a/src/librustdoc/passes/check_custom_code_classes.rs b/src/librustdoc/passes/check_custom_code_classes.rs index c5a4249c1026..31a3aaca92f1 100644 --- a/src/librustdoc/passes/check_custom_code_classes.rs +++ b/src/librustdoc/passes/check_custom_code_classes.rs @@ -7,7 +7,7 @@ use super::Pass; use crate::clean::{Crate, Item}; use crate::core::DocContext; use crate::fold::DocFolder; -use crate::html::markdown::{find_codes, ErrorCodes, LangString}; +use crate::html::markdown::{find_codes, ErrorCodes, LangString, MdRelLine}; use rustc_errors::StashKey; use rustc_feature::GateIssue; @@ -47,7 +47,7 @@ struct TestsWithCustomClasses { } impl crate::doctest::DoctestVisitor for TestsWithCustomClasses { - fn visit_test(&mut self, _: String, config: LangString, _: usize) { + fn visit_test(&mut self, _: String, config: LangString, _: MdRelLine) { self.custom_classes_found.extend(config.added_classes); } } diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index fdafb225f17a..c01bfad7a181 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -10,7 +10,7 @@ use crate::clean; use crate::clean::utils::inherits_doc_hidden; use crate::clean::*; use crate::core::DocContext; -use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString}; +use crate::html::markdown::{find_testable_code, ErrorCodes, Ignore, LangString, MdRelLine}; use crate::visit::DocVisitor; use rustc_hir as hir; use rustc_middle::lint::LintLevelSource; @@ -45,7 +45,7 @@ pub(crate) struct Tests { } impl crate::doctest::DoctestVisitor for Tests { - fn visit_test(&mut self, _: String, config: LangString, _: usize) { + fn visit_test(&mut self, _: String, config: LangString, _: MdRelLine) { if config.rust && config.ignore == Ignore::None { self.found_tests += 1; }