Skip to content
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

Fix module resolution in inner modules with paths #4194

Merged
merged 1 commit into from
May 22, 2020

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented May 21, 2020

Path attributes with relative paths were previously not treated as
relative to the containing file but as relative to the current working
directory of rustfmt.

The SubModKind enum had a dedicated variant for inner modules with
paths. However, the ordinary variant for inner modules also handles the
presence of path attributes and treats them correctly. Therefore the fix
is to simply remove the dedicated enum variant.

Fixes #3901
Fixes #4076

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! The code looks good to me. Could you make the following changes to the test code?

  • Move inner-module-path/ under rustfmt-lib/tests/target/.
  • Delete inner_module_path.
  • Add // rustfmt-recursive: true to the beginning of lib.rs. This is equivalent to setting OperationSetting.recursive to true.

I'm sorry for the confusion.

Path attributes with relative paths were previously not treated as
relative to the containing file but as relative to the current working
directory of rustfmt.

The SubModKind enum had a dedicated variant for inner modules with
paths. However, the ordinary variant for inner modules also handles the
presence of path attributes and treats them correctly. Therefore the fix
is to simply remove the dedicated enum variant.

Fixes rust-lang#3901
Fixes rust-lang#4076
@mahkoh mahkoh force-pushed the inner-module-path branch from be98597 to 48961c8 Compare May 22, 2020 08:24
@mahkoh
Copy link
Contributor Author

mahkoh commented May 22, 2020

Done

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you for the update!

@topecongiro
Copy link
Contributor

The AppVeyor failure seems to be a network issue, merging.

@karyon
Copy link
Contributor

karyon commented Oct 27, 2021

backported in #4411

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustfmt is not able to find files internally Error writing files: io error: Failed to find module
3 participants