Fix handling of invalid conversion characters

[isidentical]

No description provided.

[isidentical]

We only have a single f-string conversion character related error left after this PR which is the following: "f'{3!ª}'". The problem here is that, it used to fail because the old f-string parser did not normalize identifiers (or rather treat the character after the ! as an identifier) . Now we treat it as an identifier and when we are checking if it is a/s/r, we don't actually get to see the real token's contents (since pegen already decodes it for us and gives us the normalized version).

We can make them soft keywords (? I think they aren't normalized but not super sure, @pablogsal or @lysnikolaou you can probably correct me on this) OR we simply allow this edge case (or hard-code it in the tokenizer which might look a bit ugly I think).

[pablogsal]

We only have a single f-string conversion character related error left after this PR which is the following: "f'{3!ª}'". The problem here is that, it used to fail because the old f-string parser did not normalize identifiers (or rather treat the character after the ! as an identifier) . Now we treat it as an identifier and when we are checking if it is a/s/r, we don't actually get to see the real token's contents (since pegen already decodes it for us and gives us the normalized version).

We can make them soft keywords (? I think they aren't normalized but not super sure, @pablogsal or @lysnikolaou you can probably correct me on this) OR we simply allow this edge case (or hard-code it in the tokenizer which might look a bit ugly I think).

Let's simply allow the edge case, I don't want to add crazy complexity just for this particular case.

[pablogsal]

Hummm, actually this PR makes a lot more tests fail so I think I am going to revert it for now

[pablogsal]

30 failures -> 52 failures

[isidentical]

Uh, in my local branch it was something like 28 to 24? (On test_fstring). Will double check

[isidentical]

@pablogsal I can't seem to reproduce your findings 🤔 Is there a chance I'm missing something when trying this?

Before (current revision at the fstring-grammar-rebased-after-sprint)

❯ ./python -m test test_fstring
test test_fstring failed -- multiple errors occurred; run in verbose mode for details
test_fstring failed (30 failures)

== Tests result: FAILURE ==

1 test failed:
    test_fstring

Total duration: 863 ms
Tests result: FAILURE

After (actually before your revert / after this PR):

❯ git checkout HEAD^
HEAD is now at e1940e5e77 Fix test_type_comments
❯ [...]
❯ ./python -m test test_fstring
test test_fstring failed -- multiple errors occurred; run in verbose mode for details
test_fstring failed (24 failures)

== Tests result: FAILURE ==

1 test failed:
    test_fstring

Total duration: 820 ms
Tests result: FAILURE

[pablogsal]

Hummmm, let me check in the main CI if I revert your revert

[pablogsal]

Does #62 include this PR as well?

[isidentical]

Yep, with the above edge case fixed as well.