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

Check for ast.Attributes when finding occurrences in f-strings #751

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

sandratsy
Copy link

@sandratsy sandratsy commented Jan 23, 2024

Description

Given the following piece of code:

def xyz(self):
    print(f'{self.abc}')
    return self.abc

If I try to Rename attribute/property abc, the abc in the return statement is correctly renamed, while the abc in the f-string is not. This is because _search_in_f_string() only looks for ast.Names, while self.abc is an ast.Attribute. I have updated _search_in_f_string() to look for ast.Attributes as well.

(Ofc, this is not fool-proof, since devs can put whatever they like in f-strings, but I suppose devs are more likely to put Names and Attributes in f-strings than entire function calls.)

The node.col_offset for an ast.Attribute points to the start of self.abc rather than abc, however. To get around this, I've taken the liberty to change _search_in_f_string() to return an offset instead of the node. node.end_col_offset - len(self.name) will correctly return the index of abc.

I used node.end_col_offset - len(self.name) instead of node.col_offset + f_string.index(self.name) in case the attribute is named abc.abc. We want the offset of the 2nd abc, not the 1st.

Checklist (delete if not relevant):

  • I have updated CHANGELOG.md

@sandratsy
Copy link
Author

Hello, any updates on this? Is there anything else I should do?

@sandratsy sandratsy closed this Mar 3, 2024
@sandratsy sandratsy reopened this Mar 3, 2024
@lieryan
Copy link
Member

lieryan commented Mar 5, 2024

Thanks for the contribution @sandratsy, this is great. I've added some test cases and type hints but the fix looks sensible.

@lieryan lieryan enabled auto-merge March 5, 2024 10:44
@lieryan lieryan merged commit faf4456 into python-rope:master Mar 5, 2024
18 checks passed
@lieryan
Copy link
Member

lieryan commented Mar 5, 2024

@all-contributors please add @sandratsy for code

Copy link
Contributor

@lieryan

I've put up a pull request to add @sandratsy! 🎉

@sandratsy
Copy link
Author

Thanks v much!

@lieryan lieryan added this to the 1.13.0 milestone Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants