Add rules for how to wrap attributes on property declarations

[calda]

Summary

We currently have a rule for how to wrap attributes on function and type declarations (link), but not property declarations.

This PR adds rules for how to wrap attributes on property declarations. This is split into three groups:

1. Place attributes for computed properties on the line above the declaration.
2. For stored properties:
   • Place simple attributes on the same line as the rest of the declaration.
   • Complex attributes with named arguments, or more than one unnamed argument, should be placed on the previous line.

Taken as a whole (including the rules for functions, types, and properties), our rules for wrapping attributes can be summed up as:

attributes should be placed on the previous line, excluding simple attributes attached to stored properties

Autocorrect is implemented in the SwiftFormat wrapAttributes rule, supported by new options added in nicklockwood/SwiftFormat#1588, nicklockwood/SwiftFormat#1591, and nicklockwood/SwiftFormat#1595.

Examples

Computed properties

// WRONG
@ViewBuilder var controlPanel: some View {
  // ...
}

// RIGHT
@ViewBuilder
var controlPanel: some View {
  // ...
}

Stored properties

// WRONG
struct SpaceshipDashboardView {

  @State
  private var warpDriveEnabled: Bool

  @ObservedObject
  private var lifeSupportService: LifeSupportService

  @Environment(\.controlPanelStyle) 
  private var controlPanelStyle

}

// RIGHT
struct SpaceshipDashboardView {

  @State private var warpDriveEnabled: Bool

  @ObservedObject private var lifeSupportService: LifeSupportService

  @Environment(\.controlPanelStyle) private var controlPanelStyle

}

// WRONG
struct RocketFactory {

  @available(*, unavailable, message: "No longer in production") var saturn5Builder: Saturn5Builder

  @available(*, deprecated, message: "To be retired by 2030") var atlas5Builder: Atlas5Builder

}

// RIGHT
struct RocketFactory {

  @available(*, unavailable, message: "No longer in production")
  var saturn5Builder: Saturn5Builder

  @available(*, deprecated, message: "To be retired by 2030")
  var atlas5Builder: Atlas5Builder

}

Reasoning

Based on existing usage within our codebase / the iOS community, these are the most common ways to format attributes.

SwiftUI stored property attributes, like @State and @Environment, are almost always written on the same line as the declaration. This applies within our codebase, but also within all first-party example code from Apple.

More rationale discussed below in #254 (comment) and #254 (comment).

Please react with 👍/👎 if you agree or disagree with this proposal.

[dmiluski]

Is there a link/thread to why we chose to inline state, observed, environment where as Viewbuilder/Availability go above? Is it just to match existing usage vs building consistency?

Simple vs Complex?

This feels like a cognitive load to differentiate

[calda]

These rules try to match the most common formatting used by the community for each of these different use cases. I think the formatting in each of these cases is individually well-motivated and reasonable:

SwiftUI property wrappers

If you look at Apple's first party SwiftUI sample code, the SwiftUI property wrappers (@State, @Binding, @Environment) are always written on the same line as the rest of the declaration. Here are some examples:

• @State documentation
• @Binding documentation
• @Environment documentation

Result builders

For result builders like @ViewBuilder and the Epoxy @SectionModelBuilder, there is not a single universal formatting preferred by the entire iOS community (based on a bit of brief googling I did).

Within our codebase, placing these on the previous line is by far the most common pattern. With a quick regex search in our codebase, I see 1000+ uses of @ViewBuilder on the line before a var and zero uses of @ViewBuilder on the same line as the var.

One reason I think this formatting makes sense is because it means that all uses of @ViewBuilder (across both functions and computed variables) are consistent:

// Consistent
@ViewBuilder
var dashboard: some View {
  // ...
}

@ViewBuilder
func dashboard(padding: Double) -> some View {
  // ...
}

// Inconsistent
@ViewBuilder var dashboard: some View {
  // ...
}

@ViewBuilder
func dashboard(padding: Double) -> some View {
  // ...
}

@available

