fix backslash in f-string

[LiuYinCarl]

Fix #1836

[the-13th-letter]

With these changes (applied manually to 7.6.1), the synthetic extra lines in #1836 are now gone. (Tested with Python 3.11 and 3.12.)

[LiuYinCarl]

@the-13th-letter Thanks for your careful tests in #1828 (comment) !

But I don't understand very much because I am not a native English speaker.

In #1828 (comment), do you mean the same as the table below ?

Python Version	coverage version	use PR1838	Still has bug
3.11	7.6.1	No	No
3.11	7.6.1	Yes	No
3.12	7.6.1	No	Yes
3.12	7.6.1	Yes	No

I tested these four scenarios below and It seems like this PR(#1838) can fix your problem in both Python3.11 and Python3.12.

So can I understand you like this:

1. This PR(fix backslash in f-string #1838) can fix your problem now.
2. You think coveragepy need more test to include your bug report about f-string.

Please let me know if I'm wrong.

[the-13th-letter]

@the-13th-letter Thanks for your careful tests in #1828 (comment) !

But I don't understand very much because I am not a native English speaker.

In #1828 (comment), do you mean the same as the table below ?

[...]

@LiuYinCarl, no, that's not what I mean. In #1828, I can trigger the opposite effect: backslash continuation sometimes disappears in the HTML report, so a line break is missing, and coverage info gets "shifted upwards".

Specifically, in the following pytest-style file test_bug.py, the HTML report contains synthetic joined lines (e.g. the first two lines of the x variable are one line in the HTML report) by removing a backslash continuation from the input.

test_bug.py

def test_bug():
    prog_name = 'bug.py'
    err_msg = f"""\
{prog_name}: ERROR: This is the first line of the error.
{prog_name}: ERROR: This is the second line of the error.
\
{prog_name}: ERROR: This is the third line of the error.
"""
    if (lambda: None)():  # just some non-constant always-false predicate
        'This line intentionally is not covered.'
    'This line is covered again.'

    # test cases from https://github.com/nedbat/coveragepy/pull/1828
    def f(a, b):
        pass
    a = ["aaa", \
         "bbb \
         ccc"]
    b = ("a",\
         "b \
         c")
    c = "xxx"\
        "yyy \
        zzz"
    f(100,\
      "hello \
      world")
    d = """\
    hello
    """

    # work-alikes
    x = ["""aaa""", \
         """bbb \
         ccc"""]
    y = ("""a""",\
         """b \
         c""")
    z = """xxx"""\
        """yyy \
        zzz"""
    f(100,\
      """hello \
      world""")
    A = [f"""aaa""", \
         f"""bbb \
         ccc"""]
    B = (f"""a""",\
         f"""b \
         c""")
    C = f"""xxx"""\
        f"""yyy \
        zzz"""
    f(100,\
      f"""hello \
      world""")
    D = f"""\
    hello
    """
    AA = [f"""\
          aaa""", \
          f"""\
          bbb \
          ccc"""]
    BA = [f"""\
          {x}""", \
          f"""\
          {y} \
          {z}"""]

    if (lambda: None)():
        'This is not covered.'
    'This is covered.'

if __name__ == '__main__':  # if not running under pytest
    test_bug()

(See the attached ZIP file for full coverage output: Py3.11 unpatched, Py3.11 patched with 01779db, Py3.12 unpatched, and Py3.12 patched with 01779db. I do not know why the same behavior does not also happen with your cov.py file contents.)

All of the above has nothing to do with this PR, which is why I commented on #1828 instead of here. The only connection is that while reviewing this PR, I was reading up on #1828 to understand the surrounding code and comments. And then I realized that the tests added in dc819ff looked weird, so I commented on that.

So can I understand you like this:

1. This PR(fix backslash in f-string #1838) can fix your problem now.

Yes.

2. You think coveragepy need more test to include your bug report about f-string.

I was trying to make a different point: I think that the tests in dc819ff are wrong. They seem to expect HTML-rendered output or tokenizer output that is clearly different from the input.

[the-13th-letter]

The attached patch contains a pytest test function to check for #1836.
issue_1836_tests.diff.txt

[the-13th-letter]

This code passes my recent test file on Python 3.11 and 3.12.

LGTM.

[nedbat]

@LiuYinCarl @the-13th-letter Does one of you want to apply that new test to this pull request? Then we'd be able to merge it.

@the-13th-letter Why use {{prog_name}} in the test? It seems like you intended it to be substituted, but it's not, and now you have literal {prog_name} in the results. You can make a pull request into this pull request to apply the test to it.

[nedbat]

@the-13th-letter also, maybe we can do without {quote} by using triple-single-quotes? The escaping is a lot to puzzle through.

[nedbat]

Sorry, I went ahead and made a pull request with the test: LiuYinCarl#1

[LiuYinCarl]

@nedbat, your PR was merged and I add a annotation in this line to make it pass the tox checks. https://github.com/nedbat/coveragepy/pull/1838/files#diff-29c93c7c1984a5716eb3a992a5e15cdcf5b7471539f0cc2831f141cc780a0885R1210

[nedbat]

I cleaned up the commits and merged this as commit 515d99c and a77bba7. Thanks!

[the-13th-letter]

Just to follow up:

@the-13th-letter Why use {{prog_name}} in the test? It seems like you intended it to be substituted, but it's not, and now you have literal {prog_name} in the results.

It's a transcription of my original bug.py sample file in the original post of #1836, which included a "true" f-string. Since we're generating that as an output, and using an f-string to format the "f"/"fr"/whatever string leader, interpolations in the inner f-string like {prog_name} need to be double-escaped, as {{prog_name}}. It's very much on purpose, though admittedly born of the perhaps pointless desire of reusing the original demonstration file for this bug.

@the-13th-letter also, maybe we can do without {quote} by using triple-single-quotes? The escaping is a lot to puzzle through.

Sure, go ahead. It'd be a style violation in my corner of the world to not use double quotes, but of course that doesn't necessarily apply here. (It does mean that this is the first solution that comes to mind, though.)

[nedbat]

Looking at the coverage reports for phystokens.py, there are some partial branches in the new checks:

Is that because it's impossible for those conditions to be false, or because we are missing some test cases in the test suite? Either way, it would be great to resolve so that we don't have partial branches in the coverage report.

[LiuYinCarl]

@nedbat It's my opinion below.

It is impossible for the condition on line 74 to be false because if last_line and last_line.endswith("\\\n"):
prevent other FSTRING_MIDDLE which is not end with \\\n into the condition.

I'm not sure about line 69, but I have tested the cases in #1828 and #1838, and it is impossible for any tests on line 69 to fail.

So, I think we can just replace the two conditions with inject_backslash = False

[the-13th-letter]

@nedbat I too am mostly convinced that the condition on line 74 must always be true because of the if last_line and last_line.endswith("\\\n") guard on line 40. I would probably settle on leaving the condition inside the code, as an assertion.

Like @LiuYinCarl, I'm not so sure about line 69 either. I did however try to design (failing) test cases for that condition, without success. Thus, pragmatically, I would convert that condition into an assertion as well, and leave a note à la "We failed to design a test case that doesn't fulfill this condition.".

attempted test inputs that don't improve branch coverage

space = " "
string = f"""\
line{space}1\
 and still line{space}1
line 2 \
and still line 2\
 and yet still line 2
line{space}3\
{space}and still line 3
"""

space = " "
weird_formatting = {"empty_string": """\
""", "nonempty_string": """ \
\

""",\
"chunked_string": "hello"\
" "\
"world"}
weird_formatting_with_fstrings = {f"empty_string": f"""\
""", f"nonempty_string": f""" \
\

""",\
f"chunked_string": f"hello"\
f"{space}"\
f"world"}

[nedbat]

Thanks to both of you for continuing to work on this. I really appreciate it! <3

[nedbat]

Updated in commit 595ea22, coverage changed from 95.256% to 95.266% :)

[nedbat]

This is now released as part of coverage 7.6.2.