Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can Bowler rename the variable in the format string literals? #87

Open
sangwoo-joh opened this issue May 23, 2019 · 11 comments
Open

Can Bowler rename the variable in the format string literals? #87

sangwoo-joh opened this issue May 23, 2019 · 11 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@sangwoo-joh
Copy link

Hi.
I have some code that uses format string literals and .format() like below:

def foo(bar, zar):
    print(f'bar is {bar}, not {zar}')
    print('bar is {bar}, not {zar}'.format(bar=bar, zar=zar))

I want to rename the variable name bar to bar_new so the rename.py script is ready:

from bowler import Query

Query('.').select_var('bar').rename('bar_new').idiff()

I hope Bowler recognize the placeholder in string literal, {bar}, but it wasn't:

--- ./format_string.py
+++ ./format_string.py
@@ -1,3 +1,3 @@
-def foo(bar, zar):
+def foo(bar_new, zar):
   print(f'bar is {bar}, not {zar}')
-  print('bar is {bar}, not {zar}'.format(bar=bar, zar=zar))
+  print('bar is {bar}, not {zar}'.format(bar_new=bar_new, zar=zar))
Apply this hunk [y,N,q,a,d,?]? 

I want Bowler to rename

  • f'bar is {bar}, not {zar}'f'bar is {bar_new}, not {zar}'
  • .format(bar=bar, zar=zar)'.format(bar=bar_new, zar=zar)

and this is what exactly the Refactor → Rename function does in PyCharm.

Is there a way to refactor my code like this?
Thank you for your great tool.

@thatch
Copy link
Contributor

thatch commented May 28, 2019

The first part -- renaming the variable within an f-string -- is not currently supported, but I would be glad to review a PR if you feel like adding it. @jreese is that second part a bug, renaming the kwarg in a call? The docs are unclear at https://pybowler.io/docs/api-selectors#select-var

@amyreese
Copy link
Contributor

That looks like a side effect of the way the select_var selector is written, because it basically captures any name element in the tree, which in this case also matches the kwarg. I'm not sure I would call it a "bug" per se, but potentially unexpected behavior in most cases.

As for modifying contents of string literals, we would need to add support for parsing all string literals for f-string-like syntax, which itself would require more than what lib2to3 can offer. I believe something like typed_ast is capable of doing this, but I'm not certain, and at the very least would require a fair amount of work to fit this second method of parsing and transformations into the Bowler workflow.

@amyreese amyreese added enhancement New feature or request help wanted Extra attention is needed labels May 28, 2019
@sangwoo-joh
Copy link
Author

@thatch , @jreese Thanks for the answer!
I agree that Bowler need to parse all f-string-like literals, as @jreese said.
After I understand the core logic of Bowler itself, I hope I can contribute.
Thank you.

@sangwoo-joh
Copy link
Author

I found this issue about "f-strings are parsed just as regular strings" on python bug tracker, and it is not resolved yet. Since the mechanism of Bowler's select depends on how lib2to3 parse code, I think this issue should be resolved first... so that we can add a f-string related patterns in select_var.

@thatch
Copy link
Contributor

thatch commented Oct 30, 2019

We could either hack around this in Bowler while we use fissix (lib2to3) or wait until Bowler2 uses LibCST, at which point we can have first-party support for using them. I'm inclined to wait, mainly because I've tried to upstream things to lib2to3 before and it didn't go well.

@sangwoo-joh
Copy link
Author

@thatch Thanks for your update.
I agree with you that we have to wait.
And I'm very excited to hear about the Bowler2 using LibCST!! 😍
Always thank you for the great tool.

@Omar-Elrefaei
Copy link

Any progress or plans on Bowler2.

@sangwoo-joh Did you manage to rename vars in fstrings some other way?

@sangwoo-joh
Copy link
Author

@Omar-Elrefaei
I've just solved my issue using libCST. 😅

@Omar-Elrefaei
Copy link

Omar-Elrefaei commented Aug 24, 2020

@Omar-Elrefaei
I've just solved my issue using libCST. sweat_smile

If you have shareable code that renames variables, I would be in your debt 😳.

@sangwoo-joh
Copy link
Author

sangwoo-joh commented Aug 26, 2020

@Omar-Elrefaei
It's a rough example and maybe not suitable for your case, but you can find a sample from my gist: https://gist.github.com/sangwoo-joh/26e9007ebc2de256b0b3deeda051772d

It is true that libcst can parse the f-strings, but it has a tricky thing too: it does not provide APIs to decide whether to visit the child attributes or not; only for child nodes. Because of this lack, recklessly using leave_Name to transform the original code would rename all the Name nodes of the keyword attributes in normal format string (not f-string) such as "some-string".format(keyword=value). I don't want this, so as my gist code does, just tracking each change of the keyword attribute and restoring it after leaving was enough for my issue. But I don't know your case, so be careful about it.
I hope it would help you.

@Omar-Elrefaei
Copy link

Omar-Elrefaei commented Aug 27, 2020

@sangwoo-joh Appreciate it. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants