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.

Sign up for GitHub

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

Jump to bottom

Data class rule: removing several cases from the scope of the inspection #1471

Merged
merged 8 commits into from
Jul 25, 2022
Merged

Data class rule: removing several cases from the scope of the inspection #1471

merged 8 commits into from
Jul 25, 2022

Conversation

orchestr7
Copy link
Member

What's done:

  • Removed annotation, value, sealed, inline, inner classes from the scope
  • Added tests

@orchestr7 orchestr7 added this to the 1.2.2 milestone Jul 22, 2022
@orchestr7 orchestr7 linked an issue Jul 22, 2022 that may be closed by this pull request
USE_DATA_CLASS: False positive for Annotations #1469
Closed
@orchestr7 orchestr7 mentioned this pull request Jul 22, 2022
Closed
@codecov
Copy link

codecov bot commented Jul 22, 2022 •
edited
Loading

Codecov Report

Merging #1471 (90db394) into master (632ac74) will increase coverage by 0.02%.
The diff coverage is 80.00%.

@@             Coverage Diff              @@
##             master    #1471      +/-   ##
============================================
+ Coverage     83.25%   83.27%   +0.02%     
- Complexity     2561     2569       +8     
============================================
  Files           110      110              
  Lines          7661     7661              
  Branches       2107     2107              
============================================
+ Hits           6378     6380       +2     
+ Misses          392      387       -5     
- Partials        891      894       +3
Flag Coverage Δ
unittests 83.27% <80.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
.../ruleset/rules/chapter6/classes/DataClassesRule.kt 85.07% <80.00%> (+2.98%) ⬆️

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 632ac74...90db394. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2022 •
edited
Loading

JUnit Tests (macOS, EnricoMi/publish-unit-test-result-action@v1)

1 317 tests   1 301 ✔️  2m 37s ⏱️
   160 suites       16 💤
   160 files           0 ❌

Results for commit 90db394.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 22, 2022 •
edited
Loading

JUnit Tests (Windows, EnricoMi/publish-unit-test-result-action@v1)

1 331 tests   1 316 ✔️  2m 28s ⏱️
   161 suites       15 💤
   161 files           0 ❌

Results for commit 90db394.

♻️ This comment has been updated with latest results.

@orchestr7
Data class rule: removing several cases from the scope of the inspection
20b3e2c 
### What's done:
- Removed annotation, value, sealed, inline, inner classes from the scope
- Added tests
@orchestr7
Merge branch 'master' into bugfix/data-class-rules
44e469d
@orchestr7
Data class rule: removing several cases from the scope of the inspection
70f46ca 
### What's done:
- Removed annotation, value, sealed, inline, inner classes from the scope
- Added tests
petertrr
petertrr reviewed Jul 25, 2022
View reviewed changes
diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt Outdated
@@ -128,6 +127,12 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
return true
}

/** we do not exclude inner classes and enums here as if they have no
Copy link
Member

@petertrr petertrr Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think we filter out inner classes and enums elsewhere:

diktat/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt

Lines 99 to 100 in 70f46ca

return list.getChildren(null)
.none { it.elementType in badModifiers } &&

diktat/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/chapter6/classes/DataClassesRule.kt

Line 154 in 70f46ca

private val badModifiers = listOf(OPEN_KEYWORD, ABSTRACT_KEYWORD, INNER_KEYWORD, SEALED_KEYWORD, ENUM_KEYWORD)

Copy link
Member Author

@orchestr7 orchestr7 Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

kek

Copy link
Member Author

Choose a reason for hiding this comment

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

also I have no idea, why "open" is prohibited

Copy link
Member

Choose a reason for hiding this comment

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

image

Copy link
Member Author

@orchestr7 orchestr7 Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

but what is the idea? You have open class only with data and properties?
Why not abstract class in this case?

Copy link
Member

Choose a reason for hiding this comment

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

If there are no abstract fields, than diktat will suggest to change abstract to open ¯\(ツ)/¯

@orchestr7 orchestr7 merged commit 40a51a9 into master Jul 25, 2022
@orchestr7 orchestr7 deleted the bugfix/data-class-rules branch July 25, 2022 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petertrr petertrr petertrr approved these changes

@0x6675636b796f75676974687562 0x6675636b796f75676974687562 0x6675636b796f75676974687562 approved these changes

@sanyavertolet sanyavertolet sanyavertolet approved these changes

@nulls nulls nulls approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.2
Development

Successfully merging this pull request may close these issues.

USE_DATA_CLASS: False positive for Annotations
5 participants
@orchestr7 @nulls @petertrr @sanyavertolet @0x6675636b796f75676974687562