Auto merge of #85457 - jyn514:remove-doc-include, r=GuillaumeGomez

Remove `doc(include)`

This nightly feature is redundant now that `extended_key_value_attributes` is stable (#83366). `@rust-lang/rustdoc` not sure if you think this needs FCP; there was already an FCP in #82539, but technically it was for deprecating, not removing the feature altogether.

This should not be merged before #83366.

cc `@petrochenkov`

Author: bors

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -318,7 +318,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                 }}
 
                 gate_doc!(
-                    include => external_doc
                     cfg => doc_cfg
                     masked => doc_masked
                     notable_trait => doc_notable_trait

compiler/rustc_expand/src/base.rs

@@ -1068,11 +1068,11 @@ impl<'a> ExtCtxt<'a> {
         self.resolver.check_unused_macros();
     }
 
-    /// Resolves a path mentioned inside Rust code.
+    /// Resolves a `path` mentioned inside Rust code, returning an absolute path.
     ///
-    /// This unifies the logic used for resolving `include_X!`, and `#[doc(include)]` file paths.
+    /// This unifies the logic used for resolving `include_X!`.
     ///
-    /// Returns an absolute path to the file that `path` refers to.
+    /// FIXME: move this to `rustc_builtin_macros` and make it private.
     pub fn resolve_path(
         &self,
         path: impl Into<PathBuf>,

compiler/rustc_expand/src/expand.rs

@@ -12,11 +12,11 @@ use rustc_ast::ptr::P;
 use rustc_ast::token;
 use rustc_ast::tokenstream::TokenStream;
 use rustc_ast::visit::{self, AssocCtxt, Visitor};
-use rustc_ast::{AstLike, AttrItem, Block, Inline, ItemKind, LitKind, MacArgs};
+use rustc_ast::{AstLike, Block, Inline, ItemKind, MacArgs};
 use rustc_ast::{MacCallStmt, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem};
 use rustc_ast::{NodeId, PatKind, Path, StmtKind, Unsafe};
 use rustc_ast_pretty::pprust;
-use rustc_attr::{self as attr, is_builtin_attr};
+use rustc_attr::is_builtin_attr;
 use rustc_data_structures::map_in_place::MapInPlace;
 use rustc_data_structures::stack::ensure_sufficient_stack;
 use rustc_data_structures::sync::Lrc;
@@ -28,15 +28,14 @@ use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS;
 use rustc_session::lint::BuiltinLintDiagnostics;
 use rustc_session::parse::{feature_err, ParseSess};
 use rustc_session::Limit;
-use rustc_span::symbol::{sym, Ident, Symbol};
-use rustc_span::{ExpnId, FileName, Span, DUMMY_SP};
+use rustc_span::symbol::{sym, Ident};
+use rustc_span::{ExpnId, FileName, Span};
 
 use smallvec::{smallvec, SmallVec};
-use std::io::ErrorKind;
 use std::ops::DerefMut;
 use std::path::PathBuf;
 use std::rc::Rc;
-use std::{iter, mem, slice};
+use std::{iter, mem};
 
 macro_rules! ast_fragments {
     (
@@ -1524,139 +1523,6 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
         noop_flat_map_generic_param(param, self)
     }
 
-    fn visit_attribute(&mut self, at: &mut ast::Attribute) {
-        // turn `#[doc(include="filename")]` attributes into `#[doc(include(file="filename",
-        // contents="file contents")]` attributes
-        if !self.cx.sess.check_name(at, sym::doc) {
-            return noop_visit_attribute(at, self);
-        }
-
-        if let Some(list) = at.meta_item_list() {
-            if !list.iter().any(|it| it.has_name(sym::include)) {
-                return noop_visit_attribute(at, self);
-            }
-
-            let mut items = vec![];
-
-            for mut it in list {
-                if !it.has_name(sym::include) {
-                    items.push({
-                        noop_visit_meta_list_item(&mut it, self);
-                        it
-                    });
-                    continue;
-                }
-
-                if let Some(file) = it.value_str() {
-                    let err_count = self.cx.sess.parse_sess.span_diagnostic.err_count();
-                    self.check_attributes(slice::from_ref(at));
-                    if self.cx.sess.parse_sess.span_diagnostic.err_count() > err_count {
-                        // avoid loading the file if they haven't enabled the feature
-                        return noop_visit_attribute(at, self);
-                    }
-
-                    let filename = match self.cx.resolve_path(&*file.as_str(), it.span()) {
-                        Ok(filename) => filename,
-                        Err(mut err) => {
-                            err.emit();
-                            continue;
-                        }
-                    };
-
-                    match self.cx.source_map().load_file(&filename) {
-                        Ok(source_file) => {
-                            let src = source_file
-                                .src
-                                .as_ref()
-                                .expect("freshly loaded file should have a source");
-                            let src_interned = Symbol::intern(src.as_str());
-
-                            let include_info = vec![
-                                ast::NestedMetaItem::MetaItem(attr::mk_name_value_item_str(
-                                    Ident::with_dummy_span(sym::file),
-                                    file,
-                                    DUMMY_SP,
-                                )),
-                                ast::NestedMetaItem::MetaItem(attr::mk_name_value_item_str(
-                                    Ident::with_dummy_span(sym::contents),
-                                    src_interned,
-                                    DUMMY_SP,
-                                )),
-                            ];
-
-                            let include_ident = Ident::with_dummy_span(sym::include);
-                            let item = attr::mk_list_item(include_ident, include_info);
-                            items.push(ast::NestedMetaItem::MetaItem(item));
-                        }
-                        Err(e) => {
-                            let lit_span = it.name_value_literal_span().unwrap();
-
-                            if e.kind() == ErrorKind::InvalidData {
-                                self.cx
-                                    .struct_span_err(
-                                        lit_span,
-                                        &format!("{} wasn't a utf-8 file", filename.display()),
-                                    )
-                                    .span_label(lit_span, "contains invalid utf-8")
-                                    .emit();
-                            } else {
-                                let mut err = self.cx.struct_span_err(
-                                    lit_span,
-                                    &format!("couldn't read {}: {}", filename.display(), e),
-                                );
-                                err.span_label(lit_span, "couldn't read file");
-
-                                err.emit();
-                            }
-                        }
-                    }
-                } else {
-                    let mut err = self
-                        .cx
-                        .struct_span_err(it.span(), "expected path to external documentation");
-
-                    // Check if the user erroneously used `doc(include(...))` syntax.
-                    let literal = it.meta_item_list().and_then(|list| {
-                        if list.len() == 1 {
-                            list[0].literal().map(|literal| &literal.kind)
-                        } else {
-                            None
-                        }
-                    });
-
-                    let (path, applicability) = match &literal {
-                        Some(LitKind::Str(path, ..)) => {
-                            (path.to_string(), Applicability::MachineApplicable)
-                        }
-                        _ => (String::from("<path>"), Applicability::HasPlaceholders),
-                    };
-
-                    err.span_suggestion(
-                        it.span(),
-                        "provide a file path with `=`",
-                        format!("include = \"{}\"", path),
-                        applicability,
-                    );
-
-                    err.emit();
-                }
-            }
-
-            let meta = attr::mk_list_item(Ident::with_dummy_span(sym::doc), items);
-            *at = ast::Attribute {
-                kind: ast::AttrKind::Normal(
-                    AttrItem { path: meta.path, args: meta.kind.mac_args(meta.span), tokens: None },
-                    None,
-                ),
-                span: at.span,
-                id: at.id,
-                style: at.style,
-            };
-        } else {
-            noop_visit_attribute(at, self)
-        }
-    }
-
     fn visit_id(&mut self, id: &mut ast::NodeId) {
         if self.monotonic {
             debug_assert_eq!(*id, ast::DUMMY_NODE_ID);

compiler/rustc_feature/src/active.rs

@@ -370,9 +370,6 @@ declare_features! (
     /// Allows `#[doc(masked)]`.
     (active, doc_masked, "1.21.0", Some(44027), None),
 
-    /// Allows `#[doc(include = "some-file")]`.
-    (active, external_doc, "1.22.0", Some(44732), None),
-
     /// Allows using `crate` as visibility modifier, synonymous with `pub(crate)`.
     (active, crate_visibility_modifier, "1.23.0", Some(53120), None),
 

compiler/rustc_feature/src/removed.rs

@@ -140,6 +140,10 @@ declare_features! (
     (removed, const_fn, "1.54.0", Some(57563), None,
      Some("split into finer-grained feature gates")),
 
+    /// Allows `#[doc(include = "some-file")]`.
+    (removed, external_doc, "1.54.0", Some(44732), None,
+     Some("use #[doc = include_str!(\"filename\")] instead, which handles macro invocations")),
+
     // -------------------------------------------------------------------------
     // feature-group-end: removed features
     // -------------------------------------------------------------------------

compiler/rustc_lint/src/builtin.rs

@@ -489,7 +489,7 @@ fn has_doc(sess: &Session, attr: &ast::Attribute) -> bool {
 
     if let Some(list) = attr.meta_item_list() {
         for meta in list {
-            if meta.has_name(sym::include) || meta.has_name(sym::hidden) {
+            if meta.has_name(sym::hidden) {
                 return true;
             }
         }

compiler/rustc_passes/src/check_attr.rs

@@ -705,7 +705,7 @@ impl CheckAttrVisitor<'tcx> {
         let mut is_valid = true;
 
         if let Some(list) = attr.meta().and_then(|mi| mi.meta_item_list().map(|l| l.to_vec())) {
-            for meta in list {
+            for meta in &list {
                 if let Some(i_meta) = meta.meta_item() {
                     match i_meta.name_or_empty() {
                         sym::alias
@@ -757,7 +757,6 @@ impl CheckAttrVisitor<'tcx> {
                         | sym::html_no_source
                         | sym::html_playground_url
                         | sym::html_root_url
-                        | sym::include
                         | sym::inline
                         | sym::issue_tracker_base_url
                         | sym::keyword
@@ -792,6 +791,30 @@ impl CheckAttrVisitor<'tcx> {
                                         );
                                         diag.note("`doc(spotlight)` is now a no-op");
                                     }
+                                    if i_meta.has_name(sym::include) {
+                                        if let Some(value) = i_meta.value_str() {
+                                            // if there are multiple attributes, the suggestion would suggest deleting all of them, which is incorrect
+                                            let applicability = if list.len() == 1 {
+                                                Applicability::MachineApplicable
+                                            } else {
+                                                Applicability::MaybeIncorrect
+                                            };
+                                            let inner = if attr.style == AttrStyle::Inner {
+                                                "!"
+                                            } else {
+                                                ""
+                                            };
+                                            diag.span_suggestion(
+                                                attr.meta().unwrap().span,
+                                                "use `doc = include_str!` instead",
+                                                format!(
+                                                    "#{}[doc = include_str!(\"{}\")]",
+                                                    inner, value
+                                                ),
+                                                applicability,
+                                            );
+                                        }
+                                    }
                                     diag.emit();
                                 },
                             );

compiler/rustc_save_analysis/src/lib.rs

@@ -826,20 +826,6 @@ impl<'tcx> SaveContext<'tcx> {
                 // FIXME: Should save-analysis beautify doc strings itself or leave it to users?
                 result.push_str(&beautify_doc_string(val).as_str());
                 result.push('\n');
-            } else if self.tcx.sess.check_name(attr, sym::doc) {
-                if let Some(meta_list) = attr.meta_item_list() {
-                    meta_list
-                        .into_iter()
-                        .filter(|it| it.has_name(sym::include))
-                        .filter_map(|it| it.meta_item_list().map(|l| l.to_owned()))
-                        .flat_map(|it| it)
-                        .filter(|meta| meta.has_name(sym::contents))
-                        .filter_map(|meta| meta.value_str())
-                        .for_each(|val| {
-                            result.push_str(&val.as_str());
-                            result.push('\n');
-                        });
-                }
             }
         }
 

library/core/src/lib.rs

@@ -159,7 +159,6 @@
 #![feature(const_fn_transmute)]
 #![feature(abi_unadjusted)]
 #![feature(adx_target_feature)]
-#![feature(external_doc)]
 #![feature(associated_type_bounds)]
 #![feature(const_caller_location)]
 #![feature(slice_ptr_get)]

src/doc/rustdoc/src/documentation-tests.md

@@ -424,9 +424,7 @@ without including it in your main documentation. For example, you could write th
 `lib.rs` to test your README as part of your doctests:
 
 ```rust,no_run
-#![feature(external_doc)]
-
-#[doc(include = "../README.md")]
+#[doc = include_str!("../README.md")]
 #[cfg(doctest)]
 pub struct ReadmeDoctests;
 ```

src/doc/rustdoc/src/unstable-features.md

@@ -131,22 +131,6 @@ Book][unstable-masked] and [its tracking issue][issue-masked].
 [unstable-masked]: ../unstable-book/language-features/doc-masked.html
 [issue-masked]: https://github.com/rust-lang/rust/issues/44027
 
-### Include external files as API documentation
-
-As designed in [RFC 1990], Rustdoc can read an external file to use as a type's documentation. This
-is useful if certain documentation is so long that it would break the flow of reading the source.
-Instead of writing it all inline, writing `#[doc(include = "sometype.md")]` will ask Rustdoc to
-instead read that file and use it as if it were written inline.
-
-[RFC 1990]: https://github.com/rust-lang/rfcs/pull/1990
-
-`#[doc(include = "...")]` currently requires the `#![feature(external_doc)]` feature gate. For more
-information, see [its chapter in the Unstable Book][unstable-include] and [its tracking
-issue][issue-include].
-
-[unstable-include]: ../unstable-book/language-features/external-doc.html
-[issue-include]: https://github.com/rust-lang/rust/issues/44732
-
 ## Unstable command-line arguments
 
 These features are enabled by passing a command-line flag to Rustdoc, but the flags in question are