Warn on unclosed script tags (#6704)

Should this be user-facing by default? It seems annoying because then
it's unavoidable if you (for whatever reason) have an intentionally
unclosed tag.

Motivated by https://github.com/astral-sh/uv/issues/6700.
This commit is contained in:
Charlie Marsh 2024-08-27 13:47:11 -04:00 committed by GitHub
parent eb14056e9c
commit a8f4e08d5b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 42 additions and 9 deletions

View file

@ -5,10 +5,10 @@ use std::str::FromStr;
use std::sync::LazyLock; use std::sync::LazyLock;
use memchr::memmem::Finder; use memchr::memmem::Finder;
use pep440_rs::VersionSpecifiers;
use serde::Deserialize; use serde::Deserialize;
use thiserror::Error; use thiserror::Error;
use pep440_rs::VersionSpecifiers;
use pep508_rs::PackageName; use pep508_rs::PackageName;
use pypi_types::VerbatimParsedUrl; use pypi_types::VerbatimParsedUrl;
use uv_settings::{GlobalOptions, ResolverInstallerOptions}; use uv_settings::{GlobalOptions, ResolverInstallerOptions};
@ -41,13 +41,14 @@ impl Pep723Script {
}; };
// Extract the `script` tag. // Extract the `script` tag.
let Some(ScriptTag { let ScriptTag {
prelude, prelude,
metadata, metadata,
postlude, postlude,
}) = ScriptTag::parse(&contents)? } = match ScriptTag::parse(&contents) {
else { Ok(Some(tag)) => tag,
return Ok(None); Ok(None) => return Ok(None),
Err(err) => return Err(err),
}; };
// Parse the metadata. // Parse the metadata.
@ -152,6 +153,8 @@ pub struct ToolUv {
#[derive(Debug, Error)] #[derive(Debug, Error)]
pub enum Pep723Error { pub enum Pep723Error {
#[error("An opening tag (`# /// script`) was found without a closing tag (`# ///`). Ensure that every line between the opening and closing tags (including empty lines) starts with a leading `#`.")]
UnclosedBlock,
#[error(transparent)] #[error(transparent)]
Io(#[from] io::Error), Io(#[from] io::Error),
#[error(transparent)] #[error(transparent)]
@ -272,7 +275,7 @@ impl ScriptTag {
// //
// The latter `///` is the closing pragma // The latter `///` is the closing pragma
let Some(index) = toml.iter().rev().position(|line| *line == "///") else { let Some(index) = toml.iter().rev().position(|line| *line == "///") else {
return Ok(None); return Err(Pep723Error::UnclosedBlock);
}; };
let index = toml.len() - index; let index = toml.len() - index;
@ -364,7 +367,7 @@ fn serialize_metadata(metadata: &str) -> String {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use crate::{serialize_metadata, ScriptTag}; use crate::{serialize_metadata, Pep723Error, ScriptTag};
#[test] #[test]
fn missing_space() { fn missing_space() {
@ -374,7 +377,10 @@ mod tests {
# /// # ///
"}; "};
assert_eq!(ScriptTag::parse(contents.as_bytes()).unwrap(), None); assert!(matches!(
ScriptTag::parse(contents.as_bytes()),
Err(Pep723Error::UnclosedBlock)
));
} }
#[test] #[test]
@ -388,7 +394,10 @@ mod tests {
# ] # ]
"}; "};
assert_eq!(ScriptTag::parse(contents.as_bytes()).unwrap(), None); assert!(matches!(
ScriptTag::parse(contents.as_bytes()),
Err(Pep723Error::UnclosedBlock)
));
} }
#[test] #[test]

View file

@ -359,6 +359,30 @@ fn run_pep723_script() -> Result<()> {
Because there are no versions of add and you require add, we can conclude that your requirements are unsatisfiable. Because there are no versions of add and you require add, we can conclude that your requirements are unsatisfiable.
"###); "###);
// If the script contains an unclosed PEP 723 tag, we should error.
let test_script = context.temp_dir.child("main.py");
test_script.write_str(indoc! { r#"
# /// script
# requires-python = ">=3.11"
# dependencies = [
# "iniconfig",
# ]
# ///
import iniconfig
"#
})?;
uv_snapshot!(context.filters(), context.run().arg("--no-project").arg("main.py"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: An opening tag (`# /// script`) was found without a closing tag (`# ///`). Ensure that every line between the opening and closing tags (including empty lines) starts with a leading `#`.
"###);
Ok(()) Ok(())
} }