fix: check: recentassertions: drop the cleared requirement (#1899)

It was causing trouble and complicating things too much.
I wanted it as another signal of being "caught up", but YAGNI.
This commit is contained in:
Simon Michael 2022-08-01 16:30:39 +01:00
parent fa31612b55
commit 910b37abf8
6 changed files with 64 additions and 80 deletions

View File

@ -7,6 +7,7 @@ Helpers for making error messages.
module Hledger.Data.Errors ( module Hledger.Data.Errors (
makeTransactionErrorExcerpt, makeTransactionErrorExcerpt,
makePostingErrorExcerpt, makePostingErrorExcerpt,
makePostingAccountErrorExcerpt,
makeBalanceAssertionErrorExcerpt, makeBalanceAssertionErrorExcerpt,
transactionFindPostingIndex, transactionFindPostingIndex,
) )
@ -22,6 +23,7 @@ import Hledger.Data.Types
import Hledger.Utils import Hledger.Utils
import Data.Maybe import Data.Maybe
import Safe (headMay) import Safe (headMay)
import Hledger.Data.Posting (isVirtual)
-- | Given a problem transaction and a function calculating the best -- | Given a problem transaction and a function calculating the best
-- column(s) for marking the error region: -- column(s) for marking the error region:
@ -103,6 +105,17 @@ transactionFindPostingIndex :: (Posting -> Bool) -> Transaction -> Maybe Int
transactionFindPostingIndex ppredicate = transactionFindPostingIndex ppredicate =
fmap fst . find (ppredicate.snd) . zip [1..] . tpostings 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 -- | From the given posting, make an error excerpt showing the transaction with
-- the balance assertion highlighted. -- the balance assertion highlighted.
makeBalanceAssertionErrorExcerpt :: Posting -> (FilePath, Int, Maybe (Int, Maybe Int), Text) makeBalanceAssertionErrorExcerpt :: Posting -> (FilePath, Int, Maybe (Int, Maybe Int), Text)

View File

@ -32,7 +32,7 @@ import Hledger.Data.JournalChecks.Uniqueleafnames
import Hledger.Data.Posting (isVirtual, postingDate, postingStatus) import Hledger.Data.Posting (isVirtual, postingDate, postingStatus)
import Hledger.Data.Types import Hledger.Data.Types
import Hledger.Data.Amount (amountIsZero, amountsRaw, missingamt) 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.Time (Day, diffDays)
import Data.List.Extra import Data.List.Extra
import Hledger.Utils (chomp, textChomp, sourcePosPretty) 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)" ,"account %s ; type:A ; (L,E,R,X,C,V)"
]) f l ex (show a) a a ]) f l ex (show a) a a
where where
(f,l,_mcols,ex) = makePostingErrorExcerpt p finderrcols (f,l,_mcols,ex) = makePostingAccountErrorExcerpt p
-- 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
-- | Check that all the commodities used in this journal's postings have been declared -- | Check that all the commodities used in this journal's postings have been declared
-- by commodity directives, returning an error message otherwise. -- by commodity directives, returning an error message otherwise.
@ -191,8 +184,10 @@ balanceAssertionInfo ps =
where where
ps' = sortOn postingDate ps ps' = sortOn postingDate ps
mlatestp = lastMay 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 maxlag = 7
-- | The number of days between this balance assertion and the latest posting in its account. -- | 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 :: BalanceAssertionInfo -> Day
-- baiLagOkDate BAI{..} = addDays (-7) baiLatestPostingDate -- 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 -- | Check that all the journal's accounts with balance assertions have
-- an assertion no more than 7 days before their latest posting. -- an assertion no more than 7 days before their latest posting.
-- Today's date is provided for error messages. -- Today's date is provided for error messages.
@ -213,15 +222,14 @@ journalCheckRecentAssertions today j =
in in
case mapM_ checkRecentAssertion acctassertioninfos of case mapM_ checkRecentAssertion acctassertioninfos of
Right () -> Right () Right () -> Right ()
Left (bai@BAI{..}, msg) -> Left errmsg Left (BAI{..}, msg) -> Left errmsg
where where
errmsg = chomp $ printf errmsg = chomp $ printf
(unlines [ (unlines [
"%s:", "%s:",
"%s\n", "%s\n",
-- "In balance-asserted account %s,", "The recentassertions check is enabled, so accounts with balance assertions must",
"The recentassertions check is enabled, so accounts with balance assertions", "have a balance assertion no more than %d days before their latest posting date.",
"must have an assertion no more than %d days before their latest posting date.",
"In account %s,", "In account %s,",
"%s", "%s",
"", "",
@ -236,50 +244,11 @@ journalCheckRecentAssertions today j =
recommendation recommendation
where where
(_,_,_,excerpt) = makeBalanceAssertionErrorExcerpt baiLatestAssertionPosting (_,_,_,excerpt) = makeBalanceAssertionErrorExcerpt baiLatestAssertionPosting
recommendation recommendation = unlines [
| baiLag bai > maxlag = unlines [
"Consider adding a more recent balance assertion for this account. Eg:", "Consider adding a more recent balance assertion for this account. Eg:",
"", "",
printf "%s *\n %s $0 = $0 ; <- adjust" (show today) baiAccount 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
-- -- | Print the last balance assertion date & status of all accounts with balance assertions. -- -- | Print the last balance assertion date & status of all accounts with balance assertions.
-- printAccountLastAssertions :: Day -> [BalanceAssertionInfo] -> IO () -- printAccountLastAssertions :: Day -> [BalanceAssertionInfo] -> IO ()
@ -287,6 +256,6 @@ checkRecentAssertion bai@BAI{..}
-- forM_ acctassertioninfos $ \BAI{..} -> do -- forM_ acctassertioninfos $ \BAI{..} -> do
-- putStr $ printf "%-30s %s %s, %d days ago\n" -- putStr $ printf "%-30s %s %s, %d days ago\n"
-- baiAccount -- baiAccount
-- (if baiLatestAssertionStatus==Unmarked then " " else show baiLatestAssertionStatus) -- (if baiLatestClearedAssertionStatus==Unmarked then " " else show baiLatestClearedAssertionStatus)
-- (show baiLatestAssertionDate) -- (show baiLatestClearedAssertionDate)
-- (diffDays today baiLatestAssertionDate) -- (diffDays today baiLatestClearedAssertionDate)

View File

@ -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) - **payees** - all payees used by transactions [have been declared](#declaring-payees)
- **recentassertions** - all accounts with balance assertions have a - **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 - **uniqueleafnames** - all account leaf names are unique
@ -80,15 +80,16 @@ See: Cookbook -> [Scripting](scripting.html).
### More about specific checks ### More about specific checks
`hledger check recentassertions` will complain if any balance-asserted account does not have `hledger check recentassertions` will complain if any balance-asserted account
a (cleared) balance assertion within 7 days before its latest posting. 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, This aims to prevent the situation where you are regularly updating your journal,
but forgetting to check your balances against the real world, but forgetting to check your balances against the real world,
then one day must dig back through months of data to find an error. 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. 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, That may not be true if you auto-generate balance assertions from bank data;
but hopefully your auto-generated transactions are uncleared, and before manually marking them in that case, I recommend to import transactions uncleared,
cleared you will remember to check the latest assertions against the real-world balances. 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 [add-on commands]: #add-on-commands
[balance assertions]: #balance-assertions [balance assertions]: #balance-assertions

View File

@ -156,7 +156,7 @@ Click error names to see an example. The table headings mean:
<!-- GENERATED: --> <!-- GENERATED: -->
hledger 1.26.99-g99825d37f-20220731 error messages: hledger 1.26.99-ge777ae46f-20220803 error messages:
### accounts ### accounts
``` ```
@ -188,9 +188,9 @@ this balance was asserted: 1
but the calculated balance is: 0 but the calculated balance is: 0
a difference of: 1 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 ### 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 * | 2022-01-01 *
4 | a 0 = 0 5 | a 0 = 0
| ^^^ | ^^^
The recentassertions check is enabled, so accounts with balance assertions The recentassertions check is enabled, so accounts with balance assertions must
must have an assertion no more than 7 days before their latest posting date. have a balance assertion no more than 7 days before their latest posting date.
In account a, In account a,
the last balance assertion (2022-01-01) was 8 days before the last balance assertion (2022-01-01) was 8 days before
the latest posting (2022-01-09). the latest posting (2022-01-09).
Consider adding a more recent balance assertion for this account. Eg: Consider adding a more recent balance assertion for this account. Eg:
2022-07-31 * 2022-08-03 *
a $0 = $0 ; <- adjust a $0 = $0 ; <- adjust
``` ```

View File

@ -1,4 +1,5 @@
#!/usr/bin/env -S hledger check recentassertions -f #!/usr/bin/env -S hledger check recentassertions -f
# Latest balance assertion more than 7 days behind latest posting.
2022-01-01 * 2022-01-01 *
a 0 = 0 a 0 = 0

View File

@ -1,11 +1,11 @@
$$$ hledger check recentassertions -f recentassertions.j $$$ hledger check recentassertions -f recentassertions.j
>>>2 /hledger: Error: .*recentassertions.j:4:8: >>>2 /hledger: Error: .*recentassertions.j:5:8:
\| 2022-01-01 \* \| 2022-01-01 \*
4 \| a 0 = 0 5 \| a 0 = 0
\| \^\^\^ \| \^\^\^
The recentassertions check is enabled, so accounts with balance assertions The recentassertions check is enabled, so accounts with balance assertions must
must have an assertion no more than 7 days before their latest posting date. have a balance assertion no more than 7 days before their latest posting date.
In account a, In account a,
the las/ /
>>>= 1 >>>= 1