cli: load a package.json and name a lockfile load step from one place

mordant-baseline.toml

@@ -44,8 +44,6 @@
 "reimplemented_helper:src/runtime/api/bun/Terminal.rs" = 1
 "reimplemented_helper:src/runtime/hw_exports.rs" = 1
 "same_match_twice:src/runtime/api/bun/h2_frame_parser.rs" = 1
-"same_match_twice:src/runtime/cli/pack_command.rs" = 1
-"same_match_twice:src/runtime/cli/update_interactive_command.rs" = 2
 "same_match_twice:src/runtime/server/RequestContext.rs" = 4
 "same_match_twice:src/runtime/shell/builtin/cat.rs" = 1
 "same_match_twice:src/runtime/webcore/Blob.rs" = 2

src/install/PackageManager/WorkspacePackageJSONCache.rs

@@ -114,14 +114,36 @@ pub enum GetResult<'a> {
     ParseErr(Error),
 }
 
+/// The step of [`WorkspacePackageJSONCache::get_with_path`] that failed.
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub enum GetStep {
+    Read,
+    Parse,
+}
+
+impl GetStep {
+    /// The word for the step in a "failed to ... package.json" message.
+    pub fn verb(self) -> &'static str {
+        match self {
+            GetStep::Read => "read",
+            GetStep::Parse => "parse",
+        }
+    }
+}
+
 impl<'a> GetResult<'a> {
-    pub(crate) fn unwrap(self) -> Result<&'a mut MapEntry, Error> {
+    /// The entry, or the step that failed and its error.
+    pub fn entry(self) -> Result<&'a mut MapEntry, (GetStep, Error)> {
         match self {
             GetResult::Entry(entry) => Ok(entry),
-            GetResult::ReadErr(err) => Err(err),
-            GetResult::ParseErr(err) => Err(err),
+            GetResult::ReadErr(err) => Err((GetStep::Read, err)),
+            GetResult::ParseErr(err) => Err((GetStep::Parse, err)),
         }
     }
+
+    pub(crate) fn unwrap(self) -> Result<&'a mut MapEntry, Error> {
+        self.entry().map_err(|(_, err)| err)
+    }
 }
 
 #[derive(Default)]

src/install/lockfile.rs

