Make attribute lookups return Attribute

Summary:
This commit consolidates the `attr::Attribute` datatype as the general way
to encode an attribute lookup; both instance and class attributes now use
this.

By making this change, and eliminating all of the direct calls that rely
on an attribute being a `Type`, we now finally have enough plumbing in the
right places to be able to represent cases where the get and set action
behave differently.

Reviewed By: ndmitchell

Differential Revision: D68687292

fbshipit-source-id: 4ec00b87d36fe710b83bd3dc1fa95561fbd7b480

pyre2/pyre2/bin/alt/attr.rs

@@ -72,12 +72,11 @@ pub enum InternalError {
 }
 
 impl Attribute {
-    fn access_allowed(ty: Type) -> Self {
+    pub fn access_allowed(ty: Type) -> Self {
         Attribute(AttributeAccess::allowed(ty))
     }
-
-    fn with_access(access: AttributeAccess) -> Self {
-        Attribute(access)
+    pub fn access_not_allowed(reason: AccessNotAllowed) -> Self {
+        Attribute(AttributeAccess::not_allowed(reason))
     }
 
     pub fn get(&self) -> &AttributeAccess {
@@ -197,13 +196,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
         match self.as_attribute_base(ty.clone(), self.stdlib) {
             Some(AttributeBase::ClassInstance(class)) => {
                 match self.get_instance_attribute(&class, attr_name) {
-                    Some(attr) => LookupResult::found_type(attr),
+                    Some(attr) => LookupResult::Found(attr),
                     None => LookupResult::NotFound(NotFound::Attribute(class)),
                 }
             }
             Some(AttributeBase::ClassObject(class)) => {
                 match self.get_class_attribute(&class, attr_name) {
-                    Some(access) => LookupResult::Found(Attribute::with_access(access)),
+                    Some(attr) => LookupResult::Found(attr),
                     None => {
                         // Classes are instances of their metaclass, which defaults to `builtins.type`.
                         let metadata = self.get_metadata_for_class(&class);
@@ -214,7 +213,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                             }
                         };
                         match instance_attr {
-                            Some(attr) => LookupResult::found_type(attr),
+                            Some(attr) => LookupResult::Found(attr),
                             None => LookupResult::NotFound(NotFound::ClassAttribute(class)),
                         }
                     }
@@ -232,16 +231,17 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                 } else {
                     let class = q.as_value(self.stdlib);
                     match self.get_instance_attribute(&class, attr_name) {
-                        Some(attr) => LookupResult::found_type(attr),
+                        Some(attr) => LookupResult::Found(attr),
                         None => LookupResult::NotFound(NotFound::Attribute(class)),
                     }
                 }
             }
             Some(AttributeBase::TypeAny(style)) => {
-                let class = self.stdlib.builtins_type();
+                let builtins_type_classtype = self.stdlib.builtins_type();
                 LookupResult::found_type(
-                    self.get_instance_attribute(&class, attr_name)
-                        .map_or_else(|| style.propagate(), |attr| attr),
+                    self.get_instance_attribute(&builtins_type_classtype, attr_name)
+                        .and_then(|attr| attr.get_type().cloned())
+                        .map_or_else(|| style.propagate(), |ty| ty),
                 )
             }
             Some(AttributeBase::Any(style)) => LookupResult::found_type(style.propagate()),

pyre2/pyre2/bin/alt/classes.rs

@@ -26,7 +26,7 @@ use starlark_map::small_set::SmallSet;
 use crate::alt::answers::AnswersSolver;
 use crate::alt::answers::LookupAnswer;
 use crate::alt::attr::AccessNotAllowed;
-use crate::alt::attr::AttributeAccess;
+use crate::alt::attr::Attribute;
 use crate::ast::Ast;
 use crate::binding::binding::ClassFieldInitialization;
 use crate::binding::binding::Key;
@@ -761,9 +761,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
             })
     }
 
-    pub fn get_instance_attribute(&self, cls: &ClassType, name: &Name) -> Option<Type> {
+    pub fn get_instance_attribute(&self, cls: &ClassType, name: &Name) -> Option<Attribute> {
         self.get_instance_attribute_with_defining_class(cls, name)
-            .map(|attr| attr.value)
+            .map(|attr| Attribute::access_allowed(attr.value))
     }
 
     fn depends_on_class_type_parameter(&self, cls: &Class, ty: &Type) -> bool {
@@ -780,24 +780,24 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
     ///
     /// Access is disallowed for instance-only attributes and for attributes whose
     /// type contains a class-scoped type parameter - e.g., `class A[T]: x: T`.
-    pub fn get_class_attribute(&self, cls: &Class, name: &Name) -> Option<AttributeAccess> {
+    pub fn get_class_attribute(&self, cls: &Class, name: &Name) -> Option<Attribute> {
         let (member, _) = self.get_class_member(cls, name)?;
         if self.depends_on_class_type_parameter(cls, &member.ty) {
-            Some(AttributeAccess::not_allowed(
+            Some(Attribute::access_not_allowed(
                 AccessNotAllowed::ClassAttributeIsGeneric(cls.clone()),
             ))
         } else {
             match member.initialization {
-                ClassFieldInitialization::Instance => Some(AttributeAccess::not_allowed(
+                ClassFieldInitialization::Instance => Some(Attribute::access_not_allowed(
                     AccessNotAllowed::ClassUseOfInstanceAttribute(cls.clone()),
                 )),
                 ClassFieldInitialization::Class => {
                     if let Some(e) = self.get_enum(&self.promote_silently(cls))
                         && let Some(member) = e.get_member(name)
                     {
-                        Some(AttributeAccess::allowed(Type::Literal(member)))
+                        Some(Attribute::access_allowed(Type::Literal(member)))
                     } else {
-                        Some(AttributeAccess::allowed(member.ty))
+                        Some(Attribute::access_allowed(member.ty))
                     }
                 }
             }

pyre2/pyre2/bin/alt/expr.rs

@@ -291,6 +291,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
             Type::TypeAlias(ta) => self.as_call_target(ta.as_value(self.stdlib)),
             Type::ClassType(cls) => self
                 .get_instance_attribute(&cls, &dunder::CALL)
+                .and_then(|attr| attr.get_type().cloned())
                 .and_then(|ty| self.as_call_target(ty)),
             _ => None,
         }

pyre2/pyre2/bin/type_order.rs

@@ -13,11 +13,11 @@ use starlark_map::small_map::SmallMap;
 
 use crate::alt::answers::AnswersSolver;
 use crate::alt::answers::LookupAnswer;
+use crate::alt::attr::Attribute;
 use crate::alt::classes::ClassField;
 use crate::types::class::Class;
 use crate::types::class::ClassType;
 use crate::types::stdlib::Stdlib;
-use crate::types::types::Type;
 
 /// `TypeOrder` provides a minimal API allowing `Subset` to request additional
 /// information about types that may be required for solving bindings
@@ -61,7 +61,7 @@ impl<'a, Ans: LookupAnswer> TypeOrder<'a, Ans> {
         self.0.get_all_members(cls)
     }
 
-    pub fn get_instance_attribute(self, cls: &ClassType, name: &Name) -> Option<Type> {
+    pub fn get_instance_attribute(self, cls: &ClassType, name: &Name) -> Option<Attribute> {
         self.0.get_instance_attribute(cls, name)
     }
 }