-
Notifications
You must be signed in to change notification settings - Fork 304
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
base: master
Are you sure you want to change the base?
Changes from all commits
d343fb2
3750e2c
5a39cdc
39f921f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @calda how do we handle/guide on the case where you want to tell the language that a literal should be interpreted in a special way? For example, one way to get a floating-point value of 100 is to write let myFloatingPointValue: Double = 100 which will produce a different result than let myFloatingPointValue = 100 // implicitly of type `Int` In this example, another option is to write let myFloatingPointValue = 100.0 // implicitly of type `Double` but even then it's necessary to write an explicit type in some cases, like if you explicitly wanted a let myFloatingPointValue: Float = 100 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am curious how our autocorrection handles this case where removing the type can change the meaning of the code, and I also think that situations like this may warrant some discussion in the prose of this rule. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my testing I see that the linter is smart enough to correct There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw I also saw as I continued to review that this is covered in more detail in the next rule, though I wonder if we should move some of the content from the next rule up to this one since this rule is the one focused on removing types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I implemented this back in nicklockwood/SwiftFormat#921! Here's the code: https://github.com/nicklockwood/SwiftFormat/pull/921/files#diff-1f5cbb52e333eb61c8d6fef2bd765e3e8e6fa8f3101f91f98a9dfce9e1ef480dR1363 It special-cases support for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Good idea. I think the next rule probably goes in to too much detail about literals, and that content would be a better fit for the discussion about the |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
``` | ||
|
||
</details> | ||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!