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

rustfmt: Use edition form Cargo.toml rather than always 2015 (DO NOT MERGE) #5232

Closed
wants to merge 2 commits into from

Conversation

nickbp
Copy link

@nickbp nickbp commented Feb 20, 2022

This patch shows an example solution to #4074, where rustfmt will fetch the edition from the source of truth: Cargo.toml. This is meant as a point of discussion rather than a serious fix.

For example, this allows rustfmt to be run against files in the rust-lang/rustfmt repository itself and automatically detect the correct edition of 2018 as configured in Cargo.toml, rather than using the incorrect default of 2015 like it does today.

From discussion in #4074, it sounds like the current preference is to not have rustfmt be the user-facing tool for formatting individual files and to instead have a different command for that? But in the meantime this provides a reference for one way that it could be done.

Note that if this were a serious PR I would also have written tests for the change, including the missing tests for the current scan for rustfmt.toml, but again this is just meant to be an example to show how the lookup logic could work and per discussion it isn't expected to be accepted.

As of this patch, the edition selection order is as follows:

  1. --edition provided as an argument
  2. edition from any rustfmt.toml in the directory tree or on the host system (IMO this should be deprecated)
  3. package.edition from Cargo.toml in the directory tree (NEW)
  4. 2015

…(DO NOT MERGE)

This patch shows an example solution to #4074, where `rustfmt` will fetch the `edition` from the source of truth: `Cargo.toml`. This is meant as a point of discussion rather than a serious fix.

For example, this allows `rustfmt` to be run against files in the `rustfmt` repository itself and automatically detect the correct edition of `2018` as configured in Cargo.toml, rather than using the incorrect default of `2015` like it does today.

From discussion in #4074, it sounds like the current preference is to not have `rustfmt` detect the `edition` automatically and to instead have a different command for end users to format individual files, but in the meantime this provides a reference for one way that it could be done.

Note that if this were a serious PR I would also have written tests for the change, including adding some sorely needed coverage of `resolve_config_files` that is currently missing, but again this is just meant to be an example to show how the lookup logic could work.

As of this patch, the `edition` selection order is in the following priority:
1. `--edition` provided as an argument
2. `edition` in "nearest" `rustfmt.toml` to the file
3. `package.edition` in "nearest" `Cargo.toml` to the file (NEW!)
4. `edition` in system `rustfmt.toml`s (IMO this is wrong - causes problems when different repos have different editions)
5. `2015`
@nickbp
Copy link
Author

nickbp commented Feb 20, 2022

Closing this example in favor of #5071 which implements support at the cargo fmt level instead

@nickbp nickbp closed this Feb 20, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Feb 20, 2022

@nickbp Thanks for opening this PR to share your solution to this problem (I see that you just closed this). Everyone's feedback is much appreciated so please leave any comments you have about the proposed feature on #5071!

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

Successfully merging this pull request may close these issues.

None yet

2 participants