Option<Ty> -> Ty

Reviewed By: ndmitchell, JakobDegen

Differential Revision: D47173482

fbshipit-source-id: a2e4a807a88139545c8c696ebb9b6cf86ece4ccd

Author: stepancheg

starlark/src/docs/markdown.rs

@@ -275,7 +275,7 @@ const MAX_LENGTH_BEFORE_MULTILINE: usize = 80;
 /// produce a function prototype.
 enum TypeRenderer<'a> {
     /// A general "type".
-    Type(&'a Option<Ty>),
+    Type(&'a Ty),
     /// A function, with some extra formatting options.
     Function {
         /// The function name in the prototype as well.
@@ -286,18 +286,14 @@ enum TypeRenderer<'a> {
 
 impl<'a> RenderMarkdown for TypeRenderer<'a> {
     fn render_markdown_opt(&self, flavor: MarkdownFlavor) -> Option<String> {
-        fn raw_type(t: &Option<Ty>) -> String {
-            match t {
-                Some(t) => t.to_string(),
-                _ => "\"\"".to_owned(),
-            }
+        fn raw_type(t: &Ty) -> String {
+            t.to_string()
         }
 
-        fn raw_type_prefix(prefix: &str, t: &Option<Ty>) -> String {
-            if t.is_some() {
-                format!("{prefix}{}", raw_type(t))
-            } else {
-                String::new()
+        fn raw_type_prefix(prefix: &str, t: &Ty) -> String {
+            match t {
+                Ty::Any => String::new(),
+                t => format!("{prefix}{}", raw_type(t)),
             }
         }
 

starlark/src/docs/mod.rs

@@ -365,7 +365,7 @@ impl DocModule {
 }
 
 /// Documents a single function.
-#[derive(Debug, Clone, PartialEq, Serialize, Default, Allocative)]
+#[derive(Debug, Clone, PartialEq, Default, Serialize, Allocative)]
 pub struct DocFunction {
     /// Documentation for the function. If parsed, this should generally be the first statement
     /// of a function's body if that statement is a string literal. Any sections like "Args:",
@@ -445,10 +445,8 @@ impl DocFunction {
                 ds
             })
             .unwrap_or_default();
-        let ret = self
-            .ret
-            .typ
-            .as_ref()
+        let ret = Some(&self.ret.typ)
+            .filter(|t| t != &&Ty::Any)
             .map(|t| format!(" -> {}", t))
             .unwrap_or_default();
 
@@ -468,7 +466,7 @@ impl DocFunction {
     pub fn from_docstring(
         kind: DocStringKind,
         mut params: Vec<DocParam>,
-        return_type: Option<Ty>,
+        return_type: Ty,
         raw_docstring: Option<&str>,
         dot_type: Option<String>,
     ) -> Self {
@@ -581,7 +579,7 @@ pub enum DocParam {
         name: String,
         docs: Option<DocString>,
         #[serde(rename = "type")]
-        typ: Option<Ty>,
+        typ: Ty,
         /// If present, this parameter has a default value. This is the `repr()` of that value.
         default_value: Option<String>,
     },
@@ -594,14 +592,14 @@ pub enum DocParam {
         name: String,
         docs: Option<DocString>,
         #[serde(rename = "type")]
-        typ: Option<Ty>,
+        typ: Ty,
     },
     /// Represents the "**kwargs" style of argument.
     Kwargs {
         name: String,
         docs: Option<DocString>,
         #[serde(rename = "type")]
-        typ: Option<Ty>,
+        typ: Ty,
     },
 }
 
@@ -626,31 +624,38 @@ impl DocParam {
                 typ,
                 default_value,
                 ..
-            } => match (typ.as_ref(), default_value.as_ref()) {
-                (Some(t), Some(default)) => format!("{}: {} = {}", name, t, default),
-                (Some(t), None) => format!("{}: {}", name, t),
-                (None, Some(default)) => format!("{} = {}", name, default),
-                (None, None) => name.clone(),
+            } => match (typ, default_value.as_ref()) {
+                (Ty::Any, Some(default)) => format!("{} = {}", name, default),
+                (Ty::Any, None) => name.clone(),
+                (t, Some(default)) => format!("{}: {} = {}", name, t, default),
+                (t, None) => format!("{}: {}", name, t),
             },
             DocParam::NoArgs => "*".to_owned(),
             DocParam::OnlyPosBefore => "/".to_owned(),
-            DocParam::Args { name, typ, .. } | DocParam::Kwargs { name, typ, .. } => {
-                match typ.as_ref() {
-                    Some(typ) => format!("{}: {}", name, typ),
-                    None => name.clone(),
-                }
-            }
+            DocParam::Args { name, typ, .. } | DocParam::Kwargs { name, typ, .. } => match typ {
+                Ty::Any => name.clone(),
+                typ => format!("{}: {}", name, typ),
+            },
         }
     }
 }
 
 /// Details about the return value of a function.
-#[derive(Debug, Clone, PartialEq, Serialize, Default, Allocative)]
+#[derive(Debug, Clone, PartialEq, Serialize, Allocative)]
 pub struct DocReturn {
     /// Extra semantic details around the returned value's meaning.
     pub docs: Option<DocString>,
     #[serde(rename = "type")]
-    pub typ: Option<Ty>,
+    pub typ: Ty,
+}
+
+impl Default for DocReturn {
+    fn default() -> Self {
+        DocReturn {
+            docs: None,
+            typ: Ty::Any,
+        }
+    }
 }
 
 impl DocReturn {
@@ -660,17 +665,17 @@ impl DocReturn {
 }
 
 /// A single property of an object. These are explicitly not functions (see [`DocMember`]).
-#[derive(Debug, Clone, PartialEq, Serialize, Default, Allocative)]
+#[derive(Debug, Clone, PartialEq, Serialize, Allocative)]
 pub struct DocProperty {
     pub docs: Option<DocString>,
     #[serde(rename = "type")]
-    pub typ: Option<Ty>,
+    pub typ: Ty,
 }
 
 impl DocProperty {
     fn render_as_code(&self, name: &str) -> String {
         match (
-            self.typ.as_ref(),
+            &self.typ,
             self.docs.as_ref().map(DocString::render_as_quoted_code),
         ) {
             // TODO(nmj): The starlark syntax needs to be updated to support type
@@ -680,13 +685,12 @@ impl DocProperty {
             //     format!("{}\n_{}: {} = None", ds, name, t.raw_type)
             // }
             // (Some(t), None) => format!(r#"_{}: {} = None"#, name, t.raw_type),
-            (Some(t), Some(ds)) => {
+            (Ty::Any, Some(ds)) => format!("{}\n_{} = None", ds, name),
+            (Ty::Any, None) => format!("_{} = None", name),
+            (t, Some(ds)) => {
                 format!("{}\n# type: {}\n_{} = None", ds, t, name)
             }
-            (Some(t), None) => format!("# type: {}\n_{} = None", t, name),
-
-            (None, Some(ds)) => format!("{}\n_{} = None", ds, name),
-            (None, None) => format!("_{} = None", name),
+            (t, None) => format!("# type: {}\n_{} = None", t, name),
         }
     }
 }
@@ -708,7 +712,7 @@ impl DocMember {
             Some(DocItem::Property(x)) => DocMember::Property(x),
             _ => DocMember::Property(DocProperty {
                 docs: None,
-                typ: Some(value.get_type_starlark_repr()),
+                typ: value.get_type_starlark_repr(),
             }),
         }
     }
@@ -1134,7 +1138,7 @@ mod tests {
         DocParam::Arg {
             name: name.to_owned(),
             docs: None,
-            typ: None,
+            typ: Ty::Any,
             default_value: None,
         }
     }
@@ -1158,20 +1162,20 @@ mod tests {
         "#;
 
         let kind = DocStringKind::Starlark;
-        let return_type = Some(Ty::int());
+        let return_type = Ty::int();
         let expected = DocFunction {
             docs: DocString::from_docstring(kind, "This is an example docstring\n\nDetails here"),
             params: vec![
                 DocParam::Arg {
                     name: "**kwargs".to_owned(),
                     docs: DocString::from_docstring(kind, "Docs for kwargs"),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
                 DocParam::Arg {
                     name: "*args".to_owned(),
                     docs: DocString::from_docstring(kind, "Docs for args"),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
                 DocParam::Arg {
@@ -1184,13 +1188,13 @@ mod tests {
                             "over three lines"
                         ),
                     ),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
                 DocParam::Arg {
                     name: "arg_foo".to_owned(),
                     docs: DocString::from_docstring(kind, "The argument named foo"),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
             ],
@@ -1234,7 +1238,7 @@ mod tests {
         "#;
 
         let kind = DocStringKind::Rust;
-        let return_type = Some(Ty::int());
+        let return_type = Ty::int();
         let expected = DocFunction {
             docs: DocString::from_docstring(kind, "This is an example docstring\n\nDetails here"),
             params: vec![
@@ -1248,13 +1252,13 @@ mod tests {
                             "over three lines"
                         ),
                     ),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
                 DocParam::Arg {
                     name: "arg_foo".to_owned(),
                     docs: DocString::from_docstring(kind, "The argument named foo"),
-                    typ: None,
+                    typ: Ty::Any,
                     default_value: None,
                 },
             ],

starlark/src/eval/compiler/def.rs

@@ -550,7 +550,7 @@ impl<'v, T1: ValueLike<'v>> DefGen<T1> {
             .map(|(idx, _, ty)| (idx.0 as usize, ty.as_ty()))
             .collect();
 
-        let return_type = self.return_type.map(|r| r.as_ty());
+        let return_type = self.return_type.map_or(Ty::Any, |r| r.as_ty());
 
         let function_docs = DocFunction::from_docstring(
             DocStringKind::Starlark,

starlark/src/eval/runtime/params.rs

@@ -664,7 +664,7 @@ impl<'v, V: ValueLike<'v>> ParametersSpec<V> {
             .iter_params()
             .enumerate()
             .map(|(i, (name, kind))| {
-                let typ = parameter_types.remove(&i);
+                let typ = parameter_types.remove(&i).unwrap_or(Ty::Any);
                 let docs = parameter_docs.remove(name).flatten();
                 if pos_only == i
                     && !matches!(kind, ParameterKind::Args | ParameterKind::KWargs)
@@ -859,18 +859,18 @@ mod tests {
             DocParam::Args {
                 name: "*args".to_owned(),
                 docs: None,
-                typ: None,
+                typ: Ty::Any,
             },
             DocParam::Arg {
                 name: "a".to_owned(),
                 docs: None,
-                typ: Some(Ty::int()),
+                typ: Ty::int(),
                 default_value: Some("_".to_owned()),
             },
             DocParam::Arg {
                 name: "b".to_owned(),
                 docs: DocString::from_docstring(DocStringKind::Rust, "param b docs"),
-                typ: None,
+                typ: Ty::Any,
                 default_value: Some("_".to_owned()),
             },
         ];

starlark/src/tests/docs/golden/module.golden.md

@@ -79,7 +79,7 @@ And some assertions:
 ## notypes
 
 ```python
-def notypes(a: "") -> ""
+def notypes(a)
 ```
 
 ---

starlark/src/typing/ty.rs

@@ -809,13 +809,13 @@ impl Ty {
 
     pub(crate) fn from_docs_member(member: &DocMember) -> Self {
         match member {
-            DocMember::Property(x) => Self::from_docs_type(&x.typ),
+            DocMember::Property(x) => x.typ.clone(),
             DocMember::Function(x) => Self::from_docs_function(x),
         }
     }
 
     pub(crate) fn from_docs_property(property: &DocProperty) -> Self {
-        Self::from_docs_type(&property.typ)
+        property.typ.clone()
     }
 
     pub(crate) fn from_docs_function(function: &DocFunction) -> Self {
@@ -830,9 +830,9 @@ impl Ty {
                     ..
                 } => {
                     let mut r = if seen_no_args {
-                        Param::name_only(name, Ty::from_docs_type(typ))
+                        Param::name_only(name, typ.clone())
                     } else {
-                        Param::pos_or_name(name, Ty::from_docs_type(typ))
+                        Param::pos_or_name(name, typ.clone())
                     };
                     if default_value.is_some() {
                         r = r.optional();
@@ -849,24 +849,17 @@ impl Ty {
                 DocParam::NoArgs => seen_no_args = true,
                 DocParam::Args { typ, .. } => {
                     seen_no_args = true;
-                    params.push(Param::args(Ty::from_docs_type(typ)))
+                    params.push(Param::args(typ.clone()))
                 }
-                DocParam::Kwargs { typ, .. } => params.push(Param::kwargs(Ty::from_docs_type(typ))),
+                DocParam::Kwargs { typ, .. } => params.push(Param::kwargs(typ.clone())),
             }
         }
-        let result = Self::from_docs_type(&function.ret.typ);
+        let result = function.ret.typ.clone();
         match &function.dot_type {
             None => Ty::function(params, result),
             Some(type_attr) => Ty::ctor_function(type_attr, params, result),
         }
     }
-
-    pub(crate) fn from_docs_type(ty: &Option<Ty>) -> Self {
-        match ty {
-            None => Ty::Any,
-            Some(x) => x.clone(),
-        }
-    }
 }
 
 impl Display for Ty {

starlark/src/values/types/function.rs

@@ -168,7 +168,7 @@ impl NativeCallableRawDocs {
             DocStringKind::Rust,
             self.signature
                 .documentation(self.parameter_types.clone(), HashMap::new()),
-            Some(self.return_type.clone()),
+            self.return_type.clone(),
             self.rust_docstring,
             self.dot_type.map(ToOwned::to_owned),
         )
@@ -408,7 +408,7 @@ impl<'v> StarlarkValue<'v> for NativeAttribute {
             .docstring
             .as_ref()
             .and_then(|ds| DocString::from_docstring(DocStringKind::Rust, ds));
-        let typ = Some(self.typ.clone());
+        let typ = self.typ.clone();
         Some(DocItem::Property(DocProperty { docs: ds, typ }))
     }
 }