Feature: Configure sectioning of `orderedImports`

[amja]

Right now, enabling the orderedImports rule will put different types of imports (@testable, @_implementationOnly, etc.) in their own sections, separated by blank lines.

Would people be opposed if I raised a PR to add a configuration option to disable the sectioning behaviour? Just wanted to get some sense this would be supported by reviewers :)

My personal rationale: there are lots of testing scenarios where a list of imports will have one @testable import for the module under test and all others as normal imports. In those cases, I thought it seemed like overkill to put the 1 import in its own section. If we don't want to be too prescriptive in swift-format, it might be nice to offer the option.

Just let me know if people would be happy with this and I'll work on a PR for it.

[allevato]

[T]here are lots of testing scenarios where a list of imports will have one @testable import for the module under test and all others as normal imports. In those cases, I thought it seemed like overkill to put the 1 import in its own section.

IMO, this was explicitly a desired feature. I would expect most tests to only use @testable import for one import (the companion module to that test module) and putting @testable in its own group makes the module under test stand out clearly instead of being lost among a bunch of other imports.

In the past I could see some room for customization here in general, but I also wonder if imports are getting to be too complex for the grouping to make sense. When the rule was first written, there was really only import and @testable import. Now we have a bunch of modifiers and attributes possible on import: public/package/internal/private, @preconcurrency, @_implementationOnly, @_exported, @_spi(...), and probably others I'm forgetting. And those aren't exclusive—they can be combined—and in some cases there are compiler bugs that make imports order-dependent (at least two I know of: one involves @preconcurrency, and ClangImporter can get confused in the face of not-quite-modular C modules when dealing with forward declarations and such).

And then on top of that, a lot of real world Swift code uses conditional imports in #if blocks.

So these days I start to wonder if grouping imports is another combinatorial-explosion problem (where does a @_preconcurrency public import go? What about a @preconcurrency @_spi(...) public import? etc.), and we should consider dialing it back to just alphabetical (except for #if blocks, which we can't really do much about).

We can't really remove the groupings we have today, but we might regret adding more at this rate.

[amja]

Thank you, this is a thoughtful clarification. I wonder then, and this is definitely a self serving argument, whether the increasingly complex situation makes a configuration option to disable grouping even more appealing. If we are keeping these existing groups for backwards compatibility, people may or may not like how it interacts with the categories we don’t handle in the logic. So providing the option lets them keep the benefits of ordering without the weirdness of an incomplete categorisation algorithm.

To be clear, I’m not advocating for any more customisation beyond a Boolean config flag to disable grouping in the import ordering logic.

[allevato]

Yes, I think adding a Boolean to OrderedImportsConfiguration that disables grouping (including @testable for consistency†) and just sorts all of the imports by module name makes sense. If you'd like to work on that, I'm happy to review it!

† I'm less opinionated about this now because IMO @testable should be replaced in most cases by giving non-public APIs under test package level access, which doesn't have the issue of requiring that code be built in a different mode. But that's not really relevant to this discussion.

[amja]

Sounds good :) I’ll add the PR here when I make it. Good to know about the package access modifier 👀. I hadn’t thought about its testing implications.