diff --git a/hledger-lib/Hledger/Data/Transaction.hs b/hledger-lib/Hledger/Data/Transaction.hs index 4eac78d00..4c9cc0979 100644 --- a/hledger-lib/Hledger/Data/Transaction.hs +++ b/hledger-lib/Hledger/Data/Transaction.hs @@ -59,6 +59,7 @@ module Hledger.Data.Transaction ( ) where import Data.List +import Data.List.Extra (nubSort) import Data.Maybe import Data.Text (Text) import qualified Data.Text as T @@ -339,31 +340,55 @@ transactionPostingBalances t = (sumPostings $ realPostings t ,sumPostings $ virtualPostings t ,sumPostings $ balancedVirtualPostings t) --- | Check that this transaction appears balanced when rendered, --- returning an appropriate error message if it is not. --- In more detail: after converting amounts to cost using explicit --- transaction prices if any; and summing the real postings; and --- summing the balanced virtual postings; and applying the given --- display styles, if provided (maybe affecting decimal places): --- do both posting totals appear to be zero when rendered ? +-- | Check that this transaction would appear balanced to a human when displayed, +-- and return an appropriate error message otherwise. +-- +-- In more detail: +-- For the real postings, and separately for the balanced virtual postings: +-- +-- 1. Convert amounts to cost where possible +-- +-- 2. When there are multiple amounts which appear non-zero when displayed +-- (using the given display styles if provided), +-- are they a mix of positives and negatives ? +-- This is checked separately to give a clearer error message. +-- (Best effort, could be confused by postings with multicommodity amounts.) +-- +-- 3. Does the amounts' sum appear non-zero when displayed ? +-- (using the given display styles if provided) +-- transactionCheckBalanced :: Maybe (M.Map CommoditySymbol AmountStyle) -> Transaction -> Maybe String transactionCheckBalanced mstyles t - | rbalanced && bvbalanced = Nothing - | otherwise = Just $ printf "could not balance this transaction (%s%s%s)" rmsg sep bvmsg + | rsignsok && bvsignsok && rzerosum && bvzerosum = Nothing + | otherwise = Just msg where - (rsum, _, bvsum) = transactionPostingBalances t + (rps, bvps) = (realPostings t, balancedVirtualPostings t) + canonicalise = maybe id canonicaliseMixedAmount mstyles + + -- check mixed signs, detecting nonzeros at display precision + nonZeros ps = filter (not.isZeroMixedAmount) $ map (canonicalise.costOfMixedAmount.pamount) ps + signsOk ps = + case nonZeros ps of + nzs | length nzs > 1 -> length (nubSort $ mapMaybe isNegativeMixedAmount nzs) > 1 + _ -> True + (rsignsok, bvsignsok) = (signsOk rps, signsOk bvps) + + -- check zero sum, at display precision + (rsum, bvsum) = (sumPostings rps, sumPostings bvps) (rsumcost, bvsumcost) = (costOfMixedAmount rsum, costOfMixedAmount bvsum) (rsumdisplay, bvsumdisplay) = (canonicalise rsumcost, canonicalise bvsumcost) - where canonicalise = maybe id canonicaliseMixedAmount mstyles - -- for checking balanced, use the display precision - (rbalanced, bvbalanced) = (isZeroMixedAmount rsumdisplay, isZeroMixedAmount bvsumdisplay) - -- for selecting and generating error messages, use the uncanonicalised full precision - -- XXX always correct ? - rmsg | isReallyZeroMixedAmount rsumcost = "" - | otherwise = "real postings are off by " ++ showMixedAmount rsumcost - bvmsg | isReallyZeroMixedAmount bvsumcost = "" - | otherwise = "balanced virtual postings are off by " ++ showMixedAmount bvsumcost - sep = if not (null rmsg) && not (null bvmsg) then "; " else "" :: String + (rzerosum, bvzerosum) = (isZeroMixedAmount rsumdisplay, isZeroMixedAmount bvsumdisplay) + + -- select & generate error messages, showing amounts with their original precision + rmsg + | not rsignsok = "real postings all have the same sign" + | not rzerosum = "real postings' sum should be 0 but is: " ++ showMixedAmount rsumcost + | otherwise = "" + bvmsg + | not bvsignsok = "balanced virtual postings all have the same sign" + | not bvzerosum = "balanced virtual postings' sum should be 0 but is: " ++ showMixedAmount bvsumcost + | otherwise = "" + msg = intercalate "\n" $ ["could not balance this transaction:"] ++ filter (not.null) [rmsg, bvmsg] -- | Legacy form of transactionCheckBalanced. isTransactionBalanced :: Maybe (M.Map CommoditySymbol AmountStyle) -> Transaction -> Bool @@ -403,7 +428,7 @@ balanceTransactionHelper mstyles t = do Just err -> Left $ annotateErrorWithTransaction t' err annotateErrorWithTransaction :: Transaction -> String -> String -annotateErrorWithTransaction t s = intercalate "\n" [showGenericSourcePos $ tsourcepos t, s, showTransaction t] +annotateErrorWithTransaction t s = unlines [showGenericSourcePos $ tsourcepos t, s, showTransaction t] -- | Infer up to one missing amount for this transactions's real postings, and -- likewise for its balanced virtual postings, if needed; or return an error diff --git a/tests/journal/parse-errors.test b/tests/journal/parse-errors.test index 87295df28..fa4aa6310 100644 --- a/tests/journal/parse-errors.test +++ b/tests/journal/parse-errors.test @@ -19,23 +19,18 @@ expecting date separator or digit # A journal with an unbalanced transaction. < -2018/1/1 - a 1 +2009/1/1 + a 1 -# 2. When read from stdin with no reader prefix, the journal reader is used, -# and fails here. (Before 1.17, all readers were tried and the timedot reader -# would succeed.) +# 2. When read from stdin with no reader prefix, the journal reader is used +# (before 1.17, all readers were tried and the timedot reader would succeed), +# giving an unbalanced error in this case. $ hledger -f - print ->2 /could not balance/ +>2 /real postings' sum should be 0 but is: 1/ >=1 -# 3. So in these tests we must sometimes force the desired format, like so. -# Now we see the error from the journal reader. -$ hledger -f journal:- print ->2 /could not balance this transaction \(real postings are off by 1\)/ ->=1 - -# 4. We expect to have reference to line number with last posting +# 3. We expect to have reference to line number with last posting +# NB: \< and \> match start and end of a word. $ hledger -f journal:- print >2 /\<2\>/ >=1 @@ -45,7 +40,7 @@ $ hledger -f journal:- print 2018/1/1 (a) 1 -# 5. hledger doesn't detect this as an error directly, it parses account name "(a) 1" and +# 4. hledger doesn't detect this as an error directly, it parses account name "(a) 1" and # amount 0 here. $ hledger -f - print -x 2018-01-01 @@ -53,7 +48,7 @@ $ hledger -f - print -x >= -# 6. Two (or more) postings with implicit amount cannot be balanced. +# 5. Two (or more) postings with implicit amount cannot be balanced. < 2018/1/1 a 1 @@ -64,7 +59,7 @@ $ hledger -f journal:- print >2 /could not balance this transaction - can't have more than one real posting with no amount/ >=1 -# 7. Two (or more) virtual postings with implicit amount cannot be balanced. +# 6. Two (or more) virtual postings with implicit amount cannot be balanced. # (And the error message contains line numbers). < 2018/1/1 @@ -75,3 +70,64 @@ $ hledger -f journal:- print $ hledger -f journal:- print >2 /lines 1-4/ >=1 + +# 7. Balancing error messages show the recorded precision, not the display precision. +< +2010/1/1 + a 1C @ $1.0049 + a $-1.000 +$ hledger -f- print +>2 /real postings' sum should be 0 but is: \$0.0049/ +>=1 + +# 8. Multiple postings all with the same sign gets a distinct error message. +< +2020-01-01 + a 1A + b 1A +$ hledger -f- print +>2 /real postings all have the same sign/ +>=1 + +# 9. Zeros don't trigger the same sign error. +< +2020-01-01 + a A 0 + b B 0 +$ hledger -f- print +2020-01-01 + a 0 + b 0 + +>=0 + +# 10. An inferred multicommodity amount doesn't trigger the same sign error. +< +2020-01-01 + a A 1 + b B 1 + c +$ hledger -f- print +2020-01-01 + a A 1 + b B 1 + c + +>=0 + +# 11. For multiple postings all with the same sign but in different commodities, +# the error should show the original amounts, without inferring transaction prices. +< +2020-01-01 + a 1A + b 1B +$ hledger -f- print +>2 +hledger: "-" (lines 1-3) +could not balance this transaction: +real postings all have the same sign +2020-01-01 + a 1A @@ 1B + b 1B + +>=1 diff --git a/tests/journal/precision.test b/tests/journal/precision.test index 2a5907b98..d6a83d8d3 100644 --- a/tests/journal/precision.test +++ b/tests/journal/precision.test @@ -38,7 +38,7 @@ hledger -f - balance --no-total --cost --empty 2010/1/1 a 1C @ $1.0049 a $-1.000 ->>>2 /off by \$0.0049/ +>>>2 /should be 0 but is: \$0.0049/ >>>= 1 # 4. with $'s display precision at 2 or less, this txn should balance diff --git a/tests/journal/unbalanced.test b/tests/journal/unbalanced.test deleted file mode 100644 index 393e839a9..000000000 --- a/tests/journal/unbalanced.test +++ /dev/null @@ -1,7 +0,0 @@ -hledger -f - register -<<< -2009/1/1 a - b 1.1 - c -1 ->>>2 /could not balance this transaction \(real postings are off by 0.1\)/ ->>>= 1