codefactor-io / codefactor

Automated static analysis for C#, C++, Java, CSS, JS, Go, Python, Ruby, TypeScript, Scala, CoffeeScript, Groovy, C, SCSS, Less, PHP, Dockerfile, Bash, YAML and Swift.
https://www.codefactor.io
62 stars 31 forks source link

Improve provenance display of "complex code" issues #38

Closed LinqLover closed 3 years ago

LinqLover commented 3 years ago

In my 640 lines long file I have the following codefactor issue:

image https://github.com/LinqLover/downstream-repository-mining/blob/c3a039c2cd922125c1c720e8aef4b50963d93125/src/references.ts

Without a line number or interval, this is not very helpful for me. I did not find out yet from which part of the file the warning comes.

(Btw, I also noted that the slashes are not used consistently. In the warning right below, a backslash is used instead of a forward slash:

image)

cordis-dev commented 3 years ago

@LinqLover on slashes, you should now see them consistently across the issues.

Regarding Complex Code warning, this was created to indicate that the file contains methods that may not be complex themselves but have high combined complexity.

E.g. src\core\references.ts has collectEsmBindings method with complexity of 10 based on https://eslint.org/docs/rules/complexity. This does not nessecerally mean that it's too complex and should be refactored, this is why we are hesitant to provide lines number and/or intervals for this type of issue (if method has high enough complexity it will generate a separate issue with line numbers).

It may be equally valid to ignore such issue by developers - https://support.codefactor.io/i18-ignoring-issues-and-patterns - or to e.g. split containing file it into multiple files to resolve it.

Thanks for the feedback, we'll monitor if such issue is too noisy for others and remove it or disable it by default if needed.

LinqLover commented 3 years ago

Thanks for the reply, @cordis-dev!

This does not nessecerally mean that it's too complex and should be refactored, this is why we are hesitant to provide lines number and/or intervals for this type of issue (if method has high enough complexity it will generate a separate issue with line numbers).

Tbh I don't go with this argument. :-) Trying to "fix" this warning is pretty hard, not to say impossible for longer files without any indication where complexity is worst. I was literally stabbing around in the dark in this case, but I am interested in keeping my code clean, so ignoring the warning is also not an option for me ...

kaskas commented 3 years ago

Dear @LinqLover - thank you for the feedback. We have contemplated internally multiple times about the concern that you have raised. We believe that this is still a code quality issue and it has been identified correctly by CodeFactor as such. It is understandable that developers and dev teams choose not to fix 100% of code quality issues and there are series of good reasons for that. High complexity of a fix and low value that the fix could create, could be a good reason to ignore. This is where you as a developer or dev team would make a choice to either ignore or go for a fix.

sergei-mironov commented 1 year ago

@kaskas Can we ask Codefactor to skip a particular place in code by using a magic comment?