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

Categorize various flavors of if in Ruby as if nodes #1361

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

noahd1
Copy link
Member

@noahd1 noahd1 commented Dec 22, 2024

In https://github.com/orgs/qltysh/discussions/1313 @nk-ty noted that qlty and codeclimate CLIs calculate the cognitive complexity of a method with trailing unless statements differently. After some investigation, I determined that both were incorrectly calculating cognitive complexity. This PR is concerned with the cognitive complexity calculation in qlty.

The following are equivalent ways of expressing an if conditional in Ruby:

if !foo
    puts "hi"
end
unless foo
    puts "foo"
end
puts "hi" if !foo
puts "hi" unless foo

The first was correctly being calculated. The second was an obvious oversight. And the 3rd and 4th ones were less obvious oversights (the parser calls them if_modifier / unless_modifier nodes not if / unless nodes.

This PR categorizes all 4 as if_nodes which the cognitive complexity calculator uses, and correctly penalizes all 4 as 1 point.

@noahd1 noahd1 requested a review from brynary December 22, 2024 03:46
Copy link
Contributor

qltysh bot commented Dec 22, 2024

The code coverage on the diff in this pull request is 100.0%

Drilldown
Path File Coverage Δ
qlty-config/src/library.rs -0.7
qlty-smells/src/metrics/metrics/cognitive.rs 0.0
qlty-analysis/src/lang/ruby.rs 0.0

Copy link
Member

@brynary brynary left a comment

Choose a reason for hiding this comment

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

Nicely done!

@noahd1 noahd1 merged commit 7ac9ca2 into main Dec 23, 2024
17 checks passed
@noahd1 noahd1 deleted the nd-cognitive-complexity branch December 23, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants