logical_line for call with f-string can result in invalid ast under Python 3.12

[stephenfin]

how did you install flake8?

$ pip install flake8

unmodified output of flake8 --bug-report

{}

describe the problem

what I expected to happen

Some of the plugins in hacking use a combo of ast.parse and logical_line to generate an AST for an individual line that is then fed into an ast.NodeVisitor subclass. These work just fine on Python 3.11 and earlier, but I've noticed they fail under specific circumstances on Python 3.12. I've included a minimal reproducer below. There's a chance I am "holding it wrong" but I'd like to confirm this first.

sample code

test.py

import unittest


class TestFoo(unittest.TestCase):

    def test_foo(self):
        response_key = 'foo'
        response_val = 'bar'
        body_output = ''
        self.assertEqual(
            f'{{"{response_key}": "{response_val}"}}', body_output
        )

checks.py:

class NoneArgChecker(ast.NodeVisitor):
    def __init__(self, func_name, num_args=2):
        self.func_name = func_name
        self.num_args = num_args
        self.none_found = False

    def visit_Call(self, node):
        # snipped
        ...


def foo(logical_line, noqa):
    if noqa:
        return

    for func_name in (
        'assertEqual', 'assertIs', 'assertNotEqual', 'assertIsNot'
    ):
        try:
            start = logical_line.index('.%s(' % func_name) + 1
        except ValueError:
            continue
        checker = NoneArgChecker(func_name)
        # print(logical_line)
        checker.visit(ast.parse(logical_line))
        continue
        if checker.none_found:
            yield start, "H203: Use assertIs(Not)None to check for None"

tox.ini:

[flake8:local-plugins]
extension =
    X123 = checks:foo
paths = .

commands ran

$ flake8 test.py
wow.py:1:23: E999 SyntaxError: invalid syntax. Perhaps you forgot a comma?

If I comment out the print statement, I see different results for Python 3.12 compared Pythons 3.10 and 3.11. Under 3.12:

self.assertEqual(f'x{x{response_key}xxxx{response_val}xx}', body_output)

Under 3.10 and 3.11:

self.assertEqual(f'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx', body_output)

additional information

The E999 error is associated with line 1 of the file, despite the error actually coming from a plugin. This led me on a wild goose chase as I tried to figure out why flake8 thought my file was invalid Python but Python itself did not. I suspect this might also be user error and I should be handling the potential exception from ast.parse in my plugin but again, I just wanted to confirm that this was expected practice.

[stephenfin]

I think this may be a bug introduced in 43266a2. Simplifying the above reproducer:

test.py

key = 'foo'
val = 'bar'
print(f'{{"{key}": "{val}"}}'

checks.py

def foo(logical_line):
    print(logical_line)

If we run this we get:

❯ flake8 test.py
key = 'xxx'
val = 'xxx'
print(f'x{x{key}xxxx{val}xx}')

I think we want to get:

❯ flake8 test.py
key = 'xxx'
val = 'xxx'
print(f'xxxxxxxxxxxxxxxxxxxx')

Right?

[asottile]

nope! the method is meant to redact string contents and not code

[stephenfin]

Then we should have:

print(f'xxx{key}xxxx{val}xxx')

?

[asottile]

ah yes I see the quirk. ugh. the FSTRING_MIDDLE token is misleading for curly braces -- should be an easy fix (pycodestyle will likely need the same fix)

want to try a patch?

[stephenfin]

want to try a patch?

Done. Apologies in advance for the fuzzy wording: I have no idea what the below code is intended to account for:

flake8/src/flake8/processor.py

Lines 207 to 218 in 2a811cc

	if previous_row:
	(start_row, start_column) = start
	if previous_row != start_row:
	row_index = previous_row - 1
	column_index = previous_column - 1
	previous_text = self.lines[row_index][column_index]
	if previous_text == "," or (
	previous_text not in "{[(" and text not in "}])"
	):
	text = f" {text}"
	elif previous_column != start_column:
	text = line[previous_column:start_column] + text

(and that's after git blameing my way back as far as 23c9091...)

[sigmavirus24]

want to try a patch?

Done. Apologies in advance for the fuzzy wording: I have no idea what the below code is intended to account for:

flake8/src/flake8/processor.py

Lines 207 to 218 in 2a811cc

	if previous_row:
	(start_row, start_column) = start
	if previous_row != start_row:
	row_index = previous_row - 1
	column_index = previous_column - 1
	previous_text = self.lines[row_index][column_index]
	if previous_text == "," or (
	previous_text not in "{[(" and text not in "}])"
	):
	text = f" {text}"
	elif previous_column != start_column:
	text = line[previous_column:start_column] + text

(and that's after git blameing my way back as far as 23c9091...)

Not looking at where you blamed back to but a bunch of this was implemented to keep compat with pycodestyle. If you want reasoning it's likely in that project