Skip to content

Commit

Permalink
fix(npm): use more relaxed package.json version constraint parsing (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret authored and bartlomieju committed Jun 18, 2024
1 parent 22f2779 commit b0fc111
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 22 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ deno_lint = { version = "=0.60.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.21.4"
deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_semver = "=0.5.4"
deno_semver = "=0.5.6"
deno_task_shell = "=0.16.1"
deno_terminal.workspace = true
eszip = "=0.71.0"
Expand Down
34 changes: 27 additions & 7 deletions cli/args/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ use deno_core::anyhow::bail;
use deno_core::error::AnyError;
use deno_npm::registry::parse_dep_entry_name_and_raw_version;
use deno_runtime::deno_node::PackageJson;
use deno_semver::npm::NpmVersionReqParseError;
use deno_semver::package::PackageReq;
use deno_semver::VersionReq;
use deno_semver::VersionReqSpecifierParseError;
use indexmap::IndexMap;
use thiserror::Error;

#[derive(Debug, Error, Clone)]
pub enum PackageJsonDepValueParseError {
#[error(transparent)]
Specifier(#[from] VersionReqSpecifierParseError),
VersionReq(#[from] NpmVersionReqParseError),
#[error("Not implemented scheme '{scheme}'")]
Unsupported { scheme: String },
}
Expand Down Expand Up @@ -75,13 +75,13 @@ pub fn get_local_package_json_version_reqs(
});
}
let (name, version_req) = parse_dep_entry_name_and_raw_version(key, value);
let result = VersionReq::parse_from_specifier(version_req);
let result = VersionReq::parse_from_npm(version_req);
match result {
Ok(version_req) => Ok(PackageReq {
name: name.to_string(),
version_req,
}),
Err(err) => Err(PackageJsonDepValueParseError::Specifier(err)),
Err(err) => Err(PackageJsonDepValueParseError::VersionReq(err)),
}
}

Expand Down Expand Up @@ -208,7 +208,7 @@ mod test {
let mut package_json = PackageJson::empty(PathBuf::from("/package.json"));
package_json.dependencies = Some(IndexMap::from([(
"test".to_string(),
"1.x - 1.3".to_string(),
"%*(#$%()".to_string(),
)]));
let map = get_local_package_json_version_reqs_for_tests(&package_json);
assert_eq!(
Expand All @@ -217,8 +217,8 @@ mod test {
"test".to_string(),
Err(
concat!(
"Invalid specifier version requirement. Unexpected character.\n",
" - 1.3\n",
"Invalid npm version requirement. Unexpected character.\n",
" %*(#$%()\n",
" ~"
)
.to_string()
Expand All @@ -227,6 +227,26 @@ mod test {
);
}

#[test]
fn test_get_local_package_json_version_reqs_range() {
let mut package_json = PackageJson::empty(PathBuf::from("/package.json"));
package_json.dependencies = Some(IndexMap::from([(
"test".to_string(),
"1.x - 1.3".to_string(),
)]));
let map = get_local_package_json_version_reqs_for_tests(&package_json);
assert_eq!(
map,
IndexMap::from([(
"test".to_string(),
Ok(PackageReq {
name: "test".to_string(),
version_req: VersionReq::parse_from_npm("1.x - 1.3").unwrap()
})
)])
);
}

#[test]
fn test_get_local_package_json_version_reqs_skips_certain_specifiers() {
let mut package_json = PackageJson::empty(PathBuf::from("/package.json"));
Expand Down
17 changes: 8 additions & 9 deletions cli/standalone/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use deno_core::futures::AsyncSeekExt;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_npm::NpmSystemInfo;
use deno_semver::npm::NpmVersionReqParseError;
use deno_semver::package::PackageReq;
use deno_semver::VersionReqSpecifierParseError;
use log::Level;
Expand Down Expand Up @@ -55,15 +56,15 @@ const MAGIC_TRAILER: &[u8; 8] = b"d3n0l4nd";

#[derive(Serialize, Deserialize)]
enum SerializablePackageJsonDepValueParseError {
Specifier(String),
VersionReq(String),
Unsupported { scheme: String },
}

impl SerializablePackageJsonDepValueParseError {
pub fn from_err(err: PackageJsonDepValueParseError) -> Self {
match err {
PackageJsonDepValueParseError::Specifier(err) => {
Self::Specifier(err.source.to_string())
PackageJsonDepValueParseError::VersionReq(err) => {
Self::VersionReq(err.source.to_string())
}
PackageJsonDepValueParseError::Unsupported { scheme } => {
Self::Unsupported { scheme }
Expand All @@ -73,12 +74,10 @@ impl SerializablePackageJsonDepValueParseError {

pub fn into_err(self) -> PackageJsonDepValueParseError {
match self {
SerializablePackageJsonDepValueParseError::Specifier(source) => {
PackageJsonDepValueParseError::Specifier(
VersionReqSpecifierParseError {
source: monch::ParseErrorFailureError::new(source),
},
)
SerializablePackageJsonDepValueParseError::VersionReq(source) => {
PackageJsonDepValueParseError::VersionReq(NpmVersionReqParseError {
source: monch::ParseErrorFailureError::new(source),
})
}
SerializablePackageJsonDepValueParseError::Unsupported { scheme } => {
PackageJsonDepValueParseError::Unsupported { scheme }
Expand Down
2 changes: 1 addition & 1 deletion tests/specs/npm/npmrc_basic_auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "npmrc_test",
"version": "0.0.1",
"dependencies": {
"@denotest/basic": "1.0.0",
"@denotest/basic": "^=1.0.0",
"@denotest2/basic": "1.0.0"
}
}
2 changes: 1 addition & 1 deletion tests/testdata/package_json/invalid_value/error.ts.out
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
error: Parsing version constraints in the application-level package.json is more strict at the moment.

Invalid specifier version requirement. Unexpected character.
Invalid npm version requirement. Unexpected character.
invalid stuff that won't parse
~
at file:///[WILDCARD]/error.ts:2:23
2 changes: 1 addition & 1 deletion tests/testdata/package_json/invalid_value/task.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Warning Ignoring dependency '@denotest/cjs-default-export' in package.json because its version requirement failed to parse: Invalid specifier version requirement. Unexpected character.
Warning Ignoring dependency '@denotest/cjs-default-export' in package.json because its version requirement failed to parse: Invalid npm version requirement. Unexpected character.
invalid stuff that won't parse
~
Task test echo 1
Expand Down

0 comments on commit b0fc111

Please sign in to comment.