search

ranger / ranger

A VIM-inspired filemanager for the console
https://ranger.fm
GNU General Public License v3.0
15.55k stars 892 forks source link

Static Analysis #1578

Open martinberg212 opened 5 years ago

martinberg212 commented 5 years ago

Hi,

I wanted to try out this static code analysis tool and run ranger through it as a test. There might be some helpful things there to improve ranger. https://sonarcloud.io/dashboard?id=martinberg212_ranger

Runtime Environment

toonn commented 5 years ago

Hmm, interesting. Looks to me like ranger scores pretty good. The "duplications" are all false positives btw, could you mark them as such? Similarly most of the "Rename class" code smells are false positives, it'd be great if they could be removed from the list.

The code smells list looks like a great source of beginner contributions tbh. It'd be great if someone grouped them together and submitted issues. For example, group all the "integrate if statement in surrounding if," and make an issue listing them together. If no one steps up I'll get around to this soon™.

martinberg212 commented 5 years ago

I haven't figured out yet how to mark the duplicates as false positives. If this tool is useful, I recommend setting it up yourself as I do not plan to keep track of it (just testing). You can login via github and it took me literally only 3 minutes to run the analysis, so it might be the better option to create the project yourself if you plan to work with it.

martinberg212 commented 5 years ago

I removed the duplicates by deleting the doc folder and marked the 'Rename Class' as false positives. Not sure what can be done about the 'Cognitive Complexity' issues. Probably best starting with the reduction of if statements first.

Most of the command injection issues look like false positives too but I would like more eyes on it before marking them as such. Executing OS command is generally a feature of ranger but we should make sure there are no unintended command injection issues.

The use of regular expression might affect performance too and should get a closer look as well.

Are there any plans on creating more tests? Coverage is at 0% unfortunately.

toonn commented 5 years ago

No concrete plans. I'm writing some for some difficult PRs. Problem is that most of the code has to do with display and is hard to test reliably. Another problem is it's hard to test some things in isolation. I got stuck on the tests I'm writing because they need quite a bit of scaffolding, just shy of a full ranger instance really.