nicklockwood / SwiftFormat

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

markTypes rule does not work correctly. #1511

Open FrolovskiyKirill opened 1 year ago

FrolovskiyKirill commented 1 year ago

Hi!

We use the markTypes rule for mark extensions. Our codestyle suggests marking extensions with protocols like this:

// MARK: - URLFailureProtocol

extension OrderListPresenter: URLFailureProtocol {

When formatting swiftFormat we encountered the following problems:

we use settings like this:

--enable markTypes --marktypes never --extensionmark MARK: - %c

nicklockwood commented 1 year ago

@FrolovskiyKirill I'm not able to reproduce these issues with a simple example. Can you supply a sample file that demonstrates the problem?

FrolovskiyKirill commented 1 year ago

maybe the video will make more sense

https://github.com/nicklockwood/SwiftFormat/assets/97293194/ee4062a3-f9f7-4460-9074-a52ac5a671c3

and expected like this:

// MARK: - Test1

extension TestClass: Test1 { }

// MARK: - Test2

extension TestClass: Test2 { }

// MARK: - Analytics

extension TestClass: Test3 { }

(with dash between MARK: and protocol and the MARK: that was there doesn't change)

nicklockwood commented 1 year ago

@FrolovskiyKirill thank you - I'm now able to reproduce it. It seems to be caused by having the class definition immediately before the extension.

nicklockwood commented 1 year ago

@FrolovskiyKirill OK, it seems this is not actually a bug. For extensions that are grouped together with the type being extended, you need to use the --groupedextension template instead of --extensionmark.

@calda This does seem a little unintuitive. Any thoughts on how this might be improved? Maybe something like, defaulting the --groupedextension value to match --extensionmark if the latter is overridden (and/or already doesn't include the type name)?

calda commented 1 year ago

That could be a reasonable behavior, but right now those two values have different defaults.

I think the default behavior is pretty good, because it is more selective in where it adds the full ----- mark in Xcode.

e.g. comparing

image
// MARK: - MyComponent

struct MyComponent { }

// MARK: SomeProtocol

extension MyComponent: SomeProtocol { }

// MARK: - AnotherComponent

struct AnotherComponent { }

// MARK: SomeProtocol

extension AnotherComponent: SomeProtocol { }

vs

Screenshot 2023-08-19 at 7 42 28 AM
// MARK: - MyComponent

struct MyComponent { }

// MARK: - MyComponent + SomeProtocol

extension MyComponent: SomeProtocol { }

// MARK: - AnotherComponent

struct AnotherComponent { }

// MARK: - AnotherComponent + SomeProtocol

extension AnotherComponent: SomeProtocol { }

I find the result to be better when extensions following the main type declaration to be less emphasized so it forms a single group. This is why the behavior for these "grouped extensions" is different from other extensions (e.g. if you added an unrelated extension String { } to the file).