SE-0036 (3/3): Provide errors when referencing enum elements on instances

CHANGELOG.md

@@ -246,6 +246,9 @@ Swift 3.0
     `NS[Mutable]Dictionary` are still imported as nongeneric classes for
     the time being.
 
+* [SE-0036](https://github.com/apple/swift-evolution/blob/master/proposals/0036-enum-dot.md): 
+  Enum elements can no longer be accessed as instance members in instance methods.
+
 * As part of the changes for SE-0055 (see below), the *pointee* types of
   imported pointers (e.g. the `id` in `id *`) are no longer assumed to always
   be `_Nullable` even if annotated otherwise. However, an implicit or explicit

include/swift/AST/DiagnosticsSema.def

@@ -93,9 +93,8 @@ ERROR(could_not_use_type_member,none,
 ERROR(could_not_use_type_member_on_instance,none,
       "static member %1 cannot be used on instance of type %0",
       (Type, DeclName))
-WARNING(could_not_use_enum_element_on_instance,none,
-      "referencing enum element %0 as instance member is deprecated and will "
-      "be removed in Swift 3",
+ERROR(could_not_use_enum_element_on_instance,none,
+      "enum element %0 cannot be referenced as an instance member",
       (DeclName))
 ERROR(could_not_use_type_member_on_existential,none,
       "static member %1 cannot be used on protocol metatype %0",

include/swift/AST/Expr.h

@@ -1031,13 +1031,6 @@ class TypeExpr : public Expr {
   TypeLoc Info;
   TypeExpr(Type Ty);
 public:
-  /// Whether this type reference actually references an instance but has been
-  /// promoted to a type reference to access an enum element
-  ///
-  /// This is purely transitional and will be removed when referencing enum
-  /// elements on instance members becomes an error
-  bool IsPromotedInstanceRef = false;
-
   // Create a TypeExpr with location information.
   TypeExpr(TypeLoc Ty);
 
@@ -1058,8 +1051,7 @@ class TypeExpr : public Expr {
 
   /// Return a TypeExpr for a TypeDecl and the specified location.
   static TypeExpr *createForDecl(SourceLoc Loc, TypeDecl *D,
-                                 bool isImplicit,
-                                 bool isPromotedInstanceRef = false);
+                                 bool isImplicit);
   static TypeExpr *createForSpecializedDecl(SourceLoc Loc, TypeDecl *D,
                                             ArrayRef<TypeRepr*> args,
                                             SourceRange angleLocs);

include/swift/AST/NameLookup.h

@@ -41,13 +41,6 @@ struct UnqualifiedLookupResult {
   ValueDecl *Value;
 
 public:
-  /// Whether this should actually reference an instance but has been promoted
-  /// to a type reference to access an enum element
-  ///
-  /// This is purely transitional and will be removed when referencing enum
-  /// elements on instance members becomes an error
-  bool IsPromotedInstanceRef = false;
-
   UnqualifiedLookupResult(ValueDecl *value) : Base(nullptr), Value(value) { }
 
   UnqualifiedLookupResult(ValueDecl *base, ValueDecl *value)

lib/AST/Expr.cpp

@@ -1345,14 +1345,12 @@ Type TypeExpr::getInstanceType() const {
 
 /// Return a TypeExpr for a simple identifier and the specified location.
 TypeExpr *TypeExpr::createForDecl(SourceLoc Loc, TypeDecl *Decl,
-                                  bool isImplicit,
-                                  bool isPromotedInstanceRef) {
+                                  bool isImplicit) {
   ASTContext &C = Decl->getASTContext();
   assert(Loc.isValid());
   auto *Repr = new (C) SimpleIdentTypeRepr(Loc, Decl->getName());
   Repr->setValue(Decl);
   auto result = new (C) TypeExpr(TypeLoc(Repr, Type()));
-  result->IsPromotedInstanceRef = isPromotedInstanceRef;
   if (isImplicit)
     result->setImplicit();
   return result;

lib/AST/NameLookup.cpp

@@ -563,11 +563,7 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
             if (FD->isStatic() && !isMetatypeType)
               continue;
           } else if (isa<EnumElementDecl>(Result)) {
-            auto lookupRes = UnqualifiedLookupResult(MetaBaseDecl, Result);
-            if (!BaseDecl->getType()->is<MetatypeType>()) {
-              lookupRes.IsPromotedInstanceRef = true;
-            }
-            Results.push_back(lookupRes);
+            Results.push_back(UnqualifiedLookupResult(BaseDecl, Result));
             continue;
           }
 

lib/Sema/CSApply.cpp

@@ -2545,24 +2545,6 @@ namespace {
 
   public:
     Expr *visitUnresolvedDotExpr(UnresolvedDotExpr *expr) {
-      if (auto ty = dyn_cast<TypeExpr>(expr->getBase())) {
-        if (ty->IsPromotedInstanceRef) {
-          // An enum element was looked up on an instance. Issue a warning
-          auto enumMetatype = ty->getType()->castTo<AnyMetatypeType>();
-          auto enumType = enumMetatype->getInstanceType()->castTo<EnumType>();
-
-          SmallString<32> enumTypeName;
-          llvm::raw_svector_ostream typeNameStream(enumTypeName);
-          typeNameStream << enumType->getDecl()->getName();
-          typeNameStream << ".";
-
-          TypeChecker &tc = cs.getTypeChecker();
-          tc.diagnose(expr->getLoc(),
-                      diag::could_not_use_enum_element_on_instance,
-                      expr->getName())
-            .fixItInsert(expr->getLoc(), typeNameStream.str());
-        }
-      }
       return applyMemberRefExpr(expr, expr->getBase(), expr->getDotLoc(),
                                 expr->getNameLoc(), expr->isImplicit());
     }

lib/Sema/CSDiag.cpp

@@ -1859,6 +1859,11 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
   /// the exact expression kind).
   bool diagnoseGeneralMemberFailure(Constraint *constraint);
 
+  /// Diagnose the lookup of an enum element as instance member where only a
+  /// static member is allowed
+  void diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
+                                        SourceLoc loc);
+
   /// Given a result of name lookup that had no viable results, diagnose the
   /// unviable ones.
   void diagnoseUnviableLookupResults(MemberLookupResult &lookupResults,
@@ -2228,6 +2233,70 @@ bool FailureDiagnosis::diagnoseGeneralMemberFailure(Constraint *constraint) {
   return false;
 }
 
+void FailureDiagnosis::
+diagnoseEnumInstanceMemberLookup(EnumElementDecl *enumElementDecl,
+                                 SourceLoc loc) {
+  auto diag = diagnose(loc, diag::could_not_use_enum_element_on_instance,
+                       enumElementDecl->getName());
+  auto parentEnum = enumElementDecl->getParentEnum();
+  auto enumMetatype = parentEnum->getType()->castTo<AnyMetatypeType>();
+
+  // Determine the contextual type of the expression
+  Type contextualType;
+  for (auto iterateCS = CS;
+         contextualType.isNull() && iterateCS;
+         iterateCS = iterateCS->baseCS) {
+    contextualType = iterateCS->getContextualType();
+  }
+
+  // Try to provide a fix-it that only contains a '.'
+  if (contextualType) {
+    if (enumMetatype->getInstanceType()->isEqual(contextualType)) {
+      diag.fixItInsert(loc, ".");
+      return;
+    }
+  }
+
+  // Check if the expression is the matching operator ~=, most often used in
+  // case statements. If so, try to provide a single dot fix-it
+  const Expr *contextualTypeNode;
+  for (auto iterateCS = CS; iterateCS; iterateCS = iterateCS->baseCS) {
+    contextualTypeNode = iterateCS->getContextualTypeNode();
+  }
+
+  // The '~=' operator is an overloaded decl ref inside a binaryExpr
+  if (auto binaryExpr = dyn_cast<BinaryExpr>(contextualTypeNode)) {
+    if (auto overloadedFn
+          = dyn_cast<OverloadedDeclRefExpr>(binaryExpr->getFn())) {
+      if (overloadedFn->getDecls().size() > 0) {
+        // Fetch any declaration to check if the name is '~='
+        ValueDecl *decl0 = overloadedFn->getDecls()[0];
+
+        if (decl0->getName() == decl0->getASTContext().Id_MatchOperator) {
+          assert(binaryExpr->getArg()->getElements().size() == 2);
+
+          // If the rhs of '~=' is the enum type, a single dot suffices
+          // since the type can be inferred
+          Type secondArgType = binaryExpr->getArg()->getElement(1)->getType();
+          if (secondArgType->isEqual(enumMetatype->getInstanceType())) {
+            diag.fixItInsert(loc, ".");
+            return;
+          }
+        }
+      }
+    }
+  }
+
+  // Fall back to a fix-it with a full type qualifier
+  SmallString<32> enumTypeName;
+  llvm::raw_svector_ostream typeNameStream(enumTypeName);
+  typeNameStream << parentEnum->getName();
+  typeNameStream << ".";
+
+  diag.fixItInsert(loc, typeNameStream.str());
+  return;
+}
+
 
 /// Given a result of name lookup that had no viable results, diagnose the
 /// unviable ones.
@@ -2365,6 +2434,21 @@ diagnoseUnviableLookupResults(MemberLookupResult &result, Type baseObjTy,
       } else {
         // Otherwise the static member lookup was invalid because it was
         // called on an instance
+
+        // Handle enum element lookup on instance type
+        auto lookThroughBaseObjTy = baseObjTy->lookThroughAllAnyOptionalTypes();
+        if (lookThroughBaseObjTy->is<EnumType>()
+            || lookThroughBaseObjTy->is<BoundGenericEnumType>()) {
+          for (auto cand : result.UnviableCandidates) {
+            ValueDecl *decl = cand.first;
+            if (auto enumElementDecl = dyn_cast<EnumElementDecl>(decl)) {
+              diagnoseEnumInstanceMemberLookup(enumElementDecl, loc);
+              return;
+            }
+          }
+        }
+
+        // Provide diagnostic other static member lookups on instance type
         diagnose(loc, diag::could_not_use_type_member_on_instance,
                  baseObjTy, memberName)
           .highlight(baseRange).highlight(nameLoc.getSourceRange());
@@ -3035,7 +3119,7 @@ typeCheckChildIndependently(Expr *subExpr, Type convertType,
   bool hadError = CS->TC.typeCheckExpression(subExpr, CS->DC,
                                              TypeLoc::withoutLoc(convertType),
                                              convertTypePurpose, TCEOptions,
-                                             listener);
+                                             listener, CS);
 
   // This is a terrible hack to get around the fact that typeCheckExpression()
   // might change subExpr to point to a new OpenExistentialExpr. In that case,

lib/Sema/ConstraintSystem.h

@@ -899,6 +899,9 @@ class ConstraintSystem {
   /// Note: this is only used to support ObjCSelectorExpr at the moment.
   llvm::SmallPtrSet<Expr *, 2> UnevaluatedRootExprs;
 
+  /// The original CS if this CS was created as a simplification of another CS
+  ConstraintSystem *baseCS = nullptr;
+
 private:
 
   /// \brief Allocator used for all of the related constraint systems.
@@ -1248,6 +1251,10 @@ class ConstraintSystem {
     return contextualType;
   }
 
+  const Expr *getContextualTypeNode() const {
+    return contextualTypeNode;
+  }
+
   ContextualTypePurpose getContextualTypePurpose() const {
     return contextualTypePurpose;
   }

lib/Sema/TypeCheckConstraints.cpp

@@ -551,7 +551,6 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
 
   ResultValues.clear();
   bool AllMemberRefs = true;
-  bool PromotedInstanceRef = false;
   ValueDecl *Base = 0;
   for (auto Result : Lookup) {
     // Track the base for member declarations.
@@ -562,9 +561,6 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
         break;
       }
 
-      if (Result.IsPromotedInstanceRef) {
-        PromotedInstanceRef = true;
-      }
       Base = Result.Base;
       continue;
     }
@@ -576,8 +572,7 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
   if (AllMemberRefs) {
     Expr *BaseExpr;
     if (auto NTD = dyn_cast<NominalTypeDecl>(Base)) {
-      BaseExpr = TypeExpr::createForDecl(Loc, NTD, /*implicit=*/true,
-                                         PromotedInstanceRef);
+      BaseExpr = TypeExpr::createForDecl(Loc, NTD, /*implicit=*/true);
     } else {
       BaseExpr = new (Context) DeclRefExpr(Base, UDRE->getNameLoc(),
                                            /*implicit=*/true);
@@ -1421,14 +1416,16 @@ bool TypeChecker::typeCheckExpression(Expr *&expr, DeclContext *dc,
                                       TypeLoc convertType,
                                       ContextualTypePurpose convertTypePurpose,
                                       TypeCheckExprOptions options,
-                                      ExprTypeCheckListener *listener) {
+                                      ExprTypeCheckListener *listener,
+                                      ConstraintSystem *baseCS) {
   PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);
 
   // Construct a constraint system from this expression.
   ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes;
   if (options.contains(TypeCheckExprFlags::PreferForceUnwrapToOptional))
     csOptions |= ConstraintSystemFlags::PreferForceUnwrapToOptional;
   ConstraintSystem cs(*this, dc, csOptions);
+  cs.baseCS = baseCS;
   CleanupIllFormedExpressionRAII cleanup(Context, expr);
   ExprCleanser cleanup2(expr);
 

lib/Sema/TypeCheckNameLookup.cpp

@@ -103,12 +103,7 @@ namespace {
     ///
     /// \param foundInType The type through which we found the
     /// declaration.
-    ///
-    /// \param promotedInstanceRef true if the lookup result to be added was
-    /// actually looked up on an instance but promted to a type to look up an
-    /// enum element
-    void add(ValueDecl *found, ValueDecl *base, Type foundInType,
-             bool promotedInstanceRef = false) {
+    void add(ValueDecl *found, ValueDecl *base, Type foundInType) {
       // If we only want types, AST name lookup should not yield anything else.
       assert(!Options.contains(NameLookupFlags::OnlyTypes) ||
              isa<TypeDecl>(found));
@@ -144,7 +139,7 @@ namespace {
           isa<GenericTypeParamDecl>(found) ||
           (isa<FuncDecl>(found) && cast<FuncDecl>(found)->isOperator())) {
         if (Known.insert({{found, base}, false}).second) {
-          Result.add({found, base, promotedInstanceRef});
+          Result.add({found, base});
           FoundDecls.push_back(found);
         }
         return;
@@ -177,7 +172,7 @@ namespace {
         // default implementations in protocols.
         if (witness && !isa<ProtocolDecl>(witness->getDeclContext())) {
           if (Known.insert({{witness, base}, false}).second) {
-            Result.add({witness, base, promotedInstanceRef});
+            Result.add({witness, base});
             FoundDecls.push_back(witness);
           }
         }
@@ -234,8 +229,7 @@ LookupResult TypeChecker::lookupUnqualified(DeclContext *dc, DeclName name,
       assert(foundInType && "bogus base declaration?");
     }
 
-    builder.add(found.getValueDecl(), found.getBaseDecl(), foundInType,
-                found.IsPromotedInstanceRef);
+    builder.add(found.getValueDecl(), found.getBaseDecl(), foundInType);
   }
   return result;
 }
@@ -572,7 +566,7 @@ void TypeChecker::performTypoCorrection(DeclContext *DC, DeclRefKind refKind,
   entries.filterMaxScoreRange(MaxCallEditDistanceFromBestCandidate);
 
   for (auto &entry : entries)
-    result.add({ entry.Value, nullptr, false });
+    result.add({ entry.Value, nullptr });
 }
 
 static InFlightDiagnostic