Inconsistent `use-fstring-for-concatenation` refactoring

[diceroll123]

Issue description or question

I've found a handful of weird unexpected non-refactory cases, I've done my best to group them together somewhat. I can't begin to consider how/why these things happen, so I'll leave that to the Sourcery team! 😂

def test(blah: str = "blah"):
    # use-fstring-for-concatenation suggested
    assert "blah123" == blah + "123"

def ext_blah123() -> str:
    return "blah123"

def test2(num: str = "123"):
    # use-fstring-for-concatenation suggested
    assert ext_blah123() == "blah" + num

def test3(blah: str = "blah"):
    # no sourcery refactor suggestion
    def blah123() -> str:  # unaccessed method
        return "blah123"

    assert "blah123" == blah + "123"

def test4(blah: str = "blah"):
    # no sourcery refactor suggestion
    def blah123() -> str:
        return "blah123"

    assert blah123() == blah + "123"

---

# don't ask. I don't know how I found this.
# at first I thought the length of the variable name was to blame, and honestly it might be

def test(xxxxxxxxxxxxxxxxxxx: str):
    # no sourcery refactor suggestion
    assert "test(xxxxxxxxxxxxxxxxxxx='True')" == "blah" + xxxxxxxxxxxxxxxxxxx

def test2(xxxxxxxxxxxxxxxxxx: str):
    # use-fstring-for-concatenation suggested
    # the only difference between test2 and test1 is the xxx... variable has one less "x"
    assert "test(xxxxxxxxxxxxxxxxxx='True')" == "blah" + xxxxxxxxxxxxxxxxxx

def test3(xxxxxxxxxxxxxxxxxxx: str):
    # use-fstring-for-concatenation suggested
    # the only difference between test3 and test1 is the True in the left-hand string is a 1
    assert "test(xxxxxxxxxxxxxxxxxxx='1')" == "blah" + xxxxxxxxxxxxxxxxxxx

Good luck! 😅

Sourcery Version

v0.11.2

Code editor or IDE name and version

VSCode Version: 1.66.2 (user setup) Commit: dfd34e8260c270da74b5c2d86d61aee4b6d56977 Date: 2022-04-11T07:46:01.075Z Electron: 17.2.0 Chromium: 98.0.4758.109 Node.js: 16.13.0 V8: 9.8.177.11-electron.0 OS: Windows_NT x64 10.0.22598

[diceroll123]

Bonus inconsistency, but I can't be bothered continuing trying to repro without using my own code because I ended up finding the above ones before being able to repro this one:

The methods in the screenshot above, besides the assert line in the second one being spread across 4 lines, are exactly the same. If black is run on this code, both of the inner code would look like that of the first function, and there would be no Sourcery refactory suggestions.

[brendanator]

Thanks for reporting these @diceroll123!

We have fixes for your examples with many xs in a row. We'll continue to look into the other cases

[brendanator]

On further investigation it seems like the fix for the xs also fixes your screenshot example 👍

Your top example is a current limitation with Sourcery. Our analysis is not always correct when handling nested functions so for safety reasons we never propose anything within a function containing a nested function. Here's a simplified example:

def f():
    def g():
        pass

    if True: # Sourcery would normal remove this
        print('refactor this')

[brendanator]

See also - https://github.com/sourcery-ai/sourcery/issues/24

[brendanator]

Everything except handling nested functions is now fixed