Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

High cyclomatic complexity detector added #1618

Merged
merged 3 commits into from
Feb 15, 2023
Merged

Conversation

bart1e
Copy link
Contributor

@bart1e bart1e commented Jan 21, 2023

Detector for high cyclomatic complexity (> 11) added, as requested in #1591.
Needs #1617 to be merged before due to a bug in the calculation of cyclomatic complexity.

cc = compute_cyclomatic_complexity(f)
if cc > 11:
info = (
"Function "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use [ f, f" has a high cyclomatic complexity ({cc})"] here.

generate_results has inbuilt that will transform slither object (i.e. f here) in a human readable format (ex: adding the source mapping directly)

def _detect(self):
results = []

for f in self.compilation_unit.functions:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should iterate over contracts and then contract.functions_declared here. The reason is that the same function at the solidity source code level might appear multiple times in compilation_unit.functions.

This is because we create one function object per contract's instance to ensure proper IR conversion (ex: if a function uses super, the function that will be actually called can depend on the inheritance order).

I will put a note to improve our documentation there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback. I will change it in a while. But, iterating only over contract functions will miss top level functions. Should I also iterate over self.compilation_unit.functions_top_level? I don't think they are widely used, but solidity compiler allows using top level functions, so it may be a good idea to take them into account as well.

@montyly
Copy link
Member

montyly commented Jan 23, 2023

This is nice, thanks @bart1e

@bart1e
Copy link
Contributor Author

bart1e commented Jan 23, 2023

@montyly I have added the changes you requested. I am also iterating over top level functions - if we don't need it, just let me know and I will remove it.

@montyly montyly merged commit 24df353 into crytic:dev Feb 15, 2023
@montyly
Copy link
Member

montyly commented Feb 15, 2023

I merged the PR with #1660. thanks again for the contribution @bart1e. If you are curious, I made minor changes: d0c0a17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants