Skip to content

Commit 8842c43

Browse files
topecongirotopecongiro
authored
Improve error message when module resolution failed (#4198)
1 parent 4633930 commit 8842c43

File tree

5 files changed

+69
-45
lines changed

5 files changed

+69
-45
lines changed

‎rustfmt-core/rustfmt-bin/src/bin/main.rs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -323,16 +323,10 @@ pub enum OperationError {
323323
#[error("The `--print-config=minimal` option doesn't work with standard input.")]
324324
MinimalPathWithStdin,
325325
/// An io error during reading or writing.
326-
#[error("io error: {0}")]
326+
#[error("{0}")]
327327
IoError(IoError),
328328
}
329329

330-
impl From<IoError> for OperationError {
331-
fn from(e: IoError) -> OperationError {
332-
OperationError::IoError(e)
333-
}
334-
}
335-
336330
impl CliOptions for Opt {
337331
fn apply_to(&self, config: &mut Config) {
338332
config.set().file_lines(self.file_lines.clone());

‎rustfmt-core/rustfmt-lib/src/formatting.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
// High level formatting functions.
22

3-
use std::io;
43
use std::time::{Duration, Instant};
54

65
use rustc_ast::ast;
@@ -117,8 +116,7 @@ fn format_project(
117116
directory_ownership.unwrap_or(DirectoryOwnership::UnownedViaMod),
118117
!input_is_stdin && operation_setting.recursive,
119118
)
120-
.visit_crate(&krate)
121-
.map_err(|e| OperationError::IoError(io::Error::new(io::ErrorKind::Other, e)))?;
119+
.visit_crate(&krate)?;
122120

123121
for (path, module) in files {
124122
let should_ignore = !input_is_stdin && parse_session.ignore_file(&path);

‎rustfmt-core/rustfmt-lib/src/formatting/modules.rs‎

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ use std::path::{Path, PathBuf};
55
use rustc_ast::ast;
66
use rustc_ast::visit::Visitor;
77
use rustc_span::symbol::{self, sym, Symbol};
8+
use thiserror::Error;
89

910
use crate::config::FileName;
1011
use crate::formatting::{
1112
attr::MetaVisitor,
1213
items::is_mod_decl,
13-
syntux::parser::{Directory, DirectoryOwnership, ModulePathSuccess, Parser},
14+
syntux::parser::{Directory, DirectoryOwnership, ModulePathSuccess, Parser, ParserError},
1415
syntux::session::ParseSess,
1516
utils::contains_skip,
1617
};
@@ -31,6 +32,24 @@ pub(crate) struct ModResolver<'ast, 'sess> {
3132
recursive: bool,
3233
}
3334

35+
/// Represents errors while trying to resolve modules.
36+
#[error("failed to resolve mod `{module}`: {kind}")]
37+
#[derive(Debug, Error)]
38+
pub struct ModuleResolutionError {
39+
module: String,
40+
kind: ModuleResolutionErrorKind,
41+
}
42+
43+
#[derive(Debug, Error)]
44+
pub(crate) enum ModuleResolutionErrorKind {
45+
/// Find a file that cannot be parsed.
46+
#[error("cannot parse {file}")]
47+
ParseError { file: PathBuf },
48+
/// File cannot be found.
49+
#[error("{file} does not exist")]
50+
NotFound { file: PathBuf },
51+
}
52+
3453
#[derive(Clone)]
3554
enum SubModKind<'a, 'ast> {
3655
/// `mod foo;`
@@ -63,7 +82,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
6382
pub(crate) fn visit_crate(
6483
mut self,
6584
krate: &'ast ast::Crate,
66-
) -> Result<FileModMap<'ast>, String> {
85+
) -> Result<FileModMap<'ast>, ModuleResolutionError> {
6786
let root_filename = self.parse_sess.span_to_filename(krate.span);
6887
self.directory.path = match root_filename {
6988
FileName::Real(ref p) => p.parent().unwrap_or_else(|| Path::new("")).to_path_buf(),
@@ -81,7 +100,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
81100
}
82101

83102
/// Visit `cfg_if` macro and look for module declarations.
84-
fn visit_cfg_if(&mut self, item: Cow<'ast, ast::Item>) -> Result<(), String> {
103+
fn visit_cfg_if(&mut self, item: Cow<'ast, ast::Item>) -> Result<(), ModuleResolutionError> {
85104
let mut visitor = visitor::CfgIfVisitor::new(self.parse_sess);
86105
visitor.visit_item(&item);
87106
for module_item in visitor.mods() {
@@ -93,7 +112,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
93112
}
94113

95114
/// Visit modules defined inside macro calls.
96-
fn visit_mod_outside_ast(&mut self, module: ast::Mod) -> Result<(), String> {
115+
fn visit_mod_outside_ast(&mut self, module: ast::Mod) -> Result<(), ModuleResolutionError> {
97116
for item in module.items {
98117
if is_cfg_if(&item) {
99118
self.visit_cfg_if(Cow::Owned(item.into_inner()))?;
@@ -108,7 +127,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
108127
}
109128

110129
/// Visit modules from AST.
111-
fn visit_mod_from_ast(&mut self, module: &'ast ast::Mod) -> Result<(), String> {
130+
fn visit_mod_from_ast(&mut self, module: &'ast ast::Mod) -> Result<(), ModuleResolutionError> {
112131
for item in &module.items {
113132
if is_cfg_if(item) {
114133
self.visit_cfg_if(Cow::Borrowed(item))?;
@@ -125,7 +144,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
125144
&mut self,
126145
item: &'c ast::Item,
127146
sub_mod: Cow<'ast, ast::Mod>,
128-
) -> Result<(), String> {
147+
) -> Result<(), ModuleResolutionError> {
129148
let old_directory = self.directory.clone();
130149
let sub_mod_kind = self.peek_sub_mod(item, &sub_mod)?;
131150
if let Some(sub_mod_kind) = sub_mod_kind {
@@ -141,7 +160,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
141160
&self,
142161
item: &'c ast::Item,
143162
sub_mod: &Cow<'ast, ast::Mod>,
144-
) -> Result<Option<SubModKind<'c, 'ast>>, String> {
163+
) -> Result<Option<SubModKind<'c, 'ast>>, ModuleResolutionError> {
145164
if contains_skip(&item.attrs) {
146165
return Ok(None);
147166
}
@@ -160,7 +179,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
160179
&mut self,
161180
sub_mod_kind: SubModKind<'c, 'ast>,
162181
_sub_mod: Cow<'ast, ast::Mod>,
163-
) -> Result<(), String> {
182+
) -> Result<(), ModuleResolutionError> {
164183
match sub_mod_kind {
165184
SubModKind::External(mod_path, _, sub_mod) => {
166185
self.file_map
@@ -183,7 +202,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
183202
&mut self,
184203
sub_mod: Cow<'ast, ast::Mod>,
185204
sub_mod_kind: SubModKind<'c, 'ast>,
186-
) -> Result<(), String> {
205+
) -> Result<(), ModuleResolutionError> {
187206
match sub_mod_kind {
188207
SubModKind::External(mod_path, directory_ownership, sub_mod) => {
189208
let directory = Directory {
@@ -213,7 +232,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
213232
&mut self,
214233
sub_mod: Cow<'ast, ast::Mod>,
215234
directory: Option<Directory>,
216-
) -> Result<(), String> {
235+
) -> Result<(), ModuleResolutionError> {
217236
if let Some(directory) = directory {
218237
self.directory = directory;
219238
}
@@ -229,7 +248,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
229248
mod_name: symbol::Ident,
230249
attrs: &[ast::Attribute],
231250
sub_mod: &Cow<'ast, ast::Mod>,
232-
) -> Result<Option<SubModKind<'c, 'ast>>, String> {
251+
) -> Result<Option<SubModKind<'c, 'ast>>, ModuleResolutionError> {
233252
let relative = match self.directory.ownership {
234253
DirectoryOwnership::Owned { relative } => relative,
235254
DirectoryOwnership::UnownedViaBlock | DirectoryOwnership::UnownedViaMod => None,
@@ -239,15 +258,19 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
239258
return Ok(None);
240259
}
241260
return match Parser::parse_file_as_module(self.parse_sess, &path, sub_mod.inner) {
242-
Some(m) => Ok(Some(SubModKind::External(
261+
Ok(m) => Ok(Some(SubModKind::External(
243262
path,
244263
DirectoryOwnership::Owned { relative: None },
245264
Cow::Owned(m),
246265
))),
247-
None => Err(format!(
248-
"Failed to find module {} in {:?} {:?}",
249-
mod_name, self.directory.path, relative,
250-
)),
266+
Err(ParserError::ParseError) => Err(ModuleResolutionError {
267+
module: mod_name.to_string(),
268+
kind: ModuleResolutionErrorKind::ParseError { file: path },
269+
}),
270+
Err(..) => Err(ModuleResolutionError {
271+
module: mod_name.to_string(),
272+
kind: ModuleResolutionErrorKind::NotFound { file: path },
273+
}),
251274
};
252275
}
253276

@@ -277,21 +300,25 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
277300
}
278301
}
279302
match Parser::parse_file_as_module(self.parse_sess, &path, sub_mod.inner) {
280-
Some(m) if outside_mods_empty => {
303+
Ok(m) if outside_mods_empty => {
281304
Ok(Some(SubModKind::External(path, ownership, Cow::Owned(m))))
282305
}
283-
Some(m) => {
306+
Ok(m) => {
284307
mods_outside_ast.push((path.clone(), ownership, Cow::Owned(m)));
285308
if should_insert {
286309
mods_outside_ast.push((path, ownership, sub_mod.clone()));
287310
}
288311
Ok(Some(SubModKind::MultiExternal(mods_outside_ast)))
289312
}
290-
None if outside_mods_empty => Err(format!(
291-
"Failed to find module {} in {:?} {:?}",
292-
mod_name, self.directory.path, relative,
293-
)),
294-
None => {
313+
Err(ParserError::ParseError) => Err(ModuleResolutionError {
314+
module: mod_name.to_string(),
315+
kind: ModuleResolutionErrorKind::ParseError { file: path },
316+
}),
317+
Err(..) if outside_mods_empty => Err(ModuleResolutionError {
318+
module: mod_name.to_string(),
319+
kind: ModuleResolutionErrorKind::NotFound { file: path },
320+
}),
321+
Err(..) => {
295322
if should_insert {
296323
mods_outside_ast.push((path, ownership, sub_mod.clone()));
297324
}
@@ -305,10 +332,12 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
305332
}
306333
Err(mut e) => {
307334
e.cancel();
308-
Err(format!(
309-
"Failed to find module {} in {:?} {:?}",
310-
mod_name, self.directory.path, relative,
311-
))
335+
Err(ModuleResolutionError {
336+
module: mod_name.to_string(),
337+
kind: ModuleResolutionErrorKind::NotFound {
338+
file: self.directory.path.clone(),
339+
},
340+
})
312341
}
313342
}
314343
}
@@ -367,8 +396,8 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
367396

368397
let m = match Parser::parse_file_as_module(self.parse_sess, &actual_path, sub_mod.inner)
369398
{
370-
Some(m) => m,
371-
None => continue,
399+
Ok(m) => m,
400+
Err(..) => continue,
372401
};
373402

374403
result.push((

‎rustfmt-core/rustfmt-lib/src/formatting/syntux/parser.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ impl<'a> Parser<'a> {
167167
sess: &'a ParseSess,
168168
path: &Path,
169169
span: Span,
170-
) -> Option<ast::Mod> {
170+
) -> Result<ast::Mod, ParserError> {
171171
let result = catch_unwind(AssertUnwindSafe(|| {
172172
let mut parser = new_parser_from_file(sess.inner(), &path, Some(span));
173173

@@ -192,8 +192,10 @@ impl<'a> Parser<'a> {
192192
}
193193
}));
194194
match result {
195-
Ok(Some(m)) => Some(m),
196-
_ => None,
195+
Ok(Some(m)) => Ok(m),
196+
Ok(None) => Err(ParserError::ParseError),
197+
Err(..) if path.exists() => Err(ParserError::ParseError),
198+
Err(_) => Err(ParserError::ParsePanicError),
197199
}
198200
}
199201

‎rustfmt-core/rustfmt-lib/src/result.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use rustc_span::Span;
44
use thiserror::Error;
55

6+
use crate::formatting::modules::ModuleResolutionError;
67
use crate::{formatting::ParseSess, FileName};
78

89
/// Represents the specific error kind of [`FormatError`].
@@ -110,9 +111,9 @@ pub enum OperationError {
110111
/// satisfy that requirement.
111112
#[error("version mismatch")]
112113
VersionMismatch,
113-
/// An io error during reading or writing.
114-
#[error("io error: {0}")]
115-
IoError(std::io::Error),
114+
/// Error during module resolution.
115+
#[error("{0}")]
116+
ModuleResolutionError(#[from] ModuleResolutionError),
116117
/// Invalid glob pattern in `ignore` configuration option.
117118
#[error("invalid glob pattern found in ignore list: {0}")]
118119
InvalidGlobPattern(ignore::Error),

0 commit comments

Comments
 (0)