SE-0036: Requiring Leading Dot Prefixes for Enum Instance Member Implementations (once again)

CHANGELOG.md

@@ -54,6 +54,8 @@ 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 like 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,6 +93,9 @@ 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))
+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",
       (Type, DeclName))

lib/AST/ASTDumper.cpp

@@ -1733,7 +1733,7 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
       OS << '\n';
       printRec(E->getArgument());
     }
-    OS << "')";
+    OS << ")";
   }
   void visitDotSelfExpr(DotSelfExpr *E) {
     printCommon(E, "dot_self_expr");

lib/AST/NameLookup.cpp

@@ -559,7 +559,7 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
             if (FD->isStatic() && !isMetatypeType)
               continue;
           } else if (isa<EnumElementDecl>(Result)) {
-            Results.push_back(UnqualifiedLookupResult(MetaBaseDecl, Result));
+            Results.push_back(UnqualifiedLookupResult(BaseDecl, Result));
             continue;
           }
 

lib/Sema/CSDiag.cpp

@@ -1850,6 +1850,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,
@@ -2210,6 +2215,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.
@@ -2298,6 +2367,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());
@@ -2933,7 +3017,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

@@ -895,6 +895,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.
@@ -1244,6 +1247,10 @@ class ConstraintSystem {
     return contextualType;
   }
 
+  const Expr *getContextualTypeNode() const {
+    return contextualTypeNode;
+  }
+
   ContextualTypePurpose getContextualTypePurpose() const {
     return contextualTypePurpose;
   }

lib/Sema/TypeCheckConstraints.cpp

@@ -1330,14 +1330,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/TypeCheckPattern.cpp

@@ -65,11 +65,16 @@ filterForEnumElement(LookupResult foundElements) {
   EnumElementDecl *foundElement = nullptr;
   VarDecl *foundConstant = nullptr;
 
-  for (ValueDecl *e : foundElements) {
+  for (swift::LookupResult::Result result : foundElements) {
+    ValueDecl *e = result.Decl;
     assert(e);
     if (e->isInvalid()) {
       continue;
     }
+    // Skip if the enum element was referenced as an instance member
+    if (!result.Base || !result.Base->getType()->is<MetatypeType>()) {
+      continue;
+    }
 
     if (auto *oe = dyn_cast<EnumElementDecl>(e)) {
       // Ambiguities should be ruled out by parsing.

lib/Sema/TypeCheckStmt.cpp

@@ -359,7 +359,7 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
   template<typename StmtTy>
   bool typeCheckStmt(StmtTy *&S) {
     StmtTy *S2 = cast_or_null<StmtTy>(visit(S));
-    if (S2 == 0) return true;
+    if (S2 == nullptr) return true;
     S = S2;
     performStmtDiagnostics(TC, S);
     return false;

lib/Sema/TypeChecker.h

@@ -1130,12 +1130,17 @@ class TypeChecker final : public LazyResolver {
   /// events in the type checking of this expression, and which can introduce
   /// additional constraints.
   ///
+  /// \param baseCS If this type checking process is the simplification of
+  /// another constraint system, set the original constraint system. \c null
+  /// otherwise
+  ///
   /// \returns true if an error occurred, false otherwise.
   bool typeCheckExpression(Expr *&expr, DeclContext *dc,
                            TypeLoc convertType = TypeLoc(),
                            ContextualTypePurpose convertTypePurpose =CTP_Unused,
                            TypeCheckExprOptions options =TypeCheckExprOptions(),
-                           ExprTypeCheckListener *listener = nullptr);
+                           ExprTypeCheckListener *listener = nullptr,
+                           constraints::ConstraintSystem *baseCS = nullptr);
 
   bool typeCheckExpression(Expr *&expr, DeclContext *dc,
                            ExprTypeCheckListener *listener) {

stdlib/private/StdlibCollectionUnittest/MinimalCollections.swift.gyb

@@ -157,23 +157,23 @@ internal enum _CollectionOperation : Equatable {
     var newElementsIds = elementsLastMutatedStateIds
     var newEndIndexId = endIndexLastMutatedStateId
     switch self {
-    case reserveCapacity:
+    case .reserveCapacity:
       let invalidIndices = newElementsIds.indices
       newElementsIds.replaceSubrange(
         Range(invalidIndices),
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case append:
+    case .append:
       newElementsIds.append(nextStateId)
       newEndIndexId = nextStateId
 
-    case appendContentsOf(let count):
+    case .appendContentsOf(let count):
       newElementsIds.append(contentsOf:
         repeatElement(nextStateId, count: count))
       newEndIndexId = nextStateId
 
-    case replaceRange(let subRange, let replacementCount):
+    case .replaceRange(let subRange, let replacementCount):
       newElementsIds.replaceSubrange(
         subRange,
         with: repeatElement(nextStateId, count: replacementCount))
@@ -184,7 +184,7 @@ internal enum _CollectionOperation : Equatable {
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case insert(let atIndex):
+    case .insert(let atIndex):
       newElementsIds.insert(nextStateId, at: atIndex)
 
       let invalidIndices = atIndex..<newElementsIds.endIndex
@@ -193,7 +193,7 @@ internal enum _CollectionOperation : Equatable {
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case insertContentsOf(let atIndex, let count):
+    case .insertContentsOf(let atIndex, let count):
       newElementsIds.insert(
         contentsOf: repeatElement(nextStateId, count: count),
         at: atIndex)
@@ -204,7 +204,7 @@ internal enum _CollectionOperation : Equatable {
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case removeAtIndex(let index):
+    case .removeAtIndex(let index):
       newElementsIds.remove(at: index)
 
       let invalidIndices = index..<newElementsIds.endIndex
@@ -213,11 +213,11 @@ internal enum _CollectionOperation : Equatable {
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case removeLast:
+    case .removeLast:
       newElementsIds.removeLast()
       newEndIndexId = nextStateId
 
-    case removeRange(let subRange):
+    case .removeRange(let subRange):
       newElementsIds.removeSubrange(subRange)
 
       let invalidIndices = subRange.lowerBound..<newElementsIds.endIndex
@@ -226,7 +226,7 @@ internal enum _CollectionOperation : Equatable {
         with: repeatElement(nextStateId, count: invalidIndices.count))
       newEndIndexId = nextStateId
 
-    case removeAll(let keepCapacity):
+    case .removeAll(let keepCapacity):
       newElementsIds.removeAll(keepingCapacity: keepCapacity)
       newEndIndexId = nextStateId
     }