@@ -324,7 +324,8 @@ pub enum LoadStep {
 }
 
 impl LoadStep {
-    pub(crate) fn verb(self) -> &'static str {
+    /// The word for the step in a "failed to ... lockfile" message.
+    pub fn verb(self) -> &'static str {
         match self {
             LoadStep::OpenFile => "open",
             LoadStep::ReadFile => "read",

src/runtime/cli/outdated_command.rs

@@ -7,8 +7,8 @@ use bun_core::strings;
 use bun_core::{Global, Output};
 use bun_glob as glob;
 use bun_install::dependency::{self, Behavior};
+use bun_install::lockfile::LoadResult;
 use bun_install::lockfile::package::PackageColumns as _;
-use bun_install::lockfile::{LoadResult, LoadStep};
 use bun_install::package_manager::{
     LogLevel, ManifestLoad, Subcommand, WorkspaceFilter, populate_manifest_cache,
 };
@@ -118,24 +118,10 @@ impl OutdatedCommand {
                 if not_silent
                     && !bun_install::migration::reported_unsupported_lockfile_version(&cause)
                 {
-                    match cause.step {
-                        LoadStep::OpenFile => Output::err_generic(
-                            "failed to open lockfile: {s}",
-                            (cause.value.name(),),
-                        ),
-                        LoadStep::ParseFile => Output::err_generic(
-                            "failed to parse lockfile: {s}",
-                            (cause.value.name(),),
-                        ),
-                        LoadStep::ReadFile => Output::err_generic(
-                            "failed to read lockfile: {s}",
-                            (cause.value.name(),),
-                        ),
-                        LoadStep::Migrating => Output::err_generic(
-                            "failed to migrate lockfile: {s}",
-                            (cause.value.name(),),
-                        ),
-                    }
+                    Output::err_generic(
+                        "failed to {s} lockfile: {s}",
+                        (cause.step.verb(), cause.value.name()),
+                    );
                     if ctx.log_ref().has_errors() {
                         // SAFETY: `log_ptr` aliases `manager.log` which is the
                         // `*logger.Log` borrowed from `Command::Context`; no

src/runtime/cli/pack_command.rs

@@ -1890,46 +1890,54 @@ fn opt_pack_gzip_level(m: &PackageManager) -> Option<&[u8]> {
 // `Some` only when FOR_PUBLISH == true.
 pub(crate) type PackReturn<'a, const FOR_PUBLISH: bool> = Option<Publish::Context<'a, true>>;
 
-pub(crate) fn pack<const FOR_PUBLISH: bool>(
-    ctx: &mut Context<'_>,
+/// The package.json being packed, through the manager's cache; a package.json
+/// that cannot be read or parsed ends the command.
+///
+/// `workspace_package_json_cache` and `log` are disjoint fields on
+/// `PackageManager`; route through raw-pointer field projections so the two
+/// `&mut` borrows don't conflict.
+fn package_json_entry<'a>(
+    manager_ptr: *mut PackageManager,
     abs_package_json_path: &ZStr,
-) -> Result<PackReturn<'static, FOR_PUBLISH>, PackError<FOR_PUBLISH>> {
-    // Raw pointer for the `pm_workspace_cache`/`pm_log` disjoint-field
-    // projections and the `'static` lifetime extension when returning
-    // `Publish::Context`.
-    let manager_ptr: *mut PackageManager = &raw mut *ctx.manager;
-    let log_level = ctx.manager.options.log_level;
-    let bump = pack_bump();
-    // Note: `workspace_package_json_cache` and `log` are disjoint fields on
-    // `PackageManager`; route through raw-pointer field projections so the
-    // two `&mut` borrows don't conflict.
-    let mut json = match pm_workspace_cache(manager_ptr).get_with_path(
+) -> &'a mut WorkspacePackageJSONCache::MapEntry {
+    let result = pm_workspace_cache(manager_ptr).get_with_path(
         pm_log(manager_ptr),
         abs_package_json_path.as_bytes(),
         WorkspacePackageJSONCache::GetJSONOptions {
             guess_indentation: true,
             ..Default::default()
         },
-    ) {
-        WorkspacePackageJSONCache::GetResult::ReadErr(err) => {
-            Output::err(
-                err,
-                "failed to read package.json: {}",
-                format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
-            );
-            Global::crash();
-        }
-        WorkspacePackageJSONCache::GetResult::ParseErr(err) => {
+    );
+    match result.entry() {
+        Ok(entry) => entry,
+        Err((step, err)) => {
             Output::err(
                 err,
-                "failed to parse package.json: {}",
-                format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
+                "failed to {} package.json: {}",
+                (
+                    step.verb(),
+                    bstr::BStr::new(abs_package_json_path.as_bytes()),
+                ),
             );
-            let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
+            if step == WorkspacePackageJSONCache::GetStep::Parse {
+                let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
+            }
             Global::crash();
         }
-        WorkspacePackageJSONCache::GetResult::Entry(entry) => entry,
-    };
+    }
+}
+
+pub(crate) fn pack<const FOR_PUBLISH: bool>(
+    ctx: &mut Context<'_>,
+    abs_package_json_path: &ZStr,
+) -> Result<PackReturn<'static, FOR_PUBLISH>, PackError<FOR_PUBLISH>> {
+    // Raw pointer for the `pm_workspace_cache`/`pm_log` disjoint-field
+    // projections and the `'static` lifetime extension when returning
+    // `Publish::Context`.
+    let manager_ptr: *mut PackageManager = &raw mut *ctx.manager;
+    let log_level = ctx.manager.options.log_level;
+    let bump = pack_bump();
+    let mut json = package_json_entry(manager_ptr, abs_package_json_path);
 
     if FOR_PUBLISH {
         if let Some(config) = json.root.get(b"publishConfig") {
@@ -2156,34 +2164,7 @@ pub(crate) fn pack<const FOR_PUBLISH: bool>(
         let cache_key: &[u8] = abs_package_json_path.as_bytes();
         let _ = pm_workspace_cache(manager_ptr).map.remove(cache_key);
 
-        // Re-read package.json from disk
-        json = match pm_workspace_cache(manager_ptr).get_with_path(
-            pm_log(manager_ptr),
-            abs_package_json_path.as_bytes(),
-            WorkspacePackageJSONCache::GetJSONOptions {
-                guess_indentation: true,
-                ..Default::default()
-            },
-        ) {
-            WorkspacePackageJSONCache::GetResult::ReadErr(err) => {
-                Output::err(
-                    err,
-                    "failed to read package.json: {}",
-                    format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
-                );
-                Global::crash();
-            }
-            WorkspacePackageJSONCache::GetResult::ParseErr(err) => {
-                Output::err(
-                    err,
-                    "failed to parse package.json: {}",
-                    format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())),
-                );
-                let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer()));
-                Global::crash();
-            }
-            WorkspacePackageJSONCache::GetResult::Entry(entry) => entry,
-        };
+        json = package_json_entry(manager_ptr, abs_package_json_path);
 
         // Re-validate private flag after scripts may have modified it.
         if FOR_PUBLISH {

src/runtime/cli/package_manager_command.rs

@@ -6,7 +6,7 @@ use bun_core::fmt::PathSep;
 use bun_core::strings;
 use bun_core::{Global, Output, env_var, fmt as bun_fmt};
 use bun_install::dependency::Dependency;
-use bun_install::lockfile::{LoadResult, LoadStep, Lockfile, package::PackageColumns as _, tree};
+use bun_install::lockfile::{LoadResult, Lockfile, package::PackageColumns as _, tree};
 use bun_install::npm as Npm;
 use bun_install::package_manager_real::{
     CommandLineArguments, Subcommand, fetch_cache_directory_path, get_cache_directory,
@@ -51,15 +51,6 @@ impl<'a> ByName<'a> {
     }
 }
 
-fn load_step_verb(step: LoadStep) -> &'static str {
-    match step {
-        LoadStep::OpenFile => "open",
-        LoadStep::ReadFile => "read",
-        LoadStep::ParseFile => "parse",
-        LoadStep::Migrating => "migrate",
-    }
-}
-
 pub(crate) struct PackageManagerCommand;
 
 impl PackageManagerCommand {
@@ -93,7 +84,7 @@ impl PackageManagerCommand {
                 if not_silent && !migration::reported_unsupported_lockfile_version(err) {
                     Output::err_generic(
                         "failed to {s} lockfile: {s}",
-                        (load_step_verb(err.step), err.value.name()),
+                        (err.step.verb(), err.value.name()),
                     );
                 }
                 Global::exit(1);