sourcery-ai / sourcery

Instant AI code reviews
https://sourcery.ai
MIT License
1.53k stars 66 forks source link

Semantics-breaking refactoring suggested by Sourcery "Merge dictionary updates via the union operator" #264

Closed bwagner closed 2 years ago

bwagner commented 2 years ago

Semantics-breaking refactoring suggested by Sourcery "Merge dictionary updates via the union operator"

Code before:

a = {'a': 'b'}
a.update({k: a[v] for k, v in {'c': 'a'}.items()})

When refactoring is applied, we get invalid code (a is referred to before it exists):

a = {'a': 'b'} | {k: a[v] for k, v in {'c': 'a'}.items()}

Sourcery Version

Often available in Sourcery settings: Not in my version, but it's current.

Code editor or IDE name and version

PyCharm 2022.2 (Professional Edition) Build #PY-222.3345.131, built on July 27, 2022

OS name and version

OSX / macOS: 10.14.6 (Mojave)

brendanator commented 2 years ago

Thanks for reporting this @bwagner. I have reproduced and have a fix for the next release

bwagner commented 2 years ago

Still present in Sourcery 4.57

ruancomelli commented 2 years ago

Hi, @bwagner!

A fix for this issue was released in Sourcery v0.12.7. The expected behavior now is for Sourcery to refactor

a = {'a': 'b'}
a.update({k: a[v] for k, v in {'c': 'a'}.items()})

as

a = {'a': 'b'}
a |= {k: a[v] for k, v in {'c': 'a'}.items()}

which preserves semantics.

Can you make sure you are running v0.12.7 and check again?

ruancomelli commented 2 years ago

Closing this issue as completed since a fix was released in Sourcery v0.12.7, but feel free to re-open it if you still find this bug even after updating Sourcery :grin:

Thanks again for opening this bug report!