fix(check): honor package-lock.json when installing from package_file

qlty-check/src/tool/node.rs

@@ -172,16 +172,12 @@ impl Tool for NodePackage {
 
         self.run_command(self.cmd.build(
             NPM_COMMAND,
-            vec![
-                "install",
-                "--force",
-                format!("{}@{}", name, version).as_str(),
-            ],
+            vec!["install", "--force", format!("{name}@{version}").as_str()],
         ))
     }
 
     fn package_file_install(&self, task: &ProgressTask) -> Result<()> {
-        self.update_package_json(&self.name, &self.plugin.package_file)?;
+        let lock_file_staged = self.update_package_json(&self.name, &self.plugin.package_file)?;
         task.set_dim_message(
             format!(
                 "{} install {}",
@@ -195,10 +191,17 @@ impl Tool for NodePackage {
             .as_str(),
         );
 
-        self.run_command(
-            self.cmd
-                .build(NPM_COMMAND, vec!["install", "--force", "--no-package-lock"]),
-        )
+        let mut arguments = vec!["install", "--force"];
+
+        // Honor the user's lock file when one was staged; --no-package-lock
+        // would make npm ignore it and resolve latest matching versions
+        // instead. The flag is kept otherwise so npm does not consult the
+        // lock file it generated during the initial tool installation.
+        if !lock_file_staged {
+            arguments.push("--no-package-lock");
+        }
+
+        self.run_command(self.cmd.build(NPM_COMMAND, arguments))
     }
 
     fn extra_env_paths(&self) -> Result<Vec<String>> {
@@ -400,6 +403,94 @@ pub mod test {
         });
     }
 
+    #[test]
+    fn node_package_install_with_package_file_and_lock_file() {
+        with_node_package(|pkg, temp_path, list| {
+            let pkg_file = temp_path.path().join("package.json");
+            std::fs::write(&pkg_file, r#"{"dependencies":{"other":"2.0.0"}}"#)?;
+
+            let lock_file = temp_path.path().join("package-lock.json");
+            std::fs::write(
+                &lock_file,
+                r#"{"name":"lock-test","lockfileVersion":3,"packages":{}}"#,
+            )?;
+
+            pkg.plugin.package_file = Some(pkg_file.to_str().unwrap().to_string());
+            reroute_tools_root(&temp_path, pkg);
+
+            pkg.install(&new_task())?;
+            assert_eq!(
+                list.lock().unwrap().clone(),
+                [
+                    vec![NPM_COMMAND, "install", "--force", "test@1.0.0"],
+                    vec![NPM_COMMAND, "install", "--force"]
+                ]
+            );
+
+            let staged_lock_file = Path::new(&pkg.directory()).join("package-lock.json");
+            assert!(staged_lock_file.exists());
+            Ok(())
+        });
+    }
+
+    #[test]
+    fn node_package_install_ignores_leftover_staging_lock_file() {
+        with_node_package(|pkg, temp_path, list| {
+            let pkg_file = temp_path.path().join("package.json");
+            std::fs::write(&pkg_file, r#"{"dependencies":{"other":"2.0.0"}}"#)?;
+
+            pkg.plugin.package_file = Some(pkg_file.to_str().unwrap().to_string());
+            reroute_tools_root(&temp_path, pkg);
+
+            let staged_lock_file = Path::new(&pkg.directory()).join("package-lock.json");
+            std::fs::write(
+                &staged_lock_file,
+                r#"{"name":"tool","lockfileVersion":3,"packages":{}}"#,
+            )?;
+
+            pkg.install(&new_task())?;
+            assert_eq!(
+                list.lock().unwrap().clone(),
+                [
+                    [NPM_COMMAND, "install", "--force", "test@1.0.0"],
+                    [NPM_COMMAND, "install", "--force", "--no-package-lock"]
+                ]
+            );
+            Ok(())
+        });
+    }
+
+    #[test]
+    fn node_package_install_with_lock_file_and_package_filters() {
+        with_node_package(|pkg, temp_path, list| {
+            let pkg_file = temp_path.path().join("package.json");
+            std::fs::write(&pkg_file, r#"{"dependencies":{"other":"2.0.0"}}"#)?;
+
+            let lock_file = temp_path.path().join("package-lock.json");
+            std::fs::write(
+                &lock_file,
+                r#"{"name":"lock-test","lockfileVersion":3,"packages":{}}"#,
+            )?;
+
+            pkg.plugin.package_file = Some(pkg_file.to_str().unwrap().to_string());
+            pkg.plugin.package_filters = vec![pkg.name.clone()];
+            reroute_tools_root(&temp_path, pkg);
+
+            pkg.install(&new_task())?;
+            assert_eq!(
+                list.lock().unwrap().clone(),
+                [
+                    [NPM_COMMAND, "install", "--force", "test@1.0.0"],
+                    [NPM_COMMAND, "install", "--force", "--no-package-lock"]
+                ]
+            );
+
+            let staged_lock_file = Path::new(&pkg.directory()).join("package-lock.json");
+            assert!(!staged_lock_file.exists());
+            Ok(())
+        });
+    }
+
     #[test]
     fn node_package_install_with_extra_packages() {
         with_node_package(|pkg, temp_path, list| {

qlty-check/src/tool/node/package_json.rs

@@ -22,11 +22,12 @@ impl PackageJson {
         }
     }
 
+    // Returns whether the user's lock file was copied into the staging directory
     pub fn update_package_json(
         &self,
         tool_name: &str,
         package_file: &Option<String>,
-    ) -> Result<()> {
+    ) -> Result<bool> {
         let user_file_contents =
             std::fs::read_to_string(self.plugin.package_file.as_deref().unwrap_or_default())?;
         let mut user_json = serde_json::from_str::<Value>(&user_file_contents)?;
@@ -66,6 +67,8 @@ impl PackageJson {
         let final_package_file = serde_json::to_string_pretty(&user_json)?;
         debug!("Writing {} package.json: {}", tool_name, final_package_file);
 
+        let mut lock_file_staged = false;
+
         if self.plugin.package_filters.is_empty() {
             if let Some(package_file) = &self.plugin.package_file {
                 let package_file_path = PathBuf::from(package_file);
@@ -82,14 +85,15 @@ impl PackageJson {
                             staging_lock_file.display()
                         );
                         std::fs::copy(lock_file, staging_lock_file)?;
+                        lock_file_staged = true;
                     }
                 }
             }
         }
 
         std::fs::write(staged_file, final_package_file)?;
 
-        Ok(())
+        Ok(lock_file_staged)
     }
 
     // Filter out any dependencies that don't seem related to the plugin
@@ -115,7 +119,7 @@ impl PackageJson {
                 let path = PathBuf::from(package_file.clone().unwrap_or_default());
                 let parent_path = path.parent().unwrap().to_str().unwrap();
                 *value =
-                    Value::from(version_string.replace("file:", &format!("file:{}/", parent_path)));
+                    Value::from(version_string.replace("file:", &format!("file:{parent_path}/")));
             }
         }
     }