Skip to content

Conversation

@a7medev
Copy link
Member

@a7medev a7medev commented Oct 18, 2025

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
Copy link
Member Author

a7medev commented Oct 18, 2025

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
Copy link
Member

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.

  1. 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
Copy link
Member Author

a7medev commented Oct 25, 2025

@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. 🙏🏼

Comment on lines +59 to +61
if let firstLine = fileHeader.first, firstLine.type == .blankLine {
fileHeader.removeFirst()
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Conditionally compiled imports not sorted

2 participants