Merge pull request #17350 from rudkx/diagnose-deprecated-writable-keypath

[MiscDiagnostics] Emit a deprecation warning for some writes through …

Author: rudkx

include/swift/AST/DiagnosticsSema.def

@@ -2,7 +2,7 @@
 //
 // This source file is part of the Swift.org open source project
 //
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
 // Licensed under Apache License v2.0 with Runtime Library Exception
 //
 // See https://swift.org/LICENSE.txt for license information
@@ -495,6 +495,9 @@ ERROR(expr_keypath_subscript_index_not_hashable, none,
 ERROR(expr_smart_keypath_application_type_mismatch,none,
       "key path of type %0 cannot be applied to a base of type %1",
       (Type, Type))
+WARNING(expr_deprecated_writable_keypath,none,
+        "forming a writable keypath to property %0 that is read-only in this context "
+        "is deprecated and will be removed in a future release",(DeclName))
 
 // Selector expressions.
 ERROR(expr_selector_no_objc_runtime,none,

lib/Sema/MiscDiagnostics.cpp

@@ -3927,6 +3927,62 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
   const_cast<Expr *>(E)->walk(Walker);
 }
 
+static void diagnoseDeprecatedWritableKeyPath(TypeChecker &TC, const Expr *E,
+                                              const DeclContext *DC) {
+  if (!E || isa<ErrorExpr>(E) || !E->getType())
+    return;
+
+  class DeprecatedWritableKeyPathWalker : public ASTWalker {
+    TypeChecker &TC;
+    const DeclContext *DC;
+
+    void visitKeyPathApplicationExpr(KeyPathApplicationExpr *E) {
+      if (E->hasLValueAccessKind() &&
+          E->getLValueAccessKind() == AccessKind::Read)
+        return;
+
+      if (auto *keyPathExpr = dyn_cast<KeyPathExpr>(E->getKeyPath())) {
+        auto *decl = keyPathExpr->getType()->getNominalOrBoundGenericNominal();
+        if (decl != TC.Context.getWritableKeyPathDecl() &&
+            decl != TC.Context.getReferenceWritableKeyPathDecl())
+          return;
+
+        assert(keyPathExpr->getComponents().size() > 0);
+        auto &component = keyPathExpr->getComponents().back();
+        if (component.getKind() == KeyPathExpr::Component::Kind::Property) {
+          auto *storage =
+            cast<AbstractStorageDecl>(component.getDeclRef().getDecl());
+          if (!storage->isSettable(nullptr) ||
+              !storage->isSetterAccessibleFrom(DC)) {
+            TC.diagnose(keyPathExpr->getLoc(),
+                        swift::diag::expr_deprecated_writable_keypath,
+                        storage->getFullName());
+          }
+        }
+      }
+    }
+
+    std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
+      if (!E || isa<ErrorExpr>(E) || !E->getType())
+        return {false, E};
+
+      if (auto *KPAE = dyn_cast<KeyPathApplicationExpr>(E)) {
+        visitKeyPathApplicationExpr(KPAE);
+        return {true, E};
+      }
+
+      return {true, E};
+    }
+
+  public:
+    DeprecatedWritableKeyPathWalker(TypeChecker &TC, const DeclContext *DC)
+        : TC(TC), DC(DC) {}
+  };
+
+  DeprecatedWritableKeyPathWalker Walker(TC, DC);
+  const_cast<Expr *>(E)->walk(Walker);
+}
+
 //===----------------------------------------------------------------------===//
 // High-level entry points.
 //===----------------------------------------------------------------------===//
@@ -3940,6 +3996,8 @@ void swift::performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,
   diagRecursivePropertyAccess(TC, E, DC);
   diagnoseImplicitSelfUseInClosure(TC, E, DC);
   diagnoseUnintendedOptionalBehavior(TC, E, DC);
+  if (!TC.Context.isSwiftVersionAtLeast(5))
+    diagnoseDeprecatedWritableKeyPath(TC, E, DC);
   if (!TC.getLangOpts().DisableAvailabilityChecking)
     diagAvailability(TC, E, const_cast<DeclContext*>(DC));
   if (TC.Context.LangOpts.EnableObjCInterop)

test/Constraints/keypath.swift

@@ -5,9 +5,17 @@ struct S {
 
   init() {
     let _: WritableKeyPath<S, Int> = \.i // no error for Swift 3/4
+
+    S()[keyPath: \.i] = 1
+    // expected-error@-1 {{cannot assign to immutable expression}}
   }
 }
 
 func test() {
   let _: WritableKeyPath<C, Int> = \.i // no error for Swift 3/4
+
+  C()[keyPath: \.i] = 1   // warning on write with literal keypath
+  // expected-warning@-1 {{forming a writable keypath to property}}
+
+  let _ = C()[keyPath: \.i] // no warning for a read
 }

test/Constraints/keypath_swift_5.swift

@@ -5,9 +5,17 @@ struct S {
 
   init() {
     let _: WritableKeyPath<S, Int> = \.i // expected-error {{type of expression is ambiguous without more context}}
+
+    S()[keyPath: \.i] = 1
+    // expected-error@-1 {{cannot assign to immutable expression}}
   }
 }
 
 func test() {
   let _: WritableKeyPath<C, Int> = \.i // expected-error {{type of expression is ambiguous without more context}}
+
+  C()[keyPath: \.i] = 1
+  // expected-error@-1 {{cannot assign to immutable expression}}
+
+  let _ = C()[keyPath: \.i] // no warning for a read
 }