Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rule to place doc comments before any attributes #281

Merged
merged 4 commits into from
Jul 27, 2024

Conversation

calda
Copy link
Member

@calda calda commented Jul 22, 2024

Summary

This PR adds a new rule to place doc comments for a declaration before any attributes on the declaration.

// WRONG

@MainActor
/// A spacecraft with everything you need to explore the universe.
struct Spaceship {  }

// RIGHT

/// A spacecraft with everything you need to explore the universe.
@MainActor
struct Spaceship {}

Autocorrect support is implemented in the SwiftFormat docCommentsBeforeAttributes rule, which was added in nicklockwood/SwiftFormat#1766.

Reasoning

Placing the doc comment before any attributes is the most common practice, but I noticed some examples in our codebase where the doc comment is between the attributes and the rest of the declaration.

On the tooling side, Xcode correctly handles doc comments placed above attributes (see how the doc comment shows up in the inspector on the right):

Screenshot 2024-07-22 at 4 03 49 PM

But doesn't correctly handle doc comments placed after the attributes (see how the doc comment doesn't show up in the inspector on the right):

Screenshot 2024-07-22 at 4 04 07 PM

Copy link
Member Author

@calda calda left a comment

Choose a reason for hiding this comment

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

Strong internal support for this rule 👍🏻

Copy link
Contributor

@bachand bachand left a comment

Choose a reason for hiding this comment

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

Nice!

README.md Outdated
@@ -1870,6 +1870,26 @@ _You can enable the following settings in Xcode by running [this script](resourc

</details>

* <a id='doc-comments-before-attributes'></a>(<a href='#doc-comments-before-declarations'>link</a>) **Place doc comments for a declaration before any attributes.** [![SwiftFormat: docCommentsBeforeAttributes](https://img.shields.io/badge/SwiftFormat-docCommentsBeforeAttributes-7B0051.svg)](https://github.com/nicklockwood/SwiftFormat/blob/master/Rules.md#docCommentsBeforeAttributes)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the placement of the rule below a similar rule 👍
Screenshot 2024-07-26 at 4 44 50 PM

README.md Outdated Show resolved Hide resolved
Comment on lines +1881 to +1887
/// A spacecraft with everything you need to explore the universe.
struct Spaceship { … }

// RIGHT

/// A spacecraft with everything you need to explore the universe.
@MainActor
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this apply to property wrappers and builtin attributes like @objc or @dynamicMemberLookup as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! It applies to everything that starts with an @.

(In the Swift grammar everything that starts with an @ is an attribute -- you can confirm this by searching for @ in the grammar documentation and seeing that it only appears once, as the start of an attribute.)

calda and others added 2 commits July 26, 2024 16:55
Co-authored-by: Michael Bachand <michael.bachand@airbnb.com>
@calda calda enabled auto-merge (squash) July 27, 2024 00:02
@calda calda merged commit a09950f into master Jul 27, 2024
5 checks passed
@calda calda deleted the cal--docCommentsBeforeAttributes branch July 27, 2024 00:02
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.

2 participants