LGTM.com - py/similar-function might be too sensitive

[dgw]

Description of the false positive

The two functions are similar, so I hesitate to call this a true "false positive".

But the differences are substantial enough—variations in which SQLAlchemy table is queried, which columns are used in filters, which object properties are needed, etc.—that trying to write a single function to do both jobs would (arguably) be a bigger code smell than this.

It's also worth noting that there are other pairs of functions (the set and get companions in each suite for nick and channel) not marked as similar, even though they're no more or less so than the delete functions that were detected. And there's another suite of also-similar plugin functions that weren't flagged by the analysis.

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/sopel-irc/sopel/snapshot/f6d393044ee7c7f1a7ba6fe0c4e8a152eb5948b2/files/sopel/db.py?sort=name&dir=ASC&mode=heatmap#x9fad0d3c3a380402:1

[tausbn]

Thank you for your report!

The "Similar function" query has to walk a pretty thin line between marking too many things as similar, and marking too few (or none at all) as similar. For instance, it's desirable to consider two functions to be similar if all they differ in is the choice of variable names. Thus, to a certain extent the comparison abstracts across the variable names that appear inside functions etc.

Unfortunately, this can lead to awkward situations. For instance, if we had two functions and the only difference between them was that one uses x.foo and the other uses x.bar, then we would likely report them as similar.

But as you rightly observe, although you could write a function that would abstract across the choice of attribute to use, there's no nice way of doing so, and it would likely make readability worse overall. (Because of this, we have considered not abstracting over the names of attributes, but we have no plans for implementing this at the moment.)

Regarding the getters and setters, these may be filtered out because of other reasons, e.g. their size. If we did not take size into account, then a lot of functions would end up being equivalent. (Consider for instance two functions which each do nothing but call two other functions. After abstracting away the identifiers, these would look identical!)

Probably the best solution for you would be to add an .lgtm.yml configuration file that disables the py/similar-function query. This can also be done locally in the file with a comment, but that would likely be a bit more awkward. You can find documentation on how to specify this in the config file on LGTM.com