fix: Lambda keyword args, improve errors, and nested madness

[mbhall88]

Added

• Support for retries keyword [closes feat: add retries to grammar #145]

Fixed

• Keyword argument lambdas are now allowed inside rules [closes Keyword argument in lambda causing InvalidParameterSyntax #135]
• Improve reported line number in Snakefile when black fails to parse [closes Error feedback suggestions #127]
• A fix for egde case: nesting + ruleorder + comment #126

[mbhall88]

Okay, I have been working on this locally for a day or two. What seems to be the problem (I think) is that after ruleorder, the next token that gets emitted by tokenize is the comment (# ___any comment...). But it should be a DEDENT token that gets emitted first as the indent level of ruleorder is 2 and the indent level of the comment is 1.

When I walked through the example in pycharm's debugger I noticed that tokenize explicitly skips comments https://github.com/python/cpython/blob/f9b67ad7701a4d51c0c895db4087ebd66743aa82/Lib/tokenize.py#L500.

Maybe when we get to a comment token we need to explicitly look back and see if we missed a DEDENT?

Any thoughts @bricoletc?

[codecov]

Codecov Report

Merging #136 (ebc486e) into dev (c66bb96) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #136      +/-   ##
==========================================
+ Coverage   98.20%   98.25%   +0.04%     
==========================================
  Files          12       12              
  Lines         949      973      +24     
  Branches      211      216       +5     
==========================================
+ Hits          932      956      +24     
  Misses         10       10              
  Partials        7        7              

Flag	Coverage Δ
unittests	98.15% <100.00%> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
snakefmt/parser/grammar.py	100.00% <ø> (ø)
snakefmt/parser/parser.py	96.39% <ø> (ø)
snakefmt/formatter.py	100.00% <100.00%> (ø)
snakefmt/parser/syntax.py	99.05% <100.00%> (ø)
snakefmt/snakefmt.py	95.89% <100.00%> (ø)

[mbhall88]

Okay, I got to the bottom of it....and then I made the test case more complicated and found some more bugs....and then fixed those (I hope). The crux of the issue was really that tokenize skips comments and mixed in with that was that if there is a dedent near a comment then this was causing some issues with the code indent counters.

Even though I made the MWE test from #126 more complicated, it would be great to get some more robust testing though.

[siebrenf]

Hey mbhall88,

I've run this branch on our snakemake package, and am already loving the logging improvements! The specific case of #126 is fixed (:tada:).

Unfortunately I did find more nesting-related issues. In the MWEs below I use ruleorders, but you can also use rules or checkpoints and get the same errors.

if True:
    ruleorder: A > B
    mylist = []  # you can even remove the indentation from this line
    mystr = "a"  # error occurs in the 2nd line of code below a nested ruleorder

Additional empty lines, comment lines and rules in this MWE don't contribute, except for a comment directly below a ruleorder, which prevents the error. Prompts InvalidPython: Black error: Cannot parse.

if True:
    ruleorder: A > B
    def myfunc():
        pass
    mylist=[]

Similarly, empty lines, comment lines and rules don't seem to contribute, except for a comment directly below a ruleorder, which prevents the error. Prompts 'IndentationError: unindent does not match any outer indentation level (<tokenize>, line 3)'.

Good hunting!

[mbhall88]

Thank you so much for this thorough testing @siebrenf.

I'll schedule some time next week to try and get to the bottom of this. Happy for anyone else to have a go in the meantime though.

[bricoletc]

I think we need one change to get the comment indenting right, see comments

[bricoletc]

Also here the comment above applies with respect to TAB arithmetic, and using self.cur_indent + token lookahead instead

[mbhall88]

Ok, now passing all of @siebrenf's wonderful tests!

Feel free to test again if you like @siebrenf.

[siebrenf]

All issues mentioned above were fixed! Buuuut I did find one new issue:

if True:

    rule with_run_directive:
        output:
            test.txt
        run:
            if True:
                print("this line is in the error")

    print("the indenting on this line matters")

which causes

[ERROR] In file "test.smk":  InvalidPython: Black error:
'''
Cannot parse: 8:0: print("this line is in the error")
'''

The error is gone if the first line of the run directive is a comment, or a line of code that does not prompt and indent.

[mbhall88]

@siebrenf you're a great test case generator!! Latest commit fixes your last issue. Do you mind testing again?

[siebrenf]

Thanks, I'll put that on my resume! But for real, it's awesome what you do with it!

All nesting issues have been resolved!

[mbhall88]

@johanneskoester did you change the merging options for the repository? I can only squash and merge this PR as all other options are "not enabled for this repository". I'd rather rebase this as there is quite a bit of work. Do you mind if I change it back to allow at least rebasing?

[bricoletc]

Thanks for all the testing @siebrenf :)

[bricoletc]

@mbhall88 can now rebase- totally agree this should not be a squash
(i don't know why it was disabled- now back)