Allow semicolons directly after direct URLs (#8836)

## Summary Like pip, we now allow the semicolon to directly proceed the URL (but require that it's either preceded or followed by a space): ``` # OK ./test.whl; sys_platform == 'darwin' # OK ./test.whl ;sys_platform == 'darwin' # Error ./test.whl;sys_platform == 'darwin' ``` Closes #8831.
astral-sh · Nov 5, 2024 · d238642 · d238642
1 parent 515993c
commit d238642
Show file tree

Hide file tree

Showing 11 changed files with 290 additions and 114 deletions.
diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs
@@ -709,18 +709,17 @@ fn parse_url<T: Pep508Url>(
             len += c.len_utf8();
 
             // If we see a top-level semicolon or hash followed by whitespace, we're done.
-            match c {
-                ';' if cursor.peek_char().is_some_and(char::is_whitespace) => {
-                    break;
-                }
-                '#' if cursor.peek_char().is_some_and(char::is_whitespace) => {
+            if cursor.peek_char().is_some_and(|c| matches!(c, ';' | '#')) {
+                let mut cursor = cursor.clone();
+                cursor.next();
+                if cursor.peek_char().is_some_and(char::is_whitespace) {
                     break;
                 }
-                _ => {}
             }
         }
         (start, len)
     };
+
     let url = cursor.slice(start, len);
     if url.is_empty() {
         return Err(Pep508Error {
@@ -731,19 +730,6 @@ fn parse_url<T: Pep508Url>(
         });
     }
 
-    for c in [';', '#'] {
-        if url.ends_with(c) {
-            return Err(Pep508Error {
-                message: Pep508ErrorSource::String(format!(
-                    "Missing space before '{c}', the end of the URL is ambiguous"
-                )),
-                start: start + len - 1,
-                len: 1,
-                input: cursor.to_string(),
-            });
-        }
-    }
-
     let url = T::parse_url(url, working_dir).map_err(|err| Pep508Error {
         message: Pep508ErrorSource::UrlError(err),
         start,
@@ -970,8 +956,13 @@ fn parse_pep508_requirement<T: Pep508Url>(
 
     // wsp*
     cursor.eat_whitespace();
-    if let Some((pos, char)) = cursor.next() {
-        let message = if marker.is_none() {
+
+    if let Some((pos, char)) = cursor.next().filter(|(_, c)| *c != '#') {
+        let message = if char == '#' {
+            format!(
+                r#"Expected end of input or `;`, found `{char}`; comments must be preceded by a leading space"#
+            )
+        } else if marker.is_none() {
             format!(r#"Expected end of input or `;`, found `{char}`"#)
         } else {
             format!(r#"Expected end of input, found `{char}`"#)

diff --git a/crates/uv-pep508/src/tests.rs b/crates/uv-pep508/src/tests.rs
@@ -545,42 +545,6 @@ fn error_extras_not_closed() {
     );
 }
 
-#[test]
-fn error_no_space_after_url() {
-    assert_snapshot!(
-        parse_pep508_err(r"name @ https://example.com/; extra == 'example'"),
-        @r#"
-    Missing space before ';', the end of the URL is ambiguous
-    name @ https://example.com/; extra == 'example'
-                               ^
-    "#
-    );
-}
-
-#[test]
-fn error_no_space_after_file_url() {
-    assert_snapshot!(
-        parse_pep508_err(r"name @ file:///test.whl; extra == 'example'"),
-        @r###"
-    Missing space before ';', the end of the URL is ambiguous
-    name @ file:///test.whl; extra == 'example'
-                           ^
-    "###
-    );
-}
-
-#[test]
-fn error_no_space_after_file_path() {
-    assert_snapshot!(
-        parse_pep508_err(r"name @ ./test.whl; extra == 'example'"),
-        @r###"
-    Missing space before ';', the end of the URL is ambiguous
-    name @ ./test.whl; extra == 'example'
-                     ^
-    "###
-    );
-}
-
 #[test]
 fn error_name_at_nothing() {
     assert_snapshot!(

diff --git a/crates/uv-pep508/src/unnamed.rs b/crates/uv-pep508/src/unnamed.rs
@@ -174,7 +174,11 @@ fn parse_unnamed_requirement<Url: UnnamedRequirementUrl>(
     // wsp*
     cursor.eat_whitespace();
     if let Some((pos, char)) = cursor.next() {
-        let message = if marker.is_none() {
+        let message = if char == '#' {
+            format!(
+                r#"Expected end of input or `;`, found `{char}`; comments must be preceded by a leading space"#
+            )
+        } else if marker.is_none() {
             format!(r#"Expected end of input or `;`, found `{char}`"#)
         } else {
             format!(r#"Expected end of input, found `{char}`"#)
@@ -388,15 +392,11 @@ fn parse_unnamed_url<Url: UnnamedRequirementUrl>(
             len += c.len_utf8();
 
             // If we see a top-level semicolon or hash followed by whitespace, we're done.
-            if depth == 0 {
-                match c {
-                    ';' if cursor.peek_char().is_some_and(char::is_whitespace) => {
-                        break;
-                    }
-                    '#' if cursor.peek_char().is_some_and(char::is_whitespace) => {
-                        break;
-                    }
-                    _ => {}
+            if depth == 0 && cursor.peek_char().is_some_and(|c| matches!(c, ';' | '#')) {
+                let mut cursor = cursor.clone();
+                cursor.next();
+                if cursor.peek_char().is_some_and(char::is_whitespace) {
+                    break;
                 }
             }
         }
@@ -413,19 +413,6 @@ fn parse_unnamed_url<Url: UnnamedRequirementUrl>(
         });
     }
 
-    for c in [';', '#'] {
-        if url.ends_with(c) {
-            return Err(Pep508Error {
-                message: Pep508ErrorSource::String(format!(
-                    "Missing space before '{c}', the end of the URL is ambiguous"
-                )),
-                start: start + len - 1,
-                len: 1,
-                input: cursor.to_string(),
-            });
-        }
-    }
-
     let url = preprocess_unnamed_url(url, working_dir, cursor, start, len)?;
 
     Ok(url)

diff --git a/...uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-editable.txt.snap b/...uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-editable.txt.snap
@@ -334,6 +334,106 @@ RequirementsTxt {
             ),
             hashes: [],
         },
+        RequirementEntry {
+            requirement: Unnamed(
+                UnnamedRequirement {
+                    url: VerbatimParsedUrl {
+                        parsed_url: Directory(
+                            ParsedDirectoryUrl {
+                                url: Url {
+                                    scheme: "file",
+                                    cannot_be_a_base: false,
+                                    username: "",
+                                    password: None,
+                                    host: None,
+                                    port: None,
+                                    path: "<REQUIREMENTS_DIR>/editable",
+                                    query: None,
+                                    fragment: None,
+                                },
+                                install_path: "<REQUIREMENTS_DIR>/editable",
+                                editable: true,
+                                virtual: false,
+                            },
+                        ),
+                        verbatim: VerbatimUrl {
+                            url: Url {
+                                scheme: "file",
+                                cannot_be_a_base: false,
+                                username: "",
+                                password: None,
+                                host: None,
+                                port: None,
+                                path: "<REQUIREMENTS_DIR>/editable",
+                                query: None,
+                                fragment: None,
+                            },
+                            given: Some(
+                                "./editable",
+                            ),
+                        },
+                    },
+                    extras: [],
+                    marker: true,
+                    origin: Some(
+                        File(
+                            "<REQUIREMENTS_DIR>/editable.txt",
+                        ),
+                    ),
+                },
+            ),
+            hashes: [],
+        },
+        RequirementEntry {
+            requirement: Unnamed(
+                UnnamedRequirement {
+                    url: VerbatimParsedUrl {
+                        parsed_url: Directory(
+                            ParsedDirectoryUrl {
+                                url: Url {
+                                    scheme: "file",
+                                    cannot_be_a_base: false,
+                                    username: "",
+                                    password: None,
+                                    host: None,
+                                    port: None,
+                                    path: "<REQUIREMENTS_DIR>/editable",
+                                    query: None,
+                                    fragment: None,
+                                },
+                                install_path: "<REQUIREMENTS_DIR>/editable",
+                                editable: true,
+                                virtual: false,
+                            },
+                        ),
+                        verbatim: VerbatimUrl {
+                            url: Url {
+                                scheme: "file",
+                                cannot_be_a_base: false,
+                                username: "",
+                                password: None,
+                                host: None,
+                                port: None,
+                                path: "<REQUIREMENTS_DIR>/editable",
+                                query: None,
+                                fragment: None,
+                            },
+                            given: Some(
+                                "./editable",
+                            ),
+                        },
+                    },
+                    extras: [],
+                    marker: true,
+                    origin: Some(
+                        File(
+                            "<REQUIREMENTS_DIR>/editable.txt",
+                        ),
+                    ),
+                },
+            ),
+            hashes: [],
+        },
     ],
     index_url: None,
     extra_index_urls: [],

diff --git a/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-hash.txt.snap b/crates/uv-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-hash.txt.snap
@@ -7,7 +7,7 @@ RequirementsTxtFileError {
     error: Pep508 {
         source: Pep508Error {
             message: String(
-                "Missing space before '#', the end of the URL is ambiguous",
+                "Expected end of input or `;`, found `#`; comments must be preceded by a leading space",
             ),
             start: 10,
             len: 1,

diff --git a/...v-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-semicolon.txt.snap b/...v-requirements-txt/src/snapshots/uv_requirements_txt__test__parse-unix-semicolon.txt.snap
@@ -6,14 +6,17 @@ RequirementsTxtFileError {
     file: "<REQUIREMENTS_DIR>/semicolon.txt",
     error: Pep508 {
         source: Pep508Error {
-            message: String(
-                "Missing space before ';', the end of the URL is ambiguous",
+            message: UrlError(
+                MissingExtensionPath(
+                    "./editable;python_version >= \"3.9\" and os_name == \"posix\"",
+                    Dist,
+                ),
             ),
-            start: 10,
-            len: 1,
-            input: "./editable; python_version >= \"3.9\" and os_name == \"posix\"",
+            start: 0,
+            len: 57,
+            input: "./editable;python_version >= \"3.9\" and os_name == \"posix\"",
         },
         start: 50,
-        end: 108,
+        end: 107,
     },
 }