From e9035f7bef47c21b575af517dce309d96df4eb51 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 20 Jan 2022 11:06:45 -0500 Subject: [PATCH 1/8] refactor: prepare to associate multiple spans with a module. --- compiler/rustc_ast/src/ast.rs | 15 ++++++++++++++- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_ast_lowering/src/item.rs | 2 +- compiler/rustc_builtin_macros/src/test_harness.rs | 4 +++- compiler/rustc_expand/src/expand.rs | 6 +++--- compiler/rustc_expand/src/module.rs | 4 ++-- compiler/rustc_parse/src/parser/item.rs | 7 ++++--- src/tools/rustfmt/src/parse/parser.rs | 2 +- src/tools/rustfmt/src/visitor.rs | 2 +- 9 files changed, 30 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 725499e5c78ca..517d16cd5262d 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2317,11 +2317,24 @@ pub enum ModKind { /// or with definition outlined to a separate file `mod foo;` and already loaded from it. /// The inner span is from the first token past `{` to the last token until `}`, /// or from the first to the last token in the loaded file. - Loaded(Vec>, Inline, Span), + Loaded(Vec>, Inline, ModSpans), /// Module with definition outlined to a separate file `mod foo;` but not yet loaded from it. Unloaded, } +#[derive(Clone, Encodable, Decodable, Debug)] +pub struct ModSpans { + /// `inner_span` covers the body of the module; for a file module, its the whole file. + /// For an inline module, its the span inside the `{ ... }`, not including the curly braces. + pub inner_span: Span, +} + +impl Default for ModSpans { + fn default() -> ModSpans { + ModSpans { inner_span: Default::default() } + } +} + /// Foreign module declaration. /// /// E.g., `extern { .. }` or `extern "C" { .. }`. diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index a81a227629539..8bc3afc500a48 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1009,7 +1009,7 @@ pub fn noop_visit_item_kind(kind: &mut ItemKind, vis: &mut T) { ItemKind::Mod(unsafety, mod_kind) => { visit_unsafety(unsafety, vis); match mod_kind { - ModKind::Loaded(items, _inline, inner_span) => { + ModKind::Loaded(items, _inline, ModSpans { inner_span }) => { vis.visit_span(inner_span); items.flat_map_in_place(|item| vis.flat_map_item(item)); } diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index e8fca6f04ba5b..aee117e554f17 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -263,7 +263,7 @@ impl<'hir> LoweringContext<'_, 'hir> { }) } ItemKind::Mod(_, ref mod_kind) => match mod_kind { - ModKind::Loaded(items, _, inner_span) => { + ModKind::Loaded(items, _, ModSpans { inner_span }) => { hir::ItemKind::Mod(self.lower_mod(items, *inner_span)) } ModKind::Unloaded => panic!("`mod` items should have been loaded by now"), diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index 7ee0fb9b817ca..a489b23bad823 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -129,7 +129,9 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { // We don't want to recurse into anything other than mods, since // mods or tests inside of functions will break things - if let ast::ItemKind::Mod(_, ModKind::Loaded(.., span)) = item.kind { + if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ast::ModSpans { inner_span: span })) = + item.kind + { let prev_tests = mem::take(&mut self.tests); noop_visit_item_kind(&mut item.kind, self); self.add_test_cases(item.id, span, prev_tests); diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 1b97618050939..dc4d4e90cd233 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -12,8 +12,8 @@ use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast::{AssocItemKind, AstLike, AstLikeWrapper, AttrStyle, ExprKind, ForeignItemKind}; -use rustc_ast::{Inline, ItemKind, MacArgs, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem}; -use rustc_ast::{NodeId, PatKind, StmtKind, TyKind}; +use rustc_ast::{Inline, ItemKind, MacArgs, MacStmtStyle, MetaItemKind, ModKind, ModSpans}; +use rustc_ast::{NestedMetaItem, NodeId, PatKind, StmtKind, TyKind}; use rustc_ast_pretty::pprust; use rustc_data_structures::map_in_place::MapInPlace; use rustc_data_structures::sync::Lrc; @@ -1112,7 +1112,7 @@ impl InvocationCollectorNode for P { ); } - *mod_kind = ModKind::Loaded(items, Inline::No, inner_span); + *mod_kind = ModKind::Loaded(items, Inline::No, ModSpans { inner_span }); node.attrs = attrs; if node.attrs.len() > old_attrs_len { // If we loaded an out-of-line module and added some inner attributes, diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index a6e1232628f93..a584c69fa708c 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,6 +1,6 @@ use crate::base::ModuleData; use rustc_ast::ptr::P; -use rustc_ast::{token, Attribute, Inline, Item}; +use rustc_ast::{token, Attribute, Inline, Item, ModSpans}; use rustc_errors::{struct_span_err, DiagnosticBuilder, ErrorGuaranteed}; use rustc_parse::new_parser_from_file; use rustc_parse::validate_attr; @@ -69,7 +69,7 @@ crate fn parse_external_mod( (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. - let (items, inner_span, file_path) = + let (items, ModSpans { inner_span }, file_path) = result.map_err(|err| err.report(sess, span)).unwrap_or_default(); // Extract the directory path for submodules of the module. diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index ae46bfe354083..aa0d8d6874872 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -26,7 +26,8 @@ use tracing::debug; impl<'a> Parser<'a> { /// Parses a source module as a crate. This is the main entry point for the parser. pub fn parse_crate_mod(&mut self) -> PResult<'a, ast::Crate> { - let (attrs, items, span) = self.parse_mod(&token::Eof)?; + let (attrs, items, spans) = self.parse_mod(&token::Eof)?; + let span = spans.inner_span; Ok(ast::Crate { attrs, items, span, id: DUMMY_NODE_ID, is_placeholder: false }) } @@ -51,7 +52,7 @@ impl<'a> Parser<'a> { pub fn parse_mod( &mut self, term: &TokenKind, - ) -> PResult<'a, (Vec, Vec>, Span)> { + ) -> PResult<'a, (Vec, Vec>, ModSpans)> { let lo = self.token.span; let attrs = self.parse_inner_attributes()?; @@ -71,7 +72,7 @@ impl<'a> Parser<'a> { } } - Ok((attrs, items, lo.to(self.prev_token.span))) + Ok((attrs, items, ModSpans { inner_span: lo.to(self.prev_token.span) })) } } diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index f0944a88d2f22..3b4e762b6dd15 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -113,7 +113,7 @@ impl<'a> Parser<'a> { let result = catch_unwind(AssertUnwindSafe(|| { let mut parser = new_parser_from_file(sess.inner(), path, Some(span)); match parser.parse_mod(&TokenKind::Eof) { - Ok(result) => Some(result), + Ok((a, i, ast::ModSpans { inner_span })) => Some((a, i, inner_span)), Err(mut e) => { e.emit(); if sess.can_reset_errors() { diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index 0177689958aa7..57a58c6048466 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -915,7 +915,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let ident_str = rewrite_ident(&self.get_context(), ident).to_owned(); self.push_str(&ident_str); - if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, inner_span) = mod_kind { + if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ast::ModSpans{ inner_span }) = mod_kind { match self.config.brace_style() { BraceStyle::AlwaysNextLine => { let indent_str = self.block_indent.to_string_with_newline(self.config); From b82795244e31ce1ad60bbb823c4e4b91f921c296 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 3 Mar 2022 18:45:25 -0500 Subject: [PATCH 2/8] Associate multiple with a crate too. --- compiler/rustc_ast/src/ast.rs | 2 +- compiler/rustc_ast/src/mut_visit.rs | 7 ++++--- compiler/rustc_ast_lowering/src/lib.rs | 2 +- compiler/rustc_builtin_macros/src/test_harness.rs | 2 +- compiler/rustc_expand/src/base.rs | 2 +- compiler/rustc_expand/src/expand.rs | 8 ++++---- compiler/rustc_expand/src/module.rs | 6 +++--- compiler/rustc_expand/src/placeholders.rs | 2 +- compiler/rustc_metadata/src/creader.rs | 2 +- compiler/rustc_parse/src/lib.rs | 2 +- compiler/rustc_parse/src/parser/item.rs | 3 +-- compiler/rustc_resolve/src/lib.rs | 4 ++-- src/librustdoc/passes/collect_intra_doc_links/early.rs | 2 +- src/test/ui/ast-json/ast-json-noexpand-output.stdout | 2 +- src/test/ui/ast-json/ast-json-output.stdout | 2 +- src/tools/rustfmt/src/modules.rs | 4 ++-- src/tools/rustfmt/src/visitor.rs | 3 ++- 17 files changed, 28 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 517d16cd5262d..6a74a19c83e3f 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -510,7 +510,7 @@ pub struct WhereEqPredicate { pub struct Crate { pub attrs: Vec, pub items: Vec>, - pub span: Span, + pub spans: ModSpans, /// Must be equal to `CRATE_NODE_ID` after the crate root is expanded, but may hold /// expansion placeholders or an unassigned value (`DUMMY_NODE_ID`) before that. pub id: NodeId, diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 8bc3afc500a48..9358869789243 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1108,11 +1108,12 @@ pub fn noop_visit_fn_header(header: &mut FnHeader, vis: &mut T) { } pub fn noop_visit_crate(krate: &mut Crate, vis: &mut T) { - let Crate { attrs, items, span, id, is_placeholder: _ } = krate; + let Crate { attrs, items, spans, id, is_placeholder: _ } = krate; vis.visit_id(id); visit_attrs(attrs, vis); items.flat_map_in_place(|item| vis.flat_map_item(item)); - vis.visit_span(span); + let ModSpans { inner_span } = spans; + vis.visit_span(inner_span); } // Mutates one item into possibly many items. @@ -1536,7 +1537,7 @@ impl DummyAstNode for Crate { Crate { attrs: Default::default(), items: Default::default(), - span: Default::default(), + spans: Default::default(), id: DUMMY_NODE_ID, is_placeholder: Default::default(), } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 940887280b9c1..1221dcd2bb371 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -456,7 +456,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); self.with_hir_id_owner(CRATE_NODE_ID, |lctx| { - let module = lctx.lower_mod(&c.items, c.span); + let module = lctx.lower_mod(&c.items, c.spans.inner_span); lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs); hir::OwnerNode::Crate(lctx.arena.alloc(module)) }); diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index a489b23bad823..4bbeece2bdefc 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -112,7 +112,7 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { fn visit_crate(&mut self, c: &mut ast::Crate) { let prev_tests = mem::take(&mut self.tests); noop_visit_crate(c, self); - self.add_test_cases(ast::CRATE_NODE_ID, c.span, prev_tests); + self.add_test_cases(ast::CRATE_NODE_ID, c.spans.inner_span, prev_tests); // Create a main function to run our tests c.items.push(mk_main(&mut self.cx)); diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index 1df48b7fe1520..1263f31f6aa1b 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -67,7 +67,7 @@ impl Annotatable { Annotatable::Param(ref p) => p.span, Annotatable::FieldDef(ref sf) => sf.span, Annotatable::Variant(ref v) => v.span, - Annotatable::Crate(ref c) => c.span, + Annotatable::Crate(ref c) => c.spans.inner_span, } } diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index dc4d4e90cd233..91c3e7dc1661a 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -12,7 +12,7 @@ use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast::{AssocItemKind, AstLike, AstLikeWrapper, AttrStyle, ExprKind, ForeignItemKind}; -use rustc_ast::{Inline, ItemKind, MacArgs, MacStmtStyle, MetaItemKind, ModKind, ModSpans}; +use rustc_ast::{Inline, ItemKind, MacArgs, MacStmtStyle, MetaItemKind, ModKind}; use rustc_ast::{NestedMetaItem, NodeId, PatKind, StmtKind, TyKind}; use rustc_ast_pretty::pprust; use rustc_data_structures::map_in_place::MapInPlace; @@ -364,7 +364,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } pub fn expand_crate(&mut self, krate: ast::Crate) -> ast::Crate { - let file_path = match self.cx.source_map().span_to_filename(krate.span) { + let file_path = match self.cx.source_map().span_to_filename(krate.spans.inner_span) { FileName::Real(name) => name .into_local_path() .expect("attempting to resolve a file path in an external file"), @@ -1091,7 +1091,7 @@ impl InvocationCollectorNode for P { ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. let old_attrs_len = attrs.len(); - let ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } = + let ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } = parse_external_mod( &ecx.sess, ident, @@ -1112,7 +1112,7 @@ impl InvocationCollectorNode for P { ); } - *mod_kind = ModKind::Loaded(items, Inline::No, ModSpans { inner_span }); + *mod_kind = ModKind::Loaded(items, Inline::No, spans); node.attrs = attrs; if node.attrs.len() > old_attrs_len { // If we loaded an out-of-line module and added some inner attributes, diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index a584c69fa708c..2a059f3519d1e 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -28,7 +28,7 @@ pub struct ModulePathSuccess { crate struct ParsedExternalMod { pub items: Vec>, - pub inner_span: Span, + pub spans: ModSpans, pub file_path: PathBuf, pub dir_path: PathBuf, pub dir_ownership: DirOwnership, @@ -69,13 +69,13 @@ crate fn parse_external_mod( (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. - let (items, ModSpans { inner_span }, file_path) = + let (items, spans, file_path) = result.map_err(|err| err.report(sess, span)).unwrap_or_default(); // Extract the directory path for submodules of the module. let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); - ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } + ParsedExternalMod { items, spans, file_path, dir_path, dir_ownership } } crate fn mod_dir_path( diff --git a/compiler/rustc_expand/src/placeholders.rs b/compiler/rustc_expand/src/placeholders.rs index af593e92634b0..15af5fdc5f8e2 100644 --- a/compiler/rustc_expand/src/placeholders.rs +++ b/compiler/rustc_expand/src/placeholders.rs @@ -49,7 +49,7 @@ pub fn placeholder( AstFragmentKind::Crate => AstFragment::Crate(ast::Crate { attrs: Default::default(), items: Default::default(), - span, + spans: ast::ModSpans { inner_span: span, ..Default::default() }, id, is_placeholder: true, }), diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 7343f1465f603..f667aec03c577 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -899,7 +899,7 @@ impl<'a> CrateLoader<'a> { fn report_unused_deps(&mut self, krate: &ast::Crate) { // Make a point span rather than covering the whole file - let span = krate.span.shrink_to_lo(); + let span = krate.spans.inner_span.shrink_to_lo(); // Complain about anything left over for (name, entry) in self.sess.opts.externs.iter() { if let ExternLocation::FoundInLibrarySearchDirectories = entry.location { diff --git a/compiler/rustc_parse/src/lib.rs b/compiler/rustc_parse/src/lib.rs index 5c95a9e7bb616..a3e66464fbc8f 100644 --- a/compiler/rustc_parse/src/lib.rs +++ b/compiler/rustc_parse/src/lib.rs @@ -331,7 +331,7 @@ pub fn fake_token_stream(sess: &ParseSess, nt: &Nonterminal) -> TokenStream { pub fn fake_token_stream_for_crate(sess: &ParseSess, krate: &ast::Crate) -> TokenStream { let source = pprust::crate_to_string_for_macros(krate); let filename = FileName::macro_expansion_source_code(&source); - parse_stream_from_source_str(filename, source, sess, Some(krate.span)) + parse_stream_from_source_str(filename, source, sess, Some(krate.spans.inner_span)) } pub fn parse_cfg_attr( diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index aa0d8d6874872..484a27fa59d66 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -27,8 +27,7 @@ impl<'a> Parser<'a> { /// Parses a source module as a crate. This is the main entry point for the parser. pub fn parse_crate_mod(&mut self) -> PResult<'a, ast::Crate> { let (attrs, items, spans) = self.parse_mod(&token::Eof)?; - let span = spans.inner_span; - Ok(ast::Crate { attrs, items, span, id: DUMMY_NODE_ID, is_placeholder: false }) + Ok(ast::Crate { attrs, items, spans, id: DUMMY_NODE_ID, is_placeholder: false }) } /// Parses a `mod { ... }` or `mod ;` item. diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 086f0249114ba..9eedd9839eb5e 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -1282,7 +1282,7 @@ impl<'a> Resolver<'a> { None, ModuleKind::Def(DefKind::Mod, root_def_id, kw::Empty), ExpnId::root(), - krate.span, + krate.spans.inner_span, session.contains_name(&krate.attrs, sym::no_implicit_prelude), &mut module_map, ); @@ -1295,7 +1295,7 @@ impl<'a> Resolver<'a> { &mut FxHashMap::default(), ); - let definitions = Definitions::new(session.local_stable_crate_id(), krate.span); + let definitions = Definitions::new(session.local_stable_crate_id(), krate.spans.inner_span); let root = definitions.get_root_def(); let mut visibilities = FxHashMap::default(); diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 5c11ab1d3beb9..1d28bbde79c13 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -39,7 +39,7 @@ crate fn early_resolve_intra_doc_links( // Overridden `visit_item` below doesn't apply to the crate root, // so we have to visit its attributes and reexports separately. - loader.load_links_in_attrs(&krate.attrs, krate.span); + loader.load_links_in_attrs(&krate.attrs, krate.spans.inner_span); loader.process_module_children_or_reexports(CRATE_DEF_ID.to_def_id()); visit::walk_crate(&mut loader, krate); loader.add_foreign_traits_in_scope(); diff --git a/src/test/ui/ast-json/ast-json-noexpand-output.stdout b/src/test/ui/ast-json/ast-json-noexpand-output.stdout index ab70c5b91c65c..a456c10cf470a 100644 --- a/src/test/ui/ast-json/ast-json-noexpand-output.stdout +++ b/src/test/ui/ast-json/ast-json-noexpand-output.stdout @@ -1 +1 @@ -{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"span":{"lo":0,"hi":0},"id":0,"is_placeholder":false} +{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} diff --git a/src/test/ui/ast-json/ast-json-output.stdout b/src/test/ui/ast-json/ast-json-output.stdout index f3663d9953b8a..d385c76039a56 100644 --- a/src/test/ui/ast-json/ast-json-output.stdout +++ b/src/test/ui/ast-json/ast-json-output.stdout @@ -1 +1 @@ -{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"prelude_import","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"","span":{"lo":0,"hi":0}},"kind":{"variant":"Use","fields":[{"prefix":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"{{root}}","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"std","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"prelude","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"rust_2015","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"kind":"Glob","span":{"lo":0,"hi":0}}]},"tokens":null},{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"macro_use","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"std","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null},{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"span":{"lo":0,"hi":0},"id":0,"is_placeholder":false} +{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"prelude_import","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"","span":{"lo":0,"hi":0}},"kind":{"variant":"Use","fields":[{"prefix":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"{{root}}","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"std","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"prelude","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"rust_2015","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"kind":"Glob","span":{"lo":0,"hi":0}}]},"tokens":null},{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"macro_use","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"std","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null},{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} diff --git a/src/tools/rustfmt/src/modules.rs b/src/tools/rustfmt/src/modules.rs index d4bddd957858f..64d96a5c6a6e4 100644 --- a/src/tools/rustfmt/src/modules.rs +++ b/src/tools/rustfmt/src/modules.rs @@ -124,7 +124,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { mut self, krate: &'ast ast::Crate, ) -> Result, ModuleResolutionError> { - let root_filename = self.parse_sess.span_to_filename(krate.span); + let root_filename = self.parse_sess.span_to_filename(krate.spans.inner_span); self.directory.path = match root_filename { FileName::Real(ref p) => p.parent().unwrap_or(Path::new("")).to_path_buf(), _ => PathBuf::new(), @@ -135,7 +135,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { self.visit_mod_from_ast(&krate.items)?; } - let snippet_provider = self.parse_sess.snippet_provider(krate.span); + let snippet_provider = self.parse_sess.snippet_provider(krate.spans.inner_span); self.file_map.insert( root_filename, diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index 57a58c6048466..c44b2fc6ae354 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -915,7 +915,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let ident_str = rewrite_ident(&self.get_context(), ident).to_owned(); self.push_str(&ident_str); - if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ast::ModSpans{ inner_span }) = mod_kind { + if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans) = mod_kind { + let ast::ModSpans { inner_span } = *spans; match self.config.brace_style() { BraceStyle::AlwaysNextLine => { let indent_str = self.block_indent.to_string_with_newline(self.config); From d37da1e332a77a4cd66c2f36b4a5457f40a7bfd5 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 20 Jan 2022 14:07:54 -0500 Subject: [PATCH 3/8] Adjusted diagnostic output so that if there is no `use` in a item sequence, then we just suggest the first legal position where you could inject a use. To do this, I added `inject_use_span` field to `ModSpans`, and populate it in parser (it is the span of the first token found after inner attributes, if any). Then I rewrote the use-suggestion code to utilize it, and threw out some stuff that is now unnecessary with this in place. (I think the result is easier to understand.) Then I added a test of issue 87613. --- compiler/rustc_ast/src/ast.rs | 5 +- compiler/rustc_ast/src/mut_visit.rs | 6 +- compiler/rustc_ast_lowering/src/item.rs | 2 +- .../rustc_builtin_macros/src/test_harness.rs | 5 +- compiler/rustc_parse/src/parser/item.rs | 5 +- compiler/rustc_resolve/src/lib.rs | 101 +++++++++--------- .../ast-json/ast-json-noexpand-output.stdout | 2 +- src/test/ui/ast-json/ast-json-output.stdout | 2 +- src/test/ui/proc-macro/amputate-span.rs | 63 +++++++++++ src/test/ui/proc-macro/amputate-span.stderr | 25 +++++ .../ui/proc-macro/auxiliary/amputate-span.rs | 14 +++ src/tools/rustfmt/src/parse/parser.rs | 2 +- src/tools/rustfmt/src/visitor.rs | 2 +- 13 files changed, 168 insertions(+), 66 deletions(-) create mode 100644 src/test/ui/proc-macro/amputate-span.rs create mode 100644 src/test/ui/proc-macro/amputate-span.stderr create mode 100644 src/test/ui/proc-macro/auxiliary/amputate-span.rs diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 6a74a19c83e3f..796dac57744c2 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2322,16 +2322,17 @@ pub enum ModKind { Unloaded, } -#[derive(Clone, Encodable, Decodable, Debug)] +#[derive(Copy, Clone, Encodable, Decodable, Debug)] pub struct ModSpans { /// `inner_span` covers the body of the module; for a file module, its the whole file. /// For an inline module, its the span inside the `{ ... }`, not including the curly braces. pub inner_span: Span, + pub inject_use_span: Span, } impl Default for ModSpans { fn default() -> ModSpans { - ModSpans { inner_span: Default::default() } + ModSpans { inner_span: Default::default(), inject_use_span: Default::default() } } } diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 9358869789243..b87637d2dde6e 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1009,8 +1009,9 @@ pub fn noop_visit_item_kind(kind: &mut ItemKind, vis: &mut T) { ItemKind::Mod(unsafety, mod_kind) => { visit_unsafety(unsafety, vis); match mod_kind { - ModKind::Loaded(items, _inline, ModSpans { inner_span }) => { + ModKind::Loaded(items, _inline, ModSpans { inner_span, inject_use_span }) => { vis.visit_span(inner_span); + vis.visit_span(inject_use_span); items.flat_map_in_place(|item| vis.flat_map_item(item)); } ModKind::Unloaded => {} @@ -1112,8 +1113,9 @@ pub fn noop_visit_crate(krate: &mut Crate, vis: &mut T) { vis.visit_id(id); visit_attrs(attrs, vis); items.flat_map_in_place(|item| vis.flat_map_item(item)); - let ModSpans { inner_span } = spans; + let ModSpans { inner_span, inject_use_span } = spans; vis.visit_span(inner_span); + vis.visit_span(inject_use_span); } // Mutates one item into possibly many items. diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index aee117e554f17..a7401be6db32f 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -263,7 +263,7 @@ impl<'hir> LoweringContext<'_, 'hir> { }) } ItemKind::Mod(_, ref mod_kind) => match mod_kind { - ModKind::Loaded(items, _, ModSpans { inner_span }) => { + ModKind::Loaded(items, _, ModSpans { inner_span, inject_use_span: _ }) => { hir::ItemKind::Mod(self.lower_mod(items, *inner_span)) } ModKind::Unloaded => panic!("`mod` items should have been loaded by now"), diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index 4bbeece2bdefc..e2553ab40cad4 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -129,9 +129,8 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { // We don't want to recurse into anything other than mods, since // mods or tests inside of functions will break things - if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ast::ModSpans { inner_span: span })) = - item.kind - { + if let ast::ItemKind::Mod(_, ModKind::Loaded(.., ref spans)) = item.kind { + let ast::ModSpans { inner_span: span, inject_use_span: _ } = *spans; let prev_tests = mem::take(&mut self.tests); noop_visit_item_kind(&mut item.kind, self); self.add_test_cases(item.id, span, prev_tests); diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 484a27fa59d66..c370195659d0c 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -55,6 +55,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; let attrs = self.parse_inner_attributes()?; + let post_attr_lo = self.token.span; let mut items = vec![]; while let Some(item) = self.parse_item(ForceCollect::No)? { items.push(item); @@ -71,7 +72,9 @@ impl<'a> Parser<'a> { } } - Ok((attrs, items, ModSpans { inner_span: lo.to(self.prev_token.span) })) + let inject_use_span = post_attr_lo.data().with_hi(post_attr_lo.lo()); + let mod_spans = ModSpans { inner_span: lo.to(self.prev_token.span), inject_use_span }; + Ok((attrs, items, mod_spans)) } } diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 9eedd9839eb5e..03b4f0609bfd2 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -71,7 +71,6 @@ use rustc_span::{Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; use std::cell::{Cell, RefCell}; use std::collections::BTreeSet; -use std::ops::ControlFlow; use std::{cmp, fmt, iter, mem, ptr}; use tracing::debug; @@ -315,74 +314,70 @@ impl<'a> From<&'a ast::PathSegment> for Segment { } } +#[derive(Debug)] struct UsePlacementFinder { target_module: NodeId, - span: Option, - found_use: bool, + first_legal_span: Option, + first_use_span: Option, } impl UsePlacementFinder { fn check(krate: &Crate, target_module: NodeId) -> (Option, bool) { - let mut finder = UsePlacementFinder { target_module, span: None, found_use: false }; - if let ControlFlow::Continue(..) = finder.check_mod(&krate.items, CRATE_NODE_ID) { - visit::walk_crate(&mut finder, krate); - } - (finder.span, finder.found_use) - } - - fn check_mod(&mut self, items: &[P], node_id: NodeId) -> ControlFlow<()> { - if self.span.is_some() { - return ControlFlow::Break(()); - } - if node_id != self.target_module { - return ControlFlow::Continue(()); - } - // find a use statement - for item in items { - match item.kind { - ItemKind::Use(..) => { - // don't suggest placing a use before the prelude - // import or other generated ones - if !item.span.from_expansion() { - self.span = Some(item.span.shrink_to_lo()); - self.found_use = true; - return ControlFlow::Break(()); - } - } - // don't place use before extern crate - ItemKind::ExternCrate(_) => {} - // but place them before the first other item - _ => { - if self.span.map_or(true, |span| item.span < span) - && !item.span.from_expansion() - { - self.span = Some(item.span.shrink_to_lo()); - // don't insert between attributes and an item - // find the first attribute on the item - // FIXME: This is broken for active attributes. - for attr in &item.attrs { - if !attr.span.is_dummy() - && self.span.map_or(true, |span| attr.span < span) - { - self.span = Some(attr.span.shrink_to_lo()); - } - } - } - } + let mut finder = + UsePlacementFinder { target_module, first_legal_span: None, first_use_span: None }; + finder.visit_crate(krate); + if let Some(use_span) = finder.first_use_span { + (Some(use_span), true) + } else { + (finder.first_legal_span, false) + } + } +} + +fn is_span_suitable_for_use_injection(s: Span) -> bool { + // don't suggest placing a use before the prelude + // import or other generated ones + !s.from_expansion() +} + +fn search_for_any_use_in_items(items: &[P]) -> Option { + for item in items { + if let ItemKind::Use(..) = item.kind { + if is_span_suitable_for_use_injection(item.span) { + return Some(item.span.shrink_to_lo()); } } - ControlFlow::Continue(()) } + return None; } impl<'tcx> Visitor<'tcx> for UsePlacementFinder { + fn visit_crate(&mut self, c: &Crate) { + if self.target_module == CRATE_NODE_ID { + let inject = c.spans.inject_use_span; + if is_span_suitable_for_use_injection(inject) { + self.first_legal_span = Some(inject); + } + self.first_use_span = search_for_any_use_in_items(&c.items); + return; + } else { + visit::walk_crate(self, c); + } + } + fn visit_item(&mut self, item: &'tcx ast::Item) { - if let ItemKind::Mod(_, ModKind::Loaded(items, ..)) = &item.kind { - if let ControlFlow::Break(..) = self.check_mod(items, item.id) { + if self.target_module == item.id { + if let ItemKind::Mod(_, ModKind::Loaded(items, _inline, mod_spans)) = &item.kind { + let inject = mod_spans.inject_use_span; + if is_span_suitable_for_use_injection(inject) { + self.first_legal_span = Some(inject); + } + self.first_use_span = search_for_any_use_in_items(items); return; } + } else { + visit::walk_item(self, item); } - visit::walk_item(self, item); } } diff --git a/src/test/ui/ast-json/ast-json-noexpand-output.stdout b/src/test/ui/ast-json/ast-json-noexpand-output.stdout index a456c10cf470a..746ced689d448 100644 --- a/src/test/ui/ast-json/ast-json-noexpand-output.stdout +++ b/src/test/ui/ast-json/ast-json-noexpand-output.stdout @@ -1 +1 @@ -{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} +{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0},"inject_use_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} diff --git a/src/test/ui/ast-json/ast-json-output.stdout b/src/test/ui/ast-json/ast-json-output.stdout index d385c76039a56..b0aaa663f38c8 100644 --- a/src/test/ui/ast-json/ast-json-output.stdout +++ b/src/test/ui/ast-json/ast-json-output.stdout @@ -1 +1 @@ -{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"prelude_import","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"","span":{"lo":0,"hi":0}},"kind":{"variant":"Use","fields":[{"prefix":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"{{root}}","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"std","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"prelude","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"rust_2015","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"kind":"Glob","span":{"lo":0,"hi":0}}]},"tokens":null},{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"macro_use","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"std","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null},{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} +{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"crate_type","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":{"variant":"Eq","fields":[{"lo":0,"hi":0},{"kind":{"variant":"Interpolated","fields":[{"variant":"NtExpr","fields":[{"id":0,"kind":{"variant":"Lit","fields":[{"token":{"kind":"Str","symbol":"lib","suffix":null},"kind":{"variant":"Str","fields":["lib","Cooked"]},"span":{"lo":0,"hi":0}}]},"span":{"lo":0,"hi":0},"attrs":{"0":null},"tokens":{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}}]}]},"span":{"lo":0,"hi":0}}]},"tokens":null},{"0":[[{"variant":"Token","fields":[{"kind":"Pound","span":{"lo":0,"hi":0}}]},"Joint"],[{"variant":"Token","fields":[{"kind":"Not","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Delimited","fields":[{"open":{"lo":0,"hi":0},"close":{"lo":0,"hi":0}},"Bracket",{"0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate_type",false]},"span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":"Eq","span":{"lo":0,"hi":0}}]},"Alone"],[{"variant":"Token","fields":[{"kind":{"variant":"Literal","fields":[{"kind":"Str","symbol":"lib","suffix":null}]},"span":{"lo":0,"hi":0}}]},"Alone"]]}]},"Alone"]]}]},"id":null,"style":"Inner","span":{"lo":0,"hi":0}}],"items":[{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"prelude_import","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"","span":{"lo":0,"hi":0}},"kind":{"variant":"Use","fields":[{"prefix":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"{{root}}","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"std","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"prelude","span":{"lo":0,"hi":0}},"id":0,"args":null},{"ident":{"name":"rust_2015","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"kind":"Glob","span":{"lo":0,"hi":0}}]},"tokens":null},{"attrs":[{"kind":{"variant":"Normal","fields":[{"path":{"span":{"lo":0,"hi":0},"segments":[{"ident":{"name":"macro_use","span":{"lo":0,"hi":0}},"id":0,"args":null}],"tokens":null},"args":"Empty","tokens":null},null]},"id":null,"style":"Outer","span":{"lo":0,"hi":0}}],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"std","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null},{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"kind":"Inherited","span":{"lo":0,"hi":0},"tokens":null},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":null}],"spans":{"inner_span":{"lo":0,"hi":0},"inject_use_span":{"lo":0,"hi":0}},"id":0,"is_placeholder":false} diff --git a/src/test/ui/proc-macro/amputate-span.rs b/src/test/ui/proc-macro/amputate-span.rs new file mode 100644 index 0000000000000..3ca7c847a88dd --- /dev/null +++ b/src/test/ui/proc-macro/amputate-span.rs @@ -0,0 +1,63 @@ +// aux-build:amputate-span.rs +// edition:2018 +// compile-flags: --extern amputate_span + +// This test has been crafted to ensure the following things: +// +// 1. There's a resolution error that prompts the compiler to suggest +// adding a `use` item. +// +// 2. There are no `use` or `extern crate` items in the source +// code. In fact, there is only one item, the `fn main` +// declaration. +// +// 3. The single `fn main` declaration has an attribute attached to it +// that just deletes the first token from the given item. +// +// You need all of these conditions to hold in order to replicate the +// scenario that yielded issue 87613, where the compiler's suggestion +// looks like: +// +// ``` +// help: consider importing this struct +// | +// 47 | hey */ async use std::process::Command; +// | ++++++++++++++++++++++++++ +// ``` +// +// The first condition is necessary to force the compiler issue a +// suggestion. The second condition is necessary to force the +// suggestion to be issued at a span associated with the sole +// `fn`-item of this crate. The third condition is necessary in order +// to yield the weird state where the associated span of the `fn`-item +// does not actually cover all of the original source code of the +// `fn`-item (which is why we are calling it an "amputated" span +// here). +// +// Note that satisfying conditions 2 and 3 requires the use of the +// `--extern` compile flag. +// +// You might ask yourself: What code would do such a thing? The +// answer is: the #[tokio::main] attribute does *exactly* this (as +// well as injecting some other code into the `fn main` that it +// constructs). + +#[amputate_span::drop_first_token] +/* what the +hey */ async fn main() { + Command::new("git"); //~ ERROR [E0433] +} + +// (The /* ... */ comment in the above is not part of the original +// bug. It is just meant to illustrate one particular facet of the +// original non-ideal behavior, where we were transcribing the +// trailing comment as part of the emitted suggestion, for better or +// for worse.) + +mod inner { + #[amputate_span::drop_first_token] + /* another interesting + case */ async fn foo() { + Command::new("git"); //~ ERROR [E0433] + } +} diff --git a/src/test/ui/proc-macro/amputate-span.stderr b/src/test/ui/proc-macro/amputate-span.stderr new file mode 100644 index 0000000000000..75c5cbdabc79e --- /dev/null +++ b/src/test/ui/proc-macro/amputate-span.stderr @@ -0,0 +1,25 @@ +error[E0433]: failed to resolve: use of undeclared type `Command` + --> $DIR/amputate-span.rs:48:5 + | +LL | Command::new("git"); + | ^^^^^^^ not found in this scope + | +help: consider importing this struct + | +LL | use std::process::Command; + | + +error[E0433]: failed to resolve: use of undeclared type `Command` + --> $DIR/amputate-span.rs:61:9 + | +LL | Command::new("git"); + | ^^^^^^^ not found in this scope + | +help: consider importing this struct + | +LL | use std::process::Command; + | + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0433`. diff --git a/src/test/ui/proc-macro/auxiliary/amputate-span.rs b/src/test/ui/proc-macro/auxiliary/amputate-span.rs new file mode 100644 index 0000000000000..1a82119ae95e5 --- /dev/null +++ b/src/test/ui/proc-macro/auxiliary/amputate-span.rs @@ -0,0 +1,14 @@ +// force-host +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; + +use proc_macro::TokenStream; + +#[proc_macro_attribute] +pub fn drop_first_token(attr: TokenStream, input: TokenStream) -> TokenStream { + assert!(attr.is_empty()); + input.into_iter().skip(1).collect() +} diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index 3b4e762b6dd15..6983249c15d45 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -113,7 +113,7 @@ impl<'a> Parser<'a> { let result = catch_unwind(AssertUnwindSafe(|| { let mut parser = new_parser_from_file(sess.inner(), path, Some(span)); match parser.parse_mod(&TokenKind::Eof) { - Ok((a, i, ast::ModSpans { inner_span })) => Some((a, i, inner_span)), + Ok((a, i, ast::ModSpans { inner_span, inject_use_span: _ })) => Some((a, i, inner_span)), Err(mut e) => { e.emit(); if sess.can_reset_errors() { diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index c44b2fc6ae354..dec977e98caf5 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -916,7 +916,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str(&ident_str); if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans) = mod_kind { - let ast::ModSpans { inner_span } = *spans; + let ast::ModSpans{ inner_span, inject_use_span: _ } = *spans; match self.config.brace_style() { BraceStyle::AlwaysNextLine => { let indent_str = self.block_indent.to_string_with_newline(self.config); From fda9a561df39b60196c0a6e7417a6d50b163e92a Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 4 Mar 2022 17:05:30 -0500 Subject: [PATCH 4/8] Placate tidy in submodule. --- src/tools/rustfmt/src/parse/parser.rs | 8 +++++++- src/tools/rustfmt/src/visitor.rs | 5 ++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index 6983249c15d45..ec051d9371017 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -113,7 +113,13 @@ impl<'a> Parser<'a> { let result = catch_unwind(AssertUnwindSafe(|| { let mut parser = new_parser_from_file(sess.inner(), path, Some(span)); match parser.parse_mod(&TokenKind::Eof) { - Ok((a, i, ast::ModSpans { inner_span, inject_use_span: _ })) => Some((a, i, inner_span)), + Ok((a, + i, + ast::ModSpans { + inner_span, + inject_use_span: _ + } + )) => Some((a, i, inner_span)), Err(mut e) => { e.emit(); if sess.can_reset_errors() { diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index dec977e98caf5..dcf096294f14f 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -916,7 +916,10 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str(&ident_str); if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans) = mod_kind { - let ast::ModSpans{ inner_span, inject_use_span: _ } = *spans; + let ast::ModSpans{ + inner_span, + inject_use_span: _ + } = *spans; match self.config.brace_style() { BraceStyle::AlwaysNextLine => { let indent_str = self.block_indent.to_string_with_newline(self.config); From 59d5c74c1572f176fe6b794e1ae106e66db9d33f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 4 Mar 2022 17:14:34 -0500 Subject: [PATCH 5/8] Update use_suggestion_placement.rs test, removing the FIXME that this PR fixes. (There is another issue, in that the fixed output is not ideally indented, but that is a pre-existing issue and should not block this PR.) --- src/test/ui/resolve/use_suggestion_placement.fixed | 6 +----- src/test/ui/resolve/use_suggestion_placement.rs | 6 +----- src/test/ui/resolve/use_suggestion_placement.stderr | 6 +++--- 3 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/test/ui/resolve/use_suggestion_placement.fixed b/src/test/ui/resolve/use_suggestion_placement.fixed index 63676327aa041..d1686f7fd2b9c 100644 --- a/src/test/ui/resolve/use_suggestion_placement.fixed +++ b/src/test/ui/resolve/use_suggestion_placement.fixed @@ -14,13 +14,9 @@ mod m { } mod foo { - // FIXME: UsePlacementFinder is broken because active attributes are - // removed, and thus the `derive` attribute here is not in the AST. - // An inert attribute should work, though. - // #[derive(Debug)] use std::path::Path; -#[allow(warnings)] +#[derive(Debug)] pub struct Foo; // test whether the use suggestion isn't diff --git a/src/test/ui/resolve/use_suggestion_placement.rs b/src/test/ui/resolve/use_suggestion_placement.rs index ecc74d781679d..5be91f27092fa 100644 --- a/src/test/ui/resolve/use_suggestion_placement.rs +++ b/src/test/ui/resolve/use_suggestion_placement.rs @@ -10,11 +10,7 @@ mod m { } mod foo { - // FIXME: UsePlacementFinder is broken because active attributes are - // removed, and thus the `derive` attribute here is not in the AST. - // An inert attribute should work, though. - // #[derive(Debug)] - #[allow(warnings)] + #[derive(Debug)] pub struct Foo; // test whether the use suggestion isn't diff --git a/src/test/ui/resolve/use_suggestion_placement.stderr b/src/test/ui/resolve/use_suggestion_placement.stderr index 217c08a560b77..0aadd82f6c292 100644 --- a/src/test/ui/resolve/use_suggestion_placement.stderr +++ b/src/test/ui/resolve/use_suggestion_placement.stderr @@ -1,5 +1,5 @@ error[E0412]: cannot find type `Path` in this scope - --> $DIR/use_suggestion_placement.rs:22:16 + --> $DIR/use_suggestion_placement.rs:18:16 | LL | type Bar = Path; | ^^^^ not found in this scope @@ -10,7 +10,7 @@ LL | use std::path::Path; | error[E0425]: cannot find value `A` in this scope - --> $DIR/use_suggestion_placement.rs:27:13 + --> $DIR/use_suggestion_placement.rs:23:13 | LL | let _ = A; | ^ not found in this scope @@ -21,7 +21,7 @@ LL | use m::A; | error[E0412]: cannot find type `HashMap` in this scope - --> $DIR/use_suggestion_placement.rs:32:23 + --> $DIR/use_suggestion_placement.rs:28:23 | LL | type Dict = HashMap; | ^^^^^^^ not found in this scope From 673640d26de07e37ba34f6f77542f822fa72d54e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 7 Mar 2022 16:37:35 -0500 Subject: [PATCH 6/8] placate rustfmt in rustfmt. --- src/tools/rustfmt/src/parse/parser.rs | 8 +------- src/tools/rustfmt/src/visitor.rs | 4 ++-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index ec051d9371017..268c72649a65a 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -113,13 +113,7 @@ impl<'a> Parser<'a> { let result = catch_unwind(AssertUnwindSafe(|| { let mut parser = new_parser_from_file(sess.inner(), path, Some(span)); match parser.parse_mod(&TokenKind::Eof) { - Ok((a, - i, - ast::ModSpans { - inner_span, - inject_use_span: _ - } - )) => Some((a, i, inner_span)), + Ok((a, i, spans)) => Some((a, i, spans.inner_span)), Err(mut e) => { e.emit(); if sess.can_reset_errors() { diff --git a/src/tools/rustfmt/src/visitor.rs b/src/tools/rustfmt/src/visitor.rs index dcf096294f14f..3ebfa551d1cbc 100644 --- a/src/tools/rustfmt/src/visitor.rs +++ b/src/tools/rustfmt/src/visitor.rs @@ -916,9 +916,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str(&ident_str); if let ast::ModKind::Loaded(ref items, ast::Inline::Yes, ref spans) = mod_kind { - let ast::ModSpans{ + let ast::ModSpans { inner_span, - inject_use_span: _ + inject_use_span: _, } = *spans; match self.config.brace_style() { BraceStyle::AlwaysNextLine => { From 4a0c2b2aba04f07b4351e3be93fb48460bf20dba Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 7 Mar 2022 17:17:15 -0500 Subject: [PATCH 7/8] Add a FIXME explaining how one could go about fixing this code to bring it in sync with the method added in PR 94584. --- compiler/rustc_typeck/src/check/method/suggest.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 1a345a303caea..cdad00d3c06ba 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -1936,6 +1936,10 @@ pub fn all_traits(tcx: TyCtxt<'_>) -> Vec { } fn find_use_placement<'tcx>(tcx: TyCtxt<'tcx>, target_module: LocalDefId) -> (Option, bool) { + // FIXME(#94854): this code uses an out-of-date method for inferring a span + // to suggest. It would be better to thread the ModSpans from the AST into + // the HIR, and then use that to drive the suggestion here. + let mut span = None; let mut found_use = false; let (module, _, _) = tcx.hir().get_module(target_module); From 8f4c6b039d49cbba6a1122043fac5fd29fde92ba Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 11 Mar 2022 16:57:34 -0500 Subject: [PATCH 8/8] run rust-fix in amputate-span.rs. (Thanks to ekuber for pushing me to do this.) --- src/test/ui/proc-macro/amputate-span.fixed | 69 +++++++++++++++++++++ src/test/ui/proc-macro/amputate-span.rs | 2 + src/test/ui/proc-macro/amputate-span.stderr | 4 +- 3 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/proc-macro/amputate-span.fixed diff --git a/src/test/ui/proc-macro/amputate-span.fixed b/src/test/ui/proc-macro/amputate-span.fixed new file mode 100644 index 0000000000000..1afc3501a3277 --- /dev/null +++ b/src/test/ui/proc-macro/amputate-span.fixed @@ -0,0 +1,69 @@ +// aux-build:amputate-span.rs +// run-rustfix +// edition:2018 +// compile-flags: --extern amputate_span + +// This test has been crafted to ensure the following things: +// +// 1. There's a resolution error that prompts the compiler to suggest +// adding a `use` item. +// +// 2. There are no `use` or `extern crate` items in the source +// code. In fact, there is only one item, the `fn main` +// declaration. +// +// 3. The single `fn main` declaration has an attribute attached to it +// that just deletes the first token from the given item. +// +// You need all of these conditions to hold in order to replicate the +// scenario that yielded issue 87613, where the compiler's suggestion +// looks like: +// +// ``` +// help: consider importing this struct +// | +// 47 | hey */ async use std::process::Command; +// | ++++++++++++++++++++++++++ +// ``` +// +// The first condition is necessary to force the compiler issue a +// suggestion. The second condition is necessary to force the +// suggestion to be issued at a span associated with the sole +// `fn`-item of this crate. The third condition is necessary in order +// to yield the weird state where the associated span of the `fn`-item +// does not actually cover all of the original source code of the +// `fn`-item (which is why we are calling it an "amputated" span +// here). +// +// Note that satisfying conditions 2 and 3 requires the use of the +// `--extern` compile flag. +// +// You might ask yourself: What code would do such a thing? The +// answer is: the #[tokio::main] attribute does *exactly* this (as +// well as injecting some other code into the `fn main` that it +// constructs). + +use std::process::Command; + +#[amputate_span::drop_first_token] +/* what the +hey */ async fn main() { + Command::new("git"); //~ ERROR [E0433] +} + +// (The /* ... */ comment in the above is not part of the original +// bug. It is just meant to illustrate one particular facet of the +// original non-ideal behavior, where we were transcribing the +// trailing comment as part of the emitted suggestion, for better or +// for worse.) + +#[allow(dead_code)] +mod inner { + use std::process::Command; + +#[amputate_span::drop_first_token] + /* another interesting + case */ async fn foo() { + Command::new("git"); //~ ERROR [E0433] + } +} diff --git a/src/test/ui/proc-macro/amputate-span.rs b/src/test/ui/proc-macro/amputate-span.rs index 3ca7c847a88dd..894a06dd5f661 100644 --- a/src/test/ui/proc-macro/amputate-span.rs +++ b/src/test/ui/proc-macro/amputate-span.rs @@ -1,4 +1,5 @@ // aux-build:amputate-span.rs +// run-rustfix // edition:2018 // compile-flags: --extern amputate_span @@ -54,6 +55,7 @@ hey */ async fn main() { // trailing comment as part of the emitted suggestion, for better or // for worse.) +#[allow(dead_code)] mod inner { #[amputate_span::drop_first_token] /* another interesting diff --git a/src/test/ui/proc-macro/amputate-span.stderr b/src/test/ui/proc-macro/amputate-span.stderr index 75c5cbdabc79e..9553ba3da5428 100644 --- a/src/test/ui/proc-macro/amputate-span.stderr +++ b/src/test/ui/proc-macro/amputate-span.stderr @@ -1,5 +1,5 @@ error[E0433]: failed to resolve: use of undeclared type `Command` - --> $DIR/amputate-span.rs:48:5 + --> $DIR/amputate-span.rs:49:5 | LL | Command::new("git"); | ^^^^^^^ not found in this scope @@ -10,7 +10,7 @@ LL | use std::process::Command; | error[E0433]: failed to resolve: use of undeclared type `Command` - --> $DIR/amputate-span.rs:61:9 + --> $DIR/amputate-span.rs:63:9 | LL | Command::new("git"); | ^^^^^^^ not found in this scope