For @available, one specific reason to prefer putting these on the previous line is that @available attributes are often somewhat long, especially if you include a message: argument.

I think most folks will agree that the @available here is a bit too long, and reads better split across multiple lines:

// These lines are a bit too long (even though they actually comply with our suggested 100 character max line width!)
@available(*, unavailable, message: "No longer in production") var saturn5Builder: Saturn5Builder
@available(*, deprecated, message: "To be retired by 2030") var atlas5Builder: Atlas5Builder

// Better!
@available(*, unavailable, message: "No longer in production") 
var saturn5Builder: Saturn5Builder

@available(*, deprecated, message: "To be retired by 2030")
var atlas5Builder: Atlas5Builder

@available attributes on stored properties are somewhat rare, so this shouldn't come up very often. One nice thing about this is that it's consistent with how @available attributes are written everywhere else in the language (e.g. on types, functions, and computed properties). So like with @ViewBuilder above, this rule makes it so that @available is consistently formatted everywhere.

Other attributes with arguments

There are some other less common examples of attributes with arguments that I think are still important to consider.

In our codebase I found a property wrapper called @Tweak, which lets you customize how a SwiftUI view renders at runtime. All of the examples of this property wrapper in our codebase are placed on the previous line. Subjectively, I find this looks much better than placing them on the same line.

// Some existing use cases of `@Tweak`
@Tweak(name: "Caching enabled")
private var cachingEnabled = false

@Tweak(name: "Number of columns", range: 1...10, format: .number)
private var numberOfColumns = 2

@Tweak(name: "Aspect ratio")
private var aspectRatio = AspectRatio.stretch

// Placing them on the same line instead.
// All of these lines are less than our max line width of 100 chars, but feel too long to me.
@Tweak(name: "Caching enabled") private var cachingEnabled = false

@Tweak(name: "Number of columns", range: 1...10, format: .number) private var numberOfColumns = 2

@Tweak(name: "Aspect ratio") private var aspectRatio = AspectRatio.stretch

Another example to consider is the set of property wrappers used in the first-party Swift Argument Parser library. We use Swift Argument Parser in this repo (code pointer), and in some Airbnb-internal tools. In the documentation you can see all of the examples place the attribute on the previous line as well:

@Flag(help: "Include a counter with each repetition.")
var includeCounter = false

@Option(name: .shortAndLong, help: "The number of times to repeat 'phrase'.")
var count: Int? = nil

@Argument(help: "The phrase to repeat.")
var phrase: String

Simple vs Complex?

This feels like a cognitive load to differentiate

You're definitely right that these rules are a bit nuanced and can seem complicated. I do agree we should prefer simple rules over complex rules, but I also think we should be prioritizing the most idiomatic and common formatting for each individual use case (all with differing context).

There are some factors that I think will help manage the complexity here:

1. Taken as a whole, I think our rules for attributes can actually be summed up pretty succinctly: "attributes should be placed on the previous line, excluding simple attributes attached to stored properties".
2. Including automatic code reformatting makes it so code authors don't necessarily need to know the full set of rules off the top of their head. The linter will apply any corrections automatically.

[bachand]

Overall I am supportive of this direction though I have a bit of feedback around the special case for stored properties.

[calda]

After some discussion internally, we updated the rule to:

• For simple attributes without arguments, or with a single unnamed argument, place the attribute on the same line as the declaration
• For complex attributes with named arguments or multiple unnamed arguments, place the attribute on the line before the declaration

This removes the special case for @Environment, which both simplifies and rule and generalizes it to handle more cases correctly.

@State private var warpDriveEnabled: Bool

@ObservedObject private var lifeSupportService: LifeSupportService

@Environment(\.controlPanelStyle) private var controlPanelStyle

@AppStorage("ControlsConfig") private var controlsConfig: ControlConfiguration

@AppStorage("ControlsConfig", store: myCustomUserDefaults)
private var controlsConfig: ControlConfiguration

@Tweak(name: "Aspect ratio")
private var aspectRatio = AspectRatio.stretch

