Skip to content

Commit

Permalink
refactor: reduce code duplication (#217)
Browse files Browse the repository at this point in the history
* "refactor: reduce code duplication"

* "fix: parsed wrong node"
  • Loading branch information
baszalmstra authored Oct 19, 2023
1 parent f718884 commit 58373fa
Show file tree
Hide file tree
Showing 13 changed files with 206 additions and 143 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ thiserror = "1.0.49"
tempfile = "3.8.0"
chrono = "0.4.31"
sha1 = "0.10.6"
spdx = "0.10.2"
git2 = { version = "0.18.1", features = ["vendored-openssl"] }
fs_extra = "1.3.0"
ignore = "0.4.20"
Expand Down
4 changes: 2 additions & 2 deletions src/packaging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ fn create_index_json(output: &Output) -> Result<String, PackagingError> {
arch,
platform,
subdir: Some(output.build_configuration.target_platform.to_string()),
license: recipe.about().license().map(|l| l.to_owned()),
license: recipe.about().license().map(|l| l.to_string()),
license_family: recipe.about().license_family().map(|l| l.to_owned()),
timestamp: Some(output.build_configuration.timestamp),
depends: output
Expand Down Expand Up @@ -294,7 +294,7 @@ fn create_about_json(output: &Output) -> Result<String, PackagingError> {
.cloned()
.map(|s| vec![s])
.unwrap_or_default(),
license: recipe.about().license().map(|s| s.to_owned()),
license: recipe.about().license().map(|s| s.to_string()),
license_family: recipe.about().license_family().map(|s| s.to_owned()),
summary: recipe.about().summary().map(|s| s.to_owned()),
description: recipe.about().description().map(|s| s.to_owned()),
Expand Down
79 changes: 61 additions & 18 deletions src/recipe/custom_yaml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{fmt, hash::Hash, ops};

use linked_hash_map::LinkedHashMap;
use marked_yaml::types::MarkedScalarNode;
use marked_yaml::Span;

use crate::_partialerror;
use crate::recipe::{
Expand Down Expand Up @@ -63,15 +64,6 @@ impl Node {
Self::try_from(yaml).map_err(|err| ParsingError::from_partial(src, err))
}

/// Retrieve the Span from the contained Node
pub fn span(&self) -> &marked_yaml::Span {
match self {
Self::Mapping(map) => map.span(),
Self::Scalar(scalar) => scalar.span(),
Self::Sequence(seq) => seq.span(),
}
}

pub fn is_mapping(&self) -> bool {
matches!(self, Self::Mapping(_))
}
Expand Down Expand Up @@ -109,6 +101,29 @@ impl Node {
}
}

/// A trait that defines that the implementer has an associated span.
pub trait HasSpan {
fn span(&self) -> &marked_yaml::Span;
}

impl HasSpan for Node {
fn span(&self) -> &Span {
match self {
Self::Mapping(map) => map.span(),
Self::Scalar(scalar) => scalar.span(),
Self::Sequence(seq) => seq.span(),
}
}
}

impl<'i> TryFrom<&'i Node> for &'i ScalarNode {
type Error = ();

fn try_from(value: &'i Node) -> Result<Self, Self::Error> {
value.as_scalar().ok_or(())
}
}

impl From<ScalarNode> for Node {
fn from(value: ScalarNode) -> Self {
Self::Scalar(value)
Expand Down Expand Up @@ -195,10 +210,6 @@ impl ScalarNode {
Self { span, value }
}

pub fn span(&self) -> &marked_yaml::Span {
&self.span
}

/// Treat the scalar node as a string
///
/// Since scalars are always stringish, this is always safe.
Expand Down Expand Up @@ -230,6 +241,12 @@ impl ScalarNode {
}
}

impl HasSpan for ScalarNode {
fn span(&self) -> &Span {
&self.span
}
}

impl PartialEq for ScalarNode {
fn eq(&self, other: &Self) -> bool {
self.value == other.value
Expand Down Expand Up @@ -369,10 +386,6 @@ impl SequenceNode {
Self { span, value }
}

pub fn span(&self) -> &marked_yaml::Span {
&self.span
}

/// Check if this sequence node is only conditional.
///
/// This is convenient for places that accept if-selectors but don't accept simple sequence.
Expand All @@ -383,6 +396,12 @@ impl SequenceNode {
}
}

impl HasSpan for SequenceNode {
fn span(&self) -> &marked_yaml::Span {
&self.span
}
}

impl PartialEq for SequenceNode {
fn eq(&self, other: &Self) -> bool {
self.value == other.value
Expand Down Expand Up @@ -465,8 +484,10 @@ impl MappingNode {
pub fn new(span: marked_yaml::Span, value: LinkedHashMap<ScalarNode, Node>) -> Self {
Self { span, value }
}
}

pub fn span(&self) -> &marked_yaml::Span {
impl HasSpan for MappingNode {
fn span(&self) -> &marked_yaml::Span {
&self.span
}
}
Expand Down Expand Up @@ -712,3 +733,25 @@ impl fmt::Debug for IfSelector {
.finish()
}
}

pub trait TryConvertNode<T> {
fn try_convert(&self, name: &str) -> Result<&T, PartialParsingError>;
}

impl<T> TryConvertNode<T> for T {
fn try_convert(&self, _: &str) -> Result<&T, PartialParsingError> {
Ok(self)
}
}

impl TryConvertNode<ScalarNode> for Node {
fn try_convert(&self, name: &str) -> Result<&ScalarNode, PartialParsingError> {
self.as_scalar().ok_or_else(|| {
_partialerror!(
*self.span(),
ErrorKind::ExpectedScalar,
label = format!("expected a scalar value for {name}")
)
})
}
}
31 changes: 16 additions & 15 deletions src/recipe/error.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::{fmt, str::ParseBoolError};

use miette::{Diagnostic, SourceOffset, SourceSpan};
Expand All @@ -14,15 +15,15 @@ pub struct ParsingError {
pub src: String,

/// Offset in chars of the error.
#[label("{}", label.unwrap_or("here"))]
#[label("{}", label.as_deref().unwrap_or("here"))]
pub span: SourceSpan,

/// Label text for this span. Defaults to `"here"`.
pub label: Option<&'static str>,
pub label: Option<Cow<'static, str>>,

/// Suggestion for fixing the parser error.
#[help]
pub help: Option<&'static str>,
pub help: Option<Cow<'static, str>>,

// Specific error kind for the error.
pub kind: ErrorKind,
Expand Down Expand Up @@ -101,10 +102,10 @@ pub struct PartialParsingError {
pub span: marked_yaml::Span,

/// Label text for this span. Defaults to `"here"`.
pub label: Option<&'static str>,
pub label: Option<Cow<'static, str>>,

/// Suggestion for fixing the parser error.
pub help: Option<&'static str>,
pub help: Option<Cow<'static, str>>,

// Specific error kind for the error.
pub kind: ErrorKind,
Expand Down Expand Up @@ -176,7 +177,7 @@ macro_rules! _error {
$crate::recipe::error::ParsingError {
src: $src.to_owned(),
span: $span,
label: Some($label),
label: Some($label.into()),
help: None,
kind: $kind,
}
Expand All @@ -186,7 +187,7 @@ macro_rules! _error {
src: $src.to_owned(),
span: $span,
label: None,
help: Some($help),
help: Some($help.into()),
kind: $kind,
}
}};
Expand All @@ -200,8 +201,8 @@ macro_rules! _error {
$crate::recipe::error::ParsingError {
src: $src.to_owned(),
span: $span,
label: Some($label),
help: Some($help),
label: Some($label.into()),
help: Some($help.into()),
kind: $kind,
}
}};
Expand All @@ -222,7 +223,7 @@ macro_rules! _partialerror {
($span:expr, $kind:expr, label = $label:expr $(,)?) => {{
$crate::recipe::error::PartialParsingError {
span: $span,
label: Some($label),
label: Some($label.into()),
help: None,
kind: $kind,
}
Expand All @@ -231,7 +232,7 @@ macro_rules! _partialerror {
$crate::recipe::error::PartialParsingError {
span: $span,
label: None,
help: Some($help),
help: Some($help.into()),
kind: $kind,
}
}};
Expand All @@ -244,8 +245,8 @@ macro_rules! _partialerror {
($span:expr, $kind:expr, $label:expr, $help:expr $(,)?) => {{
$crate::recipe::error::PartialParsingError {
span: $span,
label: Some($label),
help: Some($help),
label: Some($label.into()),
help: Some($help.into()),
kind: $kind,
}
}};
Expand All @@ -257,12 +258,12 @@ pub(super) fn load_error_handler(src: &str, err: marked_yaml::LoadError) -> Pars
src,
marker_to_span(src, marker(&err)),
ErrorKind::YamlParsing(err),
label = match err {
label = Cow::Borrowed(match err {
marked_yaml::LoadError::TopLevelMustBeMapping(_) => "expected a mapping here",
marked_yaml::LoadError::UnexpectedAnchor(_) => "unexpected anchor here",
marked_yaml::LoadError::UnexpectedTag(_) => "unexpected tag here",
_ => "here",
}
})
)
}

Expand Down
1 change: 1 addition & 0 deletions src/recipe/stage1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use super::{
error::{marker_span_to_span, ErrorKind, ParsingError, PartialParsingError},
};

use crate::recipe::custom_yaml::HasSpan;
use crate::{_error, _partialerror};

/// This is the raw reprentation of a recipe, without any minijinja processing done.
Expand Down
59 changes: 59 additions & 0 deletions src/recipe/stage2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@

use minijinja::Value;
use serde::Serialize;
use std::fmt::Display;
use std::str::FromStr;

use crate::recipe::custom_yaml::{ScalarNode, TryConvertNode};
use crate::{
_partialerror,
recipe::{
custom_yaml::HasSpan,
error::{ErrorKind, ParsingError, PartialParsingError},
jinja::Jinja,
stage1::RawRecipe,
Expand Down Expand Up @@ -161,3 +165,58 @@ mod tests {
dbg!(&recipe);
}
}

/// A trait to render a certain stage1 node into its final type.
trait Render<T> {
fn render(&self, jinja: &Jinja, name: &str) -> Result<T, PartialParsingError>;
}

/// A jinja rendered string
struct Rendered(String);

impl Rendered {
// Parses this rendered value into another type.
pub fn parse<F: FromStr>(&self) -> Result<F, F::Err> {
FromStr::from_str(&self.0)
}
}

impl<N: TryConvertNode<ScalarNode> + HasSpan> Render<Rendered> for N {
fn render(&self, jinja: &Jinja, name: &str) -> Result<Rendered, PartialParsingError> {
jinja
.render_str(self.try_convert(name)?.as_str())
.map_err(|err| {
_partialerror!(
*self.span(),
ErrorKind::JinjaRendering(err),
label = format!("error rendering {name}")
)
})
.map(Rendered)
}
}

impl<N: TryConvertNode<ScalarNode> + HasSpan, T: FromStr> Render<T> for N
where
T::Err: Display,
{
fn render(&self, jinja: &Jinja, name: &str) -> Result<T, PartialParsingError> {
match Rendered::parse(&self.render(jinja, name)?) {
Ok(result) => Ok(result),
Err(e) => Err(_partialerror!(
*self.span(),
ErrorKind::Other,
label = e.to_string()
)),
}
}
}

impl<N: Render<T>, T: FromStr> Render<Option<T>> for Option<N> {
fn render(&self, jinja: &Jinja, name: &str) -> Result<Option<T>, PartialParsingError> {
match self {
None => Ok(None),
Some(node) => Ok(Some(node.render(jinja, name)?)),
}
}
}
Loading

0 comments on commit 58373fa

Please sign in to comment.