From 017d3355f82560e13f927d140572ab7ebdc566fa Mon Sep 17 00:00:00 2001
From: Simon Michael <simon@joyful.com>
Date: Sat, 15 Feb 2020 13:52:59 -0800
Subject: [PATCH] csv: allow manual assignment of "expenses:unknown" again (fix
 #1192)

---
 hledger-lib/Hledger/Read/CsvReader.hs | 48 +++++++++++++++++----------
 tests/csv.test                        | 17 ++++++++++
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/hledger-lib/Hledger/Read/CsvReader.hs b/hledger-lib/Hledger/Read/CsvReader.hs
index accf33a29..899182691 100644
--- a/hledger-lib/Hledger/Read/CsvReader.hs
+++ b/hledger-lib/Hledger/Read/CsvReader.hs
@@ -739,6 +739,14 @@ transactionFromCsvRecord sourcepos rules record = t
       ,"the parse error is:      "++customErrorBundlePretty err
       ]
 
+    -- Default account names to use when one is not set.
+    unknownExpenseAccount = "expenses:unknown"
+    unknownIncomeAccount  = "income:unknown"
+    -- A temporary placeholder for the unknown account name, which
+    -- gets replaced by one of the above based on the amount's sign.
+    -- This is a value hopefully never chosen by users (cf #1192). XXX
+    unknownPlaceholderAccount = "_unknown_"
+
     parsePosting' number accountFld amountFld amountInFld amountOutFld balanceFld commentFld =
       let currency = maybe (fromMaybe "" mdefaultcurrency) render $
                       (mfieldtemplate ("currency"++number) `or `mfieldtemplate "currency")
@@ -749,18 +757,20 @@ transactionFromCsvRecord sourcepos rules record = t
           comment = T.pack $ maybe "" render $ mfieldtemplate commentFld
           account =
             case account' of
-              -- If account is explicitly "unassigned", suppress posting
-              -- Otherwise, generate posting with "expenses:unknown" account if we have amount/balance information
+              -- account is set to "" - suppress posting
               Just "" -> Nothing
+              -- account is set
               Just account -> Just account
               Nothing ->
-                -- If we have amount or balance assertion (which implies potential amount change),
-                -- but no account name, lets generate "expenses:unknown" account name.
                 case (amount, balance) of
-                  (Just _, _ ) -> Just "expenses:unknown"
-                  (_, Just _)  -> Just "expenses:unknown"
+                  -- account is not set, but an amount is set (or implied by
+                  -- balance assignment) - use "unknown" account.
+                  (Just _, _ ) -> Just unknownPlaceholderAccount
+                  (_, Just _)  -> Just unknownPlaceholderAccount
+                  -- no account, no amount
                   (Nothing, Nothing) -> Nothing
-          in
+      in
+        -- if there's an account N, make a posting N
         case account of
           Nothing -> Nothing
           Just account -> 
@@ -800,29 +810,31 @@ transactionFromCsvRecord sourcepos rules record = t
 
     postings' = catMaybes $ posting1:[ parsePosting i | x<-[2..9], let i = show x]
 
-    improveUnknownAccountName p =
-      if paccount p /="expenses:unknown"
-      then p
-      else case isNegativeMixedAmount (pamount p) of
-        Just True -> p{paccount = "income:unknown"}
-        Just False -> p{paccount = "expenses:unknown"}
-        _ -> p
-        
     postings =
       case postings' of
         -- To be compatible with the behavior of the old code which allowed two postings only, we enforce
         -- second posting when rules generated just first of them, and posting is of type that should be balanced.
-        -- When we have strictly first and second posting, but second posting does not have amount, we fill it in.
         [("1",posting1)] ->
           case ptype posting1 of
             VirtualPosting -> [posting1]
             _ ->
-              [posting1,improveUnknownAccountName (posting{paccount="expenses:unknown", pamount=costOfMixedAmount(-(pamount posting1)), ptransaction=Just t})]
+              [posting1,improveUnknownAccountName (posting{paccount=unknownPlaceholderAccount, pamount=costOfMixedAmount(-(pamount posting1)), ptransaction=Just t})]
+        -- When we have strictly first and second posting, but second posting does not have amount, we fill it in.
         [("1",posting1),("2",posting2)] ->
           case (pamount posting1 == missingmixedamt , pamount posting2 == missingmixedamt) of
             (False, True) -> [posting1, improveUnknownAccountName (posting2{pamount=costOfMixedAmount(-(pamount posting1))})]
             _  -> [posting1, posting2]
-        _ -> map snd postings'
+
+        _ -> map (improveUnknownAccountName . snd) postings'
+      where
+        improveUnknownAccountName p =
+          if paccount p == unknownPlaceholderAccount
+          then case isNegativeMixedAmount (pamount p) of
+            Just True  -> p{paccount = unknownIncomeAccount}
+            Just False -> p{paccount = unknownExpenseAccount}
+            _ -> p
+          else p
+        
         
     -- build the transaction
     t = nulltransaction{
diff --git a/tests/csv.test b/tests/csv.test
index 7fe862686..8a9b6defa 100644
--- a/tests/csv.test
+++ b/tests/csv.test
@@ -544,6 +544,23 @@ $  ./hledger-csv
 
 >=0
 
+# 26. manually setting hledger's default "expenses:unknown"/"income:unknown" names works (#1192)
+<
+2020-01-01,5
+
+RULES
+fields date, amount
+account1 a
+account2 expenses:unknown
+
+$  ./hledger-csv
+2020-01-01
+    a                              5
+    expenses:unknown              -5
+
+>=0
+
+
 ## 26. A single unbalanced posting with number other than 1 also should not generate a balancing posting.
 #<
 #2019-01-01,1