imp: check: commodities: use the standard error format (#1436)

hledger-lib/Hledger/Read/Common.hs

@@ -134,6 +134,7 @@ import Data.List.NonEmpty (NonEmpty(..))
 import Data.Maybe (catMaybes, fromMaybe, isJust, listToMaybe)
 import qualified Data.Map as M
 import qualified Data.Semigroup as Sem
+import qualified Data.Set as S
 import Data.Text (Text)
 import qualified Data.Text as T
 import Data.Time.Calendar (Day, fromGregorianValid, toGregorian)
@@ -152,6 +153,7 @@ import Hledger.Reports.ReportOptions (ReportOpts(..), queryFromFlags, rawOptsToR
 import Hledger.Utils
 import Text.Printf (printf)
 import Hledger.Read.InputOptions
+import Safe (atMay)
 
 --- ** doctest setup
 -- $setup
@@ -416,23 +418,39 @@ journalCheckCommoditiesDeclared :: Journal -> Either String ()
 journalCheckCommoditiesDeclared j =
   mapM_ checkcommodities (journalPostings j)
   where
-    checkcommodities Posting{..} =
+    checkcommodities p@Posting{ptransaction=mt,paccount=acct,pamount=amt,pbalanceassertion} =
       case mfirstundeclaredcomm of
-        Nothing -> Right ()
+        Nothing   -> Right ()
-        Just c  -> Left $
+        Just comm -> Left $
-          (printf "undeclared commodity \"%s\"\n" (T.unpack c))
+          -- we don't save the original column of amounts
-          ++ case ptransaction of
+          printf "%s:%d:\n%sundeclared commodity \"%s\"\n" f l excerpt comm
-              Nothing -> ""
+          where
-              Just t  -> printf "in transaction at: %s\n\n%s"
+            (f,l,excerpt) = case mt of
-                          (sourcePosPairPretty $ tsourcepos t)
+              Nothing -> ("-",0,"")
-                          (linesPrepend "  " . (<>"\n") . textChomp $ showTransaction t)
+              Just t  -> (tf,errabsline,ex)
+                where
+                  (SourcePos tf tl _tc) = fst $ tsourcepos t
+                  ppredicate = S.member comm . maCommodities . pamount
+                  mpindex = fmap fst $ find (ppredicate.snd) $ zip [1..] $ tpostings t
+                  tcommentlines = max 0 $ length (T.lines $ tcomment t) - 1
+                  errrelline = maybe 0 (tcommentlines+) mpindex
+                  errabsline = unPos tl + errrelline
+                  txn = showTransaction t & textChomp & (<>"\n")
+                  errcol = case mpindex of
+                    Nothing     -> 0
+                    Just pindex -> acctend + (T.length $ T.takeWhile isnotsymbol $ T.drop acctend l)
+                      where
+                        acctend = 4 + T.length acct + if isVirtual p then 2 else 0
+                        isnotsymbol c = isSpace c || isDigit c || isDigitSeparatorChar c
+                        l = fromMaybe "" $ (T.lines txn `atMay` pindex)
+                  ex = formatExcerptLikeMegaparsec errabsline errrelline errcol txn
       where
         mfirstundeclaredcomm =
           find (`M.notMember` jcommodities j)
           . map acommodity
           . (maybe id ((:) . baamount) pbalanceassertion)
           . filter (not . isIgnorable)
-          $ amountsRaw pamount
+          $ amountsRaw amt
 
     -- Ignore missing amounts and zero amounts without commodity (#1767)
     isIgnorable a = (T.null (acommodity a) && amountIsZero a) || a == missingamt

hledger/test/errors/README.md

@@ -50,8 +50,8 @@ Here is the current status
 | balanced                 |            | Y                | -                  | Y      | Y                |                 |
 | balancednoautoconversion |            | Y                | -                  | Y      | Y                |                 |
 | assertions               |            | Y                |                    | Y      | Y                | Y               |
-| accounts                 | Y          |                  |                    | Y      | Y                |                 |
+| accounts                 | Y          | Y                | Y                  | Y      | Y                |                 |
-| commodities              |            |                  |                    | Y      | Y                |                 |
+| commodities              | Y          | Y                |                    | Y      | Y                |                 |
 | payees                   |            |                  |                    | Y      | Y                | Y               |
 | ordereddates             |            |                  |                    | Y      | Y                | Y               |
 | uniqueleafnames          |            |                  |                    | Y      | Y                |                 |
@@ -81,6 +81,7 @@ Notes (see also [#1436][]):
 - includes the word "Error" and the error position on line 1
 - FILE is the file path
 - LOCATION is `LINE[-ENDLINE][:COLUMN[-ENDCOLUMN]]`
+- we may show 0 for LINE or COLUMN when unknown
 - EXCERPT is a short visual snippet whenever possible, with the error region highlighted, line numbers, and colour when supported. This section must be easy for flycheck to ignore.
 - SUMMARY is a one line description/explanation of the problem. 
   These are currently dynamic, they can include helpful contextual info.
@@ -183,11 +184,11 @@ undeclared account "a"
 
 ### commodities
 ```
-hledger: Error: undeclared commodity "A"
+hledger: Error: /Users/simon/src/hledger/hledger/test/errors/commodities.j:6:
-in transaction at: /Users/simon/src/hledger/hledger/test/errors/./commodities.j:5-6
+  | 2022-01-01
-
+6 |     (a)             A 1
-  2022-01-01
+  |                     ^
-      (a)             A 1
+undeclared commodity "A"
 
 ```