Pyupgrade: Printf string formatting

[colin99d]

A part of #827. This one is complicated so it will take me some more time.

Please Note: I purposely did NOT implement the --keep-percent-format flag because it looks like the last flag I added was removed. If you want this flag in, just let me know.

Note: the issue of the /N{dog} format will plague us in UP030 and UP032. Whatever solution we find here needs to be applied in both of those places. Right now I am just thinking of looking for it, and if it exists, only showing a warning but not attempting top fix. I have NEVER seen this syntax in practice, and I think it is reasonable to give the people using it a warning, but don't fix.

[colin99d]

@charliermarsh I have hit a blocker and I would love some assistance. Pyupgrade uses the regex "(?<!\)(?:\\)*(\N{[^}]+})" to get an outcome like below:

We cannot use this regex in rust, because the regex uses look behinds, which are not implemented in rust. I tried taking the post's advice and reimplementing the regex without look behinds, "^\*(\N{[^}]+})", but then the character before is included in the string, which causes the program to break:

This SO post seems to indicate that there is NOT a way to accomplish what I am looking to do.

After this I tried using the fancy_regex create; however, they do not implement the split method which is required. With this I am unsure of what to do next. We could check for my ugly regex, and just ignore those (or report them as issues but not fix them). But that really bothers the perfectionist in me. Let me know how you think we should handle this.

If we do go the ignore/only report a check route I believe this is the regex we should use: "(?:\\)*(\N{[^}]+})"

[charliermarsh]

Why do we need / why does the regex exist? Is it to avoid treating named unicode characters as {}-style formatting placeholders?

[colin99d]

I know its for statements like these: "%s \N{snowman}" % (a,), because they are currently causing issues in my code. However, I am not sure the specific reason they are needed.

[colin99d]

It looks like it exists so that "%s \N{snowman}" % (a,) does not turn into "%s \N{{snowman}}" % (a,). If this happens we get the following error in python:
SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 3-14: unknown Unicode character name

But we DO need to convert "brace {} %s" % (1,) => "brace {{}} {}".format(1)

[charliermarsh]

Is there a way that we could run the regex, extract the match, then run a second regex over the match to get a more precise range?

[colin99d]

Is there a way that we could run the regex, extract the match, then run a second regex over the match to get a more precise range?

Right now the regex I have developed that is the closest is "[^\\](?:\\\\)*(\\N\{[^}]+\})". It has two issues:

1. It grabs one character before the match starts (this could in theory be fixed by your solution)
2. It would never match: \N{snowman}" % (a,) because there is no character before the strong (we can't make it optional because then it would lose all effect), I am not sure how to overcome this shortcoming

Additionally, I am not sure if we could use this "double regex" with methods like split. I have been trying to think of other ways we could do this. I am sure we could do something with tokens, but not sure how we could turn this back into a string simply.

Basically all we need to confirm is that there is an odd number of \ before a N{something}

[charliermarsh]

Okay thanks. Just LMK when it's ready. I'll point to the updated RustPython branch and get it into a finished state, hopefully.

[charliermarsh]

And yeah -- I think we map %.f to {:.0f}. Same with %.s to {:.0} etc.

[colin99d]

@charliermarsh I am ready for you. Running cargo test printf_string_formatting currently results in two failures. Both are related to the precision issue.

[charliermarsh]

Working my way through this one, found at least one bug but I think it might be in our logic and not the parser.

[charliermarsh]

@colin99d - Ok, I think this is passing all tests now. I can think of a few ways that it could be improved so I might iterate on it a bit more. (For example: it'd be nice if we could avoid transforms that create really long string format calls.) It's a really significant transform, so I want it to be rock-solid before it goes out.

[colin99d]

One thing I did in another check, is that if the new string is longer I would not do the change. But sounds good! Thanks for helping me out on this. It was definitely a lot trickier than I thought it would be.

[charliermarsh]

I guess I'm also thinking about whether we should be fixing multi-line strings.

[colin99d]

I can see it either way. Pyupgrade did NOT fix the strings that you sent me earlier that broke.

[charliermarsh]

Alright, I'm gonna merge this as soon as RustPython/RustPython#4463 is merged.