Skip to content

Commit

Permalink
Add better error message for BookKeepError on particular case.
Browse files Browse the repository at this point in the history
  • Loading branch information
xkikeg committed Sep 21, 2024
1 parent 02c0046 commit ae124d1
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 18 deletions.
29 changes: 21 additions & 8 deletions core/src/report/book_keeping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use chrono::NaiveDate;

use crate::{
load,
syntax::{self, decoration::AsUndecorated},
syntax::{self, decoration::AsUndecorated, tracked::Tracked},
};

use super::{
Expand All @@ -29,8 +29,8 @@ pub enum BookKeepError {
BalanceFailure(#[from] BalanceError),
#[error("posting amount must be resolved as a simple value with commodity or zero")]
ComplexPostingAmount,
#[error("transaction cannot have multiple postings without amount: {0} {1}")]
UndeduciblePostingAmount(usize, usize),
#[error("transaction cannot have multiple postings without amount")]
UndeduciblePostingAmount(Tracked<usize>, Tracked<usize>),
#[error("transaction cannot have unbalanced postings: {0}")]
UnbalancedPostings(String),
#[error("balance assertion failed: got {0} but expected {1}")]
Expand Down Expand Up @@ -159,7 +159,7 @@ fn add_transaction<'ctx>(
txn: &syntax::tracked::Transaction,
) -> Result<Transaction<'ctx>, BookKeepError> {
let mut postings = bcc::Vec::with_capacity_in(txn.posts.len(), ctx.arena);
let mut unfilled: Option<usize> = None;
let mut unfilled: Option<Tracked<usize>> = None;
let mut balance = Amount::default();
for (i, posting) in txn.posts.iter().enumerate() {
let account = ctx.accounts.ensure(&posting.as_undecorated().account);
Expand All @@ -168,8 +168,11 @@ fn add_transaction<'ctx>(
&posting.as_undecorated().balance,
) {
(None, None) => {
if let Some(j) = unfilled.replace(i) {
Err(BookKeepError::UndeduciblePostingAmount(j, i))
if let Some(first) = unfilled.replace(Tracked::new(i, posting.span())) {
Err(BookKeepError::UndeduciblePostingAmount(
first,
Tracked::new(i, posting.span()),
))
} else {
Ok((PostingAmount::zero(), PostingAmount::zero()))
}
Expand Down Expand Up @@ -213,6 +216,7 @@ fn add_transaction<'ctx>(
});
}
if let Some(u) = unfilled {
let u = *u.as_undecorated();
let deduced: Amount = balance.clone().negate();
postings[u].amount = deduced.clone();
bal.add_amount(postings[u].account, deduced);
Expand Down Expand Up @@ -303,7 +307,10 @@ mod tests {
use pretty_assertions::assert_eq;
use rust_decimal_macros::dec;

use crate::parse::{self, testing::expect_parse_ok};
use crate::{
parse::{self, testing::expect_parse_ok},
syntax::tracked::TrackedSpan,
};

fn parse_transaction(input: &str) -> syntax::tracked::Transaction {
let (_, ret) = expect_parse_ok(parse::transaction::transaction, input);
Expand Down Expand Up @@ -493,7 +500,13 @@ mod tests {
let mut ctx = ReportContext::new(&arena);
let mut bal = Balance::default();
let got = add_transaction(&mut ctx, &mut bal, &txn).expect_err("must fail");
assert_eq!(got, BookKeepError::UndeduciblePostingAmount(0, 1));
assert_eq!(
got,
BookKeepError::UndeduciblePostingAmount(
Tracked::new(0, TrackedSpan::new(20..42)),
Tracked::new(1, TrackedSpan::new(44..66))
)
);
}

#[test]
Expand Down
37 changes: 27 additions & 10 deletions core/src/report/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
use std::{fmt::Display, path::PathBuf};

use annotate_snippets::{Level, Snippet};
use annotate_snippets::{Annotation, Level, Snippet};

use crate::{load, parse};
use crate::{
load,
parse::{self, ParsedSpan},
};

use super::book_keeping;
use super::book_keeping::{self, BookKeepError};

/// Error arised in report APIs.
#[derive(thiserror::Error, Debug)]
Expand All @@ -31,22 +34,35 @@ pub struct ErrorContext {
path: PathBuf,
line_start: usize,
text: String,
parsed_span: ParsedSpan,
}

impl ErrorContext {
fn print(
&self,
f: &mut std::fmt::Formatter<'_>,
err: &book_keeping::BookKeepError,
) -> std::fmt::Result {
fn print(&self, f: &mut std::fmt::Formatter<'_>, err: &BookKeepError) -> std::fmt::Result {
let message = err.to_string();
let path = self.path.to_string_lossy();
let annotations: Vec<Annotation> = match err {
BookKeepError::UndeduciblePostingAmount(first, second) => {
vec![
Level::Warning
.span(self.parsed_span.resolve(first.span()))
.label("first posting that requires deduce"),
Level::Error
.span(self.parsed_span.resolve(second.span()))
.label("either this or previous posting must specify the amount"),
]
}
_ => {
// TODO: Add more detailed error into this.
// Also, put these logic into BookKeepError.
vec![Level::Error.span(0..self.text.len()).label("error occured")]
}
};
let message = Level::Error.title(&message).snippet(
Snippet::source(&self.text)
.origin(&path)
.line_start(self.line_start)
.fold(true)
.annotation(Level::Error.span(0..self.text.len())),
.annotations(annotations),
);
let rendered = self.renderer.render(message);
rendered.fmt(f)
Expand All @@ -62,6 +78,7 @@ impl ErrorContext {
path,
line_start: pctx.compute_line_start(),
text: pctx.as_str().to_owned(),
parsed_span: pctx.span(),
})
}
}
6 changes: 6 additions & 0 deletions core/testdata/error/undeducible.ledger
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
; Two postings without amount nor balance are undeducible.

2024/09/01 Shopping with point
Expenses 100 CHF
Assets:Banks
Assets:Points

0 comments on commit ae124d1

Please sign in to comment.