diff --git a/hledger-lib/Hledger/Data/Errors.hs b/hledger-lib/Hledger/Data/Errors.hs index 419aaed66..83cbefb2b 100644 --- a/hledger-lib/Hledger/Data/Errors.hs +++ b/hledger-lib/Hledger/Data/Errors.hs @@ -7,6 +7,7 @@ Helpers for making error messages. module Hledger.Data.Errors ( makeTransactionErrorExcerpt, makePostingErrorExcerpt, + makePostingAccountErrorExcerpt, makeBalanceAssertionErrorExcerpt, transactionFindPostingIndex, ) @@ -22,6 +23,7 @@ import Hledger.Data.Types import Hledger.Utils import Data.Maybe import Safe (headMay) +import Hledger.Data.Posting (isVirtual) -- | Given a problem transaction and a function calculating the best -- column(s) for marking the error region: @@ -103,6 +105,17 @@ transactionFindPostingIndex :: (Posting -> Bool) -> Transaction -> Maybe Int transactionFindPostingIndex ppredicate = fmap fst . find (ppredicate.snd) . zip [1..] . tpostings +-- | From the given posting, make an error excerpt showing the transaction with +-- this posting's account part highlighted. +makePostingAccountErrorExcerpt :: Posting -> (FilePath, Int, Maybe (Int, Maybe Int), Text) +makePostingAccountErrorExcerpt p = makePostingErrorExcerpt p finderrcols + where + -- Calculate columns suitable for highlighting the synthetic excerpt. + finderrcols p _ _ = Just (col, Just col2) + where + col = 5 + if isVirtual p then 1 else 0 + col2 = col + T.length (paccount p) - 1 + -- | From the given posting, make an error excerpt showing the transaction with -- the balance assertion highlighted. makeBalanceAssertionErrorExcerpt :: Posting -> (FilePath, Int, Maybe (Int, Maybe Int), Text) diff --git a/hledger-lib/Hledger/Data/JournalChecks.hs b/hledger-lib/Hledger/Data/JournalChecks.hs index f93eddb7d..b9528f12d 100644 --- a/hledger-lib/Hledger/Data/JournalChecks.hs +++ b/hledger-lib/Hledger/Data/JournalChecks.hs @@ -32,7 +32,7 @@ import Hledger.Data.JournalChecks.Uniqueleafnames import Hledger.Data.Posting (isVirtual, postingDate, postingStatus) import Hledger.Data.Types import Hledger.Data.Amount (amountIsZero, amountsRaw, missingamt) -import Hledger.Data.Transaction (transactionPayee, showTransactionLineFirstPart, showTransaction) +import Hledger.Data.Transaction (transactionPayee, showTransactionLineFirstPart) import Data.Time (Day, diffDays) import Data.List.Extra import Hledger.Utils (chomp, textChomp, sourcePosPretty) @@ -55,14 +55,7 @@ journalCheckAccounts j = mapM_ checkacct (journalPostings j) ,"account %s ; type:A ; (L,E,R,X,C,V)" ]) f l ex (show a) a a where - (f,l,_mcols,ex) = makePostingErrorExcerpt p finderrcols - -- Calculate columns suitable for highlighting the excerpt. - -- We won't show these in the main error line as they aren't - -- accurate for the actual data. - finderrcols p _ _ = Just (col, Just col2) - where - col = 5 + if isVirtual p then 1 else 0 - col2 = col + T.length a - 1 + (f,l,_mcols,ex) = makePostingAccountErrorExcerpt p -- | Check that all the commodities used in this journal's postings have been declared -- by commodity directives, returning an error message otherwise. @@ -191,8 +184,10 @@ balanceAssertionInfo ps = where ps' = sortOn postingDate ps mlatestp = lastMay ps' - mlatestassertp = lastMay $ filter (isJust.pbalanceassertion) ps + mlatestassertp = lastMay [p | p@Posting{pbalanceassertion=Just _} <- ps'] +-- | The number of days allowed between an account's latest balance assertion +-- and latest posting. maxlag = 7 -- | The number of days between this balance assertion and the latest posting in its account. @@ -202,6 +197,20 @@ baiLag BAI{..} = diffDays baiLatestPostingDate baiLatestAssertionDate -- baiLagOkDate :: BalanceAssertionInfo -> Day -- baiLagOkDate BAI{..} = addDays (-7) baiLatestPostingDate +-- | Check that this latest assertion is close enough to the account's latest posting. +checkRecentAssertion :: BalanceAssertionInfo -> Either (BalanceAssertionInfo, String) () +checkRecentAssertion bai@BAI{..} + | lag > maxlag = + Left (bai, printf (chomp $ unlines [ + "the last balance assertion (%s) was %d days before" + ,"the latest posting (%s)." + ]) + (show baiLatestAssertionDate) lag (show baiLatestPostingDate) + ) + | otherwise = Right () + where + lag = baiLag bai + -- | Check that all the journal's accounts with balance assertions have -- an assertion no more than 7 days before their latest posting. -- Today's date is provided for error messages. @@ -213,15 +222,14 @@ journalCheckRecentAssertions today j = in case mapM_ checkRecentAssertion acctassertioninfos of Right () -> Right () - Left (bai@BAI{..}, msg) -> Left errmsg + Left (BAI{..}, msg) -> Left errmsg where errmsg = chomp $ printf (unlines [ "%s:", "%s\n", - -- "In balance-asserted account %s,", - "The recentassertions check is enabled, so accounts with balance assertions", - "must have an assertion no more than %d days before their latest posting date.", + "The recentassertions check is enabled, so accounts with balance assertions must", + "have a balance assertion no more than %d days before their latest posting date.", "In account %s,", "%s", "", @@ -236,50 +244,11 @@ journalCheckRecentAssertions today j = recommendation where (_,_,_,excerpt) = makeBalanceAssertionErrorExcerpt baiLatestAssertionPosting - recommendation - | baiLag bai > maxlag = unlines [ - "Consider adding a more recent balance assertion for this account. Eg:", - "", - printf "%s *\n %s $0 = $0 ; <- adjust" (show today) baiAccount - ] - | otherwise = unlines [ - "Consider marking this posting or transaction cleared. Eg:", - "", - case ptransaction baiLatestAssertionPosting of - Nothing -> "(no transaction)" -- shouldn't happen - Just t -> T.unpack $ showTransaction t' - where - t' = t{tstatus=tstatus', tpostings=ps'} - where - -- clear just the posting if it was marked pending, otherwise clear the whole transaction - ispunmarked = pstatus baiLatestAssertionPosting == Unmarked - tstatus' = if ispunmarked then Cleared else tstatus t - pstatus' = if ispunmarked then Unmarked else Cleared - ps' = beforeps ++ [baiLatestAssertionPosting{pstatus=pstatus'}] ++ afterps - where - beforeps = takeWhile (/= baiLatestAssertionPosting) $ tpostings t - afterps = drop (length beforeps + 1) $ tpostings t - ] - --- | Check that this latest assertion is close enough to the account's latest posting. -checkRecentAssertion :: BalanceAssertionInfo -> Either (BalanceAssertionInfo, String) () -checkRecentAssertion bai@BAI{..} - | lag > maxlag = - Left (bai, printf (chomp $ unlines [ - "the last balance assertion (%s) was %d days before" - ,"the latest posting (%s)." - ]) - (show baiLatestAssertionDate) lag (show baiLatestPostingDate) - ) - | baiLatestAssertionStatus /= Cleared = - Left (bai, printf "the last balance assertion's status is %s, should be * (cleared)" - (case baiLatestAssertionStatus of - Unmarked -> "unmarked" :: String - Pending -> "! (pending)" - Cleared -> "* (cleared)")) - | otherwise = Right () - where - lag = baiLag bai + recommendation = unlines [ + "Consider adding a more recent balance assertion for this account. Eg:", + "", + printf "%s *\n %s $0 = $0 ; <- adjust" (show today) baiAccount + ] -- -- | Print the last balance assertion date & status of all accounts with balance assertions. -- printAccountLastAssertions :: Day -> [BalanceAssertionInfo] -> IO () @@ -287,6 +256,6 @@ checkRecentAssertion bai@BAI{..} -- forM_ acctassertioninfos $ \BAI{..} -> do -- putStr $ printf "%-30s %s %s, %d days ago\n" -- baiAccount --- (if baiLatestAssertionStatus==Unmarked then " " else show baiLatestAssertionStatus) --- (show baiLatestAssertionDate) --- (diffDays today baiLatestAssertionDate) +-- (if baiLatestClearedAssertionStatus==Unmarked then " " else show baiLatestClearedAssertionStatus) +-- (show baiLatestClearedAssertionDate) +-- (diffDays today baiLatestClearedAssertionDate) diff --git a/hledger/Hledger/Cli/Commands/Check.md b/hledger/Hledger/Cli/Commands/Check.md index 95fdffced..0cf38c780 100644 --- a/hledger/Hledger/Cli/Commands/Check.md +++ b/hledger/Hledger/Cli/Commands/Check.md @@ -62,7 +62,7 @@ They are more specialised and not desirable for everyone, therefore optional: - **payees** - all payees used by transactions [have been declared](#declaring-payees) - **recentassertions** - all accounts with balance assertions have a - (cleared) assertion no more than 7 days before their latest posting + balance assertion no more than 7 days before their latest posting - **uniqueleafnames** - all account leaf names are unique @@ -80,15 +80,16 @@ See: Cookbook -> [Scripting](scripting.html). ### More about specific checks -`hledger check recentassertions` will complain if any balance-asserted account does not have -a (cleared) balance assertion within 7 days before its latest posting. +`hledger check recentassertions` will complain if any balance-asserted account +does not have a balance assertion within 7 days before its latest posting. This aims to prevent the situation where you are regularly updating your journal, but forgetting to check your balances against the real world, then one day must dig back through months of data to find an error. It assumes that adding a balance assertion requires/reminds you to check the real-world balance. -This won't be true if you auto-generate balance assertions when importing bank data, -but hopefully your auto-generated transactions are uncleared, and before manually marking them -cleared you will remember to check the latest assertions against the real-world balances. +That may not be true if you auto-generate balance assertions from bank data; +in that case, I recommend to import transactions uncleared, +then use the manual-review-and-mark-cleared phase as a reminder +to check the latest assertions against real-world balances. [add-on commands]: #add-on-commands [balance assertions]: #balance-assertions diff --git a/hledger/test/errors/README.md b/hledger/test/errors/README.md index 6a7df8346..1a3abdfb7 100644 --- a/hledger/test/errors/README.md +++ b/hledger/test/errors/README.md @@ -156,7 +156,7 @@ Click error names to see an example. The table headings mean: -hledger 1.26.99-g99825d37f-20220731 error messages: +hledger 1.26.99-ge777ae46f-20220803 error messages: ### accounts ``` @@ -188,9 +188,9 @@ this balance was asserted: 1 but the calculated balance is: 0 a difference of: 1 -Consider viewing this account's register to troubleshoot. Eg: +Consider viewing this account's calculated balances to troubleshoot. Eg: -hledger reg -I 'a$' cur:'' +hledger reg 'a$' cur: -I # -f FILE ``` @@ -304,20 +304,20 @@ payee p ### recentassertions ``` -hledger: Error: /Users/simon/src/hledger/hledger/test/errors/./recentassertions.j:4:8: +hledger: Error: /Users/simon/src/hledger/hledger/test/errors/./recentassertions.j:5:8: | 2022-01-01 * -4 | a 0 = 0 +5 | a 0 = 0 | ^^^ -The recentassertions check is enabled, so accounts with balance assertions -must have an assertion no more than 7 days before their latest posting date. +The recentassertions check is enabled, so accounts with balance assertions must +have a balance assertion no more than 7 days before their latest posting date. In account a, the last balance assertion (2022-01-01) was 8 days before the latest posting (2022-01-09). Consider adding a more recent balance assertion for this account. Eg: -2022-07-31 * +2022-08-03 * a $0 = $0 ; <- adjust ``` diff --git a/hledger/test/errors/recentassertions.j b/hledger/test/errors/recentassertions.j index 431979633..f98c56591 100755 --- a/hledger/test/errors/recentassertions.j +++ b/hledger/test/errors/recentassertions.j @@ -1,4 +1,5 @@ #!/usr/bin/env -S hledger check recentassertions -f +# Latest balance assertion more than 7 days behind latest posting. 2022-01-01 * a 0 = 0 diff --git a/hledger/test/errors/recentassertions.test b/hledger/test/errors/recentassertions.test index 90debbfc3..6145ed343 100644 --- a/hledger/test/errors/recentassertions.test +++ b/hledger/test/errors/recentassertions.test @@ -1,11 +1,11 @@ $$$ hledger check recentassertions -f recentassertions.j ->>>2 /hledger: Error: .*recentassertions.j:4:8: +>>>2 /hledger: Error: .*recentassertions.j:5:8: \| 2022-01-01 \* -4 \| a 0 = 0 +5 \| a 0 = 0 \| \^\^\^ -The recentassertions check is enabled, so accounts with balance assertions -must have an assertion no more than 7 days before their latest posting date. +The recentassertions check is enabled, so accounts with balance assertions must +have a balance assertion no more than 7 days before their latest posting date. In account a, -the las/ +/ >>>= 1