Skip to content

Commit 4a9cb2e

Browse files
committed
docs: rust: add section on imports formatting
`rustfmt`, by default, formats imports in a way that is prone to conflicts while merging and rebasing, since in some cases it condenses several items into the same line. For instance, Linus mentioned [1] that the following case: use crate::{ fmt, page::AsPageIter, }; is compressed by `rustfmt` into: use crate::{fmt, page::AsPageIter}; which is undesirable. Similarly, `rustfmt` may put several items in the same line even if the braces span already multiple lines, e.g.: use kernel::{ acpi, c_str, device::{property, Core}, of, platform, }; The options that control the formatting behavior around imports are generally unstable, and `rustfmt` releases do not allow to use nightly features, unlike the compiler and other Rust tooling [2]. For the moment, we can introduce a workaround to prevent `rustfmt` from compressing the example above -- the "trailing empty comment": use crate::{ fmt, page::AsPageIter, // }; which is reminiscent of the trailing comma behavior in other formatters. We already used empty comments for formatting purposes in the past, e.g. in commit b9b701f ("rust: clarify the language unstable features in use"). In addition, `rustfmt` actually reformats with a vertical layout (i.e. it does not put two items in the same line) when seeing such a comment, i.e. it doesn't just preserve the formatting, which is good in the sense that we can use it to easily reformat some imports, since it matches the style we generally want to have. A Git merge driver would help (suggested by Gary and Wedson), though maintainers would need to set it up, the diffs would still be larger and the formatting rules for imports would remain hard to predict. Thus document the style that we will follow in the coding guidelines by introducing a new section and explain how the trailing empty comment works there too. We discussed the issue with upstream Rust in our usual Rust <-> Rust for Linux meeting [3], and there have also been a few other discussions in parallel in issues [4][5] and Zulip [6]. We will see what happens, but upstream Rust has already created a subteam of `rustfmt` to try to overcome the bandwidth issue [7], which is a good signal, and some organization work has already started (e.g. tracking issues). We will continue our discussions with them about it. Cc: Caleb Cartwright <caleb.cartwright@outlook.com> Cc: Yacin Tmimi <yacintmimi@gmail.com> Cc: Manish Goregaokar <manishsmail@gmail.com> Cc: Deadbeef <ent3rm4n@gmail.com> Cc: Cameron Steffen <cam.steffen94@gmail.com> Cc: Jieyou Xu <jieyouxu@outlook.com> Link: https://lore.kernel.org/all/CAHk-=wgO7S_FZUSBbngG5vtejWOpzDfTTBkVvP3_yjJmFddbzA@mail.gmail.com/ [1] Link: rust-lang/rustfmt#4884 [2] Link: https://hackmd.io/iSCyY3JTTz-g8YM-nnzTTA [3] Link: rust-lang/rustfmt#4991 [4] Link: rust-lang/rustfmt#3361 [5] Link: https://rust-lang.zulipchat.com/#narrow/channel/392734-council/topic/rustfmt.20maintenance/near/543815381 [6] Link: rust-lang/team#2017 [7] Reviewed-by: Benno Lossin <lossin@kernel.org> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
1 parent 98ac9cc commit 4a9cb2e

File tree

1 file changed

+75
-0
lines changed

1 file changed

+75
-0
lines changed

Documentation/rust/coding-guidelines.rst

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,81 @@ Like ``clang-format`` for the rest of the kernel, ``rustfmt`` works on
3838
individual files, and does not require a kernel configuration. Sometimes it may
3939
even work with broken code.
4040

41+
Imports
42+
~~~~~~~
43+
44+
``rustfmt``, by default, formats imports in a way that is prone to conflicts
45+
while merging and rebasing, since in some cases it condenses several items into
46+
the same line. For instance:
47+
48+
.. code-block:: rust
49+
50+
// Do not use this style.
51+
use crate::{
52+
example1,
53+
example2::{example3, example4, example5},
54+
example6, example7,
55+
example8::example9,
56+
};
57+
58+
Instead, the kernel uses a vertical layout that looks like this:
59+
60+
.. code-block:: rust
61+
62+
use crate::{
63+
example1,
64+
example2::{
65+
example3,
66+
example4,
67+
example5, //
68+
},
69+
example6,
70+
example7,
71+
example8::example9, //
72+
};
73+
74+
That is, each item goes into its own line, and braces are used as soon as there
75+
is more than one item in a list.
76+
77+
The trailing empty comment allows to preserve this formatting. Not only that,
78+
``rustfmt`` will actually reformat imports vertically when the empty comment is
79+
added. That is, it is possible to easily reformat the original example into the
80+
expected style by running ``rustfmt`` on an input like:
81+
82+
.. code-block:: rust
83+
84+
// Do not use this style.
85+
use crate::{
86+
example1,
87+
example2::{example3, example4, example5, //
88+
},
89+
example6, example7,
90+
example8::example9, //
91+
};
92+
93+
The trailing empty comment works for nested imports, as shown above, as well as
94+
for single item imports -- this can be useful to minimize diffs within patch
95+
series:
96+
97+
.. code-block:: rust
98+
99+
use crate::{
100+
example1, //
101+
};
102+
103+
The trailing empty comment works in any of the lines within the braces, but it
104+
is preferred to keep it in the last item, since it is reminiscent of the
105+
trailing comma in other formatters. Sometimes it may be simpler to avoid moving
106+
the comment several times within a patch series due to changes in the list.
107+
108+
There may be cases where exceptions may need to be made, i.e. none of this is
109+
a hard rule. There is also code that is not migrated to this style yet, but
110+
please do not introduce code in other styles.
111+
112+
Eventually, the goal is to get ``rustfmt`` to support this formatting style (or
113+
a similar one) automatically in a stable release without requiring the trailing
114+
empty comment. Thus, at some point, the goal is to remove those comments.
115+
41116

42117
Comments
43118
--------

0 commit comments

Comments
 (0)