Skip to content

add impl-ordering-except-fn-names config field for arbitrary_source_item_ordering #14782

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6425,6 +6425,7 @@ Released 2018-09-13
[`excessive-nesting-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#excessive-nesting-threshold
[`future-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#future-size-threshold
[`ignore-interior-mutability`]: https://doc.rust-lang.org/clippy/lint_configuration.html#ignore-interior-mutability
[`impl-ordering-except-fn-names`]: https://doc.rust-lang.org/clippy/lint_configuration.html#impl-ordering-except-fn-names
[`large-error-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#large-error-threshold
[`lint-commented-code`]: https://doc.rust-lang.org/clippy/lint_configuration.html#lint-commented-code
[`literal-representation-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#literal-representation-threshold
Expand Down
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,16 @@ A list of paths to types that should be treated as if they do not contain interi
* [`mutable_key_type`](https://rust-lang.github.io/rust-clippy/master/index.html#mutable_key_type)


## `impl-ordering-except-fn-names`
A list of function names that should be ignored when ordering functions in impl blocks.

**Default Value:** `[]`

---
**Affected lints:**
* [`arbitrary_source_item_ordering`](https://rust-lang.github.io/rust-clippy/master/index.html#arbitrary_source_item_ordering)


## `large-error-threshold`
The maximum size of the `Err`-variant in a `Result` returned from a function

Expand Down
3 changes: 3 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,9 @@ define_Conf! {
/// A list of paths to types that should be treated as if they do not contain interior mutability
#[lints(borrow_interior_mutable_const, declare_interior_mutable_const, ifs_same_cond, mutable_key_type)]
ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()]),
/// A list of function names that should be ignored when ordering functions in impl blocks.
#[lints(arbitrary_source_item_ordering)]
impl_ordering_except_fn_names: Vec<String> = Vec::new(),
/// The maximum size of the `Err`-variant in a `Result` returned from a function
#[lints(result_large_err)]
large_error_threshold: u64 = 128,
Expand Down
20 changes: 20 additions & 0 deletions clippy_lints/src/arbitrary_source_item_ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,15 @@ declare_clippy_lint! {
/// module-items-ordered-within-groupings = ["UPPER_SNAKE_CASE"]
/// ```
///
/// #### Exception function names in impl blocks
///
/// Sometimes you want to exclude some functions in impl blocks from being linted.
/// You can do this by adding the function names in the config.
///
/// ```toml
/// impl-ordering-except-fn-names = ["new"]
/// ```
///
/// ### Known Problems
///
/// #### Performance Impact
Expand Down Expand Up @@ -169,6 +178,7 @@ pub struct ArbitrarySourceItemOrdering {
enable_ordering_for_trait: bool,
module_item_order_groupings: SourceItemOrderingModuleItemGroupings,
module_items_ordered_within_groupings: SourceItemOrderingWithinModuleItemGroupings,
impl_ordering_except_fn_names: Vec<String>,
}

impl ArbitrarySourceItemOrdering {
Expand All @@ -184,6 +194,7 @@ impl ArbitrarySourceItemOrdering {
enable_ordering_for_trait: conf.source_item_ordering.contains(&Trait),
module_item_order_groupings: conf.module_item_order_groupings.clone(),
module_items_ordered_within_groupings: conf.module_items_ordered_within_groupings.clone(),
impl_ordering_except_fn_names: conf.impl_ordering_except_fn_names.clone(),
}
}

Expand Down Expand Up @@ -321,6 +332,15 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
if item.span.in_external_macro(cx.sess().source_map()) {
continue;
}
// If there are exception function names, skip it.
if !self.impl_ordering_except_fn_names.is_empty()
&& convert_assoc_item_kind(item.kind) == SourceItemOrderingTraitAssocItemKind::Fn
&& self
.impl_ordering_except_fn_names
.contains(&item.ident.name.to_string())
{
continue;
}
Comment on lines +335 to +343
Copy link
Contributor

@Jarcho Jarcho May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens too early as type sorting hasn't happened yet.

You'll have to move the kind order check on line 353 to happen before the name check at line 351 and then insert this in between.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is only to skip the function names that we don't want to sort.
I don't want to care about if it is one of the two compared objects, I just skip it.
If I change the code to this:

                for item in trait_impl.items {
                    if item.span.in_external_macro(cx.sess().source_map()) {
                        continue;
                    }
                    if let Some(cur_t) = cur_t {
                        let item_kind = convert_assoc_item_kind(item.kind);
                        let item_kind_index = self.assoc_types_order.index_of(&item_kind);

                        let cur_t_kind = convert_assoc_item_kind(cur_t.kind);
                        let cur_t_kind_index = self.assoc_types_order.index_of(&cur_t_kind);

                        // If there are exception function names, skip it.
                        if !self.impl_ordering_except_fn_names.is_empty()
                            && convert_assoc_item_kind(item.kind) == SourceItemOrderingTraitAssocItemKind::Fn
                            && self
                                .impl_ordering_except_fn_names
                                .contains(&item.ident.name.to_string())
                        {
                            continue;
                        }
                        
                        if cur_t_kind == item_kind && cur_t.ident.name.as_str() > item.ident.name.as_str() {
                            Self::lint_member_name(cx, &item.ident, &cur_t.ident);
                        } else if cur_t_kind_index > item_kind_index {
                            self.lint_impl_item(cx, item, cur_t);
                        }
                    }
                    cur_t = Some(item);
                }

Then you will find a uitest error when you run command TESTNAME="ordering_exceptions" cargo uitest.

impl BasicStruct1 {
    fn new() -> Self {
        Self {}
    }
    fn a() {}
    fn b() {}
}
error: there were 1 unmatched diagnostics
  --> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:14:8
   |
14 |     fn a() {}
   |        ^ Error[clippy::arbitrary_source_item_ordering]: incorrect ordering of items (must be alphabetically ordered)
   |

The first function will always participate in the sorting.


if let Some(cur_t) = cur_t {
let cur_t_kind = convert_assoc_item_kind(cur_t.kind);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
impl-ordering-except-fn-names = ["new"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: incorrect ordering of items (must be alphabetically ordered)
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:35:8
|
LL | fn a() {}
| ^
|
note: should be placed before `b`
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:31:8
|
LL | fn b() {}
| ^
= note: `-D clippy::arbitrary-source-item-ordering` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::arbitrary_source_item_ordering)]`

error: incorrect ordering of items (must be alphabetically ordered)
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:47:8
|
LL | fn b() {}
| ^
|
note: should be placed before `c`
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:46:8
|
LL | fn c() {}
| ^

error: incorrect ordering of items (must be alphabetically ordered)
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:59:8
|
LL | fn a() {}
| ^
|
note: should be placed before `c`
--> tests/ui-toml/arbitrary_source_item_ordering/ordering_exceptions.rs:58:8
|
LL | fn c() {}
| ^

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//@aux-build:../../ui/auxiliary/proc_macros.rs
//@revisions: exceptions
//@[exceptions] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/arbitrary_source_item_ordering/exceptions

#![allow(dead_code)]
#![warn(clippy::arbitrary_source_item_ordering)]

struct BasicStruct1 {}

impl BasicStruct1 {
fn new() -> Self {
Self {}
}
fn a() {}
fn b() {}
}

struct BasicStruct2 {}

impl BasicStruct2 {
fn a() {}
fn new() -> Self {
Self {}
}
fn b() {}
}

struct BasicStruct3 {}

impl BasicStruct3 {
fn b() {}
fn new() -> Self {
Self {}
}
fn a() {}
//~^ arbitrary_source_item_ordering
}

struct BasicStruct4 {}

impl BasicStruct4 {
fn a() {}
fn new() -> Self {
Self {}
}
fn c() {}
fn b() {}
//~^ arbitrary_source_item_ordering
}

struct BasicStruct5 {}

impl BasicStruct5 {
fn new() -> Self {
Self {}
}
fn b() {}
fn c() {}
fn a() {}
//~^ arbitrary_source_item_ordering
}

fn main() {
// test code goes here
}
3 changes: 3 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
excessive-nesting-threshold
future-size-threshold
ignore-interior-mutability
impl-ordering-except-fn-names
large-error-threshold
lint-commented-code
literal-representation-threshold
Expand Down Expand Up @@ -140,6 +141,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
excessive-nesting-threshold
future-size-threshold
ignore-interior-mutability
impl-ordering-except-fn-names
large-error-threshold
lint-commented-code
literal-representation-threshold
Expand Down Expand Up @@ -233,6 +235,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
excessive-nesting-threshold
future-size-threshold
ignore-interior-mutability
impl-ordering-except-fn-names
large-error-threshold
lint-commented-code
literal-representation-threshold
Expand Down
Loading