From 460d4ac7d88528c520121a54a87a5927b8298417 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 25 Feb 2021 10:22:56 -0500 Subject: [PATCH] Deprecate `doc(include)` ## Why deprecate doc(include)? This nightly feature is a strictly less general version of `extended_key_value_attributes`, which looks like this: ``` #[doc = include_str!("docs.md")] ``` In particular, that feature allows manipulating the filename and included docs in a way that `doc(include)` cannot, due to eager evaluation: ``` #[doc = concat!( "These are my docs: ", include_str!(concat!(env!("OUT_DIR"), "generated_docs.md")) )] ``` For more about eager evaluation and how it impacts macro parsing, see petrochenkov's excellent writeup: https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455 Given that `#[doc(include)]` offers no features that `extended_key_value_attributes` cannot, it makes no sense for the language and rustdoc to maintain it indefinitely. This deprecates `doc(include)` and adds a structured suggestion to switch to switch to `doc = include_str!` instead. ## Implementation Notably, this changes attribute cleaning to pass in whether an item is local or not. This is necessary to avoid warning about `doc(include)` in dependencies (see https://github.com/rust-lang/rust/issues/82284 for the sort of trouble that can cause). --- src/librustdoc/clean/inline.rs | 13 ++++--- src/librustdoc/clean/mod.rs | 17 ++++----- src/librustdoc/clean/types.rs | 29 +++++++++++---- src/librustdoc/doctest.rs | 2 +- .../auxiliary/doc-include-deprecated.rs | 5 +++ src/test/rustdoc-ui/auxiliary/docs.md | 1 + src/test/rustdoc-ui/doc-include-deprecated.rs | 14 ++++++++ .../rustdoc-ui/doc-include-deprecated.stderr | 35 +++++++++++++++++++ 8 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 src/test/rustdoc-ui/auxiliary/doc-include-deprecated.rs create mode 100644 src/test/rustdoc-ui/auxiliary/docs.md create mode 100644 src/test/rustdoc-ui/doc-include-deprecated.rs create mode 100644 src/test/rustdoc-ui/doc-include-deprecated.stderr diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 277ec91f15ed7..bb1fbd1565ddd 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -19,6 +19,7 @@ use crate::clean::{self, Attributes, GetDefId, ToSource, TypeKind}; use crate::core::DocContext; use crate::formats::item_type::ItemType; +use super::clean_attrs; use super::Clean; type Attrs<'hir> = rustc_middle::ty::Attributes<'hir>; @@ -121,7 +122,7 @@ crate fn try_inline( }; let target_attrs = load_attrs(cx, did); - let attrs = box merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone); + let attrs = box merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone, did.is_local()); cx.inlined.insert(did); let what_rustc_thinks = clean::Item::from_def_id_and_parts(did, Some(name), kind, cx); @@ -289,6 +290,7 @@ fn merge_attrs( parent_module: Option, old_attrs: Attrs<'_>, new_attrs: Option>, + local: bool, ) -> clean::Attributes { // NOTE: If we have additional attributes (from a re-export), // always insert them first. This ensure that re-export @@ -297,14 +299,14 @@ fn merge_attrs( if let Some(inner) = new_attrs { if let Some(new_id) = parent_module { let diag = cx.sess().diagnostic(); - Attributes::from_ast(diag, old_attrs, Some((inner, new_id))) + Attributes::from_ast(diag, old_attrs, Some((inner, new_id)), local) } else { let mut both = inner.to_vec(); both.extend_from_slice(old_attrs); - both.clean(cx) + clean_attrs(&both, local, cx) } } else { - old_attrs.clean(cx) + clean_attrs(old_attrs, local, cx) } } @@ -415,7 +417,8 @@ crate fn build_impl( debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id()); - let attrs = box merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs); + let attrs = + box merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs, did.is_local()); debug!("merged_attrs={:?}", attrs); ret.push(clean::Item::from_def_id_and_attrs_and_parts( diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 217e899001ef9..9fcd35f841173 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -108,7 +108,9 @@ impl Clean for CrateNum { // rendering by delegating everything to a hash map. let mut as_primitive = |res: Res| { if let Res::Def(DefKind::Mod, def_id) = res { - let attrs = cx.tcx.get_attrs(def_id).clean(cx); + // We already warned about any attributes on the module when cleaning it. + // Don't warn a second time. + let attrs = clean_attrs(cx.tcx.get_attrs(def_id), false, cx); let mut prim = None; for attr in attrs.lists(sym::doc) { if let Some(v) = attr.value_str() { @@ -155,7 +157,7 @@ impl Clean for CrateNum { let mut as_keyword = |res: Res| { if let Res::Def(DefKind::Mod, def_id) = res { - let attrs = tcx.get_attrs(def_id).clean(cx); + let attrs = clean_attrs(tcx.get_attrs(def_id), false, cx); let mut keyword = None; for attr in attrs.lists(sym::doc) { if attr.has_name(sym::keyword) { @@ -197,7 +199,8 @@ impl Clean for CrateNum { ExternalCrate { name: tcx.crate_name(*self), src: krate_src, - attrs: tcx.get_attrs(root).clean(cx), + // The local crate was already cleaned, and all other crates are non-local. + attrs: clean_attrs(tcx.get_attrs(root), false, cx), primitives, keywords, } @@ -237,10 +240,8 @@ impl Clean for doctree::Module<'_> { } } -impl Clean for [ast::Attribute] { - fn clean(&self, cx: &mut DocContext<'_>) -> Attributes { - Attributes::from_ast(cx.sess().diagnostic(), self, None) - } +fn clean_attrs(attrs: &[ast::Attribute], local: bool, cx: &mut DocContext<'_>) -> Attributes { + Attributes::from_ast(cx.sess().diagnostic(), attrs, None, local) } impl Clean for hir::GenericBound<'_> { @@ -2124,7 +2125,7 @@ fn clean_extern_crate( // FIXME: using `from_def_id_and_kind` breaks `rustdoc/masked` for some reason vec![Item { name: Some(name), - attrs: box attrs.clean(cx), + attrs: box clean_attrs(attrs, true, cx), span: krate.span.clean(cx), def_id: crate_def_id, visibility: krate.vis.clean(cx), diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index f3c9b987eb02a..e7cafa64783b5 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -16,6 +16,7 @@ use rustc_ast::{self as ast, AttrStyle}; use rustc_attr::{ConstStability, Deprecation, Stability, StabilityLevel}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::thin_vec::ThinVec; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, DefIndex}; @@ -32,11 +33,10 @@ use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; use crate::clean::cfg::Cfg; -use crate::clean::external_path; -use crate::clean::inline::{self, print_inlined_const}; use crate::clean::types::Type::{QPath, ResolvedPath}; use crate::clean::utils::{is_literal_expr, print_const_expr, print_evaluated_const}; use crate::clean::Clean; +use crate::clean::{clean_attrs, external_path, inline}; use crate::core::DocContext; use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; @@ -154,7 +154,7 @@ impl Item { def_id, name, kind, - box cx.tcx.get_attrs(def_id).clean(cx), + box clean_attrs(cx.tcx.get_attrs(def_id), def_id.is_local(), cx), cx, ) } @@ -739,9 +739,10 @@ impl Attributes { } crate fn from_ast( - diagnostic: &::rustc_errors::Handler, + handler: &::rustc_errors::Handler, attrs: &[ast::Attribute], additional_attrs: Option<(&[ast::Attribute], DefId)>, + local: bool, ) -> Attributes { let mut doc_strings: Vec = vec![]; let mut sp = None; @@ -800,10 +801,26 @@ impl Attributes { // Extracted #[doc(cfg(...))] match Cfg::parse(cfg_mi) { Ok(new_cfg) => cfg &= new_cfg, - Err(e) => diagnostic.span_err(e.span, e.msg), + Err(e) => handler.span_err(e.span, e.msg), } } else if let Some((filename, contents)) = Attributes::extract_include(&mi) { + if local { + let mut diag = handler + .struct_span_warn(attr.span, "`doc(include)` is deprecated"); + diag.span_suggestion_verbose( + attr.span, + "use `#![feature(extended_key_value_attributes)]` instead", + format!( + "#{}[doc = include_str!(\"{}\")]", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + filename, + ), + Applicability::MaybeIncorrect, + ); + diag.emit(); + } + let line = doc_line; doc_line += contents.as_str().lines().count(); let frag = DocFragment { @@ -2001,7 +2018,7 @@ impl Constant { crate fn expr(&self, tcx: TyCtxt<'_>) -> String { match self.kind { ConstantKind::TyConst { ref expr } => expr.clone(), - ConstantKind::Extern { def_id } => print_inlined_const(tcx, def_id), + ConstantKind::Extern { def_id } => inline::print_inlined_const(tcx, def_id), ConstantKind::Local { body, .. } | ConstantKind::Anonymous { body } => { print_const_expr(tcx, body) } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 6f6ed0eb68413..7930fbca69dfe 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1093,7 +1093,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> { nested: F, ) { let attrs = self.tcx.hir().attrs(hir_id); - let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs, None); + let mut attrs = Attributes::from_ast(self.sess.diagnostic(), attrs, None, true); if let Some(ref cfg) = attrs.cfg { if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) { return; diff --git a/src/test/rustdoc-ui/auxiliary/doc-include-deprecated.rs b/src/test/rustdoc-ui/auxiliary/doc-include-deprecated.rs new file mode 100644 index 0000000000000..3fb2a7d5fa1fc --- /dev/null +++ b/src/test/rustdoc-ui/auxiliary/doc-include-deprecated.rs @@ -0,0 +1,5 @@ +#![crate_name = "inner"] +#![feature(external_doc)] +#[doc(include = "docs.md")] +pub struct HasDocInclude {} +pub struct NoDocs {} diff --git a/src/test/rustdoc-ui/auxiliary/docs.md b/src/test/rustdoc-ui/auxiliary/docs.md new file mode 100644 index 0000000000000..a32e8ca85fdf5 --- /dev/null +++ b/src/test/rustdoc-ui/auxiliary/docs.md @@ -0,0 +1 @@ +Here have some docs diff --git a/src/test/rustdoc-ui/doc-include-deprecated.rs b/src/test/rustdoc-ui/doc-include-deprecated.rs new file mode 100644 index 0000000000000..2fc825f983f98 --- /dev/null +++ b/src/test/rustdoc-ui/doc-include-deprecated.rs @@ -0,0 +1,14 @@ +// aux-build:doc-include-deprecated.rs +// check-pass +#![feature(external_doc)] +#![doc(include = "auxiliary/docs.md")] //~ WARNING deprecated + +extern crate inner; + +pub use inner::HasDocInclude; + +#[doc(include = "auxiliary/docs.md")] //~ WARNING deprecated +pub use inner::HasDocInclude as _; + +#[doc(include = "auxiliary/docs.md")] //~ WARNING deprecated +pub use inner::NoDocs; diff --git a/src/test/rustdoc-ui/doc-include-deprecated.stderr b/src/test/rustdoc-ui/doc-include-deprecated.stderr new file mode 100644 index 0000000000000..f733868117a9f --- /dev/null +++ b/src/test/rustdoc-ui/doc-include-deprecated.stderr @@ -0,0 +1,35 @@ +warning: `doc(include)` is deprecated + --> $DIR/doc-include-deprecated.rs:10:1 + | +LL | #[doc(include = "auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `#![feature(extended_key_value_attributes)]` instead + | +LL | #[doc = include_str!("auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: `doc(include)` is deprecated + --> $DIR/doc-include-deprecated.rs:13:1 + | +LL | #[doc(include = "auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `#![feature(extended_key_value_attributes)]` instead + | +LL | #[doc = include_str!("auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: `doc(include)` is deprecated + --> $DIR/doc-include-deprecated.rs:4:1 + | +LL | #![doc(include = "auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `#![feature(extended_key_value_attributes)]` instead + | +LL | #![doc = include_str!("auxiliary/docs.md")] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 3 warnings emitted +