nicklockwood / SwiftFormat

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

Update sortTypealiases rule to also remove duplicates #1522

Closed calda closed 11 months ago

calda commented 11 months ago

With the sortTypealiases rule we've found its pretty common for folks to unintentionally add a type to the protocol composition multiple times. This can be hard to notice when the typealias isn't sorted, especially if the typealias is quite long:

typealias Dependencies
    = FooProviding
    & BarProviding
    & BaazProviding
    & QuuxProviding
    & FooProviding

but is quite obvious after its sorted:

typealias Dependencies
    = BaazProviding
    & BarProviding
    & FooProviding
    & FooProviding
    & QuuxProviding

Since we're already parsing and reordering these in the sortTypealiases rule, we may as well also remove duplicates:

typealias Dependencies
    = BaazProviding
    & BarProviding
    & FooProviding
    & QuuxProviding
codecov[bot] commented 11 months ago

Codecov Report

Merging #1522 (03346a9) into develop (7466e75) will increase coverage by 0.19%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1522      +/-   ##
===========================================
+ Coverage    95.11%   95.31%   +0.19%     
===========================================
  Files           20       20              
  Lines        21432    21446      +14     
===========================================
+ Hits         20385    20441      +56     
+ Misses        1047     1005      -42     
Files Changed Coverage Δ
Sources/Rules.swift 98.62% <100.00%> (+<0.01%) :arrow_up:

... and 5 files with indirect coverage changes

nicklockwood commented 11 months ago

I might split this out into a separate rule, just so people have the option of deduping without sorting. Accepting for now though so the PR doesn't get stale.

calda commented 11 months ago

Fine with me, I suppose that would be a pretty straightforward change. The existing implementation can easily be copied and adapted into a redundantTypealiasMember rule that is the exact same but just without the .sort { ... } call.

nicklockwood commented 11 months ago

@calda I was curious if a protocol composition expression could contain a non-protocol type such as a class, and whether it would be valid for that to be ordered after the first item. It turns out that it can and is:

let CodableView = UIView & Codable // valid
let CodableView = Codable & UIView // also valid

This is weird, and I think undesirable, but since it's valid I supposed it's not a problem for the sortTypealiases rule. I wonder if we should include a way to exclude a list of known classes to exclude from sorting? There's unfortunately not really any way to detect it automatically.

calda commented 11 months ago

Yeah, UIView & Codable is a valid protocol composition. We could certainly build in support for keeping certain known classes as the first item in the list if somebody wanted that behavior. I don't really have a preference between Codable & UIView vs UIView & Codable, other than that one is alphabetized (less important on typealiases with only a few types).