From dead3a807d6031972559c67ac5d30c61c50b0067 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Thu, 11 Jun 2020 21:11:18 -0500 Subject: [PATCH] fix: backport mod resolution error handling --- src/bin/main.rs | 2 +- src/format_report_formatter.rs | 1 + src/lib.rs | 4 ++ src/modules.rs | 95 ++++++++++++++++++++++------------ src/syntux/parser.rs | 8 +-- src/test/mod.rs | 1 + tests/target/skip/foo.rs | 5 ++ tests/target/skip/main.rs | 5 ++ 8 files changed, 85 insertions(+), 36 deletions(-) create mode 100644 tests/target/skip/foo.rs create mode 100644 tests/target/skip/main.rs diff --git a/src/bin/main.rs b/src/bin/main.rs index 4a62766a85d..9101c015fb9 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -72,7 +72,7 @@ pub enum OperationError { #[error("The `--print-config=minimal` option doesn't work with standard input.")] MinimalPathWithStdin, /// An io error during reading or writing. - #[error("io error: {0}")] + #[error("{0}")] IoError(IoError), /// Attempt to use --check with stdin, which isn't currently /// supported. diff --git a/src/format_report_formatter.rs b/src/format_report_formatter.rs index 5f5d89faf39..2fc6c4e8955 100644 --- a/src/format_report_formatter.rs +++ b/src/format_report_formatter.rs @@ -162,6 +162,7 @@ fn error_kind_to_snippet_annotation_type(error_kind: &ErrorKind) -> AnnotationTy ErrorKind::LineOverflow(..) | ErrorKind::TrailingWhitespace | ErrorKind::IoError(_) + | ErrorKind::ModuleResolutionError(_) | ErrorKind::ParseError | ErrorKind::LostComment | ErrorKind::LicenseCheck diff --git a/src/lib.rs b/src/lib.rs index 5693ee85958..2ba476466ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,7 @@ use crate::comment::LineClasses; use crate::emitter::Emitter; use crate::formatting::{FormatErrorMap, FormattingError, ReportedErrors, SourceFile}; use crate::issues::Issue; +use crate::modules::ModuleResolutionError; use crate::shape::Indent; use crate::syntux::parser::DirectoryOwnership; use crate::utils::indent_next_line; @@ -110,6 +111,9 @@ pub enum ErrorKind { /// An io error during reading or writing. #[error("io error: {0}")] IoError(io::Error), + /// Error during module resolution. + #[error("{0}")] + ModuleResolutionError(#[from] ModuleResolutionError), /// Parse error occurred when parsing the input. #[error("parse error")] ParseError, diff --git a/src/modules.rs b/src/modules.rs index 915401c4682..8ecb121f9e7 100644 --- a/src/modules.rs +++ b/src/modules.rs @@ -5,11 +5,14 @@ use std::path::{Path, PathBuf}; use rustc_ast::ast; use rustc_ast::visit::Visitor; use rustc_span::symbol::{self, sym, Symbol}; +use thiserror::Error; use crate::attr::MetaVisitor; use crate::config::FileName; use crate::items::is_mod_decl; -use crate::syntux::parser::{Directory, DirectoryOwnership, ModulePathSuccess, Parser}; +use crate::syntux::parser::{ + Directory, DirectoryOwnership, ModulePathSuccess, Parser, ParserError, +}; use crate::syntux::session::ParseSess; use crate::utils::contains_skip; @@ -29,6 +32,24 @@ pub(crate) struct ModResolver<'ast, 'sess> { recursive: bool, } +/// Represents errors while trying to resolve modules. +#[error("failed to resolve mod `{module}`: {kind}")] +#[derive(Debug, Error)] +pub struct ModuleResolutionError { + module: String, + kind: ModuleResolutionErrorKind, +} + +#[derive(Debug, Error)] +pub(crate) enum ModuleResolutionErrorKind { + /// Find a file that cannot be parsed. + #[error("cannot parse {file}")] + ParseError { file: PathBuf }, + /// File cannot be found. + #[error("{file} does not exist")] + NotFound { file: PathBuf }, +} + #[derive(Clone)] enum SubModKind<'a, 'ast> { /// `mod foo;` @@ -63,7 +84,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { pub(crate) fn visit_crate( mut self, krate: &'ast ast::Crate, - ) -> Result, String> { + ) -> Result, ModuleResolutionError> { let root_filename = self.parse_sess.span_to_filename(krate.span); self.directory.path = match root_filename { FileName::Real(ref p) => p.parent().unwrap_or(Path::new("")).to_path_buf(), @@ -81,7 +102,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } /// Visit `cfg_if` macro and look for module declarations. - fn visit_cfg_if(&mut self, item: Cow<'ast, ast::Item>) -> Result<(), String> { + fn visit_cfg_if(&mut self, item: Cow<'ast, ast::Item>) -> Result<(), ModuleResolutionError> { let mut visitor = visitor::CfgIfVisitor::new(self.parse_sess); visitor.visit_item(&item); for module_item in visitor.mods() { @@ -93,7 +114,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } /// Visit modules defined inside macro calls. - fn visit_mod_outside_ast(&mut self, module: ast::Mod) -> Result<(), String> { + fn visit_mod_outside_ast(&mut self, module: ast::Mod) -> Result<(), ModuleResolutionError> { for item in module.items { if is_cfg_if(&item) { self.visit_cfg_if(Cow::Owned(item.into_inner()))?; @@ -108,7 +129,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } /// Visit modules from AST. - fn visit_mod_from_ast(&mut self, module: &'ast ast::Mod) -> Result<(), String> { + fn visit_mod_from_ast(&mut self, module: &'ast ast::Mod) -> Result<(), ModuleResolutionError> { for item in &module.items { if is_cfg_if(item) { self.visit_cfg_if(Cow::Borrowed(item))?; @@ -125,7 +146,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { &mut self, item: &'c ast::Item, sub_mod: Cow<'ast, ast::Mod>, - ) -> Result<(), String> { + ) -> Result<(), ModuleResolutionError> { let old_directory = self.directory.clone(); let sub_mod_kind = self.peek_sub_mod(item, &sub_mod)?; if let Some(sub_mod_kind) = sub_mod_kind { @@ -141,7 +162,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { &self, item: &'c ast::Item, sub_mod: &Cow<'ast, ast::Mod>, - ) -> Result>, String> { + ) -> Result>, ModuleResolutionError> { if contains_skip(&item.attrs) { return Ok(None); } @@ -165,7 +186,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { &mut self, sub_mod_kind: SubModKind<'c, 'ast>, _sub_mod: Cow<'ast, ast::Mod>, - ) -> Result<(), String> { + ) -> Result<(), ModuleResolutionError> { match sub_mod_kind { SubModKind::External(mod_path, _, sub_mod) => { self.file_map @@ -188,7 +209,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { &mut self, sub_mod: Cow<'ast, ast::Mod>, sub_mod_kind: SubModKind<'c, 'ast>, - ) -> Result<(), String> { + ) -> Result<(), ModuleResolutionError> { match sub_mod_kind { SubModKind::External(mod_path, directory_ownership, sub_mod) => { let directory = Directory { @@ -226,7 +247,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { &mut self, sub_mod: Cow<'ast, ast::Mod>, directory: Option, - ) -> Result<(), String> { + ) -> Result<(), ModuleResolutionError> { if let Some(directory) = directory { self.directory = directory; } @@ -242,7 +263,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { mod_name: symbol::Ident, attrs: &[ast::Attribute], sub_mod: &Cow<'ast, ast::Mod>, - ) -> Result>, String> { + ) -> Result>, ModuleResolutionError> { let relative = match self.directory.ownership { DirectoryOwnership::Owned { relative } => relative, DirectoryOwnership::UnownedViaBlock | DirectoryOwnership::UnownedViaMod => None, @@ -252,16 +273,20 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { return Ok(None); } return match Parser::parse_file_as_module(self.parse_sess, &path, sub_mod.inner) { - Some((_, ref attrs)) if contains_skip(attrs) => Ok(None), - Some((m, _)) => Ok(Some(SubModKind::External( + Ok((_, ref attrs)) if contains_skip(attrs) => Ok(None), + Ok((m, _)) => Ok(Some(SubModKind::External( path, DirectoryOwnership::Owned { relative: None }, Cow::Owned(m), ))), - None => Err(format!( - "Failed to find module {} in {:?} {:?}", - mod_name, self.directory.path, relative, - )), + Err(ParserError::ParseError) => Err(ModuleResolutionError { + module: mod_name.to_string(), + kind: ModuleResolutionErrorKind::ParseError { file: path }, + }), + Err(..) => Err(ModuleResolutionError { + module: mod_name.to_string(), + kind: ModuleResolutionErrorKind::NotFound { file: path }, + }), }; } @@ -291,22 +316,26 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } } match Parser::parse_file_as_module(self.parse_sess, &path, sub_mod.inner) { - Some((_, ref attrs)) if contains_skip(attrs) => Ok(None), - Some((m, _)) if outside_mods_empty => { + Ok((_, ref attrs)) if contains_skip(attrs) => Ok(None), + Ok((m, _)) if outside_mods_empty => { Ok(Some(SubModKind::External(path, ownership, Cow::Owned(m)))) } - Some((m, _)) => { + Ok((m, _)) => { mods_outside_ast.push((path.clone(), ownership, Cow::Owned(m))); if should_insert { mods_outside_ast.push((path, ownership, sub_mod.clone())); } Ok(Some(SubModKind::MultiExternal(mods_outside_ast))) } - None if outside_mods_empty => Err(format!( - "Failed to find module {} in {:?} {:?}", - mod_name, self.directory.path, relative, - )), - None => { + Err(ParserError::ParseError) => Err(ModuleResolutionError { + module: mod_name.to_string(), + kind: ModuleResolutionErrorKind::ParseError { file: path }, + }), + Err(..) if outside_mods_empty => Err(ModuleResolutionError { + module: mod_name.to_string(), + kind: ModuleResolutionErrorKind::NotFound { file: path }, + }), + Err(..) => { if should_insert { mods_outside_ast.push((path, ownership, sub_mod.clone())); } @@ -320,10 +349,12 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } Err(mut e) => { e.cancel(); - Err(format!( - "Failed to find module {} in {:?} {:?}", - mod_name, self.directory.path, relative, - )) + Err(ModuleResolutionError { + module: mod_name.to_string(), + kind: ModuleResolutionErrorKind::NotFound { + file: self.directory.path.clone(), + }, + }) } } } @@ -379,9 +410,9 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> { } let m = match Parser::parse_file_as_module(self.parse_sess, &actual_path, sub_mod.inner) { - Some((_, ref attrs)) if contains_skip(attrs) => continue, - Some((m, _)) => m, - None => continue, + Ok((_, ref attrs)) if contains_skip(attrs) => continue, + Ok((m, _)) => m, + Err(..) => continue, }; result.push(( diff --git a/src/syntux/parser.rs b/src/syntux/parser.rs index 831554829e2..89db0f5553c 100644 --- a/src/syntux/parser.rs +++ b/src/syntux/parser.rs @@ -106,7 +106,7 @@ impl<'a> Parser<'a> { sess: &'a ParseSess, path: &Path, span: Span, - ) -> Option<(ast::Mod, Vec)> { + ) -> Result<(ast::Mod, Vec), ParserError> { let result = catch_unwind(AssertUnwindSafe(|| { let mut parser = new_parser_from_file(sess.inner(), &path, Some(span)); match parser.parse_mod(&TokenKind::Eof) { @@ -119,8 +119,10 @@ impl<'a> Parser<'a> { } })); match result { - Ok(Some(m)) => Some(m), - _ => None, + Ok(Some(m)) => Ok(m), + Ok(None) => Err(ParserError::ParseError), + Err(..) if path.exists() => Err(ParserError::ParseError), + Err(_) => Err(ParserError::ParsePanicError), } } diff --git a/src/test/mod.rs b/src/test/mod.rs index 6b7a9365c6f..e6497f48e0f 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -40,6 +40,7 @@ const SKIP_FILE_WHITE_LIST: &[&str] = &[ "cfg_mod/bar.rs", "cfg_mod/foo.rs", "cfg_mod/wasm32.rs", + "skip/foo.rs", ]; fn init_log() { diff --git a/tests/target/skip/foo.rs b/tests/target/skip/foo.rs new file mode 100644 index 00000000000..776658f8fe5 --- /dev/null +++ b/tests/target/skip/foo.rs @@ -0,0 +1,5 @@ +#![rustfmt::skip] + +fn +foo() +{} diff --git a/tests/target/skip/main.rs b/tests/target/skip/main.rs new file mode 100644 index 00000000000..2d33bef9251 --- /dev/null +++ b/tests/target/skip/main.rs @@ -0,0 +1,5 @@ +mod foo; + +fn main() { + println!("Hello, world!"); +}