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

Collapse if statements with comments #825

Merged
merged 10 commits into from
Apr 9, 2021

Conversation

kgevorkyan
Copy link
Member

What's done:

  • Change collapseThenBlocks function: KtBlockExpression.statements didn't hold comments, now it's fixed
  • Monitor, which types of nodes are followed after nested if (more proper way), instead of check, that this if is the last child
  • Fix indentions in tests

### What's done:
* Change collapseThenBlocks function: KtBlockExpression.statements didn't hold comments, now it's fixed
* Monitor, which types of nodes are followed after nested if (more proper way), instead of check, that this if is the last child
* Fix indentions in tests
@kgevorkyan kgevorkyan linked an issue Apr 7, 2021 that may be closed by this pull request
@kgevorkyan kgevorkyan changed the title Prepare rule for comments processing Collapse if statements with comments Apr 7, 2021
@codecov
Copy link

codecov bot commented Apr 7, 2021

Codecov Report

Merging #825 (bcd26da) into master (44fb01d) will increase coverage by 0.07%.
The diff coverage is 83.72%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #825      +/-   ##
============================================
+ Coverage     80.91%   80.98%   +0.07%     
- Complexity     2272     2289      +17     
============================================
  Files           100      100              
  Lines          5801     5833      +32     
  Branches       1798     1813      +15     
============================================
+ Hits           4694     4724      +30     
  Misses          286      286              
- Partials        821      823       +2     
Flag Coverage Δ Complexity Δ
unittests 80.98% <83.72%> (+0.07%) 0.00 <17.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ Complexity Δ
...ruleset/rules/chapter3/CollapseIfStatementsRule.kt 83.33% <83.33%> (+5.20%) 41.00 <17.00> (+17.00)
...cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt 92.24% <100.00%> (ø) 11.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44fb01d...bcd26da. Read the comment docs.

### What's done:
* Add logic for if statements with commets and cover it with tests
@kgevorkyan kgevorkyan force-pushed the feature/collapse_if_statements_with_comments branch from 758c34f to 84dfaf2 Compare April 7, 2021 15:04
### What's done:
* Fix codestyle warning && and more tests
@kgevorkyan kgevorkyan force-pushed the feature/collapse_if_statements_with_comments branch from 08b65f8 to 8843848 Compare April 8, 2021 09:24
@kgevorkyan kgevorkyan marked this pull request as ready for review April 8, 2021 09:47
### What's done:
* Add recommendations from review and add more tests
### What's done:
Add comments about of absence of a KDOC in allowed types
### What's done:
* Improve logic, when if-statements already have comments in conditions
* Add tests
…entions rules

### What's done:
* CollapseIfStatements rule should be invoked before LineLength and Indentions rules
### What's done:
* Fix codestyle issues
@kgevorkyan
Copy link
Member Author

@petertrr I improved the way of comments processing, check please last commits

Copy link
Member

@petertrr petertrr left a comment

Choose a reason for hiding this comment

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

LGTM

@kgevorkyan kgevorkyan merged commit 4be22ab into master Apr 9, 2021
@kgevorkyan kgevorkyan deleted the feature/collapse_if_statements_with_comments branch April 9, 2021 11:42
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.

For if-statement rule need to properly handle comments
2 participants