-
-
Notifications
You must be signed in to change notification settings - Fork 784
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
Fix UseDataClass to accept classes that implement interfaces #2905
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2905 +/- ##
============================================
- Coverage 80.18% 80.15% -0.03%
- Complexity 2426 2429 +3
============================================
Files 421 421
Lines 7378 7384 +6
Branches 1350 1354 +4
============================================
+ Hits 5916 5919 +3
- Misses 762 764 +2
- Partials 700 701 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I've left a minor comment on the tests. If you could add a couple more it would be better 👍
assertThat(subject.compileAndLint(code)).hasSize(1) | ||
} | ||
|
||
it("does report a candidate class with an interface extension that overrides vals") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can I ask you to please write two other tests:
interface SimpleInterface {
val i: Int
}
data class DataClass(override val i: Int): SimpleInterface
This should not report anything as you're correctly already using a data class.
And this other test:
interface SimpleInterface {
val i: Int
}
open class BaseClass(open val j: Int)
class DataClass(override val i: Int, override val j: Int): SimpleInterface, BaseClass(j)
This should also report nothing as you're extending from another class (and an interface).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added additional tests and looks like they pass. Great test suggestions thanks!
Addresses issue #2904
Root cause is that our doesNotExtendAnything() considers interfaces.
There is not a very easy way to fix this, the fix I have done to detect interfaces is
From what I searched PSI does not give us many tools to get class from superTypeEntry