Warn about derived Hashable implementation if there's a custom Equatable

Author: JGiola

include/swift/AST/DiagnosticsSema.def

@@ -5964,6 +5964,11 @@ ERROR(override_nsobject_hash_error,none,
       "'NSObject.hash(into:)' is not overridable; "
       "did you mean to override 'NSObject.hash'?", ())
 
+WARNING(synthesized_hashable_may_not_match_custom_equatable, none,
+         "automatically generated 'Hashable' implementation for type %0 "
+         "may not match the behavior of custom '==' operator", (Type))
+NOTE(add_custom_hashable, none, "add a custom 'hash(into:)' method", ())
+
 WARNING(hashvalue_implementation,Deprecation,
         "'Hashable.hashValue' is deprecated as a protocol requirement; "
         "conform type %0 to 'Hashable' by implementing 'hash(into:)' instead",

lib/Sema/DerivedConformanceEquatableHashable.cpp

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 //
 //  This file implements implicit derivation of the Equatable and Hashable
-//  protocols. 
+//  protocols.
 //
 //===----------------------------------------------------------------------===//
 
@@ -232,7 +232,7 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
       auto rhsVar = rhsPayloadVars[varIdx];
       auto rhsExpr = new (C) DeclRefExpr(rhsVar, DeclNameLoc(),
                                          /*Implicit*/true);
-      auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C, 
+      auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C,
           lhsExpr, rhsExpr);
       statementsInCase.emplace_back(guardStmt);
     }
