Improve ContractCaller type labels in CompileErrors and LSP features (F…

…uelLabs#4361) ## Description Closes FuelLabs#4314 This PR does the following: 1. Move impl of `DisplayWithEngines` to `DebugWithEngines`, since the output of these impls is really debug output. 2. Add impls for `DisplayWithEngines` to TypeInfo, TypeArgument and others, which the LSP uses for features like inlay hints and autocomplete. 3. Remove `display_name` in favor of `DisplayWithEngines::fmt` 4. Fixes the display label for `ContractCaller` in hover and inlay hints. 5. Changes aliases to display only the alias name, rather than the string `type {alias name} = {other name}`, similar to Rust. 6. Many of the Compile Errors that display a type now use the new `DisplayWithEngines::fmt`, which is slightly more user-friendly than before. This makes it so the types shown in the errors are the same as what the user sees in the inlay hints and hover docs. For this reason, I updated a few of the e2e vm tests where necessary. Going forward, the output of `DisplayWithEngines::fmt` should always be the user-friendly name of the type that will be shown in the editor. Here's an example of the difference: before <img width="806" alt="image" src="https://user-images.githubusercontent.com/47993817/228378865-8ec62a9c-4564-4cc6-b2fd-ed680bfb108a.png"> after <img width="872" alt="image" src="https://user-images.githubusercontent.com/47993817/228378207-94993f77-ca01-414d-84ca-d800efaa73a6.png"> ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. (existing tests pass) - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Joshua Batty <[email protected]>
braveerr · Apr 3, 2023 · c16c850 · c16c850
1 parent 8ad8fd5
commit c16c850
Show file tree

Hide file tree

Showing 19 changed files with 244 additions and 62 deletions.
diff --git a/sway-core/src/control_flow_analysis/flow_graph/mod.rs b/sway-core/src/control_flow_analysis/flow_graph/mod.rs
@@ -5,7 +5,7 @@ use std::collections::HashMap;
 
 use crate::{
     decl_engine::*,
-    engine_threading::DisplayWithEngines,
+    engine_threading::DebugWithEngines,
     language::ty::{self, GetDeclIdent},
     transform, Engines, Ident,
 };
@@ -120,11 +120,13 @@ impl<'cfg> std::convert::From<&str> for ControlFlowGraphNode<'cfg> {
     }
 }
 
-impl<'cfg> DisplayWithEngines for ControlFlowGraphNode<'cfg> {
+impl<'cfg> DebugWithEngines for ControlFlowGraphNode<'cfg> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>, engines: Engines<'_>) -> std::fmt::Result {
         let text = match self {
             ControlFlowGraphNode::OrganizationalDominator(s) => s.to_string(),
-            ControlFlowGraphNode::ProgramNode { node, .. } => engines.help_out(node).to_string(),
+            ControlFlowGraphNode::ProgramNode { node, .. } => {
+                format!("{:?}", engines.help_out(node))
+            }
             ControlFlowGraphNode::EnumVariant { variant_name, .. } => {
                 format!("Enum variant {variant_name}")
             }
@@ -201,7 +203,7 @@ impl<'cfg> ControlFlowGraph<'cfg> {
     pub(crate) fn visualize(&self, engines: Engines<'_>) {
         use petgraph::dot::{Config, Dot};
         let string_graph = self.graph.filter_map(
-            |_idx, node| Some(engines.help_out(node).to_string()),
+            |_idx, node| Some(format!("{:?}", engines.help_out(node))),
             |_idx, edge| Some(edge.0.clone()),
         );
 

diff --git a/sway-core/src/engine_threading.rs b/sway-core/src/engine_threading.rs
@@ -61,12 +61,14 @@ impl<'a, T> WithEngines<'a, T> {
     }
 }
 
+/// Displays the user-friendly formatted view of `thing` using `engines` as context.
 impl<T: DisplayWithEngines> fmt::Display for WithEngines<'_, T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.thing.fmt(f, self.engines)
     }
 }
 
+/// Displays the internals of `thing` using `engines` as context. Useful for debugging.
 impl<T: DebugWithEngines> fmt::Debug for WithEngines<'_, T> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         self.thing.fmt(f, self.engines)

diff --git a/sway-core/src/language/ty/ast_node.rs b/sway-core/src/language/ty/ast_node.rs
@@ -44,13 +44,13 @@ impl HashWithEngines for TyAstNode {
     }
 }
 
