Serious bug: Code after comment in indented if-block is unindented, changing execution

[corneliusroemer]

There's a serious bug in v0.7.0 present since 0.3.0

If you have an if statement followed by an indented block, and that block contains a line comment with #, then Snakefmt unindents everything starting from the first # which changes the code execution logic. This is a fatal bug for a code formatter: code formatters must not change the code they format.

The reproducible code is the same as for #157:

Start, this is what I want:

if config.get("s3_dst"):

    include: "workflow/snakemake_rules/upload.smk"
    include: "workflow/snakemake_rules/trigger.smk"
    # Only include rules for trigger if uploading files since the trigger
    # rules depend on the outputs from upload.
    include: "workflow/snakemake_rules/trigger.smk"

After formatting:

if config.get("s3_dst"):

    include: "workflow/snakemake_rules/upload.smk"
    include: "workflow/snakemake_rules/trigger.smk"


# Only include rules for trigger if uploading files since the trigger
# rules depend on the outputs from upload.
include: "workflow/snakemake_rules/trigger.smk"

But this is still not ok per Snakefmt, if I save again:

if config.get("s3_dst"):

    include: "workflow/snakemake_rules/upload.smk"
    include: "workflow/snakemake_rules/trigger.smk"
# Only include rules for trigger if uploading files since the trigger
# rules depend on the outputs from upload.
include: "workflow/snakemake_rules/trigger.smk"

Note: the last include is now unindented, which changes the logic significantly.

[corneliusroemer]

Same problem applies to """ comment:

if config.get("s3_dst"):
    include: "workflow/snakemake_rules/upload.smk"
    include: "workflow/snakemake_rules/trigger.smk"
    """test"""
    include: "workflow/snakemake_rules/trigger.smk"

becomes

if config.get("s3_dst"):

    include: "workflow/snakemake_rules/upload.smk"
    include: "workflow/snakemake_rules/trigger.smk"


    """test"""


include: "workflow/snakemake_rules/trigger.smk"

[corneliusroemer]

This bug got introduced in v0.3.0 - before that, in version 0.2.6 one gets an error instead:

❯ snakefmt Snakefile        
TypeError: can't multiply sequence by non-int of type 'NoneType'

So this is an old bug. I noticed in code review that some code got changed in a way that it shouldn't - hard to notice when a whole Snakefile gets formatted by snakefmt.

[johanneskoester]

Good catch! I think the discussion is ongoing in #157, so let's talk there.

[mbhall88]

v0.8.0 should fix this 🤞