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`).
This commit is contained in:
Charlie Marsh 2024-01-30 10:55:11 -08:00 committed by GitHub
parent 4ad0dc8b9e
commit 7a937e0f60
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 104 additions and 1 deletions

View File

@ -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(), "<REQUIREMENTS_TXT>")]
}, {
insta::assert_display_snapshot!(errors, @"Requirement `file.txt` in `<REQUIREMENTS_TXT>` 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!(