Don't use shorthand syntax for Optional<T> if all of the following are true:

*   It is the type of a variable (file-, function-, or type-scoped)
*   It is mutable (`var`, not `let`)
*   It is not a computed property
*   It does not have an initializer expression

If all of these are true, shortening it to `T?` would introduce a useless
nil-initialization that can be problematic in situations that want to
optimize performance (see https://forums.swift.org/t/what-is-a-variable-initialization-expression/52132).

Fixes #304.

Author: allevato

Sources/SwiftFormatRules/UseShorthandTypeNames.swift

@@ -68,6 +68,10 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
         trailingTrivia: trailingTrivia)
 
     case "Optional":
+      guard !isTypeOfUninitializedStoredVar(node) else {
+        newNode = nil
+        break
+      }
       guard let typeArgument = genericArgumentList.firstAndOnly else {
         newNode = nil
         break
@@ -496,6 +500,72 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
       trailingTrivia: node.lastToken?.trailingTrivia ?? []
     )
   }
+
+  /// Returns true if the given pattern binding represents a stored property/variable (as opposed to
+  /// a computed property/variable).
+  private func isStoredProperty(_ node: PatternBindingSyntax) -> Bool {
+    guard let accessor = node.accessor else {
+      // If it has no accessors at all, it is definitely a stored property.
+      return true
+    }
+
+    guard let accessorBlock = accessor.as(AccessorBlockSyntax.self) else {
+      // If the accessor isn't an `AccessorBlockSyntax`, then it is a `CodeBlockSyntax`; i.e., the
+      // accessor an implicit `get`. So, it is definitely not a stored property.
+      assert(accessor.is(CodeBlockSyntax.self))
+      return false
+    }
+
+    for accessorDecl in accessorBlock.accessors {
+      // Look for accessors that indicate that this is a computed property. If none are found, then
+      // it is a stored property (e.g., having only observers like `willSet/didSet`).
+      switch accessorDecl.accessorKind.tokenKind {
+      case .contextualKeyword("get"),
+        .contextualKeyword("set"),
+        .contextualKeyword("unsafeAddress"),
+        .contextualKeyword("unsafeMutableAddress"),
+        .contextualKeyword("_read"),
+        .contextualKeyword("_modify"):
+        return false
+      default:
+        return true
+      }
+    }
+
+    // This should be unreachable.
+    assertionFailure("Should not have an AccessorBlock with no AccessorDecls")
+    return false
+  }
+
+  /// Returns true if the given type identifier node represents the type of a mutable variable or
+  /// stored property that does not have an initializer clause.
+  private func isTypeOfUninitializedStoredVar(_ node: SimpleTypeIdentifierSyntax) -> Bool {
+    if let typeAnnotation = node.parent?.as(TypeAnnotationSyntax.self),
+      let patternBinding = nearestAncestor(of: typeAnnotation, type: PatternBindingSyntax.self),
+      isStoredProperty(patternBinding),
+      patternBinding.initializer == nil,
+      let variableDecl = nearestAncestor(of: patternBinding, type: VariableDeclSyntax.self),
+      variableDecl.letOrVarKeyword.tokenKind == .varKeyword
+    {
+      return true
+    }
+    return false
+  }
+
+  /// Returns the node's nearest ancestor of the given type, if found; otherwise, returns nil.
+  private func nearestAncestor<NodeType: SyntaxProtocol, AncestorType: SyntaxProtocol>(
+    of node: NodeType,
+    type: AncestorType.Type = AncestorType.self
+  ) -> AncestorType? {
+    var parent: Syntax? = Syntax(node)
+    while let existingParent = parent {
+      if existingParent.is(type) {
+        return existingParent.as(type)
+      }
+      parent = existingParent.parent
+    }
+    return nil
+  }
 }
 
 extension Finding.Message {

Tests/SwiftFormatRulesTests/UseShorthandTypeNamesTests.swift

@@ -6,15 +6,15 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
       UseShorthandTypeNames.self,
       input:
         """
-        var a: Array<Int>
-        var b: Dictionary<String, Int>
-        var c: Optional<Foo>
+        var a: Array<Int> = []
+        var b: Dictionary<String, Int> = [:]
+        var c: Optional<Foo> = nil
         """,
       expected:
         """
-        var a: [Int]
-        var b: [String: Int]
-        var c: Foo?
+        var a: [Int] = []
+        var b: [String: Int] = [:]
+        var c: Foo? = nil
         """)
 
     XCTAssertDiagnosed(.useTypeShorthand(type: "Array"))
