hledger icon indicating copy to clipboard operation
hledger copied to clipboard

warn more about incomplete account type declarations which disturb bs/is/cf

Open xMAC94x opened this issue 2 years ago • 9 comments

I just compared the release binarys from this github and found a regression in them. The newer ones don't generate any output for cashflows with my setup. I produced a minimal reproduction scenario:

Reproduction scenario

Create a file called regression.journal

; Declare top level accounts, setting their types and display order;
; Replace these account names with yours; it helps commands like bs and is detect them.
account assets       ; type:A, things I own
account revenues     ; type:R, inflow categories; part of E, separated for reporting

account assets:cash:sparkasse  ; Liquide EUR auf dem Sparkassenkonto
account revenues:initial  ; Initial money before 2017
commodity 1.000,00 EUR

2016-12-31 Initial  ;  initial:
    revenues:initial
    assets:cash:sparkasse                           1337,69 EUR =   1337,69 EUR

run the following command

./hledger cf -f  ./regression.journal -s

result on 1.26.1

Cashflow Statement 2016-12-31

                       ||   2016-12-31
=======================++==============
 Cash flows            ||
-----------------------++--------------
 assets:cash:sparkasse || 1.337,69 EUR
-----------------------++--------------
                       || 1.337,69 EUR

result on 1.27

Cashflow Statement 2016-12-31

            || 2016-12-31
============++============
 Cash flows ||
------------++------------
------------++------------
            ||

Is this an regression ? Or am i doing something wrong in my journal file ? If so, is there any method to verify the journal files, that gives me some error output or error information ?

Workaround

It turns out removing the following line fixed the cashflow under 1.27 But why do i need to remove it ?

account assets       ; type:A, things I own

Operating system:

arch-linux updated 2023-08-23 14:02 GMT+2

Interesting findings:

  • The changelog mentions: "empty cashflows are skipped " maybe the check is not 100% correct ?
  • adding --anon in 1.26.1 returns no cashflow
  • the flag -s is not necessary for this bug to appear, but even in 1.26.1 no warning is shown that any of the syntax is deprecated or will be removed
  • arch linux 1.30 has the same bug, but for reproduction purposes i choose official release binaries
  • the usage of the account keywork is even showed in the current docs: https://hledger.org/1.30/hledger.html?highlight=account#journal-cheatsheet

xMAC94x avatar Aug 24 '23 09:08 xMAC94x

I didn't see anything related in the 1.27 release notes. I suspect it's this breaking change mentioned in the 1.25 release notes, but I don't know why the behaviour changes with 1.27 not 1.25.

The rule for auto-detecting "cash" (liquid asset) accounts from account names for the cashflow report has been simplified. If you have been using the cashflow report, without explicitly declaring Cash accounts, you might notice a change, and might need to declare your Cash accounts explicitly (by adding type:C tags to top-level cash account directives).

simonmichael avatar Aug 24 '23 10:08 simonmichael

Actually, that change log item was subsequently changed (I'll update the release notes):

The rule for auto-detecting "cash" (liquid asset) accounts in the cashflow report has changed: it's now "all accounts under a top-level asset account, with cash, bank, checking or saving in their name" (case insensitive, variations allowed). So if you see a change in your cashflow reports, you might need to add account directives with type:C tags, declaring your top-most cash accounts.

It might not be the issue, I'll find out.

simonmichael avatar Aug 24 '23 10:08 simonmichael

That was a red herring. ~~[...wild goose chase elided...]~~

cashflow shows changes in Cash accounts. I think declaring the Asset accounts without also declaring Cash accounts stops it detecting the latter. See https://hledger.org/dev/hledger.html#account-types :

If you declare any account types, it's a good idea to declare an account for all of the account types, because a mixture of declared and name-inferred types can disrupt certain reports.

To fix it, also declare assets:cash as type C.

If I were to guess, I'd say this 1.27 commit caused the behaviour change (related PR: #1923):

  • c966a0f41 * fix!: cbr: Remove old account type query code. (#1921)

simonmichael avatar Aug 24 '23 11:08 simonmichael

In short, I think this is a limitation warned about in docs and not a bug. Though UX improvement plans are always welcome. I can investigate the "Interesting findings" also if needed. Thanks for the report.

simonmichael avatar Aug 24 '23 11:08 simonmichael

Hi @simonmichael thanks for the quick answer. marking it as C works. Agree, seems to be more like a documentation thing and it was announced 2 version earlier. Would have loved to see warnings in versions before though that show depcrecations, maybe this is a good feature request for some day.

Btw, is the regex here in the documents still correct? https://hledger.org/1.30/hledger.html#account-types

Feel free to close the issue :)

xMAC94x avatar Aug 24 '23 15:08 xMAC94x

More warnings could be nice, if they don't get in the way. Up to now we have kept things simple - the journal is either valid or not.

What about this particular situation ? Should we raise an error if the journal declares Asset accounts but not Cash accounts ?

simonmichael avatar Aug 24 '23 17:08 simonmichael

Or more generally, if there are account declarations for some but not all account types ? Some of them are not used by everyone (eg V).

Should reports which use particular account types (like cashflow) warn/raise an error when they find no accounts, or no account type declarations, with those types ? This may interact with the fallback regexp-based account type detection. I can imagine this getting (more) complicated.

simonmichael avatar Aug 24 '23 17:08 simonmichael

in the future it would be nice if such regex changes could be detected before they apply

WARN: Type C is currently detected via default-regex, this will change in version 1.27, please set Type C explicitly. (link to doku)

Or maybe in strict mode, show an error when a cashflow is requested but not CASH is defined ?

xMAC94x avatar Aug 24 '23 18:08 xMAC94x

We may not want to force a ton of boilerplate on everyone. But strict mode sounds like a good idea. At the very least we could add a check for this, hledger check allaccounttypesdeclared or some such.

simonmichael avatar Aug 24 '23 20:08 simonmichael