JavaScript tool version detection can ignore valid fields when another package.json version field is invalid

[tomdale]

Summary

I ran into a case where JavaScript tool version detection did not read a valid version field because another, unrelated package.json version field could not be parsed.

The broader category seems to be: a parse failure in one package.json version-like field can make detection ignore another field that would have been valid for the tool currently being detected.

Reduced Reproduction

This package.json has a valid pnpm version in devEngines.packageManager, plus an unrelated invalid Node engine:

{
  "engines": {
    "node": ">= nope"
  },
  "devEngines": {
    "packageManager": {
      "name": "pnpm",
      "version": "1.2.3"
    }
  }
}

Add this test to tools/node-depman/tests/versions_test.rs under the pnpm module:

#[tokio::test(flavor = "multi_thread")]
async fn parses_dev_engines_when_unrelated_engine_is_invalid() {
    let sandbox = create_empty_proto_sandbox();
    let plugin = sandbox.create_plugin("pnpm-test").await;

    assert_eq!(
        plugin
            .parse_version_file(ParseVersionFileInput {
                content: r#"{ "engines": { "node": ">= nope" }, "devEngines": { "packageManager": { "name": "pnpm", "version": "1.2.3" } } }"#.into(),
                file: "package.json".into(),
                ..Default::default()
            })
            .await,
        ParseVersionFileOutput {
            version: Some(UnresolvedVersionSpec::parse("1.2.3").unwrap()),
        }
    );
}

Then run:

cargo build -p node_depman_tool --target wasm32-wasip1
cargo test -p node_depman_tool --no-default-features parses_dev_engines_when_unrelated_engine_is_invalid

Actual output:

test node_depman_tool::pnpm::parses_dev_engines_when_unrelated_engine_is_invalid ... FAILED

assertion `left == right` failed
  left: ParseVersionFileOutput { version: None }
 right: ParseVersionFileOutput { version: Some(Semantic(SemVer(Version { major: 1, minor: 2, patch: 3 }))) }

Expected Behavior

I would expect the dependency manager plugin to still detect pnpm 1.2.3 from devEngines.packageManager, even though engines.node is not usable.

Notes

This is related to #158, but I think it is a separate failure mode. Even if comparator whitespace is handled correctly, an unrelated invalid version field can still prevent reading a valid package manager version.

If my reading is right, a field-local extraction path might make detection more resilient:

• read the raw devEngines.runtime
• read the raw devEngines.packageManager
• read the raw engines
• read the raw volta
• read the raw packageManager

Then parse only the selected version string for the tool currently being detected.

I may be missing some context, but this seems like it would avoid unrelated package.json fields affecting each other during version detection.

[tomdale]

I opened #161 as an attempted fix for this. The PR changes JavaScript version detection to parse package.json as raw JSON first, then extract only the fields relevant to the active tool before parsing the selected version candidate.