Skip to content

chore: backport minor fixes and refactorings from abandoned branch #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jvanstraten merged 2 commits into from
Jun 2, 2022
Merged

chore: backport minor fixes and refactorings from abandoned branch #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cf0774d
chore: backport minor fixes and refactorings from abandoned branch
jvanstraten May 9, 2022
d6c746b
Merge branch 'main' into backports-from-failed-simple-extensions
jvanstraten Jun 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ mod parse;
pub mod export;
pub mod input;

mod string_util;
mod util;

use strum::IntoEnumIterator;

Expand Down
18 changes: 9 additions & 9 deletions rs/src/output/data_type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@

use crate::output::diagnostic;
use crate::output::extension;
use crate::string_util;
use crate::string_util::Describe;
use crate::util;
use crate::util::string::Describe;
use std::collections::HashSet;
use std::fmt::Write;
use std::sync::Arc;
Expand Down Expand Up @@ -37,7 +37,7 @@ impl Describe for DataType {
fn describe(
&self,
f: &mut std::fmt::Formatter<'_>,
limit: string_util::Limit,
limit: util::string::Limit,
) -> std::fmt::Result {
let mut name = String::new();
write!(&mut name, "{}", self.class)?;
Expand All @@ -51,7 +51,7 @@ impl Describe for DataType {
let (_, limit) = limit.split(name.len());
if self.class.has_parameters() {
write!(f, "<")?;
string_util::describe_sequence(
util::string::describe_sequence(
f,
&self.parameters,
limit,
Expand Down Expand Up @@ -473,7 +473,7 @@ pub trait ParameterInfo {
}

/// Type class.
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum Class {
/// Well-known simple type.
Simple(Simple),
Expand Down Expand Up @@ -556,7 +556,7 @@ impl Class {
}

/// Enumeration of simple types defined by Substrait.
#[derive(Clone, Debug, PartialEq, Display, EnumString)]
#[derive(Clone, Debug, PartialEq, Eq, Hash, Display, EnumString)]
#[strum(ascii_case_insensitive, serialize_all = "snake_case")]
pub enum Simple {
Boolean,
Expand All @@ -578,7 +578,7 @@ pub enum Simple {
}

/// Enumeration of compound types defined by Substrait.
#[derive(Clone, Debug, PartialEq, Display, EnumString)]
#[derive(Clone, Debug, PartialEq, Eq, Hash, Display, EnumString)]
#[strum(ascii_case_insensitive, serialize_all = "UPPERCASE")]
pub enum Compound {
FixedChar,
Expand Down Expand Up @@ -762,13 +762,13 @@ impl Describe for Parameter {
fn describe(
&self,
f: &mut std::fmt::Formatter<'_>,
limit: string_util::Limit,
limit: util::string::Limit,
) -> std::fmt::Result {
match self {
Parameter::Type(data_type) => data_type.describe(f, limit),
Parameter::NamedType(name, data_type) => {
let (name_limit, type_limit) = limit.split(name.len());
string_util::describe_identifier(f, name, name_limit)?;
util::string::describe_identifier(f, name, name_limit)?;
write!(f, ": ")?;
data_type.describe(f, type_limit)
}
Expand Down
2 changes: 1 addition & 1 deletion rs/src/output/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub enum Classification {
#[strum(props(Description = "missing protobuf \"any\" declaration"))]
ProtoMissingAnyDeclaration = 1006,

// YAML-reated diagnostics (group 2).
// YAML-related diagnostics (group 2).
#[strum(props(HiddenDescription = "YAML-related diagnostic"))]
Yaml = 2000,

Expand Down
23 changes: 17 additions & 6 deletions rs/src/output/extension.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use crate::output::data_type;
use crate::output::path;
use crate::output::tree;
use crate::string_util;
use crate::util;
use std::collections::HashMap;
use std::sync::Arc;

Expand All @@ -21,19 +21,23 @@ pub struct NamedReference {
}

impl PartialEq for NamedReference {
/// Named references are equal if both references have a known name and
/// those names are the same.
fn eq(&self, other: &Self) -> bool {
self.name.is_some() && other.name.is_some() && self.name == other.name
self.name == other.name
}
}

impl Eq for NamedReference {}

impl std::hash::Hash for NamedReference {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.name.hash(state);
}
}

impl std::fmt::Display for NamedReference {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(name) = &self.name {
write!(f, "{}", string_util::as_ident_or_string(name))
write!(f, "{}", util::string::as_ident_or_string(name))
} else {
write!(f, "?")
}
Expand Down Expand Up @@ -92,7 +96,7 @@ pub struct Reference<T> {

impl<T> PartialEq for Reference<T> {
/// References are equal if they refer to the same thing, regardless of how
/// they refer to it. If we're not sure4 because either reference is
/// they refer to it. If we're not sure because either reference is
/// (partially) unresolved, return false pessimistically.
fn eq(&self, other: &Self) -> bool {
self.name == other.name && self.uri == other.uri
Expand All @@ -101,6 +105,13 @@ impl<T> PartialEq for Reference<T> {

impl<T> Eq for Reference<T> {}

impl<T> std::hash::Hash for Reference<T> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.name.hash(state);
self.uri.hash(state);
}
}

impl<T> std::fmt::Display for Reference<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}::{}", self.uri, self.name)
Expand Down
10 changes: 5 additions & 5 deletions rs/src/output/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! roughly the same as [`std::path::Path`], and [`std::path::PathBuf`], but
//! for protobuf/YAML tree paths rather than filesystem paths.

use crate::string_util;
use crate::util;

/// Element of a path to some field of a protobuf message and/or YAML file.
#[derive(Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -39,15 +39,15 @@ impl std::fmt::Display for PathElement {
}
}
match self {
PathElement::Field(field) => write!(f, "{}", string_util::as_ident_or_string(field)),
PathElement::Field(field) => write!(f, "{}", util::string::as_ident_or_string(field)),
PathElement::Repeated(field, index) => {
write!(f, "{}[{index}]", string_util::as_ident_or_string(field))
write!(f, "{}[{index}]", util::string::as_ident_or_string(field))
}
PathElement::Variant(field, variant) => write!(
f,
"{}<{}>",
string_util::as_ident_or_string(field),
string_util::as_ident_or_string(variant)
util::string::as_ident_or_string(field),
util::string::as_ident_or_string(variant)
),
PathElement::Index(index) => write!(f, "[{index}]"),
}
Expand Down
28 changes: 14 additions & 14 deletions rs/src/parse/expressions/literals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::output::data_type;
use crate::output::diagnostic;
use crate::parse::context;
use crate::parse::types;
use crate::string_util;
use crate::string_util::Describe;
use crate::util;
use crate::util::string::Describe;
use std::sync::Arc;

/// The value of a literal, not including type information.
Expand Down Expand Up @@ -136,7 +136,7 @@ impl Describe for Literal {
fn describe(
&self,
f: &mut std::fmt::Formatter<'_>,
limit: string_util::Limit,
limit: util::string::Limit,
) -> std::fmt::Result {
match &self.value {
LiteralValue::Null => {
Expand Down Expand Up @@ -197,7 +197,7 @@ impl Describe for Literal {
}
Ok(())
} else {
string_util::describe_binary(f, &d.to_le_bytes(), limit)
util::string::describe_binary(f, &d.to_le_bytes(), limit)
}
}
data_type::Class::Simple(data_type::Simple::Uuid) => {
Expand All @@ -208,10 +208,10 @@ impl Describe for Literal {
b[0], b[1], b[2], b[3], b[4], b[5], b[6], b[7], b[8], b[9], b[10], b[11], b[12], b[13], b[14], b[15]
)
}
_ => string_util::describe_binary(f, &d.to_le_bytes(), limit),
_ => util::string::describe_binary(f, &d.to_le_bytes(), limit),
},
LiteralValue::String(s) => string_util::describe_string(f, s, limit),
LiteralValue::Binary(b) => string_util::describe_binary(f, b, limit),
LiteralValue::String(s) => util::string::describe_string(f, s, limit),
LiteralValue::Binary(b) => util::string::describe_binary(f, b, limit),
LiteralValue::Interval(a, b) => match self.data_type.class() {
data_type::Class::Simple(data_type::Simple::IntervalYear) => {
write!(f, "{a}y{b:+}m")
Expand All @@ -222,22 +222,22 @@ impl Describe for Literal {
LiteralValue::Items(x) => match self.data_type.class() {
data_type::Class::Compound(data_type::Compound::Struct) => {
write!(f, "(")?;
string_util::describe_sequence(f, x, limit, 20, |f, value, index, limit| {
util::string::describe_sequence(f, x, limit, 20, |f, value, index, limit| {
write!(f, ".{index}: ")?;
value.describe(f, limit)
})?;
write!(f, ")")
}
data_type::Class::Compound(data_type::Compound::NamedStruct) => {
write!(f, "(")?;
string_util::describe_sequence(f, x, limit, 20, |f, value, index, limit| {
util::string::describe_sequence(f, x, limit, 20, |f, value, index, limit| {
if let Some(name) = self
.data_type
.parameters()
.get(index)
.and_then(|x| x.get_name())
{
write!(f, ".{}: ", string_util::as_ident_or_string(name))?;
write!(f, ".{}: ", util::string::as_ident_or_string(name))?;
} else {
write!(f, ".{index}: ")?;
}
Expand All @@ -247,14 +247,14 @@ impl Describe for Literal {
}
data_type::Class::Compound(data_type::Compound::List) => {
write!(f, "[")?;
string_util::describe_sequence(f, x, limit, 20, |f, value, _, limit| {
util::string::describe_sequence(f, x, limit, 20, |f, value, _, limit| {
value.describe(f, limit)
})?;
write!(f, "]")
}
_ => {
write!(f, "(")?;
string_util::describe_sequence(f, x, limit, 20, |f, value, _, limit| {
util::string::describe_sequence(f, x, limit, 20, |f, value, _, limit| {
value.describe(f, limit)
})?;
write!(f, ")")
Expand All @@ -263,7 +263,7 @@ impl Describe for Literal {
LiteralValue::Pairs(x) => match self.data_type.class() {
data_type::Class::Compound(data_type::Compound::Map) => {
write!(f, "{{")?;
string_util::describe_sequence(
util::string::describe_sequence(
f,
x,
limit,
Expand All @@ -279,7 +279,7 @@ impl Describe for Literal {
}
_ => {
write!(f, "(")?;
string_util::describe_sequence(
util::string::describe_sequence(
f,
x,
limit,
Expand Down
4 changes: 2 additions & 2 deletions rs/src/parse/expressions/misc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::output::diagnostic;
use crate::parse::context;
use crate::parse::expressions;
use crate::parse::types;
use crate::string_util;
use crate::util;

/// Parse an enum expression. Returns a description of said expression.
pub fn parse_enum(
Expand Down Expand Up @@ -35,7 +35,7 @@ pub fn parse_enum(
y,
Misc,
"Function option variant {}",
string_util::as_ident_or_string(variant)
util::string::as_ident_or_string(variant)
);
} else {
describe!(y, Misc, "Default function option variant");
Expand Down
16 changes: 8 additions & 8 deletions rs/src/parse/expressions/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::input::proto::substrait;
use crate::output::data_type;
use crate::output::diagnostic;
use crate::parse::context;
use crate::string_util;
use crate::string_util::Describe;
use crate::util;
use crate::util::string::Describe;
use std::sync::Arc;

pub mod conditionals;
Expand Down Expand Up @@ -72,25 +72,25 @@ impl Describe for Expression {
fn describe(
&self,
f: &mut std::fmt::Formatter<'_>,
limit: string_util::Limit,
limit: util::string::Limit,
) -> std::fmt::Result {
match self {
Expression::Unresolved => write!(f, "?"),
Expression::Literal(x) => x.describe(f, limit),
Expression::Reference(x) => x.describe(f, limit),
Expression::Function(name, args) => {
let (name_limit, args_limit) = limit.split(name.len());
string_util::describe_identifier(f, name, name_limit)?;
util::string::describe_identifier(f, name, name_limit)?;
write!(f, "(")?;
string_util::describe_sequence(f, args, args_limit, 20, |f, expr, _, limit| {
util::string::describe_sequence(f, args, args_limit, 20, |f, expr, _, limit| {
expr.describe(f, limit)
})?;
write!(f, ")")
}
Expression::BigFunction(name) => string_util::describe_identifier(f, name, limit),
Expression::BigFunction(name) => util::string::describe_identifier(f, name, limit),
Expression::Tuple(items) => {
write!(f, "(")?;
string_util::describe_sequence(f, items, limit, 20, |f, expr, _, limit| {
util::string::describe_sequence(f, items, limit, 20, |f, expr, _, limit| {
expr.describe(f, limit)
})?;
write!(f, ")")
Expand All @@ -103,7 +103,7 @@ impl Describe for Expression {
expression.describe(f, expr_limit)?;
write!(f, ")")
}
Expression::EnumVariant(Some(x)) => string_util::describe_identifier(f, x, limit),
Expression::EnumVariant(Some(x)) => util::string::describe_identifier(f, x, limit),
Expression::EnumVariant(None) => write!(f, "-"),
}
}
Expand Down
8 changes: 4 additions & 4 deletions rs/src/parse/expressions/references/mask.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::input::proto::substrait;
use crate::output::data_type;
use crate::output::diagnostic;
use crate::parse::context;
use crate::string_util;
use crate::util;
use std::sync::Arc;

/// Parse a struct item.
Expand Down Expand Up @@ -91,7 +91,7 @@ fn parse_list_select_item_element(
y,
Expression,
"Select {} element",
string_util::describe_index(x.field)
util::string::describe_index(x.field)
);
Ok(())
}
Expand Down Expand Up @@ -127,8 +127,8 @@ fn parse_list_select_item_slice(
} else {
format!(
"Select {} until {} element (inclusive)",
string_util::describe_index(x.start),
string_util::describe_index(x.end)
util::string::describe_index(x.start),
util::string::describe_index(x.end)
)
};
describe!(y, Expression, "{}", description);
Expand Down
Loading