Skip to content

Commit

Permalink
Allow empty extras in pep508-rs and add more corner case to tests (a…
Browse files Browse the repository at this point in the history
…stral-sh#2128)

## Summary

Fixes astral-sh#2127, allow empty extras, and add more corner case to tests

## References

- [PEP 508 grammar](https://peps.python.org/pep-0508/#complete-grammar)
  • Loading branch information
SigureMo authored Mar 2, 2024
1 parent 782a862 commit d4f1973
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 29 deletions.
110 changes: 82 additions & 28 deletions crates/pep508-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,9 +617,48 @@ fn parse_extras(cursor: &mut Cursor) -> Result<Vec<ExtraName>, Pep508Error> {
let Some(bracket_pos) = cursor.eat_char('[') else {
return Ok(vec![]);
};
cursor.eat_whitespace();

let mut extras = Vec::new();
let mut is_first_iteration = true;

loop {
// End of the extras section. (Empty extras are allowed.)
if let Some(']') = cursor.peek_char() {
cursor.next();
break;
}

// Comma separator
match (cursor.peek(), is_first_iteration) {
// For the first iteration, we don't expect a comma.
(Some((pos, ',')), true) => {
return Err(Pep508Error {
message: Pep508ErrorSource::String(
"Expected either alphanumerical character (starting the extra name) or ']' (ending the extras section), found ','".to_string()
),
start: pos,
len: 1,
input: cursor.to_string(),
});
}
// For the other iterations, the comma is required.
(Some((_, ',')), false) => {
cursor.next();
}
(Some((pos, other)), false) => {
return Err(Pep508Error {
message: Pep508ErrorSource::String(
format!("Expected either ',' (separating extras) or ']' (ending the extras section), found '{other}'",)
),
start: pos,
len: 1,
input: cursor.to_string(),
});
}
_ => {}
}

// wsp* before the identifier
cursor.eat_whitespace();
let mut buffer = String::new();
Expand All @@ -633,7 +672,7 @@ fn parse_extras(cursor: &mut Cursor) -> Result<Vec<ExtraName>, Pep508Error> {
input: cursor.to_string(),
};

// First char of the identifier
// First char of the identifier.
match cursor.next() {
// letterOrDigit
Some((_, alphanumeric @ ('a'..='z' | 'A'..='Z' | '0'..='9'))) => {
Expand Down Expand Up @@ -673,33 +712,12 @@ fn parse_extras(cursor: &mut Cursor) -> Result<Vec<ExtraName>, Pep508Error> {
};
// wsp* after the identifier
cursor.eat_whitespace();
// end or next identifier?
match cursor.next() {
Some((_, ',')) => {
extras.push(
ExtraName::new(buffer)
.expect("`ExtraName` validation should match PEP 508 parsing"),
);
}
Some((_, ']')) => {
extras.push(
ExtraName::new(buffer)
.expect("`ExtraName` validation should match PEP 508 parsing"),
);
break;
}
Some((pos, other)) => {
return Err(Pep508Error {
message: Pep508ErrorSource::String(format!(
"Expected either ',' (separating extras) or ']' (ending the extras section), found '{other}'"
)),
start: pos,
len: other.len_utf8(),
input: cursor.to_string(),
});
}
None => return Err(early_eof_error),
}

// Add the parsed extra
extras.push(
ExtraName::new(buffer).expect("`ExtraName` validation should match PEP 508 parsing"),
);
is_first_iteration = false;
}

Ok(extras)
Expand Down Expand Up @@ -1241,6 +1259,18 @@ mod tests {
);
}

#[test]
fn error_extras_illegal_start3() {
assert_err(
"black[,]",
indoc! {"
Expected either alphanumerical character (starting the extra name) or ']' (ending the extras section), found ','
black[,]
^"
},
);
}

#[test]
fn error_extras_illegal_character() {
assert_err(
Expand Down Expand Up @@ -1271,6 +1301,30 @@ mod tests {
);
}

#[test]
fn empty_extras() {
let black = Requirement::from_str("black[]").unwrap();
assert_eq!(black.extras, vec![]);
}

#[test]
fn empty_extras_with_spaces() {
let black = Requirement::from_str("black[ ]").unwrap();
assert_eq!(black.extras, vec![]);
}

#[test]
fn error_extra_with_trailing_comma() {
assert_err(
"black[d,]",
indoc! {"
Expected an alphanumeric character starting the extra name, found ']'
black[d,]
^"
},
);
}

#[test]
fn error_parenthesized_pep440() {
assert_err(
Expand Down
2 changes: 1 addition & 1 deletion crates/requirements-txt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ mod test {
}, {
insta::assert_display_snapshot!(errors, @r###"
Couldn't parse requirement in `<REQUIREMENTS_TXT>` at position 6
Expected an alphanumeric character starting the extra name, found ','
Expected either alphanumerical character (starting the extra name) or ']' (ending the extras section), found ','
black[,abcdef]
^
"###);
Expand Down

0 comments on commit d4f1973

Please sign in to comment.