Merge pull request #36045 from xedin/handle-non-ambiguous-fixes-better

[CSFix] Handle some cases where the same fix appears in multiple solutions

Author: xedin

include/swift/Sema/CSFix.h

@@ -478,6 +478,8 @@ class MissingConformance final : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;
+
   static MissingConformance *forRequirement(ConstraintSystem &cs, Type type,
                                             Type protocolType,
                                             ConstraintLocator *locator);
@@ -489,6 +491,12 @@ class MissingConformance final : public ConstraintFix {
   Type getNonConformingType() { return NonConformingType; }
 
   Type getProtocolType() { return ProtocolType; }
+
+  bool isEqual(const ConstraintFix *other) const;
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AddConformance;
+  }
 };
 
 /// Skip same-type generic requirement constraint,
@@ -1386,11 +1394,19 @@ class MoveOutOfOrderArgument final : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;
+
+  bool isEqual(const ConstraintFix *other) const;
+
   static MoveOutOfOrderArgument *create(ConstraintSystem &cs,
                                         unsigned argIdx,
                                         unsigned prevArgIdx,
                                         ArrayRef<ParamBinding> bindings,
                                         ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::MoveOutOfOrderArgument;
+  }
 };
 
 class AllowInaccessibleMember final : public AllowInvalidMemberRef {
@@ -1512,10 +1528,18 @@ class AllowInvalidRefInKeyPath final : public ConstraintFix {
 
   bool diagnose(const Solution &solution, bool asNote = false) const override;
 
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override;
+
   /// Determine whether give reference requires a fix and produce one.
   static AllowInvalidRefInKeyPath *
   forRef(ConstraintSystem &cs, ValueDecl *member, ConstraintLocator *locator);
 
+  bool isEqual(const ConstraintFix *other) const;
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowInvalidRefInKeyPath;
+  }
+
 private:
   static AllowInvalidRefInKeyPath *create(ConstraintSystem &cs, RefKind kind,
                                           ValueDecl *member,

lib/Sema/CSFix.cpp

@@ -248,6 +248,34 @@ bool MissingConformance::diagnose(const Solution &solution, bool asNote) const {
   return failure.diagnose(asNote);
 }
 
+bool MissingConformance::diagnoseForAmbiguity(
+    CommonFixesArray commonFixes) const {
+  auto *primaryFix = commonFixes.front().second->getAs<MissingConformance>();
+  assert(primaryFix);
+
+  if (llvm::all_of(
+          commonFixes,
+          [&primaryFix](
+              const std::pair<const Solution *, const ConstraintFix *> &entry) {
+            return primaryFix->isEqual(entry.second);
+          }))
+    return diagnose(*commonFixes.front().first);
+
+  // If the location is the same but there are different requirements
+  // involved let's not attempt to diagnose that as an ambiguity.
+  return false;
+}
+
+bool MissingConformance::isEqual(const ConstraintFix *other) const {
+  auto *conformanceFix = other->getAs<MissingConformance>();
+  if (!conformanceFix)
+    return false;
+
+  return IsContextual == conformanceFix->IsContextual &&
+         NonConformingType->isEqual(conformanceFix->NonConformingType) &&
+         ProtocolType->isEqual(conformanceFix->ProtocolType);
+}
+
 MissingConformance *
 MissingConformance::forContextual(ConstraintSystem &cs, Type type,
                                   Type protocolType,
@@ -837,6 +865,29 @@ bool MoveOutOfOrderArgument::diagnose(const Solution &solution,
   return failure.diagnose(asNote);
 }
 
+bool MoveOutOfOrderArgument::diagnoseForAmbiguity(
+    CommonFixesArray commonFixes) const {
+  auto *primaryFix =
+      commonFixes.front().second->getAs<MoveOutOfOrderArgument>();
+  assert(primaryFix);
+
+  if (llvm::all_of(
+          commonFixes,
+          [&primaryFix](
+              const std::pair<const Solution *, const ConstraintFix *> &entry) {
+            return primaryFix->isEqual(entry.second);
+          }))
+    return diagnose(*commonFixes.front().first);
+
+  return false;
+}
+
+bool MoveOutOfOrderArgument::isEqual(const ConstraintFix *other) const {
+  auto OoOFix = other->getAs<MoveOutOfOrderArgument>();
+  return OoOFix ? ArgIdx == OoOFix->ArgIdx && PrevArgIdx == OoOFix->PrevArgIdx
+                : false;
+}
+
 MoveOutOfOrderArgument *MoveOutOfOrderArgument::create(
     ConstraintSystem &cs, unsigned argIdx, unsigned prevArgIdx,
     ArrayRef<ParamBinding> bindings, ConstraintLocator *locator) {
@@ -925,6 +976,29 @@ bool AllowInvalidRefInKeyPath::diagnose(const Solution &solution,
   llvm_unreachable("covered switch");
 }
 
+bool AllowInvalidRefInKeyPath::diagnoseForAmbiguity(
+    CommonFixesArray commonFixes) const {
+  auto *primaryFix =
+      commonFixes.front().second->getAs<AllowInvalidRefInKeyPath>();
+  assert(primaryFix);
+
+  if (llvm::all_of(
+          commonFixes,
+          [&primaryFix](
+              const std::pair<const Solution *, const ConstraintFix *> &entry) {
+            return primaryFix->isEqual(entry.second);
+          })) {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  return false;
+}
+
+bool AllowInvalidRefInKeyPath::isEqual(const ConstraintFix *other) const {
+  auto *refFix = other->getAs<AllowInvalidRefInKeyPath>();
+  return refFix ? Kind == refFix->Kind && Member == refFix->Member : false;
+}
+
 AllowInvalidRefInKeyPath *
 AllowInvalidRefInKeyPath::forRef(ConstraintSystem &cs, ValueDecl *member,
                                  ConstraintLocator *locator) {

validation-test/Sema/SwiftUI/rdar74447308.swift

@@ -0,0 +1,28 @@
+// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
+
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+import SwiftUI
+
+struct Item {
+  let id = UUID()
+  let text = ""
+}
+
+class ItemList: ObservableObject {
+  @Published var items = [Item]()
+}
+
+struct ContentView: View {
+  @StateObject var list = ItemList()
+
+  var body: some View {
+    List {
+      ForEach(list.items) { item in
+      // expected-error@-1 {{referencing initializer 'init(_:content:)' on 'ForEach' requires that 'Item' conform to 'Identifiable'}}
+        Text(item.text)
+      }
+    }
+  }
+}

validation-test/Sema/SwiftUI/sr14093.swift

@@ -0,0 +1,20 @@
+// RUN: %target-typecheck-verify-swift -target x86_64-apple-macosx10.15 -swift-version 5
+
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+import SwiftUI
+import Foundation
+
+struct ContentView: View {
+  @State private var date = Date()
+
+  var body: some View {
+    Group {
+      DatePicker("Enter a date", selection: $date, displayedComponents: .date, in: Date())
+      // expected-error@-1 {{argument 'in' must precede argument 'displayedComponents'}} {{78-90=}} {{52-52=in: Date(), }}
+      DatePicker("Enter a date", selection: $date, displayedComponents: .date, in: Date() ... Date().addingTimeInterval(100))
+      // expected-error@-1 {{argument 'in' must precede argument 'displayedComponents'}} {{78-125=}} {{52-52=in: Date() ... Date().addingTimeInterval(100), }}
+    }
+  }
+}