Skip to content

Commit 90a76c1

Browse files
Sweep core utils notice, npm_range, pep440, pipenv, purl (#1920)
* Start sweep of core utils notice..purl Assisted-by: Claude Code:claude-opus-5-5 * Sweep core utils notice, npm_range, pep440, pipenv, purl Inline notice's is_muted and pipenv's resolve_on_path, name npm's operator set once, and share pep440's trailing-zero trim. Drop the JsrPurlParts alias and the is_purl wrapper, and make parse_name_version private. Delete pipenv's three PATH-resolution tests (process.rs tests the shared resolver) and purl's builder, round-trip and is_purl tests, which restate a format! or repeat the parse tests. Fold purl's three test modules into one, and drop history and banner comments. Behavior is unchanged. Assisted-by: Claude Code:claude-opus-5-5 * Keep one purl builder round-trip test Each builder now round-trips through its parser with a literal + version, replacing the 14 deleted builder and round-trip tests with one that fails when a builder and its parser disagree. The pipenv probe's unused root parameter is named _root. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 193efef commit 90a76c1

6 files changed

Lines changed: 135 additions & 432 deletions

File tree

‎crates/socket-patch-core/src/utils/notice.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,15 @@ pub fn is_quiet() -> bool {
3737
SILENT.load(Ordering::Relaxed) || JSON.load(Ordering::Relaxed)
3838
}
3939

40-
/// Whether an advisory of `level` is suppressed in the current mode.
41-
pub(crate) fn is_muted(level: Notice) -> bool {
42-
match level {
43-
Notice::Info => is_quiet(),
44-
Notice::Warning => SILENT.load(Ordering::Relaxed),
45-
}
46-
}
47-
4840
/// Print `msg()` to stderr unless `level` is muted or `shown` already
4941
/// fired. The message is built lazily so a suppressed advisory costs
5042
/// nothing, and a muted call does not consume the once.
5143
pub(crate) fn notice_once(level: Notice, shown: &AtomicBool, msg: impl FnOnce() -> String) {
52-
if is_muted(level) {
44+
let muted = match level {
45+
Notice::Info => is_quiet(),
46+
Notice::Warning => SILENT.load(Ordering::Relaxed),
47+
};
48+
if muted {
5349
return;
5450
}
5551
if !shown.swap(true, Ordering::Relaxed) {

‎crates/socket-patch-core/src/utils/npm_range.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,17 @@ pub(crate) fn npm_range_admits(range: &str, version: &str) -> Option<bool> {
3030
Some(admits)
3131
}
3232

33+
fn is_operator(c: char) -> bool {
34+
matches!(c, '^' | '~' | '>' | '<' | '=')
35+
}
36+
3337
/// The comparators of one npm range alternative, an operator npm allows
3438
/// to stand apart (`>= 1.2.0`) joined to its version.
3539
fn glue_operators(alternative: &str) -> Vec<String> {
3640
let mut out: Vec<String> = Vec::new();
3741
let mut pending = String::new();
3842
for token in alternative.split_whitespace() {
39-
if token
40-
.chars()
41-
.all(|c| matches!(c, '^' | '~' | '>' | '<' | '='))
42-
{
43+
if token.chars().all(is_operator) {
4344
pending.push_str(token);
4445
} else {
4546
out.push(format!("{}{token}", std::mem::take(&mut pending)));
@@ -53,7 +54,7 @@ fn glue_operators(alternative: &str) -> Vec<String> {
5354
/// crate reads both as caret ranges.
5455
fn npm_comparator(comparator: &str) -> Option<String> {
5556
let op_len = comparator
56-
.find(|c: char| !matches!(c, '^' | '~' | '>' | '<' | '='))
57+
.find(|c: char| !is_operator(c))
5758
.unwrap_or(comparator.len());
5859
let (op, version) = comparator.split_at(op_len);
5960
let version = version.strip_prefix('v').unwrap_or(version);

‎crates/socket-patch-core/src/utils/pep440.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ fn number(digits: &str) -> String {
3535
if trimmed.is_empty() { "0" } else { trimmed }.to_owned()
3636
}
3737

38+
/// Drop trailing `0` release segments, keeping at least one.
39+
fn trim_trailing_zeros(release: &mut Vec<String>) {
40+
while release.len() > 1 && release.last().is_some_and(|segment| segment == "0") {
41+
release.pop();
42+
}
43+
}
44+
3845
#[derive(Debug, PartialEq, Eq)]
3946
enum LocalPart {
4047
Number(String),
@@ -107,13 +114,8 @@ pub(crate) fn versions_equal(a: &str, b: &str) -> bool {
107114
match (parse(a), parse(b)) {
108115
(Some(mut a), Some(mut b)) => {
109116
// Equality ignores trailing `.0` release segments.
110-
for version in [&mut a, &mut b] {
111-
while version.release.len() > 1
112-
&& version.release.last().is_some_and(|segment| segment == "0")
113-
{
114-
version.release.pop();
115-
}
116-
}
117+
trim_trailing_zeros(&mut a.release);
118+
trim_trailing_zeros(&mut b.release);
117119
a == b
118120
}
119121
_ => false,
@@ -192,9 +194,7 @@ pub(crate) fn equivalent_release_spellings(version: &str) -> Vec<String> {
192194
}
193195
let segments: Vec<String> = version.split('.').map(number).collect();
194196
let mut trimmed = segments.clone();
195-
while trimmed.len() > 1 && trimmed.last().is_some_and(|segment| segment == "0") {
196-
trimmed.pop();
197-
}
197+
trim_trailing_zeros(&mut trimmed);
198198
let mut out = Vec::new();
199199
for len in trimmed.len()..=segments.len().max(3) {
200200
let mut release = trimmed.clone();
@@ -286,7 +286,7 @@ mod tests {
286286
}
287287

288288
/// What `str(packaging.version.Version(v))` prints, which is what
289-
/// uv.lock, wheel METADATA and PyPI record (#1589).
289+
/// uv.lock, wheel METADATA and PyPI record.
290290
#[test]
291291
fn normalize_matches_packaging() {
292292
for (raw, normalized) in [

‎crates/socket-patch-core/src/utils/pipenv.rs‎

Lines changed: 4 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
//! the vendored backend refuses installers older than 2018, so both ask
44
//! [`installed_major`] once per command.
55
6-
use std::ffi::OsString;
7-
use std::path::{Path, PathBuf};
6+
use std::path::Path;
87

98
/// Pins the answer without spawning anything: CI images without pipenv on
109
/// PATH, or a project installed with a different release than the machine's
@@ -39,38 +38,24 @@ fn parse_major(output: &str) -> Option<u32> {
3938
major.parse().ok()
4039
}
4140

42-
/// The `pipenv` executable, searched on ABSOLUTE `PATH` entries only: the
43-
/// CLI runs inside the project being scanned, so a relative entry (`.`, an
44-
/// empty component) would execute a `pipenv` planted in that repository. On Windows every `PATHEXT` extension is tried,
45-
/// so `pipenv.exe` and the `pipenv.bat` / `pipenv.cmd` shims (pyenv-win,
46-
/// hand-written wrappers) are both found. The rule lives in
47-
/// [`crate::utils::process::resolve_tool_with`], shared with every other
48-
/// tool the CLI spawns inside a project (`git`, `vlt`, `node`).
49-
fn resolve_on_path(var: &impl Fn(&str) -> Option<OsString>) -> Option<PathBuf> {
50-
crate::utils::process::resolve_tool_with("pipenv", var)
51-
}
52-
5341
/// The major of the `pipenv` on PATH (`11`, `2018`, `2026`, …), or `None`
5442
/// when none is found, it does not answer within 10 s, exits non-zero or
5543
/// prints something unrecognizable. [`MAJOR_OVERRIDE_ENV`] short-circuits
5644
/// the probe.
57-
pub async fn installed_major(root: &Path) -> Option<u32> {
45+
pub async fn installed_major(_root: &Path) -> Option<u32> {
5846
if let Some(forced) = std::env::var(MAJOR_OVERRIDE_ENV)
5947
.ok()
6048
.and_then(|value| parse_override(&value))
6149
{
6250
return Some(forced);
6351
}
64-
let program = resolve_on_path(&|name| std::env::var_os(name))?;
65-
// The RESOLVED path is spawned; a Windows `.bat` / `.cmd` shim is run by
66-
// `std` itself through cmd.exe with correct quoting (see the shared
67-
// launcher's docs).
52+
// Absolute PATH entries only, so a repo-planted `pipenv` never runs.
53+
let program = crate::utils::process::resolve_tool("pipenv")?;
6854
let mut command = crate::utils::process::command_for(&program);
6955
// The version banner does not depend on a project, so the probe runs in a
7056
// NEUTRAL directory: with the scanned repository as cwd, Pipenv would read
7157
// its `.env`, `Pipfile` and `.venv` pointer — committed, attacker-shaped
7258
// inputs that must not influence (or slow down) a version check.
73-
let _ = root;
7459
command
7560
.arg("--version")
7661
.current_dir(std::env::temp_dir())
@@ -121,68 +106,6 @@ mod tests {
121106
);
122107
}
123108

124-
#[test]
125-
fn resolve_on_path_skips_relative_entries_and_finds_absolute_ones() {
126-
let tmp = tempfile::tempdir().unwrap();
127-
let bin = tmp.path().join("bin");
128-
std::fs::create_dir_all(&bin).unwrap();
129-
let leaf = if cfg!(windows) {
130-
"pipenv.exe"
131-
} else {
132-
"pipenv"
133-
};
134-
std::fs::write(bin.join(leaf), b"").unwrap();
135-
#[cfg(unix)]
136-
{
137-
use std::os::unix::fs::PermissionsExt;
138-
std::fs::set_permissions(bin.join(leaf), std::fs::Permissions::from_mode(0o755))
139-
.unwrap();
140-
}
141-
// A repo-planted `pipenv` under a RELATIVE entry must never win.
142-
let planted = tmp.path().join("planted");
143-
std::fs::create_dir_all(&planted).unwrap();
144-
std::fs::write(planted.join(leaf), b"").unwrap();
145-
let joined = std::env::join_paths([
146-
std::path::PathBuf::from("."),
147-
std::path::PathBuf::from(""),
148-
std::path::PathBuf::from("planted"),
149-
bin.clone(),
150-
])
151-
.unwrap();
152-
let var = |name: &str| (name == "PATH").then(|| joined.clone());
153-
assert_eq!(resolve_on_path(&var), Some(bin.join(leaf)));
154-
155-
let only_relative = std::env::join_paths([
156-
std::path::PathBuf::from("."),
157-
std::path::PathBuf::from("planted"),
158-
])
159-
.unwrap();
160-
let var = |name: &str| (name == "PATH").then(|| only_relative.clone());
161-
assert_eq!(resolve_on_path(&var), None);
162-
let none = |_: &str| None::<OsString>;
163-
assert_eq!(resolve_on_path(&none), None);
164-
}
165-
166-
#[cfg(unix)]
167-
#[test]
168-
fn resolve_on_path_skips_non_executable_files() {
169-
use std::os::unix::fs::PermissionsExt;
170-
let tmp = tempfile::tempdir().unwrap();
171-
let data = tmp.path().join("data");
172-
let bin = tmp.path().join("bin");
173-
std::fs::create_dir_all(&data).unwrap();
174-
std::fs::create_dir_all(&bin).unwrap();
175-
std::fs::write(data.join("pipenv"), b"not a program").unwrap();
176-
std::fs::set_permissions(data.join("pipenv"), std::fs::Permissions::from_mode(0o644))
177-
.unwrap();
178-
std::fs::write(bin.join("pipenv"), b"").unwrap();
179-
std::fs::set_permissions(bin.join("pipenv"), std::fs::Permissions::from_mode(0o755))
180-
.unwrap();
181-
let joined = std::env::join_paths([data.clone(), bin.clone()]).unwrap();
182-
let var = |name: &str| (name == "PATH").then(|| joined.clone());
183-
assert_eq!(resolve_on_path(&var), Some(bin.join("pipenv")));
184-
}
185-
186109
#[test]
187110
fn override_accepts_a_major_or_a_full_release() {
188111
assert_eq!(parse_override("11"), Some(11));
@@ -193,32 +116,4 @@ mod tests {
193116
assert_eq!(parse_override(""), None);
194117
assert_eq!(parse_override("-11"), None);
195118
}
196-
197-
#[cfg(windows)]
198-
#[test]
199-
fn resolve_on_path_honours_pathext_shims() {
200-
let tmp = tempfile::tempdir().unwrap();
201-
let bin = tmp.path().join("bin");
202-
std::fs::create_dir_all(&bin).unwrap();
203-
std::fs::write(bin.join("pipenv.bat"), b"@echo pipenv, version 2024.4.1").unwrap();
204-
let joined = std::env::join_paths([bin.clone()]).unwrap();
205-
let var = |name: &str| match name {
206-
"PATH" => Some(joined.clone()),
207-
"PATHEXT" => Some(OsString::from(".COM;.EXE;.BAT;.CMD")),
208-
_ => None,
209-
};
210-
let found = resolve_on_path(&var).unwrap();
211-
assert_eq!(found, bin.join("pipenv.bat"));
212-
// Spawned directly: std runs the .bat through cmd.exe itself, and the
213-
// banner parses like a real pipenv's.
214-
let out = crate::utils::process::command_for(&found)
215-
.arg("--version")
216-
.output()
217-
.expect("std spawns a .bat shim");
218-
assert!(out.status.success(), "{out:?}");
219-
assert_eq!(
220-
parse_major(&String::from_utf8_lossy(&out.stdout)),
221-
Some(2024)
222-
);
223-
}
224119
}

0 commit comments

Comments
 (0)