Skip to content

Add option to group @testable imports at the top or bottom - #323

Merged
nicklockwood merged 8 commits into
nicklockwood:developfrom
fdiaz:sorted-imports-testable
Nov 20, 2018
Merged

Add option to group @testable imports at the top or bottom#323
nicklockwood merged 8 commits into
nicklockwood:developfrom
fdiaz:sorted-imports-testable

Conversation

@fdiaz

@fdiaz fdiaz commented Nov 20, 2018

Copy link
Copy Markdown
Contributor

This PR adds a config option to group @testable imports at the top or bottom when sorting them.

This way instead of:

import Bar
@testable import Foo
import UIKit

we could get:

import Bar
import UIKit
@testable import Foo

or:

@testable import Foo
import Bar
import UIKit

This is a style I've seen in some projects, putting the @testable at the bottom so it's easier to know what module is being tested.

This is my first PR to SwiftFormat so I'm more than happy to revisit anything.

@nicklockwood

Copy link
Copy Markdown
Owner

@fdiaz thanks, this looks great!

The argument names are a bit verbose compared with the rest of the project, although I don't have suggestions for better ones right now. That is contributing to the test failure in the help-line-length test though.

I'll do a more thorough review later when I get a chance.

Comment thread Sources/Options.swift Outdated
}

/// Grouping for sorting imports
public enum ImportGrouping: String, Equatable {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Equatable is redundant here, I think, since String already conforms to Equatable.

@fdiaz

fdiaz commented Nov 20, 2018

Copy link
Copy Markdown
Contributor Author

How do you feel about "normal"/"sorted" instead of "alphabetically"?

@fdiaz

fdiaz commented Nov 20, 2018

Copy link
Copy Markdown
Contributor Author

I'm gonna go with alphabetized for now instead of alphabetically. I think that makes it exactly 80. Let me know if you think there's a better word and I can change it 😉

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 89.618% when pulling e177b06 on fdiaz:sorted-imports-testable into 0bef4e4 on nicklockwood:develop.

@nicklockwood

Copy link
Copy Markdown
Owner

Lgtm. Thanks!

@nicklockwood
nicklockwood merged commit 37913c1 into nicklockwood:develop Nov 20, 2018
@fdiaz
fdiaz deleted the sorted-imports-testable branch November 20, 2018 21:05
nicklockwood pushed a commit that referenced this pull request Nov 23, 2018
* Add testable import new section option

* Add simple test case

* Add typealias

* Add simple test case

* Add new sorting algorithm for testable import

* Add enum to either group testable on top or bottom

* Remove redundant Equatable conformance

* alphabetically -> alphabetized
leogdion pushed a commit to brightdigit/SwiftFormatSlim that referenced this pull request Nov 6, 2020
…ood#323)

* Add testable import new section option

* Add simple test case

* Add typealias

* Add simple test case

* Add new sorting algorithm for testable import

* Add enum to either group testable on top or bottom

* Remove redundant Equatable conformance

* alphabetically -> alphabetized
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.

3 participants