[Sema] Provide warnings when accessing enum elements as instance members as a preparation for SE-0036

Author: ahoppen

include/swift/AST/DiagnosticsSema.def

@@ -93,6 +93,10 @@ 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",
+      (DeclName))
 ERROR(could_not_use_type_member_on_existential,none,
       "static member %1 cannot be used on protocol metatype %0",
       (Type, DeclName))

include/swift/AST/Expr.h

@@ -1031,6 +1031,13 @@ 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);
 
@@ -1051,7 +1058,8 @@ class TypeExpr : public Expr {
 
   /// Return a TypeExpr for a TypeDecl and the specified location.
   static TypeExpr *createForDecl(SourceLoc Loc, TypeDecl *D,
-                                 bool isImplicit);
+                                 bool isImplicit,
+                                 bool isPromotedInstanceRef = false);
   static TypeExpr *createForSpecializedDecl(SourceLoc Loc, TypeDecl *D,
                                             ArrayRef<TypeRepr*> args,
                                             SourceRange angleLocs);

include/swift/AST/NameLookup.h

@@ -41,6 +41,13 @@ 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,12 +1345,14 @@ 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 isImplicit,
+                                  bool isPromotedInstanceRef) {
   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

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

lib/Sema/CSApply.cpp

@@ -2514,6 +2514,24 @@ 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/TypeCheckConstraints.cpp

@@ -556,6 +556,7 @@ 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.
@@ -566,6 +567,9 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
         break;
       }
 
+      if (Result.IsPromotedInstanceRef) {
+        PromotedInstanceRef = true;
+      }
       Base = Result.Base;
       continue;
     }
