A few fixes to move-analyzer and a small optimization to reuse a regex rather than creating a new one every time

Author: stefan-mysten

external-crates/move/crates/move-analyzer/src/symbols/compilation.rs

@@ -339,13 +339,19 @@ pub fn get_compiled_pkg<F: MoveFlavor>(
 
     let root_pkg = load_root_pkg::<F>(&build_config, pkg_path)?;
     let root_pkg_name = Symbol::from(root_pkg.name().to_string());
+    // the package's transitive dependencies
+    let dependencies: Vec<_> = root_pkg
+        .packages()
+        .into_iter()
+        .filter(|x| !x.is_root())
+        .collect();
     let build_plan =
         BuildPlan::create(&root_pkg, &build_config)?.set_compiler_vfs_root(overlay_fs_root.clone());
 
-    let mapped_files_data =
-        compute_mapped_files(&root_pkg, &build_config, overlay_fs_root.clone())?;
     // Hash dependencies so we can check if something has changed.
     // TODO: do we still need this?
+    let mapped_files_data =
+        compute_mapped_files(&root_pkg, &build_config, overlay_fs_root.clone())?;
     let file_paths: Arc<BTreeMap<FileHash, PathBuf>> = Arc::new(
         mapped_files_data
             .files
@@ -361,9 +367,6 @@ pub fn get_compiled_pkg<F: MoveFlavor>(
     let mut compiler_analysis_info_opt = None;
     let mut compiler_autocomplete_info_opt = None;
 
-    // TODO: we need to rework on loading the root pkg only once.
-    let dependencies = root_pkg.packages();
-
     let compiler_flags = compiler_flags(&build_config);
     let (mut caching_result, other_diags) = if let Ok(deps_package_paths) =
         make_deps_for_compiler(&mut Vec::new(), dependencies, &build_config)
@@ -435,14 +438,21 @@ pub fn get_compiled_pkg<F: MoveFlavor>(
                 )
             }
             None => {
-                let sorted_deps = root_pkg.sorted_deps();
-                let sorted_deps: Vec<PackageName> = sorted_deps.into_iter().cloned().collect();
+                // get the topologically sorted dependencies, but use the package ids instead of
+                // package names. In the new pkg system, multiple packages with the same name can
+                // exist as the package system will assign unique package ids to them, before
+                // passing them to the compiler.
+                let sorted_deps: Result<Vec<_>> = root_pkg
+                    .sorted_deps_ids()
+                    .into_iter()
+                    .map(|x| PackageName::new(x.to_string()))
+                    .collect();
                 if let Some((program_deps, dep_names)) = compute_pre_compiled_dep_data(
                     &mut cached_packages.compiled_dep_pkgs,
                     mapped_files_data.dep_pkg_paths,
                     src_deps,
                     root_pkg_name,
-                    &sorted_deps,
+                    &sorted_deps?,
                     compiler_flags,
                     overlay_fs_root.clone(),
                 ) {
@@ -509,7 +519,7 @@ pub fn get_compiled_pkg<F: MoveFlavor>(
     let mut ide_diags = lsp_empty_diagnostics(mapped_files_data.files.file_name_mapping());
     if full_compilation || !files_to_compile.is_empty() {
         build_plan.compile_with_driver(
-            // dependencies,
+            //dependencies,
             &mut std::io::sink(),
             |compiler| {
                 let compiler = compiler.set_ide_mode();
@@ -548,7 +558,6 @@ pub fn get_compiled_pkg<F: MoveFlavor>(
                         let failure = true;
                         diagnostics = Some((diags, failure));
                         eprintln!("typed AST compilation failed");
-                        eprintln!("diagnostics: {:#?}", diagnostics);
                         return Ok((files, vec![]));
                     }
                 };
@@ -827,10 +836,7 @@ fn compute_mapped_files<F: MoveFlavor>(
             if is_dep {
                 hasher.update(fhash.0);
                 dep_hashes.push(fhash);
-                dep_pkg_paths.insert(
-                    rpkg.name().as_str().into(),
-                    rpkg.path().path().to_path_buf(),
-                );
+                dep_pkg_paths.insert(rpkg.id().clone().into(), rpkg.path().path().to_path_buf());
             }
             // write to top layer of the overlay file system so that the content
             // is immutable for the duration of compilation and symbolication

external-crates/move/crates/move-package-alt-compilation/src/build_plan.rs

@@ -87,11 +87,16 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> {
         )?;
 
         let project_root = self.root_pkg.package_path();
-        let sorted_deps = self.root_pkg.sorted_deps().into_iter().cloned().collect();
+        let sorted_deps: Result<BTreeSet<_>, _> = self
+            .root_pkg
+            .sorted_deps()
+            .into_iter()
+            .map(|x| PackageName::new(x))
+            .collect();
 
         self.clean(
             &project_root.join(CompiledPackageLayout::Root.path()),
-            sorted_deps,
+            sorted_deps?,
         )?;
 
         Ok(compiled)
@@ -183,11 +188,17 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> {
         };
 
         let project_root = self.root_pkg.package_path();
-        let sorted_deps = self.root_pkg.sorted_deps().into_iter().cloned().collect();
+
+        let sorted_deps: Result<BTreeSet<_>, _> = self
+            .root_pkg
+            .sorted_deps()
+            .into_iter()
+            .map(|x| PackageName::new(x))
+            .collect();
 
         self.clean(
             &project_root.join(CompiledPackageLayout::Root.path()),
-            sorted_deps,
+            sorted_deps?,
         )?;
 
         Ok(migration)

external-crates/move/crates/move-package-alt/src/compatibility/mod.rs

@@ -8,6 +8,7 @@ use std::path::{Path, PathBuf};
 
 use anyhow::{Context, Result, bail};
 use move_core_types::account_address::{AccountAddress, AccountAddressParseError};
+use once_cell::sync::Lazy;
 use regex::Regex;
 use tracing::debug;
 
@@ -34,6 +35,9 @@ pub enum LegacySubstOrRename {
 /// The regex to detect `module <name>::<module_name>` on its different forms.
 const MODULE_REGEX: &str = r"\bmodule\s+([a-zA-Z_][\w]*)::([a-zA-Z_][\w]*)";
 
+// Compile regex once at program startup
+static MODULE_REGEX_COMPILED: Lazy<Regex> = Lazy::new(|| Regex::new(MODULE_REGEX).unwrap());
+
 /// This is a naive way to detect all module names that are part of the source code
 /// for a given package.
 ///
@@ -111,18 +115,15 @@ fn find_files(files: &mut Vec<PathBuf>, dir: &Path, extension: &str, max_depth:
 // Consider supporting the legacy `address { module {} }` format.
 fn parse_module_names(contents: &str) -> Result<HashSet<String>> {
     let clean = strip_comments(contents);
-    let mut set = HashSet::new();
+
     // This matches `module a::b {}`, and `module a::b;` cases.
     // In both cases, the match is the 2nd group (so `match.get(1)`)
-    let regex = Regex::new(MODULE_REGEX).unwrap();
-
-    for cap in regex.captures_iter(&clean) {
-        set.insert(cap[1].to_string());
-    }
-
-    Ok(set
-        .into_iter()
-        .filter(|name| !is_address_like(name.as_str()))
+    Ok(MODULE_REGEX_COMPILED
+        .captures_iter(&clean)
+        .filter_map(|cap| {
+            let name = &cap[1];
+            (!is_address_like(name)).then(|| name.to_string())
+        })
         .collect())
 }
 

external-crates/move/crates/move-package-alt/src/graph/mod.rs

@@ -26,7 +26,7 @@ use crate::{
     errors::PackageResult,
     flavor::MoveFlavor,
     package::{Package, paths::PackagePath},
-    schema::{Environment, PackageID, PackageName},
+    schema::{Environment, PackageID},
 };
 use bimap::BiBTreeMap;
 use builder::PackageGraphBuilder;
@@ -113,13 +113,19 @@ impl<F: MoveFlavor> PackageGraph<F> {
             .collect()
     }
 
+    /// Return the list of all packages that are in the package graph, sorted in topological order.
+    pub fn sorted_packages(&self) -> Vec<PackageInfo<F>> {
+        let sorted = toposort(&self.inner, None).expect("to sort the graph");
+        sorted.iter().map(|x| self.package_info(*x)).collect()
+    }
+
     /// Return the sorted list of dependencies' name
-    pub(crate) fn sorted_deps(&self) -> Vec<&PackageName> {
+    pub(crate) fn sorted_deps(&self) -> Vec<&str> {
         let sorted = toposort(&self.inner, None).expect("to sort the graph");
         sorted
             .iter()
             .flat_map(|x| self.inner.node_weight(*x))
-            .map(|x| x.name())
+            .map(|x| x.display_name())
             .collect()
     }
 

external-crates/move/crates/move-package-alt/src/package/root_package.rs

@@ -14,8 +14,8 @@ use crate::graph::PackageInfo;
 use crate::package::block_on;
 use crate::package::package_lock::PackageSystemLock;
 use crate::schema::{
-    Environment, LocalPub, LockfileDependencyInfo, ModeName, PackageID, PackageName,
-    ParsedEphemeralPubs, ParsedPublishedFile, Publication, RenderToml,
+    Environment, LocalPub, LockfileDependencyInfo, ModeName, PackageID, ParsedEphemeralPubs,
+    ParsedPublishedFile, Publication, RenderToml,
 };
 use crate::{
     errors::{PackageError, PackageResult},
@@ -486,9 +486,16 @@ impl<F: MoveFlavor + fmt::Debug> RootPackage<F> {
     // *** PATHS RELATED FUNCTIONS ***
 
     /// Return a list of sorted package names
-    pub fn sorted_deps(&self) -> Vec<&PackageName> {
+    pub fn sorted_deps(&self) -> Vec<&str> {
         self.filtered_graph.sorted_deps()
     }
+
+    /// Return a list of package ids, topologically sorted
+    pub fn sorted_deps_ids(&self) -> Vec<&PackageID> {
+        let pkgs = self.filtered_graph.sorted_packages();
+        let ids = pkgs.into_iter().map(|p| p.id()).collect();
+        ids
+    }
 }
 
 fn localpubs_to_publications<F: MoveFlavor>(