Skip to content

Commit

Permalink
sdl-encoder: fix description encodings (#742)
Browse files Browse the repository at this point in the history
This commit introduces `Description` helper to encode top-level, field-level and input-level descriptions. This helps unify some of the description logic we've had across multiple files.

This commit also checks whether a description string is a block string character or a string character which then determines whether description needs to be multilined.

Fixes #728.

Co-authored-by: Avery Harnish <[email protected]>
  • Loading branch information
lrlna and EverlastingBugstopper authored Aug 17, 2021
1 parent f0b3537 commit 1ea936a
Show file tree
Hide file tree
Showing 15 changed files with 239 additions and 167 deletions.
16 changes: 12 additions & 4 deletions crates/rover-client/src/operations/graph/introspect/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,13 @@ mod tests {
type Species implements Node {
"""The name of this species."""
name: String
"""The classification of this species, such as "mammal" or "reptile"."""
"""
The classification of this species, such as "mammal" or "reptile".
"""
classification: String
"""The designation of this species, such as "sentient"."""
"""
The designation of this species, such as "sentient".
"""
designation: String
"""The average height of this species in centimeters."""
averageHeight: Float
Expand Down Expand Up @@ -750,7 +754,9 @@ mod tests {
}
"""A single transport craft that has hyperdrive capability."""
type Starship implements Node {
"""The name of this starship. The common name, such as "Death Star"."""
"""
The name of this starship. The common name, such as "Death Star".
"""
name: String
"""
The model or official name of this starship. Such as "T-65 X-wing" or "DS-1
Expand Down Expand Up @@ -905,7 +911,9 @@ mod tests {
Transport".
"""
model: String
"""The class of this vehicle, such as "Wheeled" or "Repulsorcraft"."""
"""
The class of this vehicle, such as "Wheeled" or "Repulsorcraft".
"""
vehicleClass: String
"""The manufacturers of this vehicle."""
manufacturers: [String]
Expand Down
135 changes: 135 additions & 0 deletions crates/sdl-encoder/src/description.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
use std::fmt::{self, Display};

#[derive(Debug, PartialEq, Clone)]
/// Convenience enum to create a Description. Can be a `Top` level, a `Field`
/// level or an `Input` level. The variants are distinguished by the way they
/// get displayed, e.g. number of leading spaces.
pub enum Description {
/// Top-level description.
Top {
/// Description.
source: Option<String>,
},
/// Field-level description.
/// This description gets additional leading spaces.
Field {
/// Description.
source: Option<String>,
},
/// Input-level description.
/// This description get an additional space at the end.
Input {
/// Description.
source: Option<String>,
},
}

impl Display for Description {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Description::Top { source } => {
if let Some(description) = source {
if is_block_string_character(description) {
writeln!(f, "\"\"\"\n{}\n\"\"\"", description)?
} else {
writeln!(f, "\"\"\"{}\"\"\"", description)?
}
}
}
Description::Field { source } => {
if let Some(description) = source {
if is_block_string_character(description) {
writeln!(f, " \"\"\"\n {}\n \"\"\"", description)?
} else {
writeln!(f, " \"\"\"{}\"\"\"", description)?
}
}
}
Description::Input { source } => {
if let Some(description) = source {
if is_block_string_character(description) {
write!(f, "\"\"\"\n{}\n\"\"\" ", description)?
} else {
write!(f, "\"\"\"{}\"\"\" ", description)?
}
}
}
}
write!(f, "")
}
}

fn is_block_string_character(s: &str) -> bool {
s.contains('\n') || s.contains('"') || s.contains('\r')
}
#[cfg(test)]
mod test {
use super::*;
use pretty_assertions::assert_eq;

#[test]
fn it_encodes_description_without_block_string_character() {
let desc = Description::Top {
source: Some(
"Favourite cat nap spots include: plant corner, pile of clothes.".to_string(),
),
};

assert_eq!(
desc.to_string(),
r#""""Favourite cat nap spots include: plant corner, pile of clothes."""
"#
);
}

#[test]
fn it_encodes_description_with_quotations() {
let desc = Description::Top {
source: Some(
"Favourite \"cat\" nap spots include: plant corner, pile of clothes.".to_string(),
),
};

assert_eq!(
desc.to_string(),
r#""""
Favourite "cat" nap spots include: plant corner, pile of clothes.
"""
"#
);
}

#[test]
fn it_encodes_description_with_new_line() {
let desc = Description::Top {
source: Some(
"Favourite cat nap spots include:\nplant corner, pile of clothes.".to_string(),
),
};

assert_eq!(
desc.to_string(),
r#""""
Favourite cat nap spots include:
plant corner, pile of clothes.
"""
"#
);
}

#[test]
fn it_encodes_description_with_carriage_return() {
let desc = Description::Top {
source: Some(
"Favourite cat nap spots include:\rplant corner,\rpile of clothes.".to_string(),
),
};

assert_eq!(
desc.to_string(),
String::from(
"\"\"\"\nFavourite cat nap spots include:\rplant corner,\rpile of clothes.\n\"\"\"\n"
)
);
}
}
21 changes: 7 additions & 14 deletions crates/sdl-encoder/src/directive_def.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::InputValue;
use crate::{Description, InputValue};
use std::fmt::{self, Display};

/// The `__Directive` type represents a Directive that a service supports.
Expand Down Expand Up @@ -34,7 +34,7 @@ pub struct Directive {
// Name must return a String.
name: String,
// Description may return a String or null.
description: Option<String>,
description: Description,
// Args returns a Vector of __InputValue representing the arguments this
// directive accepts.
args: Vec<InputValue>,
Expand All @@ -48,15 +48,17 @@ impl Directive {
pub fn new(name: String) -> Self {
Self {
name,
description: None,
description: Description::Top { source: None },
args: Vec::new(),
locations: Vec::new(),
}
}

/// Set the Directive's description.
pub fn description(&mut self, description: Option<String>) {
self.description = description;
self.description = Description::Top {
source: description,
};
}

/// Set the Directive's location.
Expand All @@ -72,16 +74,7 @@ impl Directive {

impl Display for Directive {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(description) = &self.description {
// We are determing on whether to have description formatted as
// a multiline comment based on whether or not it already includes a
// \n.
match description.contains('\n') {
true => writeln!(f, "\"\"\"\n{}\n\"\"\"", description)?,
false => writeln!(f, "\"\"\"{}\"\"\"", description)?,
}
}

write!(f, "{}", self.description)?;
write!(f, "directive @{}", self.name)?;

if !self.args.is_empty() {
Expand Down
21 changes: 7 additions & 14 deletions crates/sdl-encoder/src/enum_def.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::EnumValue;
use crate::{Description, EnumValue};
use std::fmt::{self, Display};

/// Enums are special scalars that can only have a defined set of values.
Expand Down Expand Up @@ -41,7 +41,7 @@ pub struct EnumDef {
// Name must return a String.
name: String,
// Description may return a String or null.
description: Option<String>,
description: Description,
// A vector of EnumValue. There must be at least one and they must have
// unique names.
values: Vec<EnumValue>,
Expand All @@ -52,14 +52,16 @@ impl EnumDef {
pub fn new(name: String) -> Self {
Self {
name,
description: None,
description: Description::Top { source: None },
values: Vec::new(),
}
}

/// Set the Enum Definition's description.
pub fn description(&mut self, description: Option<String>) {
self.description = description;
self.description = Description::Top {
source: description,
};
}

/// Set the Enum Definitions's values.
Expand All @@ -70,16 +72,7 @@ impl EnumDef {

impl Display for EnumDef {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(description) = &self.description {
// We are determing on whether to have description formatted as
// a multiline comment based on whether or not it already includes a
// \n.
match description.contains('\n') {
true => writeln!(f, "\"\"\"\n{}\n\"\"\"", description)?,
false => writeln!(f, "\"\"\"{}\"\"\"", description)?,
}
}

write!(f, "{}", self.description)?;
write!(f, "enum {} {{", self.name)?;
for value in &self.values {
write!(f, "\n{}", value)?;
Expand Down
21 changes: 8 additions & 13 deletions crates/sdl-encoder/src/enum_value.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::fmt::{self, Display};

use crate::Description;

/// The __EnumValue type represents one of possible values of an enum.
///
/// *EnumValueDefinition*:
Expand All @@ -26,7 +28,7 @@ pub struct EnumValue {
// Name must return a String.
name: String,
// Description may return a String or null.
description: Option<String>,
description: Description,
// Deprecated returns true if this enum value should no longer be used, otherwise false.
is_deprecated: bool,
// Deprecation reason optionally provides a reason why this enum value is deprecated.
Expand All @@ -39,14 +41,16 @@ impl EnumValue {
Self {
name,
is_deprecated: false,
description: None,
description: Description::Field { source: None },
deprecation_reason: None,
}
}

/// Set the Enum Value's description.
pub fn description(&mut self, description: Option<String>) {
self.description = description;
self.description = Description::Field {
source: description,
};
}

/// Set the Enum Value's deprecation properties.
Expand All @@ -58,16 +62,7 @@ impl EnumValue {

impl Display for EnumValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(description) = &self.description {
// We are determing on whether to have description formatted as
// a multiline comment based on whether or not it already includes a
// \n.
match description.contains('\n') {
true => writeln!(f, " \"\"\"\n {}\n \"\"\"", description)?,
false => writeln!(f, " \"\"\"{}\"\"\"", description)?,
}
}

write!(f, "{}", self.description)?;
write!(f, " {}", self.name)?;

if self.is_deprecated {
Expand Down
24 changes: 7 additions & 17 deletions crates/sdl-encoder/src/field.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{InputValue, Type_};
use crate::{Description, InputValue, Type_};
use std::fmt::{self, Display};
/// The __Field type represents each field in an Object or Interface type.
///
Expand Down Expand Up @@ -35,7 +35,7 @@ pub struct Field {
// Name must return a String.
name: String,
// Description may return a String.
description: Option<String>,
description: Description,
// Args returns a List of __InputValue representing the arguments this field accepts.
args: Vec<InputValue>,
// Type must return a __Type that represents the type of value returned by this field.
Expand All @@ -50,7 +50,7 @@ impl Field {
/// Create a new instance of Field.
pub fn new(name: String, type_: Type_) -> Self {
Self {
description: None,
description: Description::Field { source: None },
name,
type_,
args: Vec::new(),
Expand All @@ -61,7 +61,9 @@ impl Field {

/// Set the Field's description.
pub fn description(&mut self, description: Option<String>) {
self.description = description;
self.description = Description::Field {
source: description,
};
}

/// Set the Field's deprecation properties.
Expand All @@ -78,19 +80,7 @@ impl Field {

impl Display for Field {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(description) = &self.description {
// Let's indent description on a field level for now, as all fields
// are always on the same level and are indented by 2 spaces.
//
// We are also determing on whether to have description formatted as
// a multiline comment based on whether or not it already includes a
// \n.
match description.contains('\n') {
true => writeln!(f, " \"\"\"\n {}\n \"\"\"", description)?,
false => writeln!(f, " \"\"\"{}\"\"\"", description)?,
}
}

write!(f, "{}", self.description)?;
write!(f, " {}", self.name)?;

if !self.args.is_empty() {
Expand Down
Loading

0 comments on commit 1ea936a

Please sign in to comment.