-
Notifications
You must be signed in to change notification settings - Fork 13.5k
suggest declaring modules when file found but module not defined #142730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
suggest declaring modules when file found but module not defined #142730
Conversation
@rustbot label A-diagnostics |
This comment has been minimized.
This comment has been minimized.
rustbot has assigned @petrochenkov. Use |
This comment has been minimized.
This comment has been minimized.
7174cb4
to
397c51b
Compare
.iter() | ||
.flatten(), | ||
) | ||
.find(|x| x.exists()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you avoid placing complex multi-line logic into if
conditions?
This whole iter-find part would probably look better as imperative for loops anyway.
&& let i = ident.as_str() | ||
// FIXME: add case where non parent using undeclared module (hard?) | ||
&& let Some(dir) = src.parent() | ||
&& let Some(src) = src.with_extension("").file_name().and_then(|x| x.to_str()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&& let Some(src) = src.with_extension("").file_name().and_then(|x| x.to_str()) | |
&& let Some(src) = src.file_stem().and_then(|x| x.to_str()) |
?
m.display() | ||
), | ||
Applicability::MaybeIncorrect, | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move this new logic to a separate function?
) | ||
.shrink_to_lo(); | ||
Some(( | ||
vec![(sp, format!("mod {ident}\n"))], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vec![(sp, format!("mod {ident}\n"))], | |
vec![(sp, format!("mod {ident};\n"))], |
{ | ||
let sp = map | ||
.span_extend_to_prev_char( | ||
map.span_extend_to_prev_char(ident.span, '\n', true), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like something UsePlacementFinder
should typically do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats for the position of a use
though, if im reading it right? not a mod
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are typically placed next to each other, so it should work.
c1a4a41
to
18ca511
Compare
suggests declaring modules when a module is found but not defined, i.e
or
which currently is just
but now would have this nice help: