sourcery-ai / sourcery

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

Refactoring `merge-dict-assign` does not trigger when the value being set is a function parameter #342

Open ruancomelli opened 1 year ago

ruancomelli commented 1 year ago

Checklist

Description

Related to https://github.com/sourcery-ai/sourcery/issues/339#issuecomment-1510428045.

Code snippet that reproduces issue

1 - Case reported in https://github.com/sourcery-ai/sourcery/issues/339

Input:

def __init__(self, toUserName, fromUserName, mediaId):
    self.__dict = {}
    self.__dict['ToUserName'] = toUserName
    self.__dict['FromUserName'] = fromUserName
    self.__dict["CreateTime"] = int(time.time())
    self.__dict["MediaId"] = mediaId

Expected refactored code:

def __init__(self, toUserName, fromUserName, mediaId):
    self.__dict = {
        'ToUserName': toUserName,
        'FromUserName': fromUserName,
        "CreateTime": int(time.time()),
        "MediaId": mediaId
    }

Actual refactored code:

def __init__(self, toUserName, fromUserName, mediaId):
    self.__dict = {
        'ToUserName': toUserName,
        'FromUserName': fromUserName,
        "CreateTime": int(time.time()),
    }
    self.__dict["MediaId"] = mediaId

2 - Simplified version

The bug can more easily be reproduced with the following snippets: Input:

def foo(mediaId):
    d = {}
    d["CreateTime"] = int(time.time())
    d["MediaId"] = mediaId

Expected refactored code:

def foo(mediaId):
    d = {"CreateTime": int(time.time()), "MediaId": mediaId}

Actual refactored code:

def foo(mediaId):
    d = {"CreateTime": int(time.time())}
    d["MediaId"] = mediaId

Debug Information

Sourcery Version: 1.2.0

ruancomelli commented 1 year ago

Relabeled this as an enhancement proposal since the refactoring is still correct. We definitely want Sourcery to keep on refactoring until all items are added to the dictionary instantiation, but this is not a bug.