Fix common lsp panics (hyperledger#1575)

These lsp crashes are concurrency issues, so writing tests from them is
hard.

If we could hand-craft lsp json-rpc messages then this would be easier,
but we only test the extension by controlling vscode.

Signed-off-by: Sean Young <sean@mess.org>

RELEASE_CHECKLIST.md

@@ -3,6 +3,9 @@
 - Update the version in `Cargo.toml`, `solang-parser/Cargo.toml`, the binary
   links in `docs/installing.rst`, and `CHANGELOG.md`. Remember to match the
   solang-parser version in the top level `Cargo.toml`.
+- Ensure the `CHANGELOG.md` and `vscode/CHANGELOG.md` are up to date.
+- If the vscode extension is going to be updated, fix the version in
+  `docs/extension.rst`.
 - Copy the contents of the CHANGELOG for this release into commit message,
   using `git commit -s --cleanup=whitespace` so the that the lines beginning
   with `#` are not removed.

docs/extension.rst

@@ -75,8 +75,8 @@ Once you have node and npm installed, you can build the extension like so:
     npm install -g vsce
     vsce package
 
-You should now have an extension file called solang-0.3.0.vsix which can be
-installed using ``code --install-extension solang-0.3.0.vsix``.
+You should now have an extension file called ``solang-0.3.3.vsix`` which can be
+installed using ``code --install-extension solang-0.3.3.vsix``.
 
 Alternatively, the extension is run from vscode itself.
 

src/abi/anchor.rs

@@ -49,7 +49,7 @@ pub fn generate_anchor_idl(contract_no: usize, ns: &Namespace, contract_version:
 
     Idl {
         version: Version::parse(contract_version).unwrap().to_string(),
-        name: ns.contracts[contract_no].name.clone(),
+        name: ns.contracts[contract_no].id.name.clone(),
         docs,
         constants: vec![],
         instructions,
@@ -261,7 +261,8 @@ impl TypeManager<'_> {
         // If the existing type was declared outside a contract or if it is from the current contract,
         // we should change the name of the type we are adding now.
         if other_contract.is_none()
-            || other_contract.as_ref().unwrap() == &self.namespace.contracts[self.contract_no].name
+            || other_contract.as_ref().unwrap()
+                == &self.namespace.contracts[self.contract_no].id.name
         {
             let new_name = if let Some(this_name) = contract {
                 format!("{this_name}_{type_name}")

src/abi/mod.rs

@@ -21,7 +21,7 @@ pub fn generate_abi(
             if verbose {
                 eprintln!(
                     "info: Generating ink! metadata for contract {}",
-                    ns.contracts[contract_no].name
+                    ns.contracts[contract_no].id
                 );
             }
 
@@ -33,7 +33,7 @@ pub fn generate_abi(
             if verbose {
                 eprintln!(
                     "info: Generating Anchor metadata for contract {}",
-                    ns.contracts[contract_no].name
+                    ns.contracts[contract_no].id
                 );
             }
 
@@ -45,7 +45,7 @@ pub fn generate_abi(
             if verbose {
                 eprintln!(
                     "info: Generating Ethereum ABI for contract {}",
-                    ns.contracts[contract_no].name
+                    ns.contracts[contract_no].id
                 );
             }
 

src/abi/polkadot.rs

@@ -309,7 +309,7 @@ pub fn gen_project(contract_no: usize, ns: &ast::Namespace) -> InkProject {
             }
         })
         .collect();
-    let contract_name = ns.contracts[contract_no].name.clone();
+    let contract_name = ns.contracts[contract_no].id.name.clone();
     let storage = Layout::Struct(StructLayout::new(contract_name, fields));
 
     let constructor_spec = |f: &Function| -> ConstructorSpec<PortableForm> {
@@ -384,7 +384,7 @@ pub fn gen_project(contract_no: usize, ns: &ast::Namespace) -> InkProject {
 
                 let t = TypeDefTuple::new_portable(fields);
 
-                let path = path!(&ns.contracts[contract_no].name, &f.name, "return_type");
+                let path = path!(&ns.contracts[contract_no].id.name, &f.name, "return_type");
 
                 let ty = registry.register_type(Type::new(
                     path,
@@ -562,7 +562,7 @@ pub fn metadata(
     );
 
     let mut builder = Contract::builder();
-    builder.name(&ns.contracts[contract_no].name);
+    builder.name(&ns.contracts[contract_no].id.name);
     let mut description = tags(contract_no, "title", ns);
     description.extend(tags(contract_no, "notice", ns));
     if !description.is_empty() {

src/bin/doc/mod.rs

@@ -186,7 +186,7 @@ pub fn generate_docs(outdir: &OsString, files: &[ast::Namespace], verbose: bool)
                 name: &event_decl.name,
                 contract: event_decl
                     .contract
-                    .map(|contract_no| file.contracts[contract_no].name.as_str()),
+                    .map(|contract_no| file.contracts[contract_no].id.name.as_str()),
                 title: get_tag("title", &event_decl.tags),
                 notice: get_tag("notice", &event_decl.tags),
                 author: get_tag("author", &event_decl.tags),
@@ -356,7 +356,7 @@ pub fn generate_docs(outdir: &OsString, files: &[ast::Namespace], verbose: bool)
                 for var in base
                     .variables
                     .iter()
-                    .map(|var| map_var(file, Some(&base.name), var))
+                    .map(|var| map_var(file, Some(&base.id.name), var))
                 {
                     base_variables.push(var);
                 }
@@ -365,7 +365,7 @@ pub fn generate_docs(outdir: &OsString, files: &[ast::Namespace], verbose: bool)
                     let f = &file.functions[*function_no];
 
                     if f.has_body {
-                        Some(map_func(file, Some(&base.name), f))
+                        Some(map_func(file, Some(&base.id.name), f))
                     } else {
                         None
                     }
@@ -376,7 +376,7 @@ pub fn generate_docs(outdir: &OsString, files: &[ast::Namespace], verbose: bool)
 
             top.contracts.push(Contract {
                 loc: contract.loc,
-                name: &contract.name,
+                name: &contract.id.name,
                 ty: format!("{}", contract.ty),
                 title: get_tag("title", &contract.tags),
                 notice: get_tag("notice", &contract.tags),

src/bin/languageserver/mod.rs

@@ -334,16 +334,17 @@ impl SolangServer {
         let files = self.files.lock().await;
         if let Some(cache) = files.caches.get(&path) {
             let f = &cache.file;
-            let offset = f.get_offset(
+            if let Some(offset) = f.get_offset(
                 params.text_document_position_params.position.line as _,
                 params.text_document_position_params.position.character as _,
-            );
-            if let Some(reference) = cache
-                .references
-                .find(offset, offset + 1)
-                .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
-            {
-                return Ok(Some(reference.val.clone()));
+            ) {
+                if let Some(reference) = cache
+                    .references
+                    .find(offset, offset + 1)
+                    .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
+                {
+                    return Ok(Some(reference.val.clone()));
+                }
             }
         }
         Ok(None)
@@ -905,7 +906,7 @@ impl<'a> Builder<'a> {
             }
             ast::Expression::ConstantVariable { loc, ty, contract_no, var_no } => {
                 let (contract, name) = if let Some(contract_no) = contract_no {
-                    let contract = format!("{}.", self.ns.contracts[*contract_no].name);
+                    let contract = format!("{}.", self.ns.contracts[*contract_no].id);
                     let name = &self.ns.contracts[*contract_no].variables[*var_no].name;
                     (contract, name)
                 } else {
@@ -944,7 +945,7 @@ impl<'a> Builder<'a> {
             ast::Expression::StorageVariable { loc, ty, contract_no, var_no } => {
                 let contract = &self.ns.contracts[*contract_no];
                 let name = &contract.variables[*var_no].name;
-                let val = format!("{} {}.{}", ty.to_string(self.ns), contract.name, name);
+                let val = format!("{} {}.{}", ty.to_string(self.ns), contract.id, name);
                 self.hovers.push((
                     loc.file_no(),
                     HoverEntry {
@@ -1082,7 +1083,7 @@ impl<'a> Builder<'a> {
                         msg
                     }).join(", ");
 
-                let contract = fnc.contract_no.map(|contract_no| format!("{}.", self.ns.contracts[contract_no].name)).unwrap_or_default();
+                let contract = fnc.contract_no.map(|contract_no| format!("{}.", self.ns.contracts[contract_no].id)).unwrap_or_default();
 
                 let val = format!("{} {}{}({}) returns ({})\n", fnc.ty, contract, fnc.name, params, rets);
 
@@ -1140,7 +1141,7 @@ impl<'a> Builder<'a> {
                         msg
                     }).join(", ");
 
-                let contract = fnc.contract_no.map(|contract_no| format!("{}.", self.ns.contracts[contract_no].name)).unwrap_or_default();
+                let contract = fnc.contract_no.map(|contract_no| format!("{}.", self.ns.contracts[contract_no].id)).unwrap_or_default();
 
                 let val = format!("{} {}{}({}) returns ({})\n", fnc.ty, contract, fnc.name, params, rets);
 
@@ -1562,7 +1563,7 @@ impl<'a> Builder<'a> {
                         stop: base.loc.exclusive_end(),
                         val: make_code_block(format!(
                             "contract {}",
-                            self.ns.contracts[base.contract_no].name
+                            self.ns.contracts[base.contract_no].id
                         )),
                     },
                 ));
@@ -1589,8 +1590,8 @@ impl<'a> Builder<'a> {
             self.hovers.push((
                 file_no,
                 HoverEntry {
-                    start: contract.loc.start(),
-                    stop: contract.loc.start() + contract.name.len(),
+                    start: contract.id.loc.start(),
+                    stop: contract.id.loc.exclusive_end(),
                     val: render(&contract.tags[..]),
                 },
             ));
@@ -1601,7 +1602,7 @@ impl<'a> Builder<'a> {
             };
 
             self.definitions
-                .insert(cdi.clone(), loc_to_range(&contract.loc, file));
+                .insert(cdi.clone(), loc_to_range(&contract.id.loc, file));
 
             let impls = contract
                 .functions
@@ -1719,7 +1720,9 @@ impl<'a> Builder<'a> {
             .collect::<HashMap<PathBuf, usize>>();
 
         for val in self.types.values_mut() {
-            val.def_path = defs_to_files[&val.def_type].clone();
+            if let Some(path) = defs_to_files.get(&val.def_type) {
+                val.def_path = path.clone();
+            }
         }
 
         for (di, range) in &self.definitions {
@@ -1729,10 +1732,13 @@ impl<'a> Builder<'a> {
                 file_no,
                 ReferenceEntry {
                     start: file
-                        .get_offset(range.start.line as usize, range.start.character as usize),
+                        .get_offset(range.start.line as usize, range.start.character as usize)
+                        .unwrap(),
                     // 1 is added to account for the fact that `Lapper` expects half open ranges of the type:  [`start`, `stop`)
                     // i.e, `start` included but `stop` excluded.
-                    stop: file.get_offset(range.end.line as usize, range.end.character as usize)
+                    stop: file
+                        .get_offset(range.end.line as usize, range.end.character as usize)
+                        .unwrap()
                         + 1,
                     val: di.clone(),
                 },
@@ -1761,7 +1767,9 @@ impl<'a> Builder<'a> {
                         .filter(|h| h.0 == i)
                         .map(|(_, i)| {
                             let mut i = i.clone();
-                            i.val.def_path = defs_to_files[&i.val.def_type].clone();
+                            if let Some(path) = defs_to_files.get(&i.val.def_type) {
+                                i.val.def_path = path.clone();
+                            }
                             i
                         })
                         .collect(),
@@ -1990,24 +1998,25 @@ impl LanguageServer for SolangServer {
         if let Ok(path) = uri.to_file_path() {
             let files = &self.files.lock().await;
             if let Some(cache) = files.caches.get(&path) {
-                let offset = cache
+                if let Some(offset) = cache
                     .file
-                    .get_offset(pos.line as usize, pos.character as usize);
-
-                // The shortest hover for the position will be most informative
-                if let Some(hover) = cache
-                    .hovers
-                    .find(offset, offset + 1)
-                    .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
+                    .get_offset(pos.line as usize, pos.character as usize)
                 {
-                    let range = get_range_exclusive(hover.start, hover.stop, &cache.file);
-
-                    return Ok(Some(Hover {
-                        contents: HoverContents::Scalar(MarkedString::from_markdown(
-                            hover.val.to_string(),
-                        )),
-                        range: Some(range),
-                    }));
+                    // The shortest hover for the position will be most informative
+                    if let Some(hover) = cache
+                        .hovers
+                        .find(offset, offset + 1)
+                        .min_by(|a, b| (a.stop - a.start).cmp(&(b.stop - b.start)))
+                    {
+                        let range = get_range_exclusive(hover.start, hover.stop, &cache.file);
+
+                        return Ok(Some(Hover {
+                            contents: HoverContents::Scalar(MarkedString::from_markdown(
+                                hover.val.to_string(),
+                            )),
+                            range: Some(range),
+                        }));
+                    }
                 }
             }
         }

src/bin/solang.rs

@@ -230,7 +230,7 @@ fn compile(compile_args: &Compile) {
             !namespaces
                 .iter()
                 .flat_map(|ns| ns.contracts.iter())
-                .any(|contract| **name == contract.name)
+                .any(|contract| **name == contract.id.name)
         })
         .collect();
 
@@ -370,15 +370,15 @@ fn contract_results(
 
     let loc = ns.loc_to_string(PathDisplay::FullPath, &resolved_contract.loc);
 
-    if let Some(other_loc) = seen_contracts.get(&resolved_contract.name) {
+    if let Some(other_loc) = seen_contracts.get(&resolved_contract.id.name) {
         eprintln!(
             "error: contract {} defined at {other_loc} and {}",
-            resolved_contract.name, loc
+            resolved_contract.id, loc
         );
         exit(1);
     }
 
-    seen_contracts.insert(resolved_contract.name.to_string(), loc);
+    seen_contracts.insert(resolved_contract.id.to_string(), loc);
 
     if let Some("cfg") = compiler_output.emit.as_deref() {
         println!("{}", resolved_contract.print_cfg(ns));
@@ -389,13 +389,13 @@ fn contract_results(
         if ns.target == solang::Target::Solana {
             eprintln!(
                 "info: contract {} uses at least {} bytes account data",
-                resolved_contract.name, resolved_contract.fixed_layout_size,
+                resolved_contract.id, resolved_contract.fixed_layout_size,
             );
         }
 
         eprintln!(
             "info: Generating LLVM IR for contract {} with target {}",
-            resolved_contract.name, ns.target
+            resolved_contract.id, ns.target
         );
     }
 
@@ -413,7 +413,7 @@ fn contract_results(
     if let Some(level) = opt.wasm_opt.filter(|_| ns.target.is_polkadot() && verbose) {
         eprintln!(
             "info: wasm-opt level '{}' for contract {}",
-            level, resolved_contract.name
+            level, resolved_contract.id
         );
     }
 

src/codegen/cfg.rs

@@ -1283,7 +1283,7 @@ impl ControlFlowGraph {
                 } else {
                     String::new()
                 },
-                ns.contracts[*contract_no].name,
+                ns.contracts[*contract_no].id,
                 self.expr_to_string(contract, ns, encoded_args),
                 if let ExternalCallAccounts::Present(accounts) = accounts {
                     self.expr_to_string(contract, ns, accounts)
@@ -1615,9 +1615,9 @@ fn function_cfg(
     let contract_name = match func.contract_no {
         Some(base_contract_no) => format!(
             "{}::{}",
-            ns.contracts[contract_no].name, ns.contracts[base_contract_no].name
+            ns.contracts[contract_no].id, ns.contracts[base_contract_no].id
         ),
-        None => ns.contracts[contract_no].name.to_string(),
+        None => ns.contracts[contract_no].id.to_string(),
     };
 
     let name = match func.ty {
@@ -1889,8 +1889,8 @@ fn generate_modifier_dispatch(
     let modifier = &ns.functions[modifier_no];
     let name = format!(
         "{}::{}::{}::modifier{}::{}",
-        &ns.contracts[contract_no].name,
-        &ns.contracts[func.contract_no.unwrap()].name,
+        &ns.contracts[contract_no].id,
+        &ns.contracts[func.contract_no.unwrap()].id,
         func.llvm_symbol(ns),
         chain_no,
         modifier.llvm_symbol(ns)
@@ -2032,7 +2032,7 @@ fn generate_modifier_dispatch(
 impl Contract {
     /// Print the entire contract; storage initializers, constructors and functions and their CFGs
     pub fn print_cfg(&self, ns: &Namespace) -> String {
-        let mut out = format!("#\n# Contract: {}\n#\n\n", self.name);
+        let mut out = format!("#\n# Contract: {}\n#\n\n", self.id);
 
         for cfg in &self.cfg {
             if !cfg.is_placeholder() {