Skip to content

Commit 8e97e9e

Browse files
Merge pull request #604 from Automattic/fix-decimals
fix(core): currency placement with certain decimal positions
2 parents f0157bd + 35cd658 commit 8e97e9e

File tree

11 files changed

+251
-120
lines changed

11 files changed

+251
-120
lines changed

harper-core/src/currency.rs

+3-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use is_macro::Is;
22
use serde::{Deserialize, Serialize};
33

4-
use crate::NumberSuffix;
4+
use crate::Number;
55

66
#[derive(Debug, Is, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Hash)]
77
pub enum Currency {
@@ -62,14 +62,10 @@ impl Currency {
6262
}
6363

6464
/// Format an amount of the specific currency.
65-
pub fn format_amount(&self, value: f64, suffix: Option<NumberSuffix>) -> String {
65+
pub fn format_amount(&self, amount: Number) -> String {
6666
let c = self.to_char();
6767

68-
let mut amount = value.to_string();
69-
70-
if let Some(suffix) = suffix {
71-
amount.extend(suffix.to_chars());
72-
}
68+
let amount = amount.to_string();
7369

7470
match self {
7571
Currency::Dollar => format!("{}{amount}", c),

harper-core/src/document.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ impl Document {
286286
if let (TokenKind::Number(..), TokenKind::Word(..)) = (a.kind, b.kind) {
287287
if let Some(found_suffix) = NumberSuffix::from_chars(self.get_span_content(b.span))
288288
{
289-
*self.tokens[idx].kind.as_mut_number().unwrap().1 = Some(found_suffix);
289+
self.tokens[idx].kind.as_mut_number().unwrap().suffix = Some(found_suffix);
290290
replace_starts.push(idx);
291291
}
292292
}

harper-core/src/lexing/mod.rs

+21-9
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use url::lex_url;
99
use self::email_address::lex_email_address;
1010
use crate::char_ext::CharExt;
1111
use crate::punctuation::{Punctuation, Quote};
12-
use crate::{TokenKind, WordMetadata};
12+
use crate::{Number, TokenKind, WordMetadata};
1313

1414
#[derive(Debug)]
1515
pub struct FoundToken {
@@ -79,8 +79,15 @@ pub fn lex_number(source: &[char]) -> Option<FoundToken> {
7979
// Find the longest possible valid number
8080
while !s.is_empty() {
8181
if let Ok(n) = s.parse::<f64>() {
82+
let precision = s.chars().rev().position(|c| c == '.').unwrap_or_default();
83+
8284
return Some(FoundToken {
83-
token: TokenKind::Number(n.into(), None),
85+
token: TokenKind::Number(Number {
86+
value: n.into(),
87+
suffix: None,
88+
radix: 10,
89+
precision,
90+
}),
8491
next_index: s.len(),
8592
});
8693
}
@@ -119,7 +126,12 @@ pub fn lex_hex_number(source: &[char]) -> Option<FoundToken> {
119126
// Should always succeed unless the logic above is broken
120127
if let Ok(n) = u64::from_str_radix(&s, 16) {
121128
return Some(FoundToken {
122-
token: TokenKind::Number(OrderedFloat(n as f64), None),
129+
token: TokenKind::Number(Number {
130+
value: OrderedFloat(n as f64),
131+
suffix: None,
132+
radix: 16,
133+
precision: 0,
134+
}),
123135
next_index: s.len() + 2,
124136
});
125137
}
@@ -232,7 +244,7 @@ mod tests {
232244
assert!(matches!(
233245
lex_hex_number(&source),
234246
Some(FoundToken {
235-
token: TokenKind::Number(_, None),
247+
token: TokenKind::Number(_),
236248
..
237249
})
238250
));
@@ -244,7 +256,7 @@ mod tests {
244256
assert!(matches!(
245257
lex_hex_number(&source),
246258
Some(FoundToken {
247-
token: TokenKind::Number(_, None),
259+
token: TokenKind::Number(_),
248260
..
249261
})
250262
));
@@ -256,7 +268,7 @@ mod tests {
256268
assert!(matches!(
257269
lex_hex_number(&source),
258270
Some(FoundToken {
259-
token: TokenKind::Number(_, None),
271+
token: TokenKind::Number(_),
260272
..
261273
})
262274
));
@@ -268,7 +280,7 @@ mod tests {
268280
assert!(matches!(
269281
lex_hex_number(&source),
270282
Some(FoundToken {
271-
token: TokenKind::Number(_, None),
283+
token: TokenKind::Number(_),
272284
..
273285
})
274286
));
@@ -280,7 +292,7 @@ mod tests {
280292
assert!(matches!(
281293
lex_hex_number(&source),
282294
Some(FoundToken {
283-
token: TokenKind::Number(_, None),
295+
token: TokenKind::Number(_),
284296
..
285297
})
286298
));
@@ -292,7 +304,7 @@ mod tests {
292304
assert!(matches!(
293305
lex_hex_number(&source),
294306
Some(FoundToken {
295-
token: TokenKind::Number(_, None),
307+
token: TokenKind::Number(_),
296308
..
297309
})
298310
));

harper-core/src/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub mod language_detection;
1212
mod lexing;
1313
pub mod linting;
1414
mod mask;
15+
mod number;
1516
pub mod parsers;
1617
pub mod patterns;
1718
mod punctuation;
@@ -34,13 +35,14 @@ pub use fat_token::FatToken;
3435
pub use ignored_lints::IgnoredLints;
3536
use linting::Lint;
3637
pub use mask::{Mask, Masker};
38+
pub use number::{Number, NumberSuffix};
3739
pub use punctuation::{Punctuation, Quote};
3840
pub use span::Span;
3941
pub use spell::{Dictionary, FstDictionary, FullDictionary, MergedDictionary};
4042
pub use sync::Lrc;
4143
pub use title_case::{make_title_case, make_title_case_str};
4244
pub use token::Token;
43-
pub use token_kind::{NumberSuffix, TokenKind};
45+
pub use token_kind::TokenKind;
4446
pub use token_string_ext::TokenStringExt;
4547
pub use vec_ext::VecExt;
4648
pub use word_metadata::{AdverbData, ConjunctionData, NounData, Tense, VerbData, WordMetadata};

harper-core/src/linting/correct_number_suffix.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::{Lint, LintKind, Linter, Suggestion};
2-
use crate::TokenStringExt;
32
use crate::{Document, NumberSuffix, Span, TokenKind};
3+
use crate::{Number, TokenStringExt};
44

55
/// Detect and warn that the sentence is too long.
66
#[derive(Debug, Clone, Copy, Default)]
@@ -15,8 +15,13 @@ impl Linter for CorrectNumberSuffix {
1515
continue;
1616
};
1717

18-
if let TokenKind::Number(number, Some(suffix)) = number_tok.kind {
19-
if let Some(correct_suffix) = NumberSuffix::correct_suffix_for(number) {
18+
if let TokenKind::Number(Number {
19+
value,
20+
suffix: Some(suffix),
21+
..
22+
}) = number_tok.kind
23+
{
24+
if let Some(correct_suffix) = NumberSuffix::correct_suffix_for(value) {
2025
if suffix != correct_suffix {
2126
output.push(Lint {
2227
span: suffix_span,

harper-core/src/linting/currency_placement.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,11 @@ fn generate_lint_for_tokens(a: Token, b: Token, document: &Document) -> Option<L
4545
.expect_punctuation();
4646
let currency = punct.as_currency()?;
4747

48-
let (value, suffix) = matched_tokens.first_number()?.kind.expect_number();
48+
let number = matched_tokens.first_number()?.kind.expect_number();
4949

5050
let span = matched_tokens.span().unwrap();
5151

52-
let correct: Vec<_> = currency
53-
.format_amount(value.into(), suffix)
54-
.chars()
55-
.collect();
52+
let correct: Vec<_> = currency.format_amount(number).chars().collect();
5653
let actual = document.get_span_content(span);
5754

5855
if correct != actual {
@@ -141,4 +138,9 @@ mod tests {
141138
"It was my $20th.",
142139
);
143140
}
141+
142+
#[test]
143+
fn seven_even_two_decimal_clean() {
144+
assert_lint_count("$7.00", CurrencyPlacement::default(), 0);
145+
}
144146
}

harper-core/src/linting/number_suffix_capitalization.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::{Lint, LintKind, Linter, Suggestion};
2-
use crate::TokenStringExt;
32
use crate::{Document, Span, TokenKind};
3+
use crate::{Number, TokenStringExt};
44

55
/// Detect and warn that the sentence is too long.
66
#[derive(Debug, Clone, Copy, Default)]
@@ -11,7 +11,7 @@ impl Linter for NumberSuffixCapitalization {
1111
let mut output = Vec::new();
1212

1313
for number_tok in document.iter_numbers() {
14-
if let TokenKind::Number(_, None) = number_tok.kind {
14+
if let TokenKind::Number(Number { suffix: None, .. }) = number_tok.kind {
1515
continue;
1616
}
1717

harper-core/src/linting/spelled_numbers.rs

+13-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::linting::{LintKind, Linter, Suggestion};
2-
use crate::{Document, Lint, TokenStringExt};
2+
use crate::{Document, Lint, Number, TokenStringExt};
33

44
/// Linter that checks to make sure small integers (< 10) are spelled
55
/// out.
@@ -11,15 +11,22 @@ impl Linter for SpelledNumbers {
1111
let mut lints = Vec::new();
1212

1313
for number_tok in document.iter_numbers() {
14-
let (number, _suffix) = number_tok.kind.number().unwrap();
15-
let number: f64 = number.into();
16-
17-
if (number - number.floor()).abs() < f64::EPSILON && number < 10. {
14+
let Number {
15+
value,
16+
suffix: None,
17+
..
18+
} = number_tok.kind.number().unwrap()
19+
else {
20+
continue;
21+
};
22+
let value: f64 = value.into();
23+
24+
if (value - value.floor()).abs() < f64::EPSILON && value < 10. {
1825
lints.push(Lint {
1926
span: number_tok.span,
2027
lint_kind: LintKind::Readability,
2128
suggestions: vec![Suggestion::ReplaceWith(
22-
spell_out_number(number as u64).unwrap().chars().collect(),
29+
spell_out_number(value as u64).unwrap().chars().collect(),
2330
)],
2431
message: "Try to spell out numbers less than ten.".to_string(),
2532
priority: 63,

0 commit comments

Comments
 (0)