Skip to content

Commit 77989a3

Browse files
committed
Refactor ModResolver.directory
Prefer replacing with a new function with_directory rather than modifying in place.
1 parent 5775113 commit 77989a3

File tree

1 file changed

+53
-48
lines changed

1 file changed

+53
-48
lines changed

src/modules.rs

Lines changed: 53 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,11 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
208208
sub_mod: Module<'_>,
209209
from_ast: bool,
210210
) -> Result<(), ModuleResolutionError> {
211-
let old_directory = self.directory.clone();
212211
let sub_mod_kind = self.peek_sub_mod(item, &sub_mod)?;
213212
if let Some(sub_mod_kind) = sub_mod_kind {
214213
self.insert_sub_mod(sub_mod_kind.clone())?;
215214
self.visit_sub_mod_inner(sub_mod, sub_mod_kind, from_ast)?;
216215
}
217-
self.directory = old_directory;
218216
Ok(())
219217
}
220218

@@ -272,39 +270,30 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
272270
path: mod_path.parent().unwrap().to_path_buf(),
273271
ownership: directory_ownership,
274272
};
275-
self.visit_sub_mod_after_directory_update(sub_mod, Some(directory), false)
273+
self.with_directory(directory, |this| this.visit_mod_outside_ast(&sub_mod.items))?;
276274
}
277275
SubModKind::Internal(item) => {
278-
self.push_inline_mod_directory(item.ident, &item.attrs);
279-
self.visit_sub_mod_after_directory_update(sub_mod, None, from_ast)
276+
let directory = self.inline_mod_directory(item.ident, &item.attrs);
277+
self.with_directory(directory, |this| {
278+
if from_ast {
279+
this.visit_mod_from_ast(&sub_mod.items)
280+
} else {
281+
this.visit_mod_outside_ast(&sub_mod.items)
282+
}
283+
})?;
280284
}
281285
SubModKind::MultiExternal(mods) => {
282286
for (mod_path, directory_ownership, sub_mod) in mods {
283287
let directory = Directory {
284288
path: mod_path.parent().unwrap().to_path_buf(),
285289
ownership: directory_ownership,
286290
};
287-
self.visit_sub_mod_after_directory_update(sub_mod, Some(directory), false)?;
291+
self.with_directory(directory, |this| {
292+
this.visit_mod_outside_ast(&sub_mod.items)
293+
})?;
288294
}
289-
Ok(())
290295
}
291296
}
292-
}
293-
294-
fn visit_sub_mod_after_directory_update(
295-
&mut self,
296-
sub_mod: Module<'_>,
297-
directory: Option<Directory>,
298-
from_ast: bool,
299-
) -> Result<(), ModuleResolutionError> {
300-
if let Some(directory) = directory {
301-
self.directory = directory;
302-
}
303-
if from_ast {
304-
self.visit_mod_from_ast(&sub_mod.items)?;
305-
} else {
306-
self.visit_mod_outside_ast(&sub_mod.items)?;
307-
}
308297
Ok(())
309298
}
310299

@@ -437,31 +426,41 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
437426
}
438427
}
439428

440-
fn push_inline_mod_directory(&mut self, id: symbol::Ident, attrs: &[ast::Attribute]) {
429+
fn inline_mod_directory(&mut self, id: symbol::Ident, attrs: &[ast::Attribute]) -> Directory {
441430
if let Some(path) = find_path_value(attrs) {
442-
self.directory.path.push(path.as_str());
443-
self.directory.ownership = DirectoryOwnership::Owned { relative: None };
444-
} else {
445-
let id = id.as_str();
446-
// We have to push on the current module name in the case of relative
447-
// paths in order to ensure that any additional module paths from inline
448-
// `mod x { ... }` come after the relative extension.
449-
//
450-
// For example, a `mod z { ... }` inside `x/y.rs` should set the current
451-
// directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`.
452-
if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership {
453-
if let Some(ident) = relative.take() {
454-
// remove the relative offset
455-
self.directory.path.push(ident.as_str());
456-
457-
// In the case where there is an x.rs and an ./x directory we want
458-
// to prevent adding x twice. For example, ./x/x
459-
if self.directory.path.exists() && !self.directory.path.join(id).exists() {
460-
return;
461-
}
462-
}
431+
return Directory {
432+
path: self.directory.path.join(path.as_str()),
433+
ownership: DirectoryOwnership::Owned { relative: None },
434+
};
435+
}
436+
let id = id.as_str();
437+
// We have to push on the current module name in the case of relative
438+
// paths in order to ensure that any additional module paths from inline
439+
// `mod x { ... }` come after the relative extension.
440+
//
441+
// For example, a `mod z { ... }` inside `x/y.rs` should set the current
442+
// directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`.
443+
let new_path = if let DirectoryOwnership::Owned {
444+
relative: Some(ident),
445+
} = self.directory.ownership
446+
{
447+
// remove the relative offset
448+
let relative = self.directory.path.join(ident.as_str());
449+
let nested = relative.join(id);
450+
451+
// In the case where there is an x.rs and an ./x directory we want
452+
// to prevent adding x twice. For example, ./x/x
453+
if relative.exists() && !nested.exists() {
454+
relative
455+
} else {
456+
nested
463457
}
464-
self.directory.path.push(id);
458+
} else {
459+
self.directory.path.join(id)
460+
};
461+
Directory {
462+
path: new_path,
463+
ownership: self.directory.ownership,
465464
}
466465
}
467466

@@ -479,8 +478,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
479478
}
480479
let mut result = vec![];
481480
for path in path_visitor.paths() {
482-
let mut actual_path = self.directory.path.clone();
483-
actual_path.push(&path);
481+
let actual_path = self.directory.path.join(path);
484482
if !actual_path.exists() {
485483
continue;
486484
}
@@ -508,6 +506,13 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
508506
}
509507
result
510508
}
509+
510+
fn with_directory<T>(&mut self, directory: Directory, f: impl FnOnce(&mut Self) -> T) -> T {
511+
let old = std::mem::replace(&mut self.directory, directory);
512+
let out = f(self);
513+
self.directory = old;
514+
out
515+
}
511516
}
512517

513518
fn path_value(attr: &ast::Attribute) -> Option<Symbol> {

0 commit comments

Comments
 (0)