Add InvestStatmentLine as possible return from parse_record to fix typing check errors.

[jaik03]

A StatementParser parse_record call should return either a StatementLine or InvestStatementLine upon call. Added missing InvestStatementLine type to return to silence typing check errors.

[kedder]

Hm, I somehow missed the moment when this InvestStatementLine was introduced (found it now in #129), but I'm not a big fan of these ambiguous return type.

What typing check errors are you trying to address with this?

[jaik03]

error: Return type "InvestStatementLine | None" of "parse_record" incompatible with return type "StatementLine | None" in supertype "StatementParser" [override]

I'm building a plugin to parse a JSON file filled with investment information, and mypy is throwing an override error due to the fact that my declaration says I'm returning an InvestStatementLine.

I neglected to notice that the | syntax was introduced in 3.10, and I think I used it incorrectly anyways. I can fix the PR, but do you want to approach this a different way? I can always have it ignore that specific type check in my parser code, but I thought it was better to address

[kedder]

Oh, I forgot about this. Basically we need to find a better way here.

I think parse_record() should return StatementLine and nothing else. There should be a different method to return InvestStatementLine. I was planning to look closer at it, but forgot.

[coveralls]

coverage: 96.05% (+0.3%) from 95.74%
when pulling 9688f8f on jaik03:AddInvestStatementLineType
into 0e20017 on kedder:master.

[jaik03]

Oh, I forgot about this. Basically we need to find a better way here.

I think parse_record() should return StatementLine and nothing else. There should be a different method to return InvestStatementLine. I was planning to look closer at it, but forgot.

Right, this was on the back burner for me and I was just fixing it to build correctly before I started digging into a better solution. I also set up a separate test environment to run through this properly so I'm not spamming you with Github workflow failures, but accidentally pushed to the wrong repo (Sorry!). Based on what you wrote I'll take a look and see if I can see any solutions that jump out at me.

[jaik03]

So I finally got around to looking at this again. I duplicated out parse_record into a parse_invest_record, and it
seems to work just fine with the plugin I'm developing.

The only part I'm unsure on is having the 'NotImplementedError' included. Since it's up to the plugin author to declare parse(), it'll be incumbent on them to declare either parse_record or parse_invest_record as appropriate, but it won't throw a typing error.

[kedder]

Hm, something is messed up. This shouldn't be part of the diff. Can you rebase on master or merge it into your branch?

[kedder]

This # type: ignore is not needed now?

FWICT, Statement class has two lists:

class Statement(Printable):
    lines: List["StatementLine"]
    invest_lines: List["InvestStatementLine"]

Plugins that parse investment statement are supposed to add lines to invest_lines. Those that parse regular bank statemrents add to lines.

Someone who added InvestStatementLine feature probably just overrode parse() and didn't use this implementation at all. And we didn't bother to add the same level of support for the investment statements that regular statements have.

That said, I think parse_invest_record is the move in right direction, we just need to use it here somehow, because right now it looks like a lonely unused method. What I suggest is to return None by default from both parse_record() and parse_invest_record(), indicating they wasn't able to parse the record.

Then, in parse itself add another couple of lines:

if stmt_line:
    ...
else:
    invest_stmt_line = self.parse_invest_record(line)
    if invest_stmt_line:
        self.statement.invest_lines.append(invest_stmt_line)

This way users can override one of the parse_*_record depending on what statements their plugins are parsing and still make use of the generic parse() method.

And we won't need these shameful # type: ignore comments as well :)

What do you think?