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 infer property types from the right-hand-side value rather than writing the type explicitly on the left-hand side #263

Merged
merged 7 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ let package = Package(

.binaryTarget(
name: "SwiftFormat",
url: "https://github.com/calda/SwiftFormat/releases/download/0.54-beta-5/SwiftFormat.artifactbundle.zip",
checksum: "7447986db45a51164d23672c07f971406a4c0589b0c423fcb85e95ed8f8e7e48"),
url: "https://github.com/calda/SwiftFormat/releases/download/0.54-beta-7/SwiftFormat.artifactbundle.zip",
checksum: "0cf117050e7838f545009bfe4a75dbda98cff737cb847a7d065a89683e9e890a"),

.binaryTarget(
name: "SwiftLintBinary",
Expand Down
120 changes: 110 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,25 +333,125 @@ _You can enable the following settings in Xcode by running [this script](resourc

```swift
// WRONG
let host: Host = Host()
let sun: Star = Star(mass: 1.989e30)
Copy link
Member Author

Choose a reason for hiding this comment

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

While I was here I expanded the list of examples used by the "Don't include types where they can be easily inferred." rule. These updates reflect the current behavior of the SwiftFormat redundantType rule.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

let earth: Planet = Planet.earth

// RIGHT
let host = Host()
let sun = Star(mass: 1.989e30)
let earth = Planet.earth

// NOT RECOMMENDED. However, since the linter doesn't have full type information, this is not enforced automatically.
let moon: Moon = earth.moon // returns `Moon`

// RIGHT
let moon = earth.moon
let moon: PlanetaryBody? = earth.moon

// WRONG: Most literals provide a default type that can be inferred.
let enableGravity: Bool = true
let numberOfPlanets: Int = 8
let sunMass: Double = 1.989e30

// RIGHT
let enableGravity = true
let numberOfPlanets = 8
let sunMass = 1.989e30

// WRONG: Types can be inferred from if/switch expressions as well if each branch has the same explicit type.
let smallestPlanet: Planet =
if treatPlutoAsPlanet {
Planet.pluto
} else {
Planet.mercury
}

// RIGHT
let smallestPlanet =
if treatPlutoAsPlanet {
Planet.pluto
} else {
Planet.mercury
}
```

</details>

* <a id='infer-property-types'></a>(<a href='#infer-property-types'>link</a>) **Prefer letting the type of a variable or property be inferred from the right-hand-side value rather than writing the type explicitly on the left-hand side.** [![SwiftFormat: preferInferredTypes](https://img.shields.io/badge/SwiftFormat-preferInferredTypes-7B0051.svg)](https://github.com/nicklockwood/SwiftFormat/blob/master/Rules.md#preferInferredTypes)

<details>

Prefer using inferred types when the right-hand-side value is a static member with a leading dot (e.g. an `init`, a `static` property / function, or an enum case). This applies to both local variables and property declarations:

```swift
enum Direction {
case left
case right
// WRONG
struct SolarSystemBuilder {
let sun: Star = .init(mass: 1.989e30)
let earth: Planet = .earth

func setUp() {
let galaxy: Galaxy = .andromeda
let system: SolarSystem = .init(sun, earth)
galaxy.add(system)
}
}

// RIGHT
struct SolarSystemBuilder {
let sun = Star(mass: 1.989e30)
let earth = Planet.earth

func someDirection() -> Direction {
// WRONG
return Direction.left
func setUp() {
let galaxy = Galaxy.andromeda
let system = SolarSystem(sun, earth)
galaxy.add(system)
}
}
```

// RIGHT
return .left
Copy link
Member Author

@calda calda Mar 13, 2024

Choose a reason for hiding this comment

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

This is a nice example and is good advice in general, but doesn't reflect the behavior of the SwiftFormat redundantType rule that we use. I don't believe we have autocorrect support for this specific example.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added an asana task to our internal board for us to look at adding autocorrect for this sort of case

Explicit types are still permitted in other cases:

```swift
// RIGHT: There is no right-hand-side value, so an explicit type is required.
let sun: Star

// RIGHT: The right-hand-side is not a static member of the left-hand type.
let moon: PlantaryBody = earth.moon
let sunMass: Float = 1.989e30
let planets: [Planet] = []
let venusMoon: Moon? = nil
```

There are some rare cases where the inferred type syntax has a different meaning than the explicit type syntax. In these cases, the explicit type syntax is still permitted:

```swift
extension String {
static let earth = "Earth"
}

// WRONG: fails with "error: type 'String?' has no member 'earth'"
let planetName = String?.earth

// RIGHT
let planetName: String? = .earth
```

```swift
struct SaturnOutline: ShapeStyle { ... }

extension ShapeStyle where Self == SaturnOutline {
static var saturnOutline: SaturnOutline {
SaturnOutline()
}
}

// WRONG: fails with "error: static member 'saturnOutline' cannot be used on protocol metatype '(any ShapeStyle).Type'"
let myShape2 = (any ShapeStyle).myShape

// RIGHT: If the property's type is an existential / protocol type, moving the type
// to the right-hand side will result in invalid code if the value is defined in an
// extension like `extension ShapeStyle where Self == SaturnOutline`.
// SwiftFormat autocorrect detects this case by checking for the existential `any` keyword.
let myShape1: any ShapeStyle = .saturnOutline
Copy link
Member Author

@calda calda Jun 8, 2024

Choose a reason for hiding this comment

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

I spent a bit of time thinking about this case in more detail, that let myShape1: ShapeStyle = .saturnOutline is allowed but let myShape1: = ShapeStyle.saturnOutline isn't. I couldn't understand why this wasn't allowed.

This syntax was introduced in SE-0299. This question came up briefly in the review thread, and Pavel mentioned that there was more discussion in the pitch thread. A found the most compelling discussion here in the pitch thread.

They intentionally don't frame this as adding a static member to the protocol type, but rather a shorthand syntax that lets you omit the name of the concrete type.

Basically, the thinking is that this code:

let myShape1: ShapeStyle = .saturnOutline

is actually implicit shorthand for this code:

let myShape1: ShapeStyle = SaturnOutline.saturnOutline

not shorthand for this code:

let myShape1: ShapeStyle = ShapeStyle.saturnOutline

This is subtle but distinct, and is a reasonable explanation for why ShapeStyle.saturnOutline isn't allowed.

Copy link
Member Author

@calda calda Jun 8, 2024

Choose a reason for hiding this comment

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

This problem should also go away eventually, once any is required in a future language mode (just not Swift 6).

```

</details>
Expand Down
17 changes: 9 additions & 8 deletions Sources/AirbnbSwiftFormatTool/airbnb.swiftformat
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@
--organizetypes class,struct,enum,extension,actor # organizeDeclarations
--extensionacl on-declarations # extensionAccessControl
--patternlet inline # hoistPatternLet
--redundanttype inferred # redundantType
--redundanttype inferred # redundantType, propertyType
--typeblanklines preserve # blankLinesAtStartOfScope, blankLinesAtEndOfScope
--emptybraces spaced # emptyBraces
--someAny disabled # opaqueGenericParameters
--elseposition same-line #elseOnSameLine
--guardelse next-line #elseOnSameLine
--onelineforeach convert #preferForLoop
--shortoptionals always #typeSugar
--semicolons never #semicolons
--doccomments preserve #docComments
--elseposition same-line # elseOnSameLine
--guardelse next-line # elseOnSameLine
--onelineforeach convert # preferForLoop
--shortoptionals always # typeSugar
--semicolons never # semicolons
--doccomments preserve # docComments

# We recommend a max width of 100 but _strictly enforce_ a max width of 130
--maxwidth 130 # wrap
Expand Down Expand Up @@ -102,4 +102,5 @@
--rules wrapMultilineConditionalAssignment
--rules blankLineAfterMultilineSwitchCase
--rules consistentSwitchStatementSpacing
--rules semicolons
--rules semicolons
--rules propertyType
Loading