Skip to content

Commit

Permalink
bed/record/name: Remove validation on parse
Browse files Browse the repository at this point in the history
This allows for more relaxed input in memory and is checked on
serialization instead.

Closes #271.
  • Loading branch information
zaeleus committed Jun 16, 2024
1 parent 2c80e82 commit 49e604a
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 51 deletions.
9 changes: 9 additions & 0 deletions noodles-bed/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

## Unreleased

### Changed

* bed/record/name: Remove validation on parse ([#271]).

This allows for more relaxed input in memory and is checked on
serialization instead.

[#271]: https://github.com/zaeleus/noodles/issues/271

### Fixed

* bed/record: Fix parse error kind returned from an invalid score ([#270]).
Expand Down
34 changes: 22 additions & 12 deletions noodles-bed/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ where
/// use noodles_bed::{self as bed, record::Name};
/// use noodles_core::Position;
///
/// let name: Name = "ndls1".parse()?;
/// let name = Name::from("ndls1");
///
/// let record = bed::Record::<4>::builder()
/// .set_reference_sequence_name("sq0")
Expand Down Expand Up @@ -548,12 +548,26 @@ fn format_bed_4_fields<const N: u8>(f: &mut fmt::Formatter<'_>, record: &Record<
where
Record<N>: BedN<3> + BedN<4>,
{
fn is_valid_name(s: &str) -> bool {
const MAX_LENGTH: usize = 255;

fn is_valid_name_char(c: char) -> bool {
matches!(c, ' '..='~')
}

s.len() <= MAX_LENGTH && s.chars().all(is_valid_name_char)
}

format_bed_3_fields(f, record)?;

f.write_char(DELIMITER)?;

if let Some(name) = record.name() {
write!(f, "{name}")
if is_valid_name(name) {
write!(f, "{name}")
} else {
Err(fmt::Error)
}
} else {
f.write_str(MISSING_STRING)
}
Expand Down Expand Up @@ -679,8 +693,6 @@ pub enum ParseError {
InvalidEndPosition(num::ParseIntError),
/// The name is missing.
MissingName,
/// The name is invalid.
InvalidName(name::ParseError),
/// The score is missing.
MissingScore,
/// The score is invalid.
Expand Down Expand Up @@ -721,7 +733,6 @@ impl error::Error for ParseError {
Self::InvalidEndPosition(e) | Self::InvalidThickEnd(e) | Self::InvalidBlockStart(e) => {
Some(e)
}
Self::InvalidName(e) => Some(e),
Self::InvalidScore(e) => Some(e),
Self::InvalidStrand(e) => Some(e),
Self::InvalidColor(e) => Some(e),
Expand All @@ -740,7 +751,6 @@ impl fmt::Display for ParseError {
Self::MissingEndPosition => f.write_str("missing end position"),
Self::InvalidEndPosition(_) => f.write_str("invalid end position"),
Self::MissingName => f.write_str("missing name"),
Self::InvalidName(_) => f.write_str("invalid name"),
Self::MissingScore => f.write_str("missing score"),
Self::InvalidScore(_) => f.write_str("invalid score"),
Self::MissingStrand => f.write_str("missing strand"),
Expand Down Expand Up @@ -960,11 +970,11 @@ where
{
fields
.next()
.ok_or(ParseError::MissingName)
.and_then(|s| match s {
MISSING_STRING => Ok(None),
_ => s.parse().map(Some).map_err(ParseError::InvalidName),
.map(|s| match s {
MISSING_STRING => None,
_ => Some(Name::from(s)),
})
.ok_or(ParseError::MissingName)
}

fn parse_score<'a, I>(fields: &mut I) -> Result<Option<Score>, ParseError>
Expand Down Expand Up @@ -1113,7 +1123,7 @@ mod tests {
assert_eq!(record.to_string(), "sq0\t7\t13\t.");

let mut standard_fields = StandardFields::new("sq0", start, end);
standard_fields.name = "ndls1".parse().map(Some)?;
standard_fields.name = Some(Name::from("ndls1"));
let record: Record<4> = Record::new(standard_fields, OptionalFields::default());
assert_eq!(record.to_string(), "sq0\t7\t13\tndls1");

Expand Down Expand Up @@ -1302,7 +1312,7 @@ mod tests {
let start = Position::try_from(8)?;
let end = Position::try_from(13)?;
let mut standard_fields = StandardFields::new("sq0", start, end);
standard_fields.name = "ndls1".parse().map(Some)?;
standard_fields.name = Some(Name::from("ndls1"));

let expected = Ok(Record::new(standard_fields, OptionalFields::default()));

Expand Down
2 changes: 1 addition & 1 deletion noodles-bed/src/record/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ where
/// use noodles_bed::{self as bed, record::Name};
/// use noodles_core::Position;
///
/// let name: Name = "ndls1".parse()?;
/// let name = Name::from("ndls1");
///
/// let record = bed::Record::<4>::builder()
/// .set_reference_sequence_name("sq0")
Expand Down
54 changes: 16 additions & 38 deletions noodles-bed/src/record/name.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
//! BED record name.

use std::{error, fmt, ops::Deref, str::FromStr};

const MAX_LENGTH: usize = 255;
use std::{fmt, ops::Deref, str::FromStr};

/// A BED record name.
#[derive(Clone, Debug, Eq, PartialEq)]
Expand All @@ -22,46 +20,24 @@ impl fmt::Display for Name {
}
}

/// An error returned when a raw BED record name fails to parse.
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum ParseError {
/// The input is empty.
Empty,
/// The input is invalid.
Invalid,
}

impl error::Error for ParseError {}

impl fmt::Display for ParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Empty => f.write_str("empty input"),
Self::Invalid => f.write_str("invalid input"),
}
}
}

impl FromStr for Name {
type Err = ParseError;
type Err = ();

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.is_empty() {
Err(ParseError::Empty)
} else if !is_valid_name(s) {
Err(ParseError::Invalid)
} else {
Ok(Self(s.into()))
}
Ok(Self(s.into()))
}
}

fn is_valid_name_char(c: char) -> bool {
matches!(c, ' '..='~')
impl From<&str> for Name {
fn from(s: &str) -> Self {
Self::from(s.to_owned())
}
}

fn is_valid_name(s: &str) -> bool {
s.len() <= MAX_LENGTH && s.chars().all(is_valid_name_char)
impl From<String> for Name {
fn from(s: String) -> Self {
Self(s)
}
}

#[cfg(test)]
Expand All @@ -76,13 +52,15 @@ mod tests {

#[test]
fn test_from_str() {
const MAX_LENGTH: usize = 255;

assert_eq!("ndls1".parse(), Ok(Name(String::from("ndls1"))));
assert_eq!(" ~".parse(), Ok(Name(String::from(" ~"))));

assert_eq!("".parse::<Name>(), Err(ParseError::Empty));
assert_eq!("🍜".parse::<Name>(), Err(ParseError::Invalid));
assert_eq!("".parse::<Name>(), Ok(Name(String::new())));
assert_eq!("🍜".parse::<Name>(), Ok(Name(String::from("🍜"))));

let s = "n".repeat(MAX_LENGTH + 1);
assert_eq!(s.parse::<Name>(), Err(ParseError::Invalid));
assert_eq!(s.parse::<Name>(), Ok(Name::from(s)));
}
}

0 comments on commit 49e604a

Please sign in to comment.