Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Cyclomatic Complexity Rule #1464

Merged
merged 13 commits into from
Sep 14, 2016
Merged

Cyclomatic Complexity Rule #1464

merged 13 commits into from
Sep 14, 2016

Conversation

bencoveney
Copy link
Contributor

Resolves #1411

Roughly matches the corresponding ESLint and JSHint rule with the exception that && is now considered a code branch.


export class Rule extends Lint.Rules.AbstractRule {

public static ANONYMOUS_FAILURE_STRING = "The cyclomatic complexity of is higher than the threshold";
Copy link
Contributor

Choose a reason for hiding this comment

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

Wording?

@jkillian
Copy link
Contributor

Looking good @bencoveney! Just a few small changes needed before this can merge

@bencoveney
Copy link
Contributor Author

bencoveney commented Aug 23, 2016

Anticipating a CI failure, I think there is some rule additions that are causing the linting to fail on unrelated code (#1508). I can re-merge and update once they are fixed.

@jkillian
Copy link
Contributor

Sorry about that @bencoveney, I believe #1511 should have fixed the issue you were seeing

@bencoveney
Copy link
Contributor Author

So I believe that's all the comments so far addressed.

The only one I have deviated from slightly is the recommendation to up the minimum threshold from 1 to 5 or similar. It makes sense to disallow 1 as this would not allow any branches in the code however 5 might be slightly too high in my opinion as there would be logical values which would be prevented for example my tests use a limit of 3.

With this considered I have increased the limit from 1 to 2 which would be the lowest reasonable value AFAICT. Whether anyone would actually want to enforce a limit of 2 on real code would probably be debatable...

@jkillian
Copy link
Contributor

jkillian commented Sep 6, 2016

Looking good @bencoveney, and apologies for the slowness in getting back to this. I'm thinking the failure messages should display the measured complexity and the threshold complexity - this would make it easier for users to adjust their code.

@bencoveney
Copy link
Contributor Author

Error messages now take the following formats:

The function has a cyclomatic complexity of 4 which is higher than the threshold of 3

The function myNamedFunction has a cyclomatic complexity of 4 which is higher than the threshold of 3

@jkillian
Copy link
Contributor

Looks good, thanks @bencoveney!

@jkillian jkillian merged commit bfe06b2 into palantir:master Sep 14, 2016
@bencoveney bencoveney deleted the complexity branch October 18, 2016 07:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants