Don't allow assignments for accounts in transaction modifiers.

[jkr]

As discussed in comments for #908, this throws an error if there is an attempt to assign to an account that is also present in a transaction modifier.

Note that this currently works whether or not --auto is set, so that it doesn't produce a regression in rewrites (which also use the jtxnmodifier list).

[simonmichael]

The code looks good.

I don't have a firm enough grasp of the problem you ran into, whether it happens in practice etc. The journal from your test works fine with hledger 1.11. Shouldn't it break in the way you reported ?

I'm still a little unsure of the benefit. Is the cure (preemptively disallowing a surprising interaction) worse than the disease (added complexity, this plus the future stuff/corner cases you mention) ?

[simonmichael]

Oh I see. It works with hledger-1.11, but not with master, because of your earlier fix. Please correct me if I'm confused.

[jkr]

Right -- it didn't use to be a problem (in some cases) because it always applied transaction modifiiers before it did the other finalization, which would miss inference and other things. But fixing that (and allowing inference before modification) creates this new problem, where assignment (done before modification) is wrong after modification.

The errors for this now are opaque: it's hard to figure out why the assignment failed. This is an attempt to explain to the user that assignment and modification don't play well (or predictably) together.

I'm not totally sure if the cure is better or worse than the disease -- but it came out of errors when trying to convert my ledger-cli journal to hledger, and discovering that something that worked there didn't work here, without being able to figure out why.

[simonmichael]

Sorry for the delay on this. What's the current situation as you see it ?

[jkr]

I guess it seems more like a form of preventative error reporting. If you run balance assignments with automated transactions, you'll either have unexpected results on the --auto side (as with hledger) or on the --actual side (as with ledger-cli). So if folks do it, we could either do this, and refuse to do it, or error out with unbalanced transactions. I'm honestly not sure which is better.

Not a big deal for me, at this point, because I got in the habit of not using balance assignments -- just assertions, which is better because it's more explicit anyway. But it does seem like it might catch people coming over from ledger-cli, and explain why a journal that worked over there doesn't work over here.

There might also be other ways to do it. If you ran modifiers transaction-by-transactiion during the first finalization, instead of mapping it over after, that might allow for better control over this -- perhaps making the error unnecessary.

So, in answer to your question... maybe, not sure?

[simonmichael]

@jkr I've been reconstructing the history here as part of writing changelogs. Does the following look right ?

In hledger 1.11-, transaction modifier (auto postings) rules ran before missing amounts were inferred. This meant amount multipliers could generate too many missing-amount postings, making the transaction unbalanceable. (#893)

This was resolved by doing amount inference (and balance assignments & balance assertions; these three are done together) earlier, before transaction modifiers. (#900, #903)

We found a new issue caused by this change: now balance assignments are generated so as to satisfy balance assertions with the unmodified transactions, but if transaction modifiers add additional postings to those accounts, the assertions can fail. (#908)

The current proposed fix is to track which accounts can be affected by transaction modifier rules, and disallow balance assignments on those accounts. (#912)

[simonmichael]

If so, I guess I'm inclined to merge this. It seems better to have this limitation than the one in hledger 1.11. What do you think @adept ?

I wondered if we could infer missing amounts, then apply transaction modifiers, then do balance assignments and assertions. But I have the impression/recollection that missing amounts/balance assignments/balance assertions are interdependent and must be done as a single step.

[simonmichael]

Merged with some fixups (1389a64). Thanks!