[Adjust Rule] SIM401 quotation issue

[jonyscathe]

Desired change

• Rule(s): SIM401
• Adjustment: The key and default value in the suggested change should have the correct type

Explanation

Makes the suggestion valid python. If the key in the if statement is a string, then it should still be a string in the suggested code. If the default value in the else-block is not a string, it should not be put in quotes. Or if it is an empty string it shouldn't get extra quotes.

Example

The following code:

if 'last_name' in test_dict:
    name = test_dict['last_name']
else:
    name = test_dict['first_name']
if 'phone_number' in test_dict:
    number = test_dict['phone_number']
else:
    number = ''

Will currently result in the following SIM401 messages:

Use name = test_dict.get(last_name, "test_dict['first_name']") instead of an if-block Use number = test_dict.get(phone_number, """") instead of an if-block

Obviously it the messages should state the following with both the key and default value in the exact same format as they were in the if-else-block. Use name = test_dict.get("last_name", test_dict['first_name']) instead of an if-block Use number = test_dict.get("phone_number", "") instead of an if-block

[MartinThoma]

Thank you for the input :hugs: I've added a test which contain your examples so that I can track progress in this issue

[MartinThoma]

Thank you very much for your help. The issue was fixed in flake8-simplify==0.14.6

[joaoe]

Hi. I just spotted this. The proposed change in this issue is incorrect.

The example

if 'last_name' in test_dict:
    name = test_dict['last_name']
else:
    name = test_dict['first_name']

the keys can be mutually exclusive. This statement name = test_dict.get(last_name, "test_dict['first_name']") can well throw a KeyError which would not happen in the original case, or run a __getitem__ with side effects. There is simply no way to trigger this warning unless the fallback is some trivial like a constant or variable.

[joaoe]

I created #177 to follow up on this and other related issues.