Skip to content

Commit b0d9ae6

Browse files
JonathanBrouwerJonathanBrouwer
authored
Rollup merge of rust-lang#156401 - shivendra02467:doc-cfg-sort-fix, r=GuillaumeGomez
rustdoc: deterministic sorting for `doc_cfg` badges Fixes rust-lang#156391 Currently, target-exclusive `doc_cfg` badges (eg. "Available on...") reuse the order of predicates as they appear in the source code. This often buries popular targets behind niche ones and leads to inconsistent UI rendering. This PR introduces a deterministic sorting mechanism to the `Cfg` AST prior to HTML/JSON rendering. **Note for Reviewers:** To provide the best UX, I implemented a lightweight tiering heuristic (prioritizing major platforms like Linux/Apple/Windows first, followed by mobile, then BSDs, and alphabetizing the rest). However, I am completely open to tweaking these priority groupings or falling back to a different sorting logic if the team prefers. Let me know what you think!
2 parents 22f9a1d + f658bf6 commit b0d9ae6

11 files changed

Lines changed: 157 additions & 39 deletions

File tree

‎src/librustdoc/clean/cfg.rs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,44 @@ impl Cfg {
187187
}
188188
}
189189

190+
/// Recursively sorts the configuration tree to ensure deterministic rendering.
191+
///
192+
/// Sorting groups predicates logically: Targets first, then Target Features,
193+
/// then Crate Features, and finally nested Any/All/Not groupings.
194+
/// Within each group, a fallback alphabetical sort is applied.
195+
pub(crate) fn sort_for_rendering(&mut self) {
196+
fn sort_cfg_entry(cfg: &mut CfgEntry) {
197+
match cfg {
198+
CfgEntry::Any(sub_cfgs, _) | CfgEntry::All(sub_cfgs, _) => {
199+
for sub_cfg in sub_cfgs.iter_mut() {
200+
sort_cfg_entry(sub_cfg);
201+
}
202+
203+
sub_cfgs.sort_by_cached_key(|a| {
204+
(
205+
cfg_category(a),
206+
Display(a, Format::LongPlain).to_string().to_ascii_lowercase(),
207+
)
208+
});
209+
}
210+
CfgEntry::Not(box_cfg, _) => sort_cfg_entry(box_cfg),
211+
_ => {}
212+
}
213+
}
214+
215+
fn cfg_category(cfg: &CfgEntry) -> u8 {
216+
match cfg {
217+
CfgEntry::NameValue { name, .. } if *name == sym::feature => 2,
218+
CfgEntry::NameValue { name, .. } if *name == sym::target_feature => 1,
219+
CfgEntry::NameValue { .. } | CfgEntry::Bool(..) => 0,
220+
CfgEntry::Any(..) | CfgEntry::All(..) | CfgEntry::Not(..) => 3,
221+
_ => 4,
222+
}
223+
}
224+
225+
sort_cfg_entry(&mut self.0);
226+
}
227+
190228
fn omit_preposition(&self) -> bool {
191229
matches!(self.0, CfgEntry::Bool(..))
192230
}
@@ -843,14 +881,20 @@ pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute>
843881
if matches!(cfg_info.current_cfg.0, CfgEntry::Bool(true, _)) {
844882
None
845883
} else {
846-
Some(Arc::new(cfg_info.current_cfg.clone()))
884+
let mut cfg = cfg_info.current_cfg.clone();
885+
cfg.sort_for_rendering();
886+
Some(Arc::new(cfg))
847887
}
848888
} else {
849889
// If `doc(auto_cfg)` feature is enabled, we want to collect all `cfg` items, we remove the
850890
// hidden ones afterward.
851891
match strip_hidden(&cfg_info.current_cfg.0, &cfg_info.hidden_cfg) {
852892
None | Some(CfgEntry::Bool(true, _)) => None,
853-
Some(cfg) => Some(Arc::new(Cfg(cfg))),
893+
Some(cfg_entry) => {
894+
let mut cfg = Cfg(cfg_entry);
895+
cfg.sort_for_rendering();
896+
Some(Arc::new(cfg))
897+
}
854898
}
855899
}
856900
}

‎src/librustdoc/clean/cfg/tests.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,3 +418,34 @@ fn test_simplify_with() {
418418
assert_eq!(foobar.simplify_with(&foobarbaz), None);
419419
});
420420
}
421+
422+
#[test]
423+
fn test_sort_for_rendering() {
424+
create_default_session_globals_then(|| {
425+
let mut cfg = cfg_any(thin_vec![
426+
name_value_cfg_e("feature", "sync"),
427+
name_value_cfg_e("target_os", "linux"),
428+
cfg_all_e(thin_vec![word_cfg_e("unix")]),
429+
name_value_cfg_e("target_feature", "sse2"),
430+
name_value_cfg_e("target_os", "android"),
431+
name_value_cfg_e("feature", "alloc"),
432+
]);
433+
434+
cfg.sort_for_rendering();
435+
436+
let expected = cfg_any(thin_vec![
437+
// Category 0: Targets (Sorted Alphabetically: Android -> Linux)
438+
name_value_cfg_e("target_os", "android"),
439+
name_value_cfg_e("target_os", "linux"),
440+
// Category 1: Target Features
441+
name_value_cfg_e("target_feature", "sse2"),
442+
// Category 2: Crate Features (Sorted Alphabetically: alloc -> sync)
443+
name_value_cfg_e("feature", "alloc"),
444+
name_value_cfg_e("feature", "sync"),
445+
// Category 3: Nested logic pushed to the end
446+
cfg_all_e(thin_vec![word_cfg_e("unix")]),
447+
]);
448+
449+
assert_eq!(cfg, expected);
450+
});
451+
}

‎tests/rustdoc-gui/item-info-overflow.goml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ assert-property: (".item-info", {"scrollWidth": "940"})
88
// Just to be sure we're comparing the correct "item-info":
99
assert-text: (
1010
".item-info",
11-
"Available on Android or Linux or Emscripten or DragonFly BSD or FreeBSD or NetBSD or OpenBSD",
11+
"Available on Android or DragonFly BSD or Emscripten or FreeBSD or Linux or NetBSD or OpenBSD",
1212
STARTS_WITH,
1313
)
1414

@@ -26,6 +26,6 @@ assert-property: (
2626
// Just to be sure we're comparing the correct "item-info":
2727
assert-text: (
2828
"#impl-SimpleTrait-for-LongItemInfo2 .item-info",
29-
"Available on Android or Linux or Emscripten or DragonFly BSD or FreeBSD or NetBSD or OpenBSD",
29+
"Available on Android or DragonFly BSD or Emscripten or FreeBSD or Linux or NetBSD or OpenBSD",
3030
STARTS_WITH,
3131
)

‎tests/rustdoc-gui/item-info.goml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ store-position: (
1919
"//*[@class='stab portability']//code[normalize-space()='Win32_System_Diagnostics']",
2020
{"x": second_line_x, "y": second_line_y},
2121
)
22-
assert: |first_line_x| != |second_line_x| && |first_line_x| == 521 && |second_line_x| == 277
23-
assert: |first_line_y| != |second_line_y| && |first_line_y| == 676 && |second_line_y| == 699
22+
assert: |first_line_x| != |second_line_x| && |first_line_x| == 509 && |second_line_x| == 277
23+
assert: |first_line_y| == |second_line_y| && |first_line_y| == 699
2424

2525
// Now we ensure that they're not rendered on the same line.
2626
set-window-size: (1100, 800)

‎tests/rustdoc-html/doc-cfg/all-targets.rs‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
//@ has all_targets/fn.foo.html \
44
// '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \
5-
// 'Available on GNU or Catalyst or Managarm C Library or MSVC or musl or Newlib or \
6-
// Neutrino 7.0 or Neutrino 7.1 or Neutrino 7.1 with io-sock or Neutrino 8.0 or \
7-
// OpenHarmony or relibc or SGX or Simulator or WASIp1 or WASIp2 or WASIp3 or \
8-
// uClibc or V5 or target_env=fake_env only.'
5+
// 'Available on target_env=fake_env or Catalyst or GNU or Managarm C Library \
6+
// or MSVC or musl or Neutrino 7.0 or Neutrino 7.1 or Neutrino 7.1 with io-sock \
7+
// or Neutrino 8.0 or Newlib or OpenHarmony or relibc or SGX or Simulator or \
8+
// uClibc or V5 or WASIp1 or WASIp2 or WASIp3 only.'
99
#[doc(cfg(any(
1010
target_env = "gnu",
1111
target_env = "macabi",
@@ -32,12 +32,12 @@ pub fn foo() {}
3232

3333
//@ has all_targets/fn.bar.html \
3434
// '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \
35-
// 'Available on AArch64 or AMD GPU or ARM or ARM64EC or AVR or BPF or C-SKY or \
36-
// Hexagon or LoongArch32 or LoongArch64 or Motorola 680x0 or MIPS or MIPS release \
37-
// 6 or MIPS-64 or MIPS-64 release 6 or MSP430 or NVidia GPU or PowerPC or \
38-
// PowerPC64 or RISC-V RV32 or RISC-V RV64 or s390x or SPARC or SPARC-64 or SPIR-V \
39-
// or WebAssembly or WebAssembly or x86 or x86-64 or Xtensa or \
40-
// target_arch=fake_arch only.'
35+
// 'Available on target_arch=fake_arch or AArch64 or AMD GPU or ARM or \
36+
// ARM64EC or AVR or BPF or C-SKY or Hexagon or LoongArch32 or LoongArch64 \
37+
// or MIPS or MIPS release 6 or MIPS-64 or MIPS-64 release 6 or Motorola 680x0 \
38+
// or MSP430 or NVidia GPU or PowerPC or PowerPC64 or RISC-V RV32 or RISC-V RV64 \
39+
// or s390x or SPARC or SPARC-64 or SPIR-V or WebAssembly or WebAssembly or x86 \
40+
// or x86-64 or Xtensa only.'
4141
#[doc(cfg(any(
4242
target_arch = "aarch64",
4343
target_arch = "amdgpu",
@@ -75,15 +75,16 @@ pub fn bar() {}
7575

7676
//@ has all_targets/fn.baz.html \
7777
// '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \
78-
// 'Available on AIX and AMD HSA and Android and CUDA and Cygwin and DragonFly \
79-
// BSD and Emscripten and ESP-IDF and FreeBSD and Fuchsia and Haiku and HelenOS \
80-
// and Hermit and Horizon and GNU/Hurd and illumos and iOS and L4Re and Linux \
81-
// and LynxOS-178 and macOS and Managarm and Motor OS and NetBSD and bare-metal \
82-
// and QNX Neutrino and NuttX and OpenBSD and Play Station Portable and Play \
83-
// Station 1 and QuRT and Redox OS and RTEMS OS and Solaris and SOLID ASP3 and \
84-
// TEEOS and Trusty and tvOS and UEFI and VEXos and visionOS and Play Station \
85-
// Vita and VxWorks and WASI and watchOS and Windows and Xous and zero knowledge \
86-
// Virtual Machine and target_os=unknown and target_os=fake_os only.'
78+
// 'Available on target_os=fake_os and target_os=unknown and AIX and AMD HSA \
79+
// and Android and bare-metal and CUDA and Cygwin and DragonFly BSD and \
80+
// Emscripten and ESP-IDF and FreeBSD and Fuchsia and GNU/Hurd and Haiku \
81+
// and HelenOS and Hermit and Horizon and illumos and iOS and L4Re and Linux \
82+
// and LynxOS-178 and macOS and Managarm and Motor OS and NetBSD and NuttX \
83+
// and OpenBSD and Play Station 1 and Play Station Portable and Play Station Vita \
84+
// and QNX Neutrino and QuRT and Redox OS and RTEMS OS and Solaris and \
85+
// SOLID ASP3 and TEEOS and Trusty and tvOS and UEFI and VEXos and visionOS \
86+
// and VxWorks and WASI and watchOS and Windows and Xous and zero knowledge \
87+
// Virtual Machine only.'
8788
#[doc(cfg(all(
8889
target_os = "aix",
8990
target_os = "amdhsa",

‎tests/rustdoc-html/doc-cfg/doc-cfg-simplification.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pub mod ratel {
4646

4747
//@ has 'globuliferous/ratel/static.NUNCIATIVE.html'
4848
//@ count - '//*[@class="stab portability"]' 1
49-
//@ matches - '//*[@class="stab portability"]' 'crate features ratel and nunciative'
49+
//@ matches - '//*[@class="stab portability"]' 'crate features nunciative and ratel'
5050
#[doc(cfg(feature = "nunciative"))]
5151
pub static NUNCIATIVE: () = ();
5252

@@ -80,7 +80,7 @@ pub mod ratel {
8080

8181
//@ has 'globuliferous/ratel/enum.Cosmotellurian.html'
8282
//@ count - '//*[@class="stab portability"]' 10
83-
//@ matches - '//*[@class="stab portability"]' 'crate features ratel and cosmotellurian'
83+
//@ matches - '//*[@class="stab portability"]' 'crate features cosmotellurian and ratel'
8484
//@ matches - '//*[@class="stab portability"]' 'crate feature biotaxy'
8585
//@ matches - '//*[@class="stab portability"]' 'crate feature xiphopagus'
8686
//@ matches - '//*[@class="stab portability"]' 'crate feature juxtapositive'
@@ -158,7 +158,7 @@ pub mod ratel {
158158

159159
//@ has 'globuliferous/ratel/trait.Aposiopesis.html'
160160
//@ count - '//*[@class="stab portability"]' 4
161-
//@ matches - '//*[@class="stab portability"]' 'crate features ratel and aposiopesis'
161+
//@ matches - '//*[@class="stab portability"]' 'crate features aposiopesis and ratel'
162162
//@ matches - '//*[@class="stab portability"]' 'crate feature umbracious'
163163
//@ matches - '//*[@class="stab portability"]' 'crate feature uakari'
164164
//@ matches - '//*[@class="stab portability"]' 'crate feature rotograph'

‎tests/rustdoc-html/doc-cfg/doc-cfg.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
//@ has doc_cfg/struct.Portable.html
44
//@ !has - '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' ''
55
//@ has - '//*[@id="method.unix_and_arm_only_function"]' 'fn unix_and_arm_only_function()'
6-
//@ has - '//*[@class="stab portability"]' 'Available on Unix and ARM only.'
6+
//@ has - '//*[@class="stab portability"]' 'Available on ARM and Unix only.'
77
//@ has - '//*[@id="method.wasi_and_wasm32_only_function"]' 'fn wasi_and_wasm32_only_function()'
88
//@ has - '//*[@class="stab portability"]' 'Available on WASI and WebAssembly only.'
99
pub struct Portable;
@@ -25,7 +25,7 @@ pub mod unix_only {
2525

2626
//@ has doc_cfg/unix_only/trait.ArmOnly.html \
2727
// '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \
28-
// 'Available on Unix and ARM only.'
28+
// 'Available on ARM and Unix only.'
2929
//@ count - '//*[@class="stab portability"]' 1
3030
#[doc(cfg(target_arch = "arm"))]
3131
pub trait ArmOnly {

‎tests/rustdoc-html/doc-cfg/duplicate-cfg.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,11 @@ pub mod bar {
2323
}
2424

2525
//@ has 'foo/baz/index.html'
26-
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features sync and send only.'
26+
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features send and sync only.'
2727
#[doc(cfg(all(feature = "sync", feature = "send")))]
2828
pub mod baz {
2929
//@ has 'foo/baz/struct.Baz.html'
30-
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features sync and send only.'
30+
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features send and sync only.'
3131
#[doc(cfg(feature = "sync"))]
3232
pub struct Baz;
3333
}
@@ -37,17 +37,17 @@ pub mod baz {
3737
#[doc(cfg(feature = "sync"))]
3838
pub mod qux {
3939
//@ has 'foo/qux/struct.Qux.html'
40-
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features sync and send only.'
40+
//@ has '-' '//*[@class="stab portability"]' 'Available on crate features send and sync only.'
4141
#[doc(cfg(all(feature = "sync", feature = "send")))]
4242
pub struct Qux;
4343
}
4444

4545
//@ has 'foo/quux/index.html'
46-
//@ has '-' '//*[@class="stab portability"]' 'Available on crate feature sync and crate feature send and foo only.'
46+
//@ has '-' '//*[@class="stab portability"]' 'Available on foo and crate feature send and crate feature sync only.'
4747
#[doc(cfg(all(feature = "sync", feature = "send", foo)))]
4848
pub mod quux {
4949
//@ has 'foo/quux/struct.Quux.html'
50-
//@ has '-' '//*[@class="stab portability"]' 'Available on crate feature sync and crate feature send and foo and bar only.'
50+
//@ has '-' '//*[@class="stab portability"]' 'Available on bar and foo and crate feature send and crate feature sync only.'
5151
#[doc(cfg(all(feature = "send", feature = "sync", bar)))]
5252
pub struct Quux;
5353
}

‎tests/rustdoc-html/doc-cfg/sort.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Tests that `doc(cfg)` badges and auto-detected `cfg` badges are sorted deterministically.
2+
//@ edition:2021
3+
//@ compile-flags: --cfg target_os="linux"
4+
5+
#![crate_name = "foo"]
6+
#![feature(doc_cfg)]
7+
8+
// TEST 1: Explicit `#[doc(cfg(...))]`
9+
// Tests that OS targets are sorted alphabetically.
10+
//@ has 'foo/fn.foo.html'
11+
//@ has - '//*[@class="stab portability"]' 'Available on Android or Apple or Cygwin \
12+
// or DragonFly BSD or FreeBSD or Linux or NetBSD or OpenBSD or QNX Neutrino only.'
13+
#[doc(cfg(any(
14+
target_os = "android",
15+
target_os = "linux",
16+
target_os = "dragonfly",
17+
target_os = "freebsd",
18+
target_os = "netbsd",
19+
target_os = "openbsd",
20+
target_os = "nto",
21+
target_vendor = "apple",
22+
target_os = "cygwin"
23+
)))]
24+
pub fn foo() {}
25+
26+
// TEST 2: Implicit `#[cfg(...)]` via auto-detection
27+
// Tests that targets are sorted alphabetically just like explicit `doc(cfg)`.
28+
//@ has 'foo/fn.bar.html'
29+
//@ has - '//*[@class="stab portability"]' 'Available on Android or Apple or Cygwin \
30+
// or DragonFly BSD or FreeBSD or Linux or NetBSD or OpenBSD or QNX Neutrino only.'
31+
#[cfg(any(
32+
target_os = "android",
33+
target_os = "linux",
34+
target_os = "dragonfly",
35+
target_os = "freebsd",
36+
target_os = "netbsd",
37+
target_os = "openbsd",
38+
target_os = "nto",
39+
target_vendor = "apple",
40+
target_os = "cygwin"
41+
))]
42+
pub fn bar() {}

‎tests/rustdoc-html/inline_cross/doc-auto-cfg.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ pub mod pre {
3737
pub mod post {
3838
// issue: <https://github.com/rust-lang/rust/issues/113982>
3939
//@ has 'it/post/index.html' '//*[@class="stab portability"]' 'extra'
40-
//@ has - '//*[@class="stab portability"]' 'extra and extension'
40+
//@ has - '//*[@class="stab portability"]' 'extension and extra'
4141
//@ has 'it/post/struct.Type.html' '//*[@class="stab portability"]' \
4242
// 'Available on crate feature extra only.'
4343
//@ has 'it/post/fn.compute.html' '//*[@class="stab portability"]' \
44-
// 'Available on crate feature extra and extension only.'
44+
// 'Available on extension and crate feature extra only.'
4545
#[cfg(feature = "extra")]
4646
pub use doc_auto_cfg::*;
4747

0 commit comments

Comments
 (0)