Order conditionally-compiled imports

[a7medev]

Recursively sort conditionally-compiled imports the same way other imports are ordered.

Conditionally-compiled imports are always placed after normal non-conditional imports as before, they're only sorted relative to the same #if block.

Resolves #1027.

[a7medev]

Some questions to consider:

1. Should this be enabled by default with "OrderedImports" or should it be a new rule/option?
2. Is a single test enough given that the specifics of ordering is shared in orderImports or do we need to add more tests for other cases?

[allevato]

Thanks for looking into this!

1. Should this be enabled by default with "OrderedImports" or should it be a new rule/option?

We have to avoid changing the default behavior for anyone who's using the existing default in something like a CI workflow (we've forgotten that before, and it causes some annoyance).

One day I want to refactor the settings to make it easier to add new configuration options to existing rules. For now, you could introduce a small struct into the configuration that's something like OrderedImportSettings and have a single boolean in it, includeConditionalImports.

2. Is a single test enough given that the specifics of ordering is shared in orderImports or do we need to add more tests for other cases?

It would be good to have a nested case as well. For example,

import A
#if FOO
  import D
  #if BAR
    import F
    import E
  #else
    import H
    import G
  #endif
  import C
#endif
import B

I would expect the output to be:

import A
import B
#if FOO
  import C
  import D
  #if BAR
    import E
    import F
  #else
    import G
    import H
  #endif
#endif

[a7medev]

@allevato I've added an option for enabling ordering conditional imports and the nested conditional imports test.

I found some issues with the original implementation related to leading trivia and file header which I fixed and added some test cases for.

I also updated the documentation.

Please recheck. 🙏🏼

[a7medev]

This handles the case in testPreservesHeaderCommentInConditionalCompilationBlock.

Previously, the newline separating the comment and #if FOO was printed as is as part of fileHeader resulting in an extra newline separating the comment and #if FOO.

[a7medev]

This is a breaking change for regular file headers if for some reason file header comments have a newline before them. I don't think this is a behavior we need to maintain. What do you think?

[allevato]

It's probably not a big deal, but can we avoid the break by passing into formatAndAppend whether we're at the top-level of the file or we're inside an IfConfigDecl?

[a7medev]

@allevato Actually looking at this again, the pretty-printer will always remove that line anyway (and looks like that isn't configurable), so there should be no breaking changes here.

[allevato]

Even better!

[a7medev]

Hi @allevato! Have you checked the last changes here? 🙏🏼

[allevato]

LGTM, but now that we have #1090 in flight, let's wait for that to land, and that will be a good test to make sure there aren't any unintended side effects across some real code before we merge.

[a7medev]

@allevato Tests and real-world project compatibility tests are now green.

[allevato]

Thanks for adding this!