i18n: clippy cleanups

This commit is contained in:
Quentin Gliech
2023-09-27 15:08:39 +02:00
parent a82fcc990d
commit be20a50acc
7 changed files with 102 additions and 125 deletions

View File

@@ -12,5 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#![warn(clippy::pedantic)]
#![deny(clippy::all)]
pub mod sprintf;
mod translations;
pub mod translations;

View File

@@ -18,16 +18,18 @@ use serde_json::Value;
/// A list of arguments that can be accessed by index or name.
#[derive(Debug, Clone, Default)]
pub struct ArgumentList {
pub struct List {
arguments: Vec<Value>,
name_index: HashMap<String, usize>,
}
impl ArgumentList {
impl List {
#[must_use]
pub fn get_by_index(&self, index: usize) -> Option<&Value> {
self.arguments.get(index)
}
#[must_use]
pub fn get_by_name(&self, name: &str) -> Option<&Value> {
self.name_index
.get(name)
@@ -35,7 +37,7 @@ impl ArgumentList {
}
}
impl<A: Into<Argument>> FromIterator<A> for ArgumentList {
impl<A: Into<Argument>> FromIterator<A> for List {
fn from_iter<T: IntoIterator<Item = A>>(iter: T) -> Self {
let mut arguments = Vec::new();
let mut name_index = HashMap::new();
@@ -87,6 +89,7 @@ impl From<(String, Value)> for Argument {
impl Argument {
/// Create a new argument with the given name and value.
#[must_use]
pub fn named(name: String, value: Value) -> Self {
Self {
name: Some(name),
@@ -95,6 +98,7 @@ impl Argument {
}
/// Set the name of the argument.
#[must_use]
pub fn with_name(mut self, name: String) -> Self {
self.name = Some(name);
self
@@ -109,7 +113,7 @@ mod tests {
#[test]
fn test_argument_list() {
let list = ArgumentList::from_iter([
let list = List::from_iter([
("hello", json!("world")),
("alice", json!(null)),
("bob", json!(42)),
@@ -125,7 +129,7 @@ mod tests {
assert_eq!(list.get_by_name("bob"), Some(&json!(42)));
assert_eq!(list.get_by_name("charlie"), None);
let list = ArgumentList::from_iter([
let list = List::from_iter([
Argument::from(json!("hello")),
Argument::named("alice".to_owned(), json!(null)),
Argument::named("bob".to_owned(), json!(42)),

View File

@@ -18,7 +18,9 @@ use serde_json::{ser::PrettyFormatter, Value};
use thiserror::Error;
use super::{ArgumentList, Message};
use crate::sprintf::message::{ArgumentReference, MessagePart, Placeholder, TypeSpecifier};
use crate::sprintf::message::{
ArgumentReference, PaddingSpecifier, Part, Placeholder, TypeSpecifier,
};
macro_rules! format_placeholder {
($value:expr, $type:literal, $placeholder:expr) => {
@@ -185,13 +187,9 @@ fn find_value<'a>(
current_index: usize,
) -> Result<&'a Value, FormatError> {
match requested_argument {
Some(ArgumentReference::Named(name)) => {
arguments
.get_by_name(name)
.ok_or(FormatError::UnknownNamedArgument {
name: name.to_owned(),
})
}
Some(ArgumentReference::Named(name)) => arguments
.get_by_name(name)
.ok_or(FormatError::UnknownNamedArgument { name: name.clone() }),
Some(ArgumentReference::Indexed(index)) => arguments
.get_by_index(*index - 1)
@@ -216,11 +214,15 @@ fn to_precision(number: f64, mut placeholder: Placeholder) -> String {
}
// This tells us how many numbers are before the decimal point
let log10 = number.abs().log10().floor() as i32;
// This lossy cast is fine because we only care about the order of magnitude,
// and special cases are handled above
#[allow(clippy::cast_possible_truncation)]
let log10 = number.abs().log10().floor() as i64;
let precision_i64 = precision.try_into().unwrap_or(i64::MAX);
// We can fit the number in the precision, so we just format it as normal
if log10 > 0 && log10 <= precision as i32 || number.abs() < 10.0 {
if log10 > 0 && log10 <= precision_i64 || number.abs() < 10.0 {
// We remove the number of digits before the decimal point from the precision
placeholder.precision = Some(precision - 1 - log10.max(0) as usize);
placeholder.precision = Some(precision - 1 - log10.try_into().unwrap_or(0usize));
format_placeholder!(number, &placeholder)
} else {
// Else in scientific notation there is always one digit before the decimal
@@ -230,6 +232,7 @@ fn to_precision(number: f64, mut placeholder: Placeholder) -> String {
}
}
#[allow(clippy::too_many_lines, clippy::match_same_arms)]
fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, FormatError> {
match (value, &placeholder.type_specifier) {
(Value::Number(number), ts @ TypeSpecifier::BinaryNumber) => {
@@ -297,10 +300,9 @@ fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, Form
if let Some(number) = number.as_u64() {
Ok(format_placeholder!(number, placeholder))
} else if let Some(number) = number.as_i64() {
// Truncate to a i32 to mimic JS's behaviour
let number = number as i32;
// Then cast to u32 to mimic JS's behaviour
let number = number as u32;
// Truncate to a i32 and then u32 to mimic JS's behaviour
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
let number = number as i32 as u32;
Ok(format_placeholder!(number, placeholder))
} else {
Err(FormatError::InvalidTypeSpecifier {
@@ -367,10 +369,9 @@ fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, Form
if let Some(number) = number.as_u64() {
Ok(format_placeholder!(number, "o", placeholder))
} else if let Some(number) = number.as_i64() {
// Truncate to a i32 to mimic JS's behaviour
let number = number as i32;
// Bit by bit cast to u32 to mimic JS's behaviour
let number = number as u32;
// Truncate to a i32 and then u32 to mimic JS's behaviour
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
let number = number as i32 as u32;
Ok(format_placeholder!(number, "o", placeholder))
} else {
Err(FormatError::InvalidTypeSpecifier {
@@ -422,10 +423,9 @@ fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, Form
if let Some(number) = n.as_u64() {
Ok(format_placeholder!(number, "x", placeholder))
} else if let Some(number) = n.as_i64() {
// Truncate to a i32 to mimic JS's behaviour
let number = number as i32;
// Then cast to u32 to mimic JS's behaviour
let number = number as u32;
// Truncate to a i32 and then u32 to mimic JS's behaviour
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
let number = number as i32 as u32;
Ok(format_placeholder!(number, "x", placeholder))
} else {
Err(FormatError::InvalidTypeSpecifier {
@@ -445,10 +445,9 @@ fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, Form
if let Some(number) = n.as_u64() {
Ok(format_placeholder!(number, "X", placeholder))
} else if let Some(number) = n.as_i64() {
// Truncate to a i32 to mimic JS's behaviour
let number = number as i32;
// Then cast to u32 to mimic JS's behaviour
let number = number as u32;
// Truncate to a i32 and then u32 to mimic JS's behaviour
#[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)]
let number = number as i32 as u32;
Ok(format_placeholder!(number, "X", placeholder))
} else {
Err(FormatError::InvalidTypeSpecifier {
@@ -484,6 +483,12 @@ fn format_value(value: &Value, placeholder: &Placeholder) -> Result<String, Form
}
impl Message {
/// Format the message with the given arguments.
///
/// # Errors
///
/// Returns an error if the message can't be formatted with the given
/// arguments.
pub fn format(&self, arguments: &ArgumentList) -> Result<String, FormatError> {
let mut buffer = String::new();
@@ -492,13 +497,13 @@ impl Message {
let mut current_placeholder = 0usize;
for part in self.parts() {
match part {
MessagePart::Percent => {
Part::Percent => {
buffer.push('%');
}
MessagePart::Text(text) => {
Part::Text(text) => {
buffer.push_str(text);
}
MessagePart::Placeholder(placeholder) => {
Part::Placeholder(placeholder) => {
let value = find_value(
arguments,
placeholder.requested_argument.as_ref(),
@@ -511,8 +516,7 @@ impl Message {
let formatted = if let Some(width) = placeholder.width {
let spacer = placeholder
.padding_specifier
.map(|p| p.char())
.unwrap_or(' ');
.map_or(' ', PaddingSpecifier::char);
let alignment = if placeholder.left_align {
Alignment::Left

View File

@@ -106,7 +106,7 @@ impl std::fmt::Display for TypeSpecifier {
Self::HexadecimalNumberUppercase => 'X',
Self::Json => 'j',
};
write!(f, "{}", specifier)
write!(f, "{specifier}")
}
}
@@ -119,8 +119,8 @@ pub enum ArgumentReference {
impl std::fmt::Display for ArgumentReference {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ArgumentReference::Indexed(index) => write!(f, "{}$", index),
ArgumentReference::Named(name) => write!(f, "({})", name),
ArgumentReference::Indexed(index) => write!(f, "{index}$"),
ArgumentReference::Named(name) => write!(f, "({name})"),
}
}
}
@@ -135,16 +135,16 @@ impl std::fmt::Display for PaddingSpecifier {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
PaddingSpecifier::Zero => write!(f, "0"),
PaddingSpecifier::Char(c) => write!(f, "'{}", c),
PaddingSpecifier::Char(c) => write!(f, "'{c}"),
}
}
}
impl PaddingSpecifier {
pub fn char(&self) -> char {
pub fn char(self) -> char {
match self {
PaddingSpecifier::Zero => '0',
PaddingSpecifier::Char(c) => *c,
PaddingSpecifier::Char(c) => c,
}
}
@@ -154,13 +154,6 @@ impl PaddingSpecifier {
PaddingSpecifier::Char(_) => false,
}
}
pub const fn is_char(self) -> bool {
match self {
PaddingSpecifier::Zero => false,
PaddingSpecifier::Char(_) => true,
}
}
}
#[derive(Debug, Clone)]
@@ -177,14 +170,7 @@ pub struct Placeholder {
impl Placeholder {
pub fn padding_specifier_is_zero(&self) -> bool {
self.padding_specifier
.map(PaddingSpecifier::is_zero)
.unwrap_or(false)
}
pub fn padding_specifier_is_char(&self) -> bool {
self.padding_specifier
.map(PaddingSpecifier::is_char)
.unwrap_or(false)
.is_some_and(PaddingSpecifier::is_zero)
}
/// Whether it should be formatted as a number for the width argument
@@ -198,7 +184,7 @@ impl std::fmt::Display for Placeholder {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "%")?;
if let Some(argument) = &self.requested_argument {
write!(f, "{}", argument)?;
write!(f, "{argument}")?;
}
if self.plus_sign {
@@ -206,7 +192,7 @@ impl std::fmt::Display for Placeholder {
}
if let Some(padding_specifier) = &self.padding_specifier {
write!(f, "{}", padding_specifier)?;
write!(f, "{padding_specifier}")?;
}
if self.left_align {
@@ -214,11 +200,11 @@ impl std::fmt::Display for Placeholder {
}
if let Some(width) = self.width {
write!(f, "{}", width)?;
write!(f, "{width}")?;
}
if let Some(precision) = self.precision {
write!(f, ".{}", precision)?;
write!(f, ".{precision}")?;
}
write!(f, "{}", self.type_specifier)
@@ -227,37 +213,28 @@ impl std::fmt::Display for Placeholder {
#[derive(Debug, Clone)]
pub struct Message {
parts: Vec<MessagePart>,
parts: Vec<Part>,
}
impl std::fmt::Display for Message {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
for part in self.parts.iter() {
write!(f, "{}", part)?;
for part in &self.parts {
write!(f, "{part}")?;
}
Ok(())
}
}
impl FromIterator<MessagePart> for Message {
fn from_iter<T: IntoIterator<Item = MessagePart>>(iter: T) -> Self {
impl FromIterator<Part> for Message {
fn from_iter<T: IntoIterator<Item = Part>>(iter: T) -> Self {
Self {
parts: iter.into_iter().collect(),
}
}
}
impl IntoIterator for Message {
type Item = MessagePart;
type IntoIter = std::vec::IntoIter<Self::Item>;
fn into_iter(self) -> Self::IntoIter {
self.parts.into_iter()
}
}
impl Message {
pub fn parts(&self) -> std::slice::Iter<'_, MessagePart> {
pub(crate) fn parts(&self) -> std::slice::Iter<'_, Part> {
self.parts.iter()
}
}
@@ -272,35 +249,35 @@ impl Serialize for Message {
impl<'de> Deserialize<'de> for Message {
fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let string = String::deserialize(deserializer)?;
Ok(string.parse().map_err(serde::de::Error::custom)?)
string.parse().map_err(serde::de::Error::custom)
}
}
#[derive(Debug, Clone)]
pub enum MessagePart {
pub(crate) enum Part {
Percent,
Text(String),
Placeholder(Placeholder),
}
impl From<Placeholder> for MessagePart {
impl From<Placeholder> for Part {
fn from(placeholder: Placeholder) -> Self {
Self::Placeholder(placeholder)
}
}
impl From<String> for MessagePart {
impl From<String> for Part {
fn from(text: String) -> Self {
Self::Text(text)
}
}
impl std::fmt::Display for MessagePart {
impl std::fmt::Display for Part {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
MessagePart::Percent => write!(f, "%%"),
MessagePart::Text(text) => write!(f, "{}", text),
MessagePart::Placeholder(placeholder) => write!(f, "{}", placeholder),
Part::Percent => write!(f, "%%"),
Part::Text(text) => write!(f, "{text}"),
Part::Placeholder(placeholder) => write!(f, "{placeholder}"),
}
}
}

View File

@@ -20,11 +20,12 @@ mod parser;
use thiserror::Error;
pub use self::{
argument::{Argument, ArgumentList},
argument::{Argument, List as ArgumentList},
message::Message,
};
macro_rules! arg_list_inner {
($var:ident |) => { };
($var:ident | $name:ident = $($arg:expr)*, $($rest:tt)*) => {{
$var.push($crate::sprintf::Argument::from((stringify!($name), ::serde_json::json!($($arg)*))));
$crate::sprintf::arg_list_inner!($var | $($rest)* );
@@ -39,12 +40,11 @@ macro_rules! arg_list_inner {
($var:ident | $($arg:expr)*) => {{
$var.push($crate::sprintf::Argument::from(::serde_json::json!($($arg)*)));
}};
($var:ident |) => { };
}
macro_rules! arg_list {
($($args:tt)*) => {{
let mut __args = Vec::new();
let mut __args = Vec::<$crate::sprintf::Argument>::new();
$crate::sprintf::arg_list_inner!(__args | $($args)* );
$crate::sprintf::ArgumentList::from_iter(__args)
}}
@@ -72,7 +72,6 @@ macro_rules! sprintf {
pub(crate) use arg_list;
pub(crate) use arg_list_inner;
pub(crate) use sprintf;
#[derive(Debug, Error)]
#[error(transparent)]

View File

@@ -19,7 +19,7 @@ use std::str::FromStr;
use pest::{error::ErrorVariant, iterators::Pair, Parser, Span};
use super::message::{
ArgumentReference, Message, MessagePart, PaddingSpecifier, Placeholder, TypeSpecifier,
ArgumentReference, Message, PaddingSpecifier, Part, Placeholder, TypeSpecifier,
};
#[derive(pest_derive::Parser)]
@@ -73,17 +73,17 @@ fn ensure_rule_type(pair: &Pair<Rule>, rule: Rule) -> Result<()> {
}
}
fn interpret_ident(pair: Pair<Rule>) -> Result<String> {
ensure_rule_type(&pair, Rule::ident)?;
fn interpret_ident(pair: &Pair<Rule>) -> Result<String> {
ensure_rule_type(pair, Rule::ident)?;
Ok(pair.as_str().to_owned())
}
fn interpret_number(pair: Pair<Rule>) -> Result<usize> {
ensure_rule_type(&pair, Rule::number)?;
fn interpret_number(pair: &Pair<Rule>) -> Result<usize> {
ensure_rule_type(pair, Rule::number)?;
pair.as_str().parse().map_err(|e| {
Error::new_from_span(
ErrorVariant::CustomError {
message: format!("Failed to parse number: {}", e),
message: format!("Failed to parse number: {e}"),
},
pair.as_span(),
)
@@ -96,7 +96,7 @@ fn interpret_arg_named(pair: Pair<Rule>) -> Result<ArgumentReference> {
let mut pairs = pair.into_inner();
let ident = next_pair(&mut pairs, span)?;
let ident = interpret_ident(ident)?;
let ident = interpret_ident(&ident)?;
ensure_end_of_pairs(&mut pairs, span)?;
Ok(ArgumentReference::Named(ident))
@@ -108,14 +108,14 @@ fn interpret_arg_indexed(pair: Pair<Rule>) -> Result<ArgumentReference> {
let mut pairs = pair.into_inner();
let number = next_pair(&mut pairs, span)?;
let number = interpret_number(number)?;
let number = interpret_number(&number)?;
ensure_end_of_pairs(&mut pairs, span)?;
Ok(ArgumentReference::Indexed(number))
}
fn interpret_padding_specifier(pair: Pair<Rule>) -> Result<PaddingSpecifier> {
ensure_rule_type(&pair, Rule::padding_specifier)?;
fn interpret_padding_specifier(pair: &Pair<Rule>) -> Result<PaddingSpecifier> {
ensure_rule_type(pair, Rule::padding_specifier)?;
let specifier: Vec<char> = pair.as_str().chars().collect();
let specifier = match specifier[..] {
@@ -124,7 +124,7 @@ fn interpret_padding_specifier(pair: Pair<Rule>) -> Result<PaddingSpecifier> {
ref specifier => {
return Err(Error::new_from_span(
ErrorVariant::CustomError {
message: format!("Unexpected padding specifier: {:?}", specifier),
message: format!("Unexpected padding specifier: {specifier:?}"),
},
pair.as_span(),
))
@@ -140,7 +140,7 @@ fn interpret_width(pair: Pair<Rule>) -> Result<usize> {
let mut pairs = pair.into_inner();
let number = next_pair(&mut pairs, span)?;
let number = interpret_number(number)?;
let number = interpret_number(&number)?;
ensure_end_of_pairs(&mut pairs, span)?;
Ok(number)
@@ -152,14 +152,14 @@ fn interpret_precision(pair: Pair<Rule>) -> Result<usize> {
let mut pairs = pair.into_inner();
let number = next_pair(&mut pairs, span)?;
let number = interpret_number(number)?;
let number = interpret_number(&number)?;
ensure_end_of_pairs(&mut pairs, span)?;
Ok(number)
}
fn interpret_type_specifier(pair: Pair<Rule>) -> Result<TypeSpecifier> {
ensure_rule_type(&pair, Rule::type_specifier)?;
fn interpret_type_specifier(pair: &Pair<Rule>) -> Result<TypeSpecifier> {
ensure_rule_type(pair, Rule::type_specifier)?;
let specifier: Vec<char> = pair.as_str().chars().collect();
let type_specifier = match specifier[..] {
@@ -218,7 +218,7 @@ fn interpret_placeholder(pair: Pair<Rule>) -> Result<Placeholder> {
};
let padding_specifier = if current_pair.as_rule() == Rule::padding_specifier {
let padding_specifier = interpret_padding_specifier(current_pair)?;
let padding_specifier = interpret_padding_specifier(&current_pair)?;
current_pair = next_pair(&mut pairs, span)?;
Some(padding_specifier)
} else {
@@ -248,7 +248,7 @@ fn interpret_placeholder(pair: Pair<Rule>) -> Result<Placeholder> {
None
};
let type_specifier = interpret_type_specifier(current_pair)?;
let type_specifier = interpret_type_specifier(&current_pair)?;
ensure_end_of_pairs(&mut pairs, span)?;
@@ -272,7 +272,7 @@ impl FromStr for Message {
.filter(|pair| pair.as_rule() != Rule::EOI)
.map(|pair| match pair.as_rule() {
Rule::text => Ok(pair.as_str().to_owned().into()),
Rule::percent => Ok(MessagePart::Percent),
Rule::percent => Ok(Part::Percent),
Rule::placeholder => Ok(interpret_placeholder(pair)?.into()),
_ => Err(unexpected_rule_error(&pair)),
})
@@ -335,9 +335,9 @@ mod tests {
"Hello %(who)s!",
];
for case in cases.into_iter() {
for case in cases {
let result: Result<Message> = case.parse();
assert!(result.is_ok(), "Failed to parse: {}", case);
assert!(result.is_ok(), "Failed to parse: {case}");
let message = result.unwrap();
assert_eq!(message.to_string(), *case);
}

View File

@@ -87,7 +87,7 @@ impl TranslationTree {
match self {
TranslationTree::Message(_) => None,
TranslationTree::Children(tree) => {
let child = tree.get(next.deref())?;
let child = tree.get(&*next)?;
child.walk_path(path)
}
}
@@ -96,14 +96,7 @@ impl TranslationTree {
fn as_message(&self) -> Option<&Message> {
match self {
TranslationTree::Message(message) => Some(message),
_ => None,
}
}
fn as_children(&self) -> Option<&BTreeMap<String, TranslationTree>> {
match self {
TranslationTree::Children(children) => Some(children),
_ => None,
TranslationTree::Children(_) => None,
}
}
}
@@ -111,7 +104,7 @@ impl TranslationTree {
#[cfg(test)]
mod tests {
use super::*;
use crate::sprintf::arg_list;
use crate::sprintf::{arg_list, ArgumentList};
#[test]
fn test_it_works() {
@@ -131,15 +124,12 @@ mod tests {
let message = tree.message("hello");
assert!(message.is_some());
let message = message.unwrap();
assert_eq!(message.format(&Default::default()).unwrap(), "world");
assert_eq!(message.format(&ArgumentList::default()).unwrap(), "world");
let message = tree.message("damals.about_x_hours_ago.one");
assert!(message.is_some());
let message = message.unwrap();
assert_eq!(
message.format(&Default::default()).unwrap(),
"about one hour ago"
);
assert_eq!(message.format(&arg_list!()).unwrap(), "about one hour ago");
let message = tree.pluralize("damals.about_x_hours_ago", PluralCategory::Other);
assert!(message.is_some());