From c4d7cc198a647abe872e4ca39c797b7684bd8e7e Mon Sep 17 00:00:00 2001 From: Simon Michael Date: Thu, 7 Nov 2019 18:58:12 -0800 Subject: [PATCH] csv: finalise and check journals generated from CSV (#1000) Invalid transactions generated from CSV will now be rejected. I updated some csv tests to avoid this, except for 21, which probably needs more cleanup. --- hledger-lib/CHANGES.md | 15 ++------ hledger-lib/Hledger/Read/CsvReader.hs | 24 +++++++----- tests/csv.test | 55 ++++++++++----------------- 3 files changed, 40 insertions(+), 54 deletions(-) diff --git a/hledger-lib/CHANGES.md b/hledger-lib/CHANGES.md index 6f162bdf8..cdd4ba5f1 100644 --- a/hledger-lib/CHANGES.md +++ b/hledger-lib/CHANGES.md @@ -6,6 +6,7 @@ For user-visible changes, see the hledger package changelog. - CSV reading improvements (#1095) - can now generate a variable number of postings, from zero to nine (#627, #1095) + - can now generate postings with balance assignments (#1000) - skip can be used in conditional blocks; so can the new "end" rule (skips all remaining) (#1076) - both the amount-in/amount-out fields having a value is now ok, if one of them is zero (#570) - line feeds/carriage returns in quoted CSV values are now converted to spaces (#416, #841) @@ -16,22 +17,14 @@ For user-visible changes, see the hledger package changelog. - fixed: show two spaces between description and comment, not three - clarified that hledger field assignments can reference csv fields only, not other hledger fields (#500) - new docs, tests - - can now generate postings with balance assignments (#1000, WIP) Migration notes: - CSV rules now give you more freedom to generate any journal - entries you want, including malformed entries or unbalanced - transactions. And, this is not checked, currently. In fact - journals generated from CSV are not fully finalised and should not - be used directly for reports. You should probably pipe their print - output through another hledger to validate. Eg: + entries you want, including malformed or unbalanced ones. hledger + validates the generated journal and will report such errors. - ```shell - $ hledger -f a.csv print | hledger -f- -I CMD` - ``` - - (#1000, WIP) + - Balance assertions generated from CSV are not checked. - When `print`ing from CSV, there is now one less space between transaction descriptions and comments. If you are comparing old diff --git a/hledger-lib/Hledger/Read/CsvReader.hs b/hledger-lib/Hledger/Read/CsvReader.hs index 1885e8e89..9a20cfcc9 100644 --- a/hledger-lib/Hledger/Read/CsvReader.hs +++ b/hledger-lib/Hledger/Read/CsvReader.hs @@ -71,7 +71,7 @@ import Text.Printf (printf) import Hledger.Data import Hledger.Utils -import Hledger.Read.Common (Reader(..),InputOpts(..),amountp, statusp, genericSourcePos) +import Hledger.Read.Common (Reader(..),InputOpts(..),amountp, statusp, genericSourcePos, finaliseJournal) type CSV = [Record] @@ -88,15 +88,21 @@ reader = Reader } -- | Parse and post-process a "Journal" from CSV data, or give an error. --- XXX currently ignores the string and reads from the file path +-- Does not check balance assertions. +-- XXX currently ignores the provided data, reads it from the file path instead. parse :: InputOpts -> FilePath -> Text -> ExceptT String IO Journal parse iopts f t = do let rulesfile = mrules_file_ iopts let separator = separator_ iopts r <- liftIO $ readJournalFromCsv separator rulesfile f t - case r of Left e -> throwError e - Right j -> return $ journalNumberAndTieTransactions j --- XXX does not use parseAndFinaliseJournal like the other readers + case r of Left e -> throwError e + Right pj -> finaliseJournal iopts{ignore_assertions_=True} f t pj' + where + -- finaliseJournal assumes the journal's items are + -- reversed, as produced by JournalReader's parser. + -- But here they are already properly ordered. So we'd + -- better preemptively reverse them once more. XXX inefficient + pj' = journalReverse pj -- | Read a Journal from the given CSV data (and filename, used for error -- messages), or return an error. Proceed as follows: @@ -163,10 +169,10 @@ readJournalFromCsv separator mrulesfile csvfile csvdata = (initialPos parsecfilename) records -- Ensure transactions are ordered chronologically. - -- First, reverse them to get same-date transactions ordered chronologically, - -- if the CSV records seem to be most-recent-first, ie if there's an explicit - -- "newest-first" directive, or if there's more than one date and the first date - -- is more recent than the last. + -- First, if the CSV records seem to be most-recent-first (because + -- there's an explicit "newest-first" directive, or there's more + -- than one date and the first date is more recent than the last): + -- reverse them to get same-date transactions ordered chronologically. txns' = (if newestfirst || mseemsnewestfirst == Just True then reverse else id) txns where diff --git a/tests/csv.test b/tests/csv.test index 174d34740..a88c7f33f 100644 --- a/tests/csv.test +++ b/tests/csv.test @@ -213,13 +213,15 @@ fields date, description, amount, balance1, balance2, amount3,comment3 date-format %d/%Y/%m currency $ account1 assets:myacct -account3 expenses:tax +account3 expenses:tax +account4 the:remainder $ ./hledger-csv 2009/09/10 Flubber Co - assets:myacct $50 = $321 - expenses:unknown = $123 + assets:myacct $50.000 = $321.000 + expenses:unknown = $123.000 expenses:tax $0.234 ; VAT + the:remainder >=0 @@ -232,13 +234,15 @@ fields date, description, amount, balance1, balance2, currency3, amount3,comment date-format %d/%Y/%m currency $ account1 assets:myacct -account3 expenses:tax +account3 expenses:tax +account4 the:remainder $ ./hledger-csv 2009/09/10 Flubber Co assets:myacct $50 = $321 expenses:unknown = $123 expenses:tax £0.234 ; VAT + the:remainder >=0 @@ -418,34 +422,15 @@ $ ./hledger-csv # 20. Test for #1001 - empty assignment to amount show not eat next line < -2018-10-15,100 -2018-10-16,200 -2018-10-17,300 +2018-10-15,1 + RULES -fields date,bal +fields date,amount +amount +comment x -balance EUR %bal -date-format %Y-%m-%d -description Assets Update -account1 assets -account2 income -if 2018 - amount - comment Dont eat me - balance - comment Dont eat me $ ./hledger-csv -2018/10/15 Assets Update ; Dont eat me - assets - income - -2018/10/16 Assets Update ; Dont eat me - assets - income - -2018/10/17 Assets Update ; Dont eat me - assets - income +2018/10/15 ; x >=0 @@ -470,6 +455,13 @@ date-format %m/%d/%Y description %description for %itemtitle if Conditional Empty Fee account3 +# XXX skip this one for now, not sure what should be done +# 2018/12/22 (987654321) Someone for Joyful Systems +# sm:assets:online:paypal $6.66 = $99.60 +# sm:expenses:unknown +# JS:expenses:banking:paypal +if ,Empty fee + skip $ ./hledger-csv 2018/12/22 (123456789) Someone for Joyful Systems @@ -477,11 +469,6 @@ $ ./hledger-csv sm:expenses:unknown JS:expenses:banking:paypal $-0.59 -2018/12/22 (987654321) Someone for Joyful Systems - sm:assets:online:paypal $6.66 = $99.60 - sm:expenses:unknown - JS:expenses:banking:paypal - 2018/12/22 (10101010101) Someone for Joyful Systems sm:assets:online:paypal $7.77 = $88.66 sm:expenses:unknown $-7.77