@@ -41,13 +41,13 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
         var h: [String: Dictionary<String, Int>]
         var i: [Dictionary<String, Int>: Dictionary<String, Int>]
 
-        var a: Optional<Array<Int>>
-        var b: Optional<Dictionary<String, Int>>
-        var c: Optional<Optional<Int>>
-        var d: Array<Int>?
-        var e: Dictionary<String, Int>?
-        var f: Optional<Int>?
-        var g: Optional<Int?>
+        let a: Optional<Array<Int>>
+        let b: Optional<Dictionary<String, Int>>
+        let c: Optional<Optional<Int>>
+        let d: Array<Int>?
+        let e: Dictionary<String, Int>?
+        let f: Optional<Int>?
+        let g: Optional<Int?>
 
         var a: Array<Optional<Int>>
         var b: Dictionary<Optional<String>, Optional<Int>>
@@ -70,13 +70,13 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
         var h: [String: [String: Int]]
         var i: [[String: Int]: [String: Int]]
 
-        var a: [Int]?
-        var b: [String: Int]?
-        var c: Int??
-        var d: [Int]?
-        var e: [String: Int]?
-        var f: Int??
-        var g: Int??
+        let a: [Int]?
+        let b: [String: Int]?
+        let c: Int??
+        let d: [Int]?
+        let e: [String: Int]?
+        let f: Int??
+        let g: Int??
 
         var a: [Int?]
         var b: [String?: Int?]
@@ -323,4 +323,94 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
         var c: Sectional<Couch> = Sectional<Couch>(from: warehouse)
         """)
   }
+
+  func testOptionalStoredVarsWithoutInitializersAreNotChangedUnlessImmutable() {
+    XCTAssertFormatting(
+      UseShorthandTypeNames.self,
+      input:
+        """
+        var a: Optional<Int>
+        var b: Optional<Int> {
+          didSet {}
+        }
+        let c: Optional<Int>
+        """,
+      expected:
+        """
+        var a: Optional<Int>
+        var b: Optional<Int> {
+          didSet {}
+        }
+        let c: Int?
+        """)
+  }
+
+  func testOptionalComputedVarsAreChanged() {
+    XCTAssertFormatting(
+      UseShorthandTypeNames.self,
+      input:
+        """
+        var a: Optional<Int> { nil }
+        var b: Optional<Int> {
+          get { 0 }
+        }
+        var c: Optional<Int> {
+          _read {}
+        }
+        var d: Optional<Int> {
+          unsafeAddress {}
+        }
+        """,
+      expected:
+        """
+        var a: Int? { nil }
+        var b: Int? {
+          get { 0 }
+        }
+        var c: Int? {
+          _read {}
+        }
+        var d: Int? {
+          unsafeAddress {}
+        }
+        """)
+  }
+
+  func testOptionalStoredVarsWithInitializersAreChanged() {
+    XCTAssertFormatting(
+      UseShorthandTypeNames.self,
+      input:
+        """
+        var a: Optional<Int> = nil
+        var b: Optional<Int> = nil {
+          didSet {}
+        }
+        let c: Optional<Int> = nil
+        """,
+      expected:
+        """
+        var a: Int? = nil
+        var b: Int? = nil {
+          didSet {}
+        }
+        let c: Int? = nil
+        """)
+  }
+
+  func testOptionalsNestedInOtherTypseInStoredVarsAreStillChanged() {
+    XCTAssertFormatting(
+      UseShorthandTypeNames.self,
+      input:
+        """
+        var c: Generic<Optional<Int>>
+        var d: [Optional<Int>]
+        var e: [String: Optional<Int>]
+        """,
+      expected:
+        """
+        var c: Generic<Int?>
+        var d: [Int?]
+        var e: [String: Int?]
+        """)
+  }
 }