From 7a937e0f60485ee85677da63a82976a3c52d41e4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 30 Jan 2024 10:55:11 -0800 Subject: [PATCH] Error when parsing `requirements.txt`-like packages in `requirements.txt` file (#1179) ## Summary Like https://github.com/astral-sh/puffin/pull/1180, this PR adds logic for `requirements.txt` parsing whereby if a requirement _looks like_ a local requirements file or an editable directory, we prompt the user to correct the error (typically, by adding `-r`). --- crates/requirements-txt/src/lib.rs | 105 ++++++++++++++++++++++++++++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index 3eea9c36a..b8825e69b 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -34,6 +34,7 @@ //! wrappable_whitespaces = whitespace ('\\\n' | whitespace)* //! ``` +use std::borrow::Cow; use std::fmt::{Display, Formatter}; use std::io; use std::path::{Path, PathBuf}; @@ -573,8 +574,49 @@ fn parse_requirement_and_hashes( break (end, false); } }; + + let requirement = &content[start..end]; + + // If the requirement looks like a `requirements.txt` file (with a missing `-r`), raise an + // error. + // + // While `requirements.txt` is a valid package name (per the spec), PyPI disallows + // `requirements.txt` and some other variants anyway. + #[allow(clippy::case_sensitive_file_extension_comparisons)] + if requirement.ends_with(".txt") || requirement.ends_with(".in") { + let path = Path::new(requirement); + let path = if path.is_absolute() { + Cow::Borrowed(path) + } else { + Cow::Owned(working_dir.join(path)) + }; + if path.is_file() { + return Err(RequirementsTxtParserError::MissingRequirementPrefix( + requirement.to_string(), + )); + } + } + + // If the requirement looks like an editable requirement (with a missing `-e`), raise an + // error. + // + // Slashes are not allowed in package names, so these would be rejected in the next step anyway. + if requirement.contains('/') || requirement.contains('\\') { + let path = Path::new(requirement); + let path = if path.is_absolute() { + Cow::Borrowed(path) + } else { + Cow::Owned(working_dir.join(path)) + }; + if path.is_dir() { + return Err(RequirementsTxtParserError::MissingEditablePrefix( + requirement.to_string(), + )); + } + } + let requirement = - Requirement::parse(&content[start..end], working_dir).map_err(|err| match err.message { + Requirement::parse(requirement, working_dir).map_err(|err| match err.message { Pep508ErrorSource::String(_) | Pep508ErrorSource::UrlError(_) => { RequirementsTxtParserError::Pep508 { source: err, @@ -664,6 +706,8 @@ pub enum RequirementsTxtParserError { }, InvalidEditablePath(String), UnsupportedUrl(String), + MissingRequirementPrefix(String), + MissingEditablePrefix(String), Parser { message: String, location: usize, @@ -708,6 +752,12 @@ impl RequirementsTxtParserError { RequirementsTxtParserError::UnsupportedUrl(url) => { RequirementsTxtParserError::UnsupportedUrl(url) } + RequirementsTxtParserError::MissingRequirementPrefix(given) => { + RequirementsTxtParserError::MissingRequirementPrefix(given) + } + RequirementsTxtParserError::MissingEditablePrefix(given) => { + RequirementsTxtParserError::MissingEditablePrefix(given) + } RequirementsTxtParserError::Parser { message, location } => { RequirementsTxtParserError::Parser { message, @@ -752,6 +802,15 @@ impl Display for RequirementsTxtParserError { RequirementsTxtParserError::UnsupportedUrl(url) => { write!(f, "Unsupported URL (expected a `file://` scheme): `{url}`") } + RequirementsTxtParserError::MissingRequirementPrefix(given) => { + write!(f, "Requirement `{given}` looks like a requirements file but was passed as a package name. Did you mean `-r {given}`?") + } + RequirementsTxtParserError::MissingEditablePrefix(given) => { + write!( + f, + "Requirement `{given}` looks like a directory but was passed as a package name. Did you mean `-e {given}`?" + ) + } RequirementsTxtParserError::Parser { message, location } => { write!(f, "{message} at position {location}") } @@ -775,6 +834,8 @@ impl std::error::Error for RequirementsTxtParserError { RequirementsTxtParserError::Url { source, .. } => Some(source), RequirementsTxtParserError::InvalidEditablePath(_) => None, RequirementsTxtParserError::UnsupportedUrl(_) => None, + RequirementsTxtParserError::MissingRequirementPrefix(_) => None, + RequirementsTxtParserError::MissingEditablePrefix(_) => None, RequirementsTxtParserError::UnsupportedRequirement { source, .. } => Some(source), RequirementsTxtParserError::Pep508 { source, .. } => Some(source), RequirementsTxtParserError::Subfile { source, .. } => Some(source.as_ref()), @@ -808,6 +869,20 @@ impl Display for RequirementsTxtFileError { self.file.normalized_display(), ) } + RequirementsTxtParserError::MissingRequirementPrefix(given) => { + write!( + f, + "Requirement `{given}` in `{}` looks like a requirements file but was passed as a package name. Did you mean `-r {given}`?", + self.file.normalized_display(), + ) + } + RequirementsTxtParserError::MissingEditablePrefix(given) => { + write!( + f, + "Requirement `{given}` in `{}` looks like a directory but was passed as a package name. Did you mean `-e {given}`?", + self.file.normalized_display(), + ) + } RequirementsTxtParserError::Parser { message, location } => { write!( f, @@ -1062,6 +1137,34 @@ mod test { Ok(()) } + #[test] + fn missing_r() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + + let file_txt = temp_dir.child("file.txt"); + file_txt.touch()?; + + let requirements_txt = temp_dir.child("requirements.txt"); + requirements_txt.write_str(indoc! {" + flask + file.txt + "})?; + + let error = RequirementsTxt::parse(requirements_txt.path(), temp_dir.path()).unwrap_err(); + let errors = anyhow::Error::new(error) + .chain() + .map(|err| err.to_string().replace('\\', "/")) + .join("\n"); + + insta::with_settings!({ + filters => vec![(requirements_txt.path().to_str().unwrap(), "")] + }, { + insta::assert_display_snapshot!(errors, @"Requirement `file.txt` in `` looks like a requirements file but was passed as a package name. Did you mean `-r file.txt`?"); + }); + + Ok(()) + } + #[test] fn editable_extra() { assert_eq!(