Implement never use implicitly unwrapped optionals (swiftlang#58)

Author: LaurenWhite

Diff for: tools/swift-format/Sources/Core/Context.swift

@@ -15,11 +15,19 @@ public class Context {
 
   /// The URL of the file being linted or formatted.
   public let fileURL: URL
+  
+  /// Indicates whether the file imports XCTest, and is test code
+  public var importsXCTest: Bool
+  
+  /// Indicates whether the visitor has already determined a value for importsXCTest
+  public var didSetImportsXCTest: Bool
 
   /// Creates a new Context with the provided configuration, diagnostic engine, and file URL.
   public init(configuration: Configuration, diagnosticEngine: DiagnosticEngine?, fileURL: URL) {
     self.configuration = configuration
     self.diagnosticEngine = diagnosticEngine
     self.fileURL = fileURL
+    self.importsXCTest = false
+    self.didSetImportsXCTest = false
   }
 }

Diff for: tools/swift-format/Sources/Rules/ImportsXCTestVisitor.swift

@@ -0,0 +1,39 @@
+import Core
+import Foundation
+import SwiftSyntax
+
+/// Visitor that determines if the target source file imports XCTest
+private final class ImportsXCTestVisitor: SyntaxVisitor {
+  let context: Context
+  
+  init(context: Context) {
+    self.context = context
+  }
+
+  override func visit(_ node: SourceFileSyntax) {
+    for statement in node.statements {
+      guard let importDecl = statement.item as? ImportDeclSyntax else { continue }
+      for component in importDecl.path {
+        guard component.name.text == "XCTest" else { continue }
+        context.importsXCTest = true
+        context.didSetImportsXCTest = true
+        return
+      }
+    }
+    context.didSetImportsXCTest = true
+  }
+}
+
+/// Sets the appropriate value of the importsXCTest field in the Context class, which
+/// indicates whether the file contains test code or not.
+///
+/// This setter will only run the visitor if another rule hasn't already called this function to
+/// determine if the source file imports XCTest.
+///
+/// - Parameters:
+///   - context: The context information of the target source file.
+///   - sourceFile: The file to be visited.
+func setImportsXCTest(context: Context, sourceFile: SourceFileSyntax) {
+  guard !context.didSetImportsXCTest else { return }
+  ImportsXCTestVisitor(context: context).visit(sourceFile)
+}

Diff for: tools/swift-format/Sources/Rules/NeverUseImplicitlyUnwrappedOptionals.swift

@@ -6,13 +6,45 @@ import SwiftSyntax
 ///
 /// Certain properties (e.g. `@IBOutlet`) tied to the UI lifecycle are ignored.
 ///
-/// This rule does not apply to test code, defined as code which matches one or more of:
-///   * Parent directory named "Tests"
+/// This rule does not apply to test code, defined as code which:
 ///   * Contains the line `import XCTest`
 ///
+/// TODO: Create exceptions for other UI elements (ex: viewDidLoad)
+///
 /// Lint: Declaring a property with an implicitly unwrapped type yields a lint error.
 ///
 /// - SeeAlso: https://google.github.io/swift#implicitly-unwrapped-optionals
 public final class NeverUseImplicitlyUnwrappedOptionals: SyntaxLintRule {
+  
+  // Checks if "XCTest" is an import statement
+  public override func visit(_ node: SourceFileSyntax) {
+    setImportsXCTest(context: context, sourceFile: node)
+    super.visit(node)
+  }
+
+  public override func visit(_ node: VariableDeclSyntax) {
+    guard !context.importsXCTest else { return }
+    // Ignores IBOutlet variables
+    if let attributes = node.attributes {
+      for attribute in attributes {
+        if attribute.attributeName.text == "IBOutlet" { return }
+      }
+    }
+    // Finds type annotation for variable(s)
+    for binding in node.bindings {
+      guard let nodeTypeAnnotation = binding.typeAnnotation else { continue }
+      diagnoseImplicitWrapViolation(nodeTypeAnnotation.type)
+    }
+  }
+
+  func diagnoseImplicitWrapViolation(_ type: TypeSyntax) {
+    guard let violation = type as? ImplicitlyUnwrappedOptionalTypeSyntax else { return }
+    diagnose(.doNotUseImplicitUnwrapping(identifier: "\(violation.wrappedType)"), on: type)
+  }
+}
 
+extension Diagnostic.Message {
+  static func doNotUseImplicitUnwrapping(identifier: String) -> Diagnostic.Message {
+    return .init(.warning, "use \(identifier) or \(identifier)? instead of \(identifier)!")
+  }
 }

Diff for: tools/swift-format/Tests/SwiftFormatTests/NeverUseImplicitlyUnwrappedOptionalsTests.swift

@@ -0,0 +1,43 @@
+import Foundation
+import SwiftSyntax
+import XCTest
+
+@testable import Rules
+
+public class NeverUseImplicitlyUnwrappedOptionalsTests: DiagnosingTestCase {
+  public func testInvalidVariableUnwrapping() {
+    let input =
+      """
+      import Core
+      import Foundation
+      import SwiftSyntax
+
+      var foo: Int?
+      var s: String!
+      var c, d, e: Float
+      @IBOutlet var button: UIButton!
+      """
+    performLint(NeverUseImplicitlyUnwrappedOptionals.self, input: input)
+    XCTAssertNotDiagnosed(.doNotUseImplicitUnwrapping(identifier: "Int"))
+    XCTAssertDiagnosed(.doNotUseImplicitUnwrapping(identifier: "String"))
+    XCTAssertNotDiagnosed(.doNotUseImplicitUnwrapping(identifier: "Float"))
+    XCTAssertNotDiagnosed(.doNotUseImplicitUnwrapping(identifier: "UIButton"))
+  }
+  public func testIgnoreTestCode() {
+    let input =
+      """
+      import XCTest
+
+      var s: String!
+      """
+    performLint(NeverUseImplicitlyUnwrappedOptionals.self, input: input)
+    XCTAssertNotDiagnosed(.doNotUseImplicitUnwrapping(identifier: "String"))
+  }
+
+#if !os(macOS)
+  static let allTests = [
+    NeverUseImplicitlyUnwrappedOptionalsTests.testInvalidVariableUnwrapping,
+  ]
+#endif
+
+}