Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Fix rustfmt during builds by reading edition from manifest #1533

Merged
merged 3 commits into from
Aug 5, 2019

Conversation

alexheretic
Copy link
Member

Not being able to run rustfmt when code is building has been getting on my nerves a bit.

This pr adds a fallback to cover cases where files_to_crates is empty causing format requests to fail as we can't currently determine the edition to use. In these cases the manifest is read directly to determine edition.

Fixes #1148

Copy link
Member

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

I have to admit that fixing the formatting was high on my to-do list. I believe #1513 already addresses most of the pain points, however input file detection should be pushed further back to parsing if possible. Right now we can't do it inside after_parsing due to internal rustc bits (disambiguator calculation, which we need, happens after).

A best effort fallback like the one here will be good for the users so that's a 👍 from me

rls/src/actions/mod.rs Outdated Show resolved Hide resolved
rls/src/actions/mod.rs Outdated Show resolved Hide resolved
@@ -425,6 +435,24 @@ impl InitActionContext {
}
}

/// Read edition from the manifest
fn edition_from_manifest<P: AsRef<Path>>(manifest_path: P) -> Option<Edition> {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not thrilled about placement of this function here but maybe it's self-contained enough that it's not a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a private function that's only used here so it seems ok to me. I'd imagine it won't be hard to move or replace later. But if you can think of a better place to put it, along with the test, then sure.

alexheretic and others added 2 commits August 4, 2019 23:55
Co-Authored-By: Igor Matuszewski <Xanewok@gmail.com>
@Xanewok
Copy link
Member

Xanewok commented Aug 5, 2019

Thank you!

@Xanewok Xanewok merged commit bdfcef7 into rust-lang:master Aug 5, 2019
@alexheretic alexheretic deleted the rustfmt-while-building branch August 6, 2019 08:48
bors added a commit to rust-lang/rust that referenced this pull request Aug 8, 2019
Update RLS

Most importantly this includes:
* Collect file -> edition mapping after AST expansion ([#1513](rust-lang/rls#1513)) (enabled by #62679)
* Fix rustfmt during builds by reading edition from manifest ([#1533](rust-lang/rls#1533))

Which fixes the annoying problem where users couldn't format via RLS while `cargo fmt` worked.

The beta cut-off is drawing near and I'd like to make sure this lands before then.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RLS won't format code until it is done building
2 participants