@@ -502,7 +502,7 @@ static CallExpr *createHasherCombineCall(ASTContext &C,
   // hasher.combine(_:)
   auto *combineCall = UnresolvedDotExpr::createImplicit(
       C, hasherExpr, C.Id_combine, {Identifier()});
-  
+
   // hasher.combine(hashable)
   auto *argList = ArgumentList::forImplicitUnlabeled(C, {hashable});
   return CallExpr::createImplicit(C, combineCall, argList);
@@ -1007,14 +1007,14 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
     if (hashValueDecl->isImplicit()) {
       // Neither hashValue nor hash(into:) is explicitly defined; we need to do
       // a full Hashable derivation.
-      
+
       // Refuse to synthesize Hashable if type isn't a struct or enum, or if it
       // has non-Hashable stored properties/associated values.
       auto hashableProto = C.getProtocol(KnownProtocolKind::Hashable);
+      auto nominalTy = Nominal->getDeclaredType();
       if (!canDeriveConformance(getConformanceContext(), Nominal,
                                 hashableProto)) {
-        ConformanceDecl->diagnose(diag::type_does_not_conform,
-                                  Nominal->getDeclaredType(),
+        ConformanceDecl->diagnose(diag::type_does_not_conform, nominalTy,
                                   hashableProto->getDeclaredInterfaceType());
         // Ideally, this would be diagnosed in
         // ConformanceChecker::resolveWitnessViaLookup. That doesn't work for
@@ -1029,6 +1029,27 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
       if (checkAndDiagnoseDisallowedContext(requirement))
         return nullptr;
 
+      // Warn the user that having a custom == almost surely require a
+      // custom Hashable conformance even if, for source stability reasons,
+      // it will be synthesized
+      auto equatableProto = C.getProtocol(KnownProtocolKind::Equatable);
+      auto eqConformance = TypeChecker::conformsToProtocol(nominalTy,
+          equatableProto, getParentModule());
+      if (eqConformance) {
+        DeclName equalsName(C, DeclBaseName(C.Id_EqualsOperator),
+                            {Identifier(), Identifier()});
+        ConcreteDeclRef witness = eqConformance.getWitnessByName(
+            nominalTy->getRValueType(), equalsName);
+
+        auto equalsDeclaration = witness.getDecl();
+        if (equalsDeclaration && !equalsDeclaration->isImplicit()) {
+          equalsDeclaration->diagnose(
+              diag::synthesized_hashable_may_not_match_custom_equatable,
+              nominalTy);
+          equalsDeclaration->diagnose(diag::add_custom_hashable);
+        }
+      }
+
       if (auto ED = dyn_cast<EnumDecl>(Nominal)) {
         std::pair<BraceStmt *, bool> (*bodySynthesizer)(
             AbstractFunctionDecl *, void *);
@@ -1044,7 +1065,7 @@ ValueDecl *DerivedConformance::deriveHashable(ValueDecl *requirement) {
                                        &deriveBodyHashable_struct_hashInto);
       else // This should've been caught by canDeriveHashable above.
         llvm_unreachable("Attempt to derive Hashable for a type other "
-                         "than a struct or enum");      
+                         "than a struct or enum");
     } else {
       // hashValue has an explicit implementation, but hash(into:) doesn't.
       // Emit a deprecation warning, then derive hash(into:) in terms of

test/Sema/Inputs/external_equatable_conformance.swift

@@ -0,0 +1,11 @@
+public struct ImplicitEquatable: ExpressibleByIntegerLiteral, Equatable {
+  public init(integerLiteral: Int) {}
+}
+
+public struct ExplicitEquatable: ExpressibleByIntegerLiteral, Equatable {
+  public init(integerLiteral: Int) {}
+
+  public static func == (lhs: ExplicitEquatable, rhs: ExplicitEquatable) -> Bool {
+    return true
+  }
+}

test/Sema/diag_custom_equatable_synthesized_hashable.swift

@@ -0,0 +1,90 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t/NormalLibrary.swiftmodule %S/Inputs/external_equatable_conformance.swift
+
+// RUN: %target-typecheck-verify-swift -I %t
+
+import NormalLibrary
+
+// All conformance synthesized no warning emitted
+struct NoCustomEquatable: Hashable {
+  let a:Int
+}
+
+// Equatable implemented and Hashable synthesized raise a warning
+enum CustomEquatable: Hashable {
+  case a
+
+  static func == (lhs: CustomEquatable, rhs: CustomEquatable) -> Bool {
+    // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'CustomEquatable' may not match the behavior of custom '==' operator}}
+    // expected-note@-2 {{add a custom 'hash(into:)' method}}
+    return true
+  }
+}
+
+// All conformance implemented no warning emitted
+enum CustomHashable: Hashable {
+  case a
+
+  func hash(into hasher: inout Hasher) {}
+
+  static func == (lhs: CustomHashable, rhs: CustomHashable) -> Bool {
+    return true
+  }
+}
+
+struct ExtendedConformance: Hashable {
+  let a:Int
+}
+
+extension ExtendedConformance {
+  static func == (lhs: ExtendedConformance, rhs: ExtendedConformance) -> Bool {
+    // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ExtendedConformance' may not match the behavior of custom '==' operator}}
+    // expected-note@-2 {{add a custom 'hash(into:)' method}}
+    return true
+  }
+}
+
+enum ImportedConformance: ImplicitEquatable {
+  case x = 1
+}
+
+enum ImportedExplicitConformance: ExplicitEquatable {
+  case x = 1
+}
+
+protocol DefaultedImplementationProtocol: Equatable {}
+extension DefaultedImplementationProtocol {
+  static func == (lhs: Self, rhs: Self) -> Bool { true }
+  // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'DefaultedImplementation' may not match the behavior of custom '==' operator}}
+  // expected-note@-2 {{add a custom 'hash(into:)' method}}
+}
+
+struct DefaultedImplementation: DefaultedImplementationProtocol, Hashable {
+  let a: Int
+}
+
+protocol ConditionalImplementationProtocol {}
+extension ConditionalImplementationProtocol where Self: Equatable {
+  static func == (lhs: Self, rhs: Self) -> Bool { true }
+  // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ConditionalImplementation' may not match the behavior of custom '==' operator}}
+  // expected-note@-2 {{add a custom 'hash(into:)' method}}
+}
+
+struct ConditionalImplementation: ConditionalImplementationProtocol, Hashable {
+  let a: Int
+}
+
+protocol ConditionalHashableImplementationProtocol {}
+extension ConditionalHashableImplementationProtocol where Self: Equatable {
+  static func == (lhs: Self, rhs: Self) -> Bool { true }
+  // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'ConditionalHashableImplementation' may not match the behavior of custom '==' operator}}
+  // expected-note@-2 {{add a custom 'hash(into:)' method}}
+}
+
+extension ConditionalHashableImplementationProtocol where Self: Hashable {
+  func hash(into hasher: Hasher) {}
+}
+
+struct ConditionalHashableImplementation: ConditionalHashableImplementationProtocol, Hashable {
+  let a: Int
+}

test/Sema/enum_conformance_synthesis.swift

@@ -220,6 +220,8 @@ public enum Medicine {
 extension Medicine : Equatable {}
 
 public func ==(lhs: Medicine, rhs: Medicine) -> Bool {
+  // expected-warning@-1 {{automatically generated 'Hashable' implementation for type 'Medicine' may not match the behavior of custom '==' operator}}
+  // expected-note@-2 {{add a custom 'hash(into:)' method}}
   return true
 }