@available(*, unavailable) 
var saturn5Builder: Saturn5Builder

@available(*, unavailable, message: "No longer in production") 
var saturn5Builder: Saturn5Builder

I also added a rationale section to the new rule:

Why?

Unlike other types of declarations, which have braces and span multiple lines, stored property declarations are often only a single line of code. Stored properties are often written sequentially without any blank lines between them. This makes the code compact without hurting readability, and allows for related properties to be grouped together in blocks:

struct SpaceshipDashboardView {
  @State private var warpDriveEnabled: Bool
  @State private var lifeSupportEnabled: Bool
  @State private var artificialGravityEnabled: Bool
  @State private var tractorBeamEnabled: Bool
  
  @Environment(\.controlPanelStyle) private var controlPanelStyle
  @Environment(\.toggleButtonStyle) private var toggleButtonStyle
}

If stored property attributes were written on the previous line (like other types of attributes), then the properties start to visually bleed together unless you add blank lines between them:

struct SpaceshipDashboardView {
  @State
  private var warpDriveEnabled: Bool
  @State
  private var lifeSupportEnabled: Bool
  @State
  private var artificialGravityEnabled: Bool
  @State
  private var tractorBeamEnabled: Bool
  
  @Environment(\.controlPanelStyle)
  private var controlPanelStyle
  @Environment(\.toggleButtonStyle)
  private var toggleButtonStyle
}

If you add blank lines, the list of properties becomes much longer and you lose the ability to group related properties together:

struct SpaceshipDashboardView {
  @State
  private var warpDriveEnabled: Bool
  
  @State
  private var lifeSupportEnabled: Bool
  
  @State
  private var artificialGravityEnabled: Bool
  
  @State
  private var tractorBeamEnabled: Bool
  
  @Environment(\.controlPanelStyle)
  private var controlPanelStyle
  
  @Environment(\.toggleButtonStyle)
  private var toggleButtonStyle
}

This doesn't apply to complex attributes with named arguments, or multiple unnamed arguments. These arguments are visually complex and typically quite long, so feel cramped and unnecessarily long when written on a single line:

// Despite being less than 100 characters long, these lines are very complex and feel unnecessarily long: 
@available(*, unavailable, message: "No longer in production") var saturn5Builder: Saturn5Builder
@available(*, deprecated, message: "To be retired by 2030") var atlas5Builder: Atlas5Builder
@available(*, iOS 17.0, tvOS 17.0, macOS 14.0, watchOS 10.0) var newRocketBuilder: NewRocketBuilder

[calda]

Will need to update this again after merging nicklockwood/SwiftFormat#1595

[bachand]

Great work @calda . I love where we landed!

[bachand]

👍

[bachand]

Suggested change

	@available(*, iOS 17.0, tvOS 17.0, macOS 14.0, watchOS 10.0) var newRocketBuilder: NewRocketBuilder
	@available(*, iOS 17.0, tvOS 17.0, macOS 14.0, watchOS 10.0) var newShepardBuilder: NewShepardBuilder

Just an idea: https://www.blueorigin.com/new-shepard#:~:text=Named%20after%20Mercury%20astronaut%20Alan,internationally%20recognized%20boundary%20of%20space.

[bachand]

If we made this change we should update elsewhere.

[calda]

Good suggestion! I went with New Glenn (also by Blue Origin) instead, since it's 8-9 years newer than the New Shepard. https://en.wikipedia.org/wiki/New_Glenn

[bachand]

In some other rules we put the "why?" before the examples but I am fine moving it after the examples here.

[bachand]

I get a build error locally if I don't fully qualify the key path but I'm OK keeping what we have as-is since it matches Apple documentation. There could be something that I'm missing in my code example.

[bachand]

With indentation this line is 101 characters. This is in conflict with the comment above.

[calda]

The indentation isn't part of the code block, since the start of the code block is itself indented by two chars. After updating this to newGlennBuilder it's also two chars shorter than before, so will be under 100 no matter how you count it.