From 82fe8662abcb658e9b97da2c9ff57ba43574cf98 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 20 Mar 2025 16:25:27 -0700 Subject: [PATCH] Error on missing argument in `requirements.txt` (#12354) Closes https://github.com/astral-sh/uv/issues/12348. --- crates/uv-requirements-txt/src/lib.rs | 69 +++++++++++++++++++++------ 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/crates/uv-requirements-txt/src/lib.rs b/crates/uv-requirements-txt/src/lib.rs index 4307ca11d..f191f2b97 100644 --- a/crates/uv-requirements-txt/src/lib.rs +++ b/crates/uv-requirements-txt/src/lib.rs @@ -522,7 +522,7 @@ fn parse_entry( let start = s.cursor(); Ok(Some(if s.eat_if("-r") || s.eat_if("--requirement") { - let filename = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = parse_value("--requirement", content, s, |c: char| !is_terminal(c))?; let filename = unquote(filename) .ok() .flatten() @@ -534,7 +534,7 @@ fn parse_entry( end, } } else if s.eat_if("-c") || s.eat_if("--constraint") { - let filename = parse_value(content, s, |c: char| !is_terminal(c))?; + let filename = parse_value("--constraint", content, s, |c: char| !is_terminal(c))?; let filename = unquote(filename) .ok() .flatten() @@ -581,7 +581,7 @@ fn parse_entry( hashes, }) } else if s.eat_if("-i") || s.eat_if("--index-url") { - let given = parse_value(content, s, |c: char| !is_terminal(c))?; + let given = parse_value("--index-url", content, s, |c: char| !is_terminal(c))?; let given = unquote(given) .ok() .flatten() @@ -612,7 +612,7 @@ fn parse_entry( }; RequirementsTxtStatement::IndexUrl(url.with_given(given)) } else if s.eat_if("--extra-index-url") { - let given = parse_value(content, s, |c: char| !is_terminal(c))?; + let given = parse_value("--extra-index-url", content, s, |c: char| !is_terminal(c))?; let given = unquote(given) .ok() .flatten() @@ -645,7 +645,7 @@ fn parse_entry( } else if s.eat_if("--no-index") { RequirementsTxtStatement::NoIndex } else if s.eat_if("--find-links") || s.eat_if("-f") { - let given = parse_value(content, s, |c: char| !is_terminal(c))?; + let given = parse_value("--find-links", content, s, |c: char| !is_terminal(c))?; let given = unquote(given) .ok() .flatten() @@ -676,7 +676,7 @@ fn parse_entry( }; RequirementsTxtStatement::FindLinks(url.with_given(given)) } else if s.eat_if("--no-binary") { - let given = parse_value(content, s, |c: char| !is_terminal(c))?; + let given = parse_value("--no-binary", content, s, |c: char| !is_terminal(c))?; let given = unquote(given) .ok() .flatten() @@ -692,7 +692,7 @@ fn parse_entry( })?; RequirementsTxtStatement::NoBinary(NoBinary::from_pip_arg(specifier)) } else if s.eat_if("--only-binary") { - let given = parse_value(content, s, |c: char| !is_terminal(c))?; + let given = parse_value("--only-binary", content, s, |c: char| !is_terminal(c))?; let given = unquote(given) .ok() .flatten() @@ -878,14 +878,14 @@ fn parse_hashes(content: &str, s: &mut Scanner) -> Result, Requireme column, }); } - let hash = parse_value(content, s, |c: char| !c.is_whitespace())?; + let hash = parse_value("--hash", content, s, |c: char| !c.is_whitespace())?; hashes.push(hash.to_string()); loop { eat_wrappable_whitespace(s); if !s.eat_if("--hash") { break; } - let hash = parse_value(content, s, |c: char| !c.is_whitespace())?; + let hash = parse_value("--hash", content, s, |c: char| !c.is_whitespace())?; hashes.push(hash.to_string()); } Ok(hashes) @@ -893,25 +893,37 @@ fn parse_hashes(content: &str, s: &mut Scanner) -> Result, Requireme /// In `-=` or `- value`, this parses the part after the key fn parse_value<'a, T>( + option: &str, content: &str, s: &mut Scanner<'a>, while_pattern: impl Pattern, ) -> Result<&'a str, RequirementsTxtParserError> { - if s.eat_if('=') { + let value = if s.eat_if('=') { // Explicit equals sign. - Ok(s.eat_while(while_pattern).trim_end()) + s.eat_while(while_pattern).trim_end() } else if s.eat_if(char::is_whitespace) { // Key and value are separated by whitespace instead. s.eat_whitespace(); - Ok(s.eat_while(while_pattern).trim_end()) + s.eat_while(while_pattern).trim_end() } else { let (line, column) = calculate_row_column(content, s.cursor()); - Err(RequirementsTxtParserError::Parser { + return Err(RequirementsTxtParserError::Parser { message: format!("Expected '=' or whitespace, found {:?}", s.peek()), line, column, - }) + }); + }; + + if value.is_empty() { + let (line, column) = calculate_row_column(content, s.cursor()); + return Err(RequirementsTxtParserError::Parser { + message: format!("`{option}` must be followed by an argument"), + line, + column, + }); } + + Ok(value) } /// Fetch the contents of a URL and return them as a string. @@ -1780,6 +1792,35 @@ mod test { Ok(()) } + #[tokio::test] + async fn missing_value() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let requirements_txt = temp_dir.child("requirements.txt"); + requirements_txt.write_str(indoc! {" + flask + --no-binary + "})?; + + let error = RequirementsTxt::parse( + requirements_txt.path(), + temp_dir.path(), + &BaseClientBuilder::new(), + ) + .await + .unwrap_err(); + let errors = anyhow::Error::new(error).chain().join("\n"); + + let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string()); + let filters = vec![(requirement_txt.as_str(), "")]; + insta::with_settings!({ + filters => filters + }, { + insta::assert_snapshot!(errors, @"`--no-binary` must be followed by an argument at :3:1"); + }); + + Ok(()) + } + #[tokio::test] async fn missing_r() -> Result<()> { let temp_dir = assert_fs::TempDir::new()?;