nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.62k stars 623 forks source link

`sortDeclarations` silent failure #1708

Closed domkm closed 3 weeks ago

domkm commented 1 month ago

sortDeclaration silently fails to organize some types. Running with --verbose does not show any warnings or errors either.

Here is an example of a type that is not being sorted:

// swiftformat:sort
struct BookReaderView {
  @Namespace private var animation
  @State private var animationContent: Bool = false
  @State private var offsetY: CGFloat = 0
  @Bindable var model: Book
  @Query(
    filter: #Predicate<Alignment> { $0.progress_ < 1 },
    sort: \.updatedAt_,
    order: .reverse
  ) private var incompleteAlignments: [Alignment]
  @Query(
    filter: #Predicate<AudioContent> { $0.progress_ < 1 },
    sort: \.updatedAt_,
    order: .reverse
  ) private var incompleteAudioContents: [AudioContent]
  @Query(
    filter: #Predicate<TextContent> { $0.progress_ < 1 },
    sort: \.updatedAt_,
    order: .reverse
  ) private var incompleteTextContents: [TextContent]
}

My .swiftformat:

--disable headerFileName
--disable redundantSelf
--enable blankLineAfterImports
--enable blankLinesBetweenChainedFunctions
--enable blankLinesBetweenImports
--enable blockComments
--enable fileHeader
--enable isEmpty
--enable organizeDeclarations
--enable sortDeclarations
--enable sortSwitchCases
--enable sortTypealiases
--enable wrapEnumCases
--enable wrapSwitchCases

--allman false
--extensionacl on-declarations
--header strip
--indent 2
--markcategories false
--organizetypes class,actor,struct,enum,extension,protocol
nicklockwood commented 3 weeks ago

@domkm the problem appears to be caused by enabling organizeDeclarations, thought I'm not yet sure why.

@calda any idea what might be going on here?

Never mind, it seems to be due to this:

// Sorting the body of a type conflicts with the `organizeDeclaration`
// keyword if enabled for this type of declaration. In that case,
// defer to the sorting implementation in `organizeDeclarations`.
if formatter.options.enabledRules.contains(FormatRules.organizeDeclarations.name),
   formatter.options.organizeTypes.contains(declarationKeyword)
{
    return
}

I think there's probably a better solution possible here where we sort first and then run organizeDeclarations after. I'll look into it.

nicklockwood commented 3 weeks ago

@calda OK, I figured it out (finally). It's because of this:

// The compiler will synthesize a memberwise init for `struct`
// declarations that don't have an `init` declaration.
// We have to take care to not reorder any properties (but reordering functions etc is ok!)
if typeDeclaration.kind == "struct",
   !typeDeclaration.body.contains(where: { $0.keyword == "init" })
{

@domkm tl;dr; we can't re-order the properties here because it would break the synthesized initializer for the struct. If you were to declare an explicit initializer it would work.

@calda it seems inconsistent that the sortDeclarations rule on its own is happy to make this breaking change, but that the organizeDeclarations rule won't do it. Maybe if the user has explicitly requested sorting via the comment directive then we can just go ahead and apply the sorting anyway?

Also the lack of indication in the output for why it didn't apply the sorting seems not ideal. I wonder if it should print a warning or something to the console?

calda commented 3 weeks ago

Ahhhhh nice find!

I think we can disable that functionality when the sort annotation is applied explicitly.

calda commented 3 weeks ago

I can post a fix. (I'm always happy to investigate and fix issues with rules I worked on -- was planning on investigating today, but you beat me to it 😛)

nicklockwood commented 3 weeks ago

@calda just adding the check for explicit sort to that if statement seems to fix it. I'll let you know if I find any other issues 👍

domkm commented 3 weeks ago

Thanks for figuring it out!

nicklockwood commented 3 weeks ago

@domkm fixed in 0.54.0