-
Notifications
You must be signed in to change notification settings - Fork 55
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
Ordering of types of groups within a module #71
Comments
Also related to (almost a dup of?) #24 |
Yeah, I almost commented on #24 but wasn't sure if it was quite on topic or not. |
I think this deserves its own issue. It's more general than #24 because it talks about every kind of item and the entire module. I was chatting about this just the other day, and the order I typically follow is like this (let me know if I am forgetting something):
That said, I'm not sure rustfmt should strictly control this ordering. Some might find that too invasive or obstructive to their desired code organization. We could provide such an ordering as a guideline in the style guide. But I would be okay with rustfmt forcing
@johnthagen I'm fairly sure this is wrong (e.g. see here) and if the order ever mattered I would consider it a bug. Can you make a reproducible test case? |
Additionally, within the mixed struct A { ... }
enum B { ... }
impl A { ... }
impl B { ... }
impl Foo for B { ... } I picked this style so all my data definitions are in one place, as opposed to the following mixed types/impls style: struct A { ... }
impl A { ... }
enum B { ... }
impl B { ... }
impl Foo for B { ... } |
There are two things that are usually above everything you've listed so far: file-scoped |
@solson I agree that rustfmt shouldn't reorder those, except perhaps for I don't agree that const and static always belong at the top; sometimes they belong next to a block of related functions that use them. Regarding struct, enum, and impl, I personally prefer the mixed style: I do agree that |
I wasn't so sure about my placement for these. I guess the place I chose is where I dump them if I have no better idea.
Agreed. I was imagining defining a set of structs/enums together which contain each other in some kind of tree structure (which comes up a lot for me), but it's not a universal rule. |
I just remembered something else: I tend to write EDIT: On the other hand, even with an order-independent |
You are absolutely correct, I was wrong about this. |
I think we could recommend:
Both in modules and functions. Beyond that I feel like there are too many special cases, or trade-offs to make a strong recommendation. I also think that by default Rustfmt should not make this change, but it would be nice if we could opt in to it. |
I'm in favor of rustfmt enforcing the order @nrc mentioned by default (leaving everything in 4 in its original order). Hiding a module-scope |
@solson mod declarations sometimes have to come after macro declarations in order for the macro to be visible. Macro uses in general might appear anywhere within the 3 groups, I think. I've occasionally put |
Ah, yeah, Maybe we can automatically move
I do consider this bad style. I like when the answer to the question "where does this name come from" is answered in a single predictable place for all names. |
There's a strong convention of putting |
Although its name varies ( We should definitely mention putting |
I think actually the name is not too important - inline mods belong with everything else, only placeholder mods should be at the top (I'd put |
I strongly prefer for the ordering of imports to be:
This ordering follows the module hierarchy, from "most" global to local. An |
In my mind, the global → local ordering is that which people suggested earlier:
|
@solson A non-inline I suspect that some of the preference for the |
I disagree with this phrasing. It's defining a submodule inside the current module and that code is nested inside the current module even though it's in a different file. I suspect our disagreement stems from the fact that I view
I prefer this ordering because to me it's the most natural expression of "external imports first, local definitions after". |
Perhaps this is part of it. But if we consider why users had an issue with this ordering, I believe it's because it feels like, and indeed is, a "use before definition". I strive to have definitions appear before their use in my source code. This is why I believe that the style imposed by |
Ah... I don't think in this way. I was quite happy that Rust did not enforce definition-use ordering. I like to put the most "high-level" things first, like
Well, I am myself an example of seeing All that said, I feel like I'm mostly disagreeing with the supporting points you've brought up, and not your actual proposal. :) I wouldn't terribly mind either order, but I'd like to see more feedback from the community at large. I feel like there are not many distinct voices on this thread so far. |
I'm +1 towards |
Expanding on @johnthagen's example, below is my preferred list including
|
I always |
So, the question from @ebkalderon is (I think) whether I'm against recommending that |
We discussed this issue at the style team meeting. The two outstanding questions are:
|
OK, I just finished analyzing all 8487 crates from crates.io. A total of 2096 crates use both 768 crates use That seems to indicate a complete lack of any standard convention. Between that and the specific cases I quoted above, I'd consider this conclusive evidence that we should not attempt to reorder |
If you'd like to do some further analysis, results.txt contains a summary of every .rs file on crates.io that contains (Note that while that file contains one entry per Rust source file, I would recommend doing the analysis by crate rather than by file, to avoid overcounting the conventions of crates with more source files.) |
Awesome, thanks @joshtriplett ! Did you notice any instances where |
Given this data and our discussion I'd like to propose the following:
Rationale for the recommendation is that this is what the (slim) majority of crates and the Rust repo does. |
I like this. Part of the reasons for a lack of standard convention could very well be because one has never been offered (the whole point of this project), so this seems like a very sensible compromise. Hopefully over time we then see a gradual conformity for a majority of projects. |
@nrc I just did a check for 7450 crates contained a file with both an Of those, I see two common patterns:
Personally, I would propose the following:
|
Thanks for collecting the data!
Yes, this is a good clarification
I think the difference here is you think rustfmt should not move out
I don't think we should have an opinion on a language issue. |
@nrc Are you OK with pulling all the And the only reason I commented on the language proposal is that eliminating |
Yes. This doesn't feel any worse than (e.g.) using block formatting where the author used visual formatting. And if they strongly object they can turn off the option.
Although you are right that it would avoid the problem, it seems like a rather large change for a rather minor problem. If that happens for other reasons, then of course we should have an opinion on the style in the resulting language, but I don't think our motivation is strong enough to try and influence the decision there. |
@nrc As part of your proposal, did you arrive at an ultimate consensus for how to handle the ordering of |
@ebkalderon I don't think we did exactly. I've been away for a few weeks and I'll try and get to this soon. |
Summary of recommendations (assume mods = out-of-line mods for all of this):
|
@nrc I think that instead of |
Curious: are all the people sure that out-of-line mod declarations should be sorted alphabetically? I think I have a pretty strong argument for preserving the order of A good logical ordering (for example. from low-level to high-level) of To me, mod declarations seems much closer to function definitions than to imports, because their ordering has the potential to expose the high-level structure of the code. This probably doesn't matter if you working with a particular code-base a lot, because you are most likely using navigation by type or file names. However, if you are just diving in into the project, figuring out which 20% of code is actually interesting can be very challenging, and here I think manually ordered mod declarations can help a lot? |
FWIW you can just put the important modules in a separate group to not get reordered with others. For example, given the following code, mod foo;
mod bar;
mod baz;
mod spams;
mod eggs; it should be formatted like: mod bar;
mod baz;
mod foo;
mod eggs;
mod spams; |
Our consensus about this sort of thing was that ordering is a weak and implicit way to supply information and we shouldn't encourage it (for one thing, the reader has to expect you to be ordering/grouping modules to recognise that there is actually information there). We thought that letting the user group modules using whitespace but not maintain order otherwise was a good compromise. |
6 years ago, there was a discussion on whether mod should come before or after use declarations. A code analysis[^1] of crates.io showed that the majority of crates prefer to use mod either before or after use declarations, but not both. Now in 2023, rust-analyzer[^2] uses mod before use declarations and rustfmt[^3] uses mod after use declarations. This shouldn't matter too much, although it does seem rather lax compared to the nature of correctness in rust. For this project specifically, mod after use enables the important use declarations to be read first before the less important module includes (which should rarely change if ever). [^1]: rust-lang/style-team#71 (comment) [^2]: https://github.com/rust-lang/rust/blob/master/src/tools/rust-analyzer/crates/ide-completion/src/lib.rs [^3]: https://github.com/rust-lang/rustfmt/blob/master/src/config/mod.rs
Is there any guidance about the general order of how things should be defined in a module? For example.
I'm unclear where
mod
declarations should go (beforeuse
or afteruse
?). I can't find if this has been discussed before. I'm not sure it's somethingrustfmt
would try to fix, but having a guideline could still be helpful. Relates to #24.Edit:
Sometimes you need to declare amod
before you can import it:The text was updated successfully, but these errors were encountered: