imp:lots: show precise posting location in unclassified lotful error

Use index-based posting lookup (makePostingErrorExcerptByIndex) instead
of equality-based search, which failed when postings were modified by
the balancer after parsing. The error now points to the specific posting
line rather than the transaction start.

Author: simonmichael

hledger-lib/Hledger/Data/Errors.hs

@@ -11,6 +11,7 @@ module Hledger.Data.Errors (
   makePriceDirectiveErrorExcerpt,
   makeTransactionErrorExcerpt,
   makePostingErrorExcerpt,
+  makePostingErrorExcerptByIndex,
   makePostingAccountErrorExcerpt,
   makeBalanceAssertionErrorExcerpt,
   transactionFindPostingIndex,
@@ -199,6 +200,24 @@ decoratePostingErrorExcerpt absline relline mcols txt =
     lineprefix = T.replicate marginw " " <> "| "
       where  marginw = length (show absline) + 1
 
+-- | Like 'makePostingErrorExcerpt', but identifies the posting by its
+-- 0-based index in the transaction rather than by equality search.
+-- This avoids false mismatches when postings have been modified after parsing
+-- (e.g. by the balancer), and is unambiguous when duplicate postings exist.
+makePostingErrorExcerptByIndex :: Transaction -> Int -> (FilePath, Int, Maybe (Int, Maybe Int), Text)
+makePostingErrorExcerptByIndex t idx = (f, errabsline, Nothing, ex)
+  where
+    (SourcePos f tl _) = fst $ tsourcepos t
+    errrelline =
+      commentExtraLines (tcomment t) +
+      sum (map postingLines $ take (idx + 1) $ tpostings t)
+      where
+        postingLines p' = 1 + commentExtraLines (pcomment p')
+        commentExtraLines c = max 0 (length (T.lines c) - 1)
+    errabsline = unPos tl + errrelline
+    txntxt = showTransaction t & textChomp & (<>"\n")
+    ex = decoratePostingErrorExcerpt errabsline errrelline Nothing txntxt
+
 -- | Find the 1-based index of the first posting in this transaction
 -- satisfying the given predicate.
 transactionFindPostingIndex :: (Posting -> Bool) -> Transaction -> Maybe Int

hledger-lib/Hledger/Data/Lots.hs

@@ -125,7 +125,7 @@ import Hledger.Data.AccountName (accountNameType)
 import Hledger.Data.Dates (showDate)
 import Hledger.Data.AccountType (isAssetType, isEquityType)
 import Hledger.Data.Amount (amountSetQuantity, amountsRaw, isNegativeAmount, maNegate, mapMixedAmount, mixedAmount, mixedAmountLooksZero, nullmixedamt, noCostFmt, showAmountWith, showMixedAmountOneLine)
-import Hledger.Data.Errors (makePostingErrorExcerpt, makeTransactionErrorExcerpt)
+import Hledger.Data.Errors (makePostingErrorExcerpt, makePostingErrorExcerptByIndex, makeTransactionErrorExcerpt)
 import Hledger.Data.Journal (journalAccountType, journalCommodityLotsMethod, journalCommodityUsesLots, journalFilePaths, journalInheritedAccountTags, journalMapTransactions, journalTieTransactions, parseReductionMethod)
 import Hledger.Data.Posting (generatedPostingTagName, hasAmount, isReal, nullposting, originalPosting, postingAddHiddenAndMaybeVisibleTag, postingStripCosts)
 import Hledger.Data.Transaction (txnTieKnot)
@@ -590,7 +590,7 @@ transactionClassifyLotPostings verbosetags lookupAccountType commodityIsLotful t
 journalCalculateLots :: Bool -> Journal -> Either String Journal
 journalCalculateLots verbosetags j
   | not $ any (any isLotPosting . tpostings) txns = do
-      mapM_ checkUnclassified [p | t <- txns, p <- tpostings t]
+      mapM_ checkUnclassified [(t, i, p) | t <- txns, (i, p) <- zip [0..] (tpostings t)]
       Right j
   | otherwise = do
       validateUserLabels txns
@@ -599,8 +599,8 @@ journalCalculateLots verbosetags j
       Right (journalTieTransactions $ j{jtxns = reverse txns'})
   where
     txns = jtxns j
-    checkUnclassified p
-      | isUnclassifiedLotfulPosting j p = Left (unclassifiedLotWarning j p)
+    checkUnclassified (t, i, p)
+      | isUnclassifiedLotfulPosting j p = Left (unclassifiedLotWarning j t i p)
       | otherwise                       = Right ()
 
 -- Disposal balancing
@@ -770,17 +770,19 @@ isUnclassifiedLotfulPosting j p =
       | hasAccountLotsTag     = any ((/= 0) . aquantity) amts
       | otherwise             = False
 
--- | Build a warning message for an unclassified lotful posting.
-unclassifiedLotWarning :: Journal -> Posting -> String
-unclassifiedLotWarning j p =
+-- | Build an error message for an unclassified lotful posting.
+-- Takes the transaction and the 0-based posting index for precise source location.
+unclassifiedLotWarning :: Journal -> Transaction -> Int -> Posting -> String
+unclassifiedLotWarning j t idx p =
   let amts = amountsRaw (pamount p)
       lotfulCommodities = [acommodity a | a <- amts, journalCommodityUsesLots j (acommodity a)]
       hasAccountTag = any ((== "lots") . T.toLower . fst) (ptags p)
       source = case (lotfulCommodities, hasAccountTag) of
         (c:_, _)   -> T.unpack c ++ " is declared lotful (commodity lots: tag)"
         ([], True)  -> T.unpack (paccount p) ++ " is declared lotful (account lots: tag)"
         _           -> "posting involves a lotful commodity or account"
-  in postingErrPrefix p
+      (f, line, _, ex) = makePostingErrorExcerptByIndex t idx
+  in printf "%s:%d:\n%s\n" f line ex
      ++ source ++ " but this posting was not classified as\n"
      ++ "acquire, dispose, or transfer. Lot state will not be updated.\n"
      ++ "Possible fixes: add a cost basis ({$X}), a price (@ $X),\n"
@@ -937,7 +939,7 @@ processTransaction verbosetags j needsLabels (ls, acc) t = do
           (st', newPs) <- processDisposePosting verbosetags j t st p
           foldMPostings st' (reverse newPs ++ acc') ps tmap
       | isUnclassifiedLotfulPosting j p =
-          Left (unclassifiedLotWarning j p)
+          Left (unclassifiedLotWarning j t i p)
       | otherwise =
           foldMPostings st (p:acc') ps tmap