-impl DisplayWithEngines for TyAstNode {
+impl DebugWithEngines for TyAstNode {
     fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
         use TyAstNodeContent::*;
         match &self.content {
-            Declaration(typed_decl) => DisplayWithEngines::fmt(typed_decl, f, engines),
-            Expression(exp) => DisplayWithEngines::fmt(exp, f, engines),
-            ImplicitReturnExpression(exp) => write!(f, "return {}", engines.help_out(exp)),
+            Declaration(typed_decl) => DebugWithEngines::fmt(typed_decl, f, engines),
+            Expression(exp) => DebugWithEngines::fmt(exp, f, engines),
+            ImplicitReturnExpression(exp) => write!(f, "return {:?}", engines.help_out(exp)),
             SideEffect(_) => f.write_str(""),
         }
     }

diff --git a/sway-core/src/language/ty/declaration/declaration.rs b/sway-core/src/language/ty/declaration/declaration.rs
@@ -407,6 +407,51 @@ impl DisplayWithEngines for TyDecl {
     }
 }
 
+impl DebugWithEngines for TyDecl {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> std::fmt::Result {
+        let type_engine = engines.te();
+        write!(
+            f,
+            "{} declaration ({})",
+            self.friendly_type_name(),
+            match self {
+                TyDecl::VariableDecl(decl) => {
+                    let TyVariableDecl {
+                        mutability,
+                        name,
+                        type_ascription,
+                        body,
+                        ..
+                    } = &**decl;
+                    let mut builder = String::new();
+                    match mutability {
+                        VariableMutability::Mutable => builder.push_str("mut"),
+                        VariableMutability::RefMutable => builder.push_str("ref mut"),
+                        VariableMutability::Immutable => {}
+                    }
+                    builder.push_str(name.as_str());
+                    builder.push_str(": ");
+                    builder.push_str(
+                        format!(
+                            "{:?}",
+                            engines.help_out(type_engine.get(type_ascription.type_id))
+                        )
+                        .as_str(),
+                    );
+                    builder.push_str(" = ");
+                    builder.push_str(format!("{:?}", engines.help_out(body)).as_str());
+                    builder
+                }
+                TyDecl::FunctionDecl { name, .. }
+                | TyDecl::TraitDecl { name, .. }
+                | TyDecl::StructDecl { name, .. }
+                | TyDecl::EnumDecl { name, .. } => name.as_str().into(),
+                _ => String::new(),
+            }
+        )
+    }
+}
+
 impl CollectTypesMetadata for TyDecl {
     // this is only run on entry nodes, which must have all well-formed types
     fn collect_types_metadata(
@@ -661,7 +706,7 @@ impl TyDecl {
                 let decl = decl_engine.get_impl_trait(decl_id);
                 let implementing_for_type_id = type_engine.get(decl.implementing_for.type_id);
                 format!(
-                    "{} for {}",
+                    "{} for {:?}",
                     self.get_decl_ident()
                         .map_or(String::from(""), |f| f.as_str().to_string()),
                     engines.help_out(implementing_for_type_id)

diff --git a/sway-core/src/language/ty/expression/expression.rs b/sway-core/src/language/ty/expression/expression.rs
@@ -75,6 +75,17 @@ impl DisplayWithEngines for TyExpression {
     }
 }
 
+impl DebugWithEngines for TyExpression {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
+        write!(
+            f,
+            "{:?} ({:?})",
+            engines.help_out(&self.expression),
+            engines.help_out(self.return_type)
+        )
+    }
+}
+
 impl CollectTypesMetadata for TyExpression {
     fn collect_types_metadata(
         &self,

diff --git a/sway-core/src/language/ty/expression/expression_variant.rs b/sway-core/src/language/ty/expression/expression_variant.rs
@@ -943,6 +943,13 @@ impl ReplaceDecls for TyExpressionVariant {
 }
 
 impl DisplayWithEngines for TyExpressionVariant {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
+        // TODO: Implement user-friendly display strings if needed.
+        DebugWithEngines::fmt(self, f, engines)
+    }
+}
+
+impl DebugWithEngines for TyExpressionVariant {
     fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
         let s = match self {
             TyExpressionVariant::Literal(lit) => format!("literal {lit}"),
@@ -958,7 +965,7 @@ impl DisplayWithEngines for TyExpressionVariant {
             TyExpressionVariant::Tuple { fields } => {
                 let fields = fields
                     .iter()
-                    .map(|field| engines.help_out(field).to_string())
+                    .map(|field| format!("{:?}", engines.help_out(field)))
                     .collect::<Vec<_>>()
                     .join(", ");
                 format!("tuple({fields})")
@@ -983,7 +990,7 @@ impl DisplayWithEngines for TyExpressionVariant {
                 ..
             } => {
                 format!(
-                    "\"{}.{}\" struct field access",
+                    "\"{:?}.{}\" struct field access",
                     engines.help_out(*resolved_type_of_parent),
                     field_to_access.name
                 )
@@ -994,7 +1001,7 @@ impl DisplayWithEngines for TyExpressionVariant {
                 ..
             } => {
                 format!(
-                    "\"{}.{}\" tuple index",
+                    "\"{:?}.{}\" tuple index",
                     engines.help_out(*resolved_type_of_parent),
                     elem_to_access_num
                 )
@@ -1018,20 +1025,20 @@ impl DisplayWithEngines for TyExpressionVariant {
             TyExpressionVariant::StorageAccess(access) => {
                 format!("storage field {} access", access.storage_field_name())
             }
-            TyExpressionVariant::IntrinsicFunction(kind) => engines.help_out(kind).to_string(),
+            TyExpressionVariant::IntrinsicFunction(kind) => format!("{:?}", engines.help_out(kind)),
             TyExpressionVariant::AbiName(n) => format!("ABI name {n}"),
             TyExpressionVariant::EnumTag { exp } => {
-                format!("({} as tag)", engines.help_out(exp.return_type))
+                format!("({:?} as tag)", engines.help_out(exp.return_type))
             }
             TyExpressionVariant::UnsafeDowncast { exp, variant } => {
                 format!(
-                    "({} as {})",
+                    "({:?} as {})",
                     engines.help_out(exp.return_type),
                     variant.name
                 )
             }
             TyExpressionVariant::WhileLoop { condition, .. } => {
-                format!("while loop on {}", engines.help_out(&**condition))
+                format!("while loop on {:?}", engines.help_out(&**condition))
             }
             TyExpressionVariant::Break => "break".to_string(),
             TyExpressionVariant::Continue => "continue".to_string(),
@@ -1062,7 +1069,7 @@ impl DisplayWithEngines for TyExpressionVariant {
                 format!("storage reassignment to {place}")
             }
             TyExpressionVariant::Return(exp) => {
-                format!("return {}", engines.help_out(&**exp))
+                format!("return {:?}", engines.help_out(&**exp))
             }
         };
         write!(f, "{s}")

diff --git a/sway-core/src/language/ty/expression/intrinsic_function.rs b/sway-core/src/language/ty/expression/intrinsic_function.rs
@@ -66,17 +66,17 @@ impl ReplaceSelfType for TyIntrinsicFunctionKind {
     }
 }
 
-impl DisplayWithEngines for TyIntrinsicFunctionKind {
+impl DebugWithEngines for TyIntrinsicFunctionKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
         let targs = self
             .type_arguments
             .iter()
-            .map(|targ| engines.help_out(targ.type_id))
+            .map(|targ| format!("{:?}", engines.help_out(targ.type_id)))
             .join(", ");
         let args = self
             .arguments
             .iter()
-            .map(|e| format!("{}", engines.help_out(e)))
+            .map(|e| format!("{:?}", engines.help_out(e)))
             .join(", ");
 
         write!(f, "{}::<{}>::({})", self.kind, targs, args)

diff --git a/sway-core/src/type_system/ast_elements/type_argument.rs b/sway-core/src/type_system/ast_elements/type_argument.rs
@@ -80,6 +80,12 @@ impl DisplayWithEngines for TypeArgument {
     }
 }
 
+impl DebugWithEngines for TypeArgument {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
+        write!(f, "{:?}", engines.help_out(engines.te().get(self.type_id)))
+    }
+}
+
 impl From<&TypeParameter> for TypeArgument {
     fn from(type_param: &TypeParameter) -> Self {
         TypeArgument {

diff --git a/sway-core/src/type_system/ast_elements/type_parameter.rs b/sway-core/src/type_system/ast_elements/type_parameter.rs
@@ -106,9 +106,14 @@ impl Spanned for TypeParameter {
     }
 }
 
-impl DisplayWithEngines for TypeParameter {
+impl DebugWithEngines for TypeParameter {
     fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
-        write!(f, "{}: {}", self.name_ident, engines.help_out(self.type_id))
+        write!(
+            f,
+            "{}: {:?}",
+            self.name_ident,
+            engines.help_out(self.type_id)
+        )
     }
 }
 

diff --git a/sway-core/src/type_system/engine.rs b/sway-core/src/type_system/engine.rs
@@ -486,7 +486,9 @@ impl TypeEngine {
         let engines = Engines::new(self, decl_engine);
         let mut builder = String::new();
         self.slab.with_slice(|elems| {
-            let list = elems.iter().map(|type_info| engines.help_out(type_info));
+            let list = elems
+                .iter()
+                .map(|type_info| format!("{:?}", engines.help_out(type_info)));
             let list = ListDisplay { list };
             write!(builder, "TypeEngine {{\n{list}\n}}").unwrap();
         });

diff --git a/sway-core/src/type_system/id.rs b/sway-core/src/type_system/id.rs
@@ -18,6 +18,12 @@ impl DisplayWithEngines for TypeId {
     }
 }
 
+impl DebugWithEngines for TypeId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>, engines: Engines<'_>) -> fmt::Result {
+        write!(f, "{:?}", engines.help_out(engines.te().get(*self)))
+    }
+}
+
 impl From<usize> for TypeId {
     fn from(o: usize) -> Self {
         TypeId(o)