merge_imports breaks apart imports

[cramertj]

merge_imports actually causes nested imports to be broken apart, not merged. This seems counterintuitive to me.

merge_imports = false
 
  1 use {                                                                                                
  2     crate::PseudoFile,                                                                               
  3     failure::Error,                                                                                  
  4     fidl::encoding::OutOfLine,                                                                       
  5     fidl::endpoints::RequestStream,                                                                  
  6     fidl_fuchsia_io::{                                                                               
  7         FileObject, FileRequest, FileRequestStream, NodeAttributes, NodeInfo, OPEN_FLAG_APPEND,      
  8         OPEN_FLAG_DESCRIBE, OPEN_FLAG_DIRECTORY, OPEN_FLAG_STATUS, OPEN_FLAG_TRUNCATE,               
  9         OPEN_RIGHT_READABLE, OPEN_RIGHT_WRITABLE,                                                    
 10     },                                                                                               
 11     fuchsia_async::Channel,                                                                          
 12     fuchsia_zircon::{                                                                                
 13         sys::{ZX_ERR_NOT_SUPPORTED, ZX_ERR_OUT_OF_RANGE, ZX_OK},                                     
 14         Status,                                                                                      
 15     },                                                                                               
 16     futures::{                                                                                       
 17         ready,                                                                                       
 18         stream::{FuturesUnordered, Stream, StreamExt, StreamFuture},                                 
 19         task::LocalWaker,                                                                            
 20         Future, FutureExt, Poll,                                                                     
 21     },                                                                                               
 22     std::pin::{Pin, Unpin},                                                                          
 23 };
 
merge_imports = true
 
  1 use crate::PseudoFile;                                                                               
  2 use failure::Error;                                                                                  
  3 use fidl::{encoding::OutOfLine, endpoints::RequestStream};                                           
  4 use fidl_fuchsia_io::{                                                                               
  5     FileObject, FileRequest, FileRequestStream, NodeAttributes, NodeInfo, OPEN_FLAG_APPEND,          
  6     OPEN_FLAG_DESCRIBE, OPEN_FLAG_DIRECTORY, OPEN_FLAG_STATUS, OPEN_FLAG_TRUNCATE,                   
  7     OPEN_RIGHT_READABLE, OPEN_RIGHT_WRITABLE,                                                        
  8 };                                                                                                   
  9 use fuchsia_async::Channel;                                                                          
 10 use fuchsia_zircon::{                                                                                
 11     sys::{ZX_ERR_NOT_SUPPORTED, ZX_ERR_OUT_OF_RANGE, ZX_OK},                                         
 12     Status,                                                                                          
 13 };                                                                                                   
 14 use futures::{                                                                                       
 15     ready,                                                                                           
 16     stream::{FuturesUnordered, Stream, StreamExt, StreamFuture},                                     
 17     task::LocalWaker,                                                                                
 18     Future, FutureExt, Poll,                                                                         
 19 };                                                                                                   
 20 use std::pin::{Pin, Unpin};

[ilya-bobyr]

Even if this is the desired behavior, documentation should be clear about it.
Currently, it is not obvious that it would happen from the example in the docs.

Current docs:

false (default):

use foo::{a, c, d};
use foo::{b, g};
use foo::{e, f};

true:

use foo::{a, b, c, d, e, f, g};

Possible example that shows all the differences:

false (default):

use foo::{a, c, d};
use foo::{b, g};
use bar::{e, f};

true:

use {
    foo::{a, b, c, d, g},
    bar::{e, f},
};

[topecongiro]

I think merge_imports should take enum, instead of a plain boolean, so users can control how deep merging should take effect.

[nrc]

Perhaps a better name would be something like normalize_import_grouping?

[nrc]

I think we need to revisit both how imports are formatted and the options available post-1.0

[mulkieran]

I do not know how closely rustfmt is integrated w/ the compiler. Would rustfmt be able to choose the shortest import path, so that it could convert:

crate::foo::bar::baz::a;

to

crate::foo::bar::a;

since a is actually re-exported from bar?

[ytmimi]

merge_imports was deprecated so we can use imports_granularity=Crate to reproduce what's happening here. However, at the Crate granularity everything is already as broken up as it could be. All that's happening is that we remove the top level use block in favor of individual use statements. Note, with imports_granularity=Preserve (the default) the imports are left unchanged.

@calebcartwright I feel like the current behavior is intended and this can be closed. Maybe we need to add a new group_imports to try and preserve the top level single use block?

[calebcartwright]

Correct @ytmimi so I'm going to close.

There's a number of variants under imports_granularity (https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#imports_granularity) with respective textual descriptions and examples. If anyone believes those can be improved/elaborated then feel free to submit a PR, but there's no need to keep this issue open for any potential documentation enhancements.

Additionally, the single-import statement flavor discussed in the OP is covered in #5360 and is better tracked there given its context in the present day configuration surface.