@@ -577,7 +581,8 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
   if (AllMemberRefs) {
     Expr *BaseExpr;
     if (auto NTD = dyn_cast<NominalTypeDecl>(Base)) {
-      BaseExpr = TypeExpr::createForDecl(Loc, NTD, /*implicit=*/true);
+      BaseExpr = TypeExpr::createForDecl(Loc, NTD, /*implicit=*/true,
+                                         PromotedInstanceRef);
     } else {
       BaseExpr = new (Context) DeclRefExpr(Base, UDRE->getNameLoc(),
                                            /*implicit=*/true);

lib/Sema/TypeCheckNameLookup.cpp

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

lib/Sema/TypeCheckPattern.cpp

@@ -64,13 +64,21 @@ extractEnumElement(TypeChecker &TC, DeclContext *DC, SourceLoc UseLoc,
 ///
 /// If there are no enum elements but there are properties, attempts to map
 /// an arbitrary property to an enum element using extractEnumElement.
+///
+/// \param isPromoted If set to anything but the \c nullptr, this will be set to
+///        \c true if the found enum element is referenced as on an instance
+///        but the lookup has been promoted to be on the type instead
+///        This is purely transitional and will be removed when referencing enum
+///        elements on instance members becomes an error
+
 static EnumElementDecl *
 filterForEnumElement(TypeChecker &TC, DeclContext *DC, SourceLoc UseLoc,
-                     LookupResult foundElements) {
+                     LookupResult foundElements, bool *isPromoted = nullptr) {
   EnumElementDecl *foundElement = nullptr;
   VarDecl *foundConstant = nullptr;
 
-  for (ValueDecl *e : foundElements) {
+  for (LookupResult::Result result : foundElements) {
+    ValueDecl *e = result.Decl;
     assert(e);
     if (e->isInvalid()) {
       continue;
@@ -80,6 +88,9 @@ filterForEnumElement(TypeChecker &TC, DeclContext *DC, SourceLoc UseLoc,
       // Ambiguities should be ruled out by parsing.
       assert(!foundElement && "ambiguity in enum case name lookup?!");
       foundElement = oe;
+      if (isPromoted != nullptr) {
+        *isPromoted = result.IsPromotedInstanceRef;
+      }
       continue;
     }
 
@@ -96,13 +107,20 @@ filterForEnumElement(TypeChecker &TC, DeclContext *DC, SourceLoc UseLoc,
 }
 
 /// Find an unqualified enum element.
+///
+/// \param IsPromoted If set to anything but the \c nullptr, this will be set to
+///        \c true if the found enum element is referenced as on an instance
+///        but the lookup has been promoted to be on the type instead
+///        This is purely transitional and will be removed when referencing enum
+///        elements on instance members becomes an error
 static EnumElementDecl *
 lookupUnqualifiedEnumMemberElement(TypeChecker &TC, DeclContext *DC,
-                                   Identifier name, SourceLoc UseLoc) {
+                                   Identifier name, SourceLoc UseLoc,
+                                   bool *IsPromoted = nullptr) {
   auto lookupOptions = defaultUnqualifiedLookupOptions;
   lookupOptions |= NameLookupFlags::KnownPrivate;
   auto lookup = TC.lookupUnqualified(DC, name, SourceLoc(), lookupOptions);
-  return filterForEnumElement(TC, DC, UseLoc, lookup);
+  return filterForEnumElement(TC, DC, UseLoc, lookup, IsPromoted);
 }
 
 /// Find an enum element in an enum type.
@@ -477,10 +495,22 @@ class ResolvePattern : public ASTVisitor<ResolvePattern,
     // rdar://20879992 is addressed.
     //
     // Try looking up an enum element in context.
+
+    // Check if the enum element was actually accessed on an instance and was
+    // promoted to be looked up on a type. If so, provide a warning
+    bool isPromotedInstance = false;
+
     if (EnumElementDecl *referencedElement
         = lookupUnqualifiedEnumMemberElement(TC, DC,
                                              ude->getName().getBaseName(),
-                                             ude->getLoc())) {
+                                             ude->getLoc(),
+                                             &isPromotedInstance)) {
+      if (isPromotedInstance) {
+        TC.diagnose(ude->getLoc(), diag::could_not_use_enum_element_on_instance,
+                                ude->getName())
+          .fixItInsert(ude->getLoc(), ".");
+      }
+
       auto *enumDecl = referencedElement->getParentEnum();
       auto enumTy = enumDecl->getDeclaredTypeInContext();
       TypeLoc loc = TypeLoc::withoutLoc(enumTy);
@@ -563,9 +593,21 @@ class ResolvePattern : public ASTVisitor<ResolvePattern,
     // If we had a single component, try looking up an enum element in context.
     if (auto compId = dyn_cast<ComponentIdentTypeRepr>(repr)) {
       // Try looking up an enum element in context.
+
+      // Check if the enum element was actually accessed on an instance and was
+      // promoted to be looked up on a type. If so, provide a warning
+      bool isPromoted = false;
       EnumElementDecl *referencedElement
         = lookupUnqualifiedEnumMemberElement(TC, DC, compId->getIdentifier(),
-                                             repr->getLoc());
+                                             repr->getLoc(), &isPromoted);
+
+      if (isPromoted) {
+        TC.diagnose(compId->getLoc(),
+                    diag::could_not_use_enum_element_on_instance,
+                    compId->getIdentifier())
+          .fixItInsert(compId->getLoc(), ".");
+      }
+
 
       if (!referencedElement)
         return nullptr;

lib/Sema/TypeChecker.h

@@ -63,6 +63,13 @@ class LookupResult {
     /// The base declaration through which we found the declaration.
     ValueDecl *Base;
 
+    /// 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;
+
     operator ValueDecl*() const { return Decl; }
     ValueDecl *operator->() const { return Decl; }
   };

test/Parse/enum.swift

@@ -433,3 +433,92 @@ enum RawValueBTest: Double, RawValueB {
 enum foo : String {
   case bar = nil // expected-error {{cannot convert nil to raw type 'String'}}
 }
+
+// Static member lookup from instance methods
+
+struct EmptyStruct {}
+
+enum EnumWithStaticMember {
+  static let staticVar = EmptyStruct()
+
+  func foo() {
+    let _ = staticVar // expected-error {{static member 'staticVar' cannot be used on instance of type 'EnumWithStaticMember'}}
+  }
+}
+
+// SE-0036:
+
+struct SE0036_Auxiliary {}
+
+enum SE0036 {
+  case A
+  case B(SE0036_Auxiliary)
+
+  static func staticReference() {
+    _ = A
+    _ = SE0036.A
+  }
+
+  func staticReferencInInstanceMethod() {
+    _ = A // expected-warning {{referencing enum element 'A' as instance member is deprecated and will be removed in Swift 3}} {{9-9=SE0036.}}
+    _ = SE0036.A
+  }
+
+  static func staticReferencInSwitchInStaticMethod() {
+    switch SE0036.A {
+    case A: break
+    case B(_): break
+    }
+  }
+
+  func staticReferencInSwitchInInstanceMethod() {
+    switch self {
+    case A: break // expected-warning {{referencing enum element 'A' as instance member is deprecated and will be removed in Swift 3}} {{10-10=.}}
+    case B(_): break // expected-warning {{referencing enum element 'B' as instance member is deprecated and will be removed in Swift 3}} {{10-10=.}}
+    }
+  }
+
+  func explicitReferencInSwitch() {
+    switch SE0036.A {
+    case SE0036.A: break
+    case SE0036.B(_): break
+    }
+  }
+
+  func dotReferencInSwitchInInstanceMethod() {
+    switch self {
+    case .A: break
+    case .B(_): break
+    }
+  }
+
+  static func dotReferencInSwitchInStaticMethod() {
+    switch SE0036.A {
+    case .A: break
+    case .B(_): break
+    }
+  }
+
+  init() {
+    self = .A
+    self = A // expected-warning {{referencing enum element 'A' as instance member is deprecated and will be removed in Swift 3}} {{12-12=SE0036.}}
+    self = SE0036.A
+    self = .B(SE0036_Auxiliary())
+    self = B(SE0036_Auxiliary()) // expected-warning {{referencing enum element 'B' as instance member is deprecated and will be removed in Swift 3}} {{12-12=SE0036.}}
+    self = SE0036.B(SE0036_Auxiliary())
+  }
+}
+
+enum SE0036_Generic<T> {
+  case A(x: T)
+
+  func foo() {
+    switch self {
+    case A(_): break // expected-warning {{referencing enum element 'A' as instance member is deprecated and will be removed in Swift 3}} {{10-10=.}}
+    }
+
+    switch self {
+    case SE0036_Generic.A(let a): print(a)
+    }
+  }
+}