Skip to main content

stern4rust/rules/source/
ordered_imports_rule.rs

1// Copyright 2025 Umberto Gotti <umberto.gotti@umbertogotti.dev>
2// Licensed under the MIT License
3// SPDX-License-Identifier: MIT
4
5use syn::Item;
6use syn::ItemUse;
7use syn::parse_file;
8use syn::spanned::Spanned;
9
10use crate::finding::model::import_path::ImportPath;
11use crate::reporting::offence::Offence;
12use crate::reporting::rule_explanation::RuleExplanation;
13use crate::rule::Rule;
14use crate::source_file::SourceFile;
15
16// Imports in `src/` run in alphabetic order.
17//
18// `test-file-structure` has asked this of `tests/` since the first release and
19// nothing asked it of the source tree, which is where `imported-paths`
20// routinely *adds* lines -- 201 of them across the sibling tools -- with nothing
21// saying where a new one lands. The result is a file whose first import block is
22// sorted and whose second is whatever order things were needed in.
23//
24// This is a scope change rather than a new question, and the stand-downs come
25// with it. `cargo fmt` runs first in the gate and orders `self`, `super`,
26// `crate` and uppercase-initial paths by rules of its own, so a rule demanding
27// the alphabet there would write a file no edit could make green -- each run
28// undoing the last. `ImportPath` already decides which pairs those are, and this
29// rule asks it rather than deciding again.
30//
31// The consequence worth stating is that the stand-down does far more work here
32// than it ever has in `tests/`. A test file imports the crate under test, so
33// `crate::` never appears; a source file usually leads with a block of it.
34// Measured on this crate, **56% of adjacent import pairs in `src/` stand down**
35// -- so more than half of what the rule appears to check, it does not.
36//
37// A block ends where the lines stop being consecutive: a blank line or a comment
38// between two imports separates them, and the first import of a block is
39// compared with nothing.
40pub struct OrderedImportsRule;
41
42impl OrderedImportsRule {
43    pub const SOURCE_ROOT: &'static str = "src/";
44
45    pub fn new() -> Self {
46        Self
47    }
48
49    fn applies_to(file: &SourceFile) -> bool {
50        file.relative_path().starts_with(Self::SOURCE_ROOT)
51    }
52
53    fn imports(items: &[Item]) -> Vec<&ItemUse> {
54        items
55            .iter()
56            .filter_map(|item| match item {
57                Item::Use(import) => Some(import),
58                _ => None,
59            })
60            .collect()
61    }
62
63    // The text as written, taken from the line rather than rebuilt from the
64    // syntax tree, so the offence quotes what the reader will search for.
65    //
66    // Anchored on the `use` keyword, not on the item. An item's span begins at
67    // its first attribute, so a gated import read from `span().start()` was
68    // judged -- and quoted -- by its `#[cfg(...)]` line instead of its path.
69    //
70    // The corrections that produced were not merely noisy, they were wrong.
71    // `embassy-logging` gates four imports on three features, and following
72    // every correction would have ordered them by feature name: `rtt_sink`
73    // above `qemu_sink` above `host_sink`, the paths in reverse, by the rule
74    // that exists to alphabetise them. It cut both ways -- two imports genuinely
75    // out of order went unreported because their attributes happened to sort.
76    //
77    // `follows` deliberately keeps the item span. An attributed import sitting
78    // directly beneath another is consecutive, and anchoring that on the `use`
79    // line too would put the attribute in the gap and stop the pair being
80    // compared at all.
81    fn text_of(file: &SourceFile, import: &ItemUse) -> String {
82        file.lines()
83            .get(import.use_token.span().start().line - 1)
84            .map(|line| line.trim().to_string())
85            .unwrap_or_default()
86    }
87
88    // Consecutive lines, so a blank line or a comment between two imports ends
89    // the block and the pair is never compared.
90    fn follows(previous: &ItemUse, import: &ItemUse) -> bool {
91        import.span().start().line == previous.span().end().line + 1
92    }
93
94    fn offence(&self, file: &SourceFile, previous: &str, import: &str, line: usize) -> Offence {
95        Offence::new(
96            file.relative_path(),
97            line,
98            self.name(),
99            format!("`{import}` is out of alphabetic order; it follows `{previous}`"),
100            format!("move `{import}` above `{previous}`"),
101        )
102        .with_subject(import)
103    }
104}
105
106impl Default for OrderedImportsRule {
107    fn default() -> Self {
108        Self::new()
109    }
110}
111
112impl Rule for OrderedImportsRule {
113    fn name(&self) -> &'static str {
114        "ordered-imports"
115    }
116
117    fn check(&self, file: &SourceFile) -> Vec<Offence> {
118        if !Self::applies_to(file) {
119            return Vec::new();
120        }
121        let Ok(syntax) = parse_file(&file.contents()) else {
122            return Vec::new();
123        };
124        let imports = Self::imports(&syntax.items);
125        imports
126            .windows(2)
127            .filter(|pair| Self::follows(pair[0], pair[1]))
128            .filter_map(|pair| {
129                let previous = Self::text_of(file, pair[0]);
130                let import = Self::text_of(file, pair[1]);
131                if ImportPath::decides_order(&previous, &import)
132                    || ImportPath::is_ordered(&previous, &import)
133                {
134                    return None;
135                }
136                Some(self.offence(file, &previous, &import, pair[1].span().start().line))
137            })
138            .collect()
139    }
140
141    fn check_workspace(&self, _files: &[SourceFile]) -> Vec<Offence> {
142        Vec::new()
143    }
144
145    fn requirement(&self) -> Option<&'static str> {
146        None
147    }
148
149    fn is_configured(&self) -> bool {
150        true
151    }
152
153    fn explanation(&self) -> RuleExplanation {
154        RuleExplanation::new(
155            self.name(),
156            "Imports in src/ run in alphabetic order.",
157            "use zzz::Zed;\nuse aaa::Alpha;",
158            "use aaa::Alpha;\nuse zzz::Zed;",
159        )
160    }
161}