Consider Imports Reorder config when merging Use Trees

[davidBar-On]

While evaluating enhancements to PR #5612, I found that merging use-trees does not take into account the imports_reorder setting and it is considered to set as true. Assuming this is an issue, this PR is an attempt to fix it. This is done by adding the imports_reorder config value as a parameter to the use-trees merge related functions.

Note that because of the added parameter, many internal test cases in imports.rs were changed. For the existing tests I added true as the imports_reorder value, and I added a version of the tests with false value.

One case that is not handled by this PR, and that was raised in a PR #5612 comment is the formatting of the following code, when imports_granularity=Preserve and reorder_imports=true:

use a::b::c::{
d::e
};

In this case, because of the Preserve, it is expected that the output will be use a::b::c::{d::e}, but it is use a::b::c::d::e.

For the record, following is a possible fix for this issue (if it is an issue). I didn't add this fix because it affects some existing test cases. The possible fix is to UseTree::normalize():

1. Add imports_granularity argument to the function (usually the passed value is context.config.imports_granularity()).
2. Change:
   

   rustfmt/src/imports.rs

   Lines 590 to 595 in ee2bed9

   	// Normalise foo::{bar} -> foo::bar
   	if let UseSegmentKind::List(ref list) = last.kind {
   	if list.len() == 1 && list[0].to_string() != "self" {
   	normalize_sole_list = true;
   	}
   	}

   
   To:

        // Normalise foo::{bar} -> foo::bar
        if let UseSegmentKind::List(ref list) = last.kind {
            if list.len() == 1 && list[0].to_string() != "self" && imports_granularity != ImportGranularity::Preserve {
                normalize_sole_list = true;
            }
        }

[ytmimi]

Thanks for the PR, and taking the time to look into this. I took a really quick look, and I don't think this is an issue. I think users expect that when we merge imports we also sort them within the new import list, but maybe someone out there would like some more explicit control with the reorder_imports option so maybe we should keep this open. Personally, my vote would be to close this. @calebcartwright what are your thoughts?

One case that is not handled by this PR, and that was raised in a PR #5162 comment is the formatting of the following code, when imports_granularity=Preserve and reorder_imports=true:

use a::b::c::{
d::e
};

In this case, because of the Preserve, it is expected that the output will be use a::b::c::{d::e}, but it is use a::b::c::d::e.

I believe the above 👆🏼 issue that's not handled by this PR is the only one that needs fixing as it's inconsistent with the definition of Preserve, which is.

Do not change the granularity of any imports and preserve the original structure written by the developer.

However, any fix would need to be version gated because reorder_imports is a stable config option and reorder_imports=true is the default.

I believe fixing the discrepancy in the case mentioned above should be simple enough, and it might make more sense to add an issue for it to the backlog and label it as good-first-issue for a new contributor to work on.

[davidBar-On]

I think users expect that when we merge imports we also sort them within the new import list ... Personally, my vote would be to close this.

My view on this (as much as it worth) is that this is the right approach, but it should be extended so imports_granularity=Crate and reorder_imports=false will be mutual exclusive. I.e. rustfmt should not allow setting these two values, and when imports_granularity=Crate is set, rustfmt should implicitly set reorder_imports to true. Otherwise non-merged parts will not be sorted and merged parts will be sorted, which seem to be a conflict.

There are probably other similar cases. E.g. imports_granularity=Preserve and reorder_imports=true may be mutual exclusive, and imports_granularity=Preserve should implicitly implyreorder_imports=false.