Feature request: reorder_type_constraints

[orenbenkiki]

Requested feature(s):

EDIT: (1) is a duplicate of #4112, which was a duplicate of #1867 - ignore this and focus on #2.

1. Use reorder_derive_items to have rustfmt change:

#[derive(PartialEq, Copy, Display, Clone)]

Into:

#[derive(Clone, Copy, Display, PartialEq)]

That is, alphabetically sort the derived types.

2. Use reorder_type_constraints to have rustfmt change:

pub fn<T: PartialEq + Copy + Debug + 'static + Clone>foo(...) { ... }

Into:

pub fn<T: 'static + Clone + Copy + Debug + PartialEq>foo(...) { ... }

That is, alphabetically sort the contsraint types (and sort lifetimes before types).

Motivation:

When there are more than 2-3 types in either derive or a constraint, it is tedius and error-prone to manually ensure they all appear in the same order. However it is useful for them to be in a deterministic order as this makes it easier to read a list, and more importantly to see the difference between two lists.

Options

Perhaps "standard" traits should be sorted before all other traits?

[ytmimi]

Thanks for the feature request!

Reordering derives was requested in #4112, which was a duplicate of #1867. Seems like there was some discussion about reordering derives and how that could lead to compilation errors. There is also an open RFC for the feature: rust-lang/style-team#154, but I think there are still some issues to work through.

Reordering type constraints seems like a new feature request (I quickly checked to see if I could find something similar). Because there's still some debate on whether derives can safely be reordered, it's probably best to limit the scope of the feature request to focus on reordering type constraints.

[orenbenkiki]

I've no idea how I managed to miss that issue... Sure, let's restrict this one to just type constraints.

It seems to me that for type constraints there's no way that the order matters - there's no issue of side-effects of the order like there is for derive macros.

[0xangelo]

This is precisely what I was looking for. It would be an amazing feature for sure! Is there anyone working on this already? I'd be willing to help; does anyone have tips on where to get started?

[ytmimi]

Hey @0xangelo thanks for your interest in this! I don't believe anyone is working on this so you can comment @rustbot claim to assign yourself to this issue.

What I think you'll need to do is sort the params before calling overflow::rewrite_with_angle_brackets

rustfmt/src/items.rs

Lines 2711 to 2726 in a37d3ab

	fn rewrite_generics(
	context: &RewriteContext<'_>,
	ident: &str,
	generics: &ast::Generics,
	shape: Shape,
	) -> Option<String> {
	// FIXME: convert bounds to where-clauses where they get too big or if
	// there is a where-clause at all.
	if generics.params.is_empty() {
	return Some(ident.to_owned());
	}
	let params = generics.params.iter();
	overflow::rewrite_with_angle_brackets(context, ident, params, shape, generics.span)
	}

You'll also likely need to introduce a new configuration option for the new feature that should be false by default, but maybe we want to introduce an enum for the new option instead of a bool? I'd check out the Configuration section of the Contributing Guide for details on adding a new configuration option.

[0xangelo]

Cool! I'll see if I can get started on it this weekend
@rustbot claim

[ytmimi]

@0xangelo one thing that's going to make this a little tricky to implement is reordering comments when you reorder the generics. This is also an issue we still have when reordering imports. I'm not sure if we'd need to retain comments for the feature to be introduced as unstable, but we'd certainly need to worry about comment reordering if we were to stabilize the option.

[calebcartwright]

I'm not sure if we'd need to retain comments for the feature to be introduced as unstable, but we'd certainly need to worry about comment reordering if we were to stabilize the option.

We can certainly introduce as unstable provided we don't completely remove comments. I can't recall offhand what the current behavior is in the face of comments, but there's a fairly common approach elsewhere which would be fine to introduce on nightly: if we detect comments are lost then simply leave the span contents as-is. Also agreed we'd need to get the comment situation resolved before stabilization.

[calebcartwright]

A broader behavior question for consideration, but I suspect we're going to have people that want to distinguish between re_grouping_ and re_ordering_. More specifically, that there would people that wanted to keep all lifetimes separate from all traits, with those respectively ordered amongst themselves, as well as another group that would want the full ordering with mixing and matching.

I would prefer to avoid having another scenario where we need 2+ config options to control behavior because that increases the config surface as well as cognitive complexity. I wonder if an enum-based config option with multiple variants would suffice?