Add rule to warn on ignored return value

[bbaldino]

Addresses #2239.

I'd consider this as draft at this point. It works for the test cases I've provided, but I'm not familiar with all of the psi element types and I worry a bit that there may be some I missed (or perhaps a better way to detect what I am detecting).

[bbaldino]

Travis tests look like they're failing because I didn't update default-detekt-yml. Not sure how that should be updated yet, though, so will leave it as-is until I hear how to proceed there.

[BraisGabin]

Travis tests look like they're failing because I didn't update default-detekt-yml. Not sure how that should be updated yet, though, so will leave it as-is until I hear how to proceed there.

Run ./gradlew generateDocumentation and those files would be updated.

[bbaldino]

Travis tests look like they're failing because I didn't update default-detekt-yml. Not sure how that should be updated yet, though, so will leave it as-is until I hear how to proceed there.

Run ./gradlew generateDocumentation and those files would be updated.

I ran this, but it didn't seem to result in any changes I could commit.

[bbaldino]

Travis tests look like they're failing because I didn't update default-detekt-yml. Not sure how that should be updated yet, though, so will leave it as-is until I hear how to proceed there.

Run ./gradlew generateDocumentation and those files would be updated.

I ran this, but it didn't seem to result in any changes I could commit.

Found it. Had to add it to PotentialBugProvider.

[bbaldino]

I've made some more tweaks, but when testing against my code base I'm running into a lot of incorrect classifications. I'm checking the PsiElement type of each one I find and adding handling for it, but it feels a bit like whack-a-mole and can be hard to differentiate in places where elements are nested multiple levels below where an assignment or something is actually done. I have a feeling my lack of knowledge of the Psi types may be making this more difficult than it needs to be? Are there some useful helpers to differentiate expressions from assignment expressions, etc.?

[codecov-io]

Codecov Report

Merging #2242 into master will increase coverage by 0.03%.
The diff coverage is 87.5%.

@@             Coverage Diff              @@
##             master    #2242      +/-   ##
============================================
+ Coverage     81.38%   81.41%   +0.03%     
- Complexity     2074     2083       +9     
============================================
  Files           343      344       +1     
  Lines          5984     6000      +16     
  Branches       1086     1091       +5     
============================================
+ Hits           4870     4885      +15     
  Misses          532      532              
- Partials        582      583       +1

Impacted Files	Coverage Δ	Complexity Δ
...sch/detekt/rules/providers/PotentialBugProvider.kt	100% <100%> (ø)	3 <0> (ø)	⬇️
...arturbosch/detekt/rules/bugs/IgnoredReturnValue.kt	85.71% <85.71%> (ø)	6 <6> (?)
...turbosch/detekt/rules/style/UnusedPrivateMember.kt	93.47% <0%> (ø)	5% <0%> (ø)	⬇️
...osch/detekt/rules/bugs/WrongEqualsTypeParameter.kt	100% <0%> (ø)	10% <0%> (+3%)	⬆️
.../gitlab/arturbosch/detekt/rules/MethodSignature.kt	70% <0%> (+10%)	0% <0%> (ø)	⬇️

---

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 5dfe7c5...ef69090. Read the comment docs.

[schalkms]

Continuing here, because it’s easier than on Slack.
The way to go would be to take a code snippet that gets flagged by your rule and debug the Psi tree with the watch functionality in IntelliJ. I don’t think there’s some pre defined helper function in the Framework, which does the trick for you.
Please correct me if I’m wrong @3flex.

[bbaldino]

Continuing here, because it’s easier than on Slack.
The way to go would be to take a code snippet that gets flagged by your rule and debug the Psi tree with the watch functionality in IntelliJ. I don’t think there’s some pre defined helper function in the Framework, which does the trick for you.
Please correct me if I’m wrong @3flex.

Ok, this is what I've been doing. Thanks for confirming.

[3flex]

Also have a look at PsiViewer plugin for IntelliJ which I've found useful.

[bbaldino]

Also have a look at PsiViewer plugin for IntelliJ which I've found useful.

Ah! This is super helpful. Thanks!

[schalkms]

@bbaldino do you have an update on this PR? Do you need assistance from our side?

[bbaldino]

@bbaldino do you have an update on this PR? Do you need assistance from our side?

Sorry for the lack of updates, work has been busy and I haven't had much time to work on this. Where I was hung up, though, was discovering all the different expressions which this should apply to; finding them all and good ways to properly detect them was proving tough.

I wondered about detekt's policy on rules: might it be interesting to have an initial version of this which was naive and only detected some cases? Or are rules only merged when they are 'complete'?

[schalkms]

I wondered about detekt's policy on rules: might it be interesting to have an initial version of this which was naive and only detected some cases? Or are rules only merged when they are 'complete'?

We haven’t had this case so far. I obviously don’t want to include rules that are buggy into detekt. In your case the rule just doesn’t flag all corner cases, which is okay. I hope that I understood you correctly.
In my opinion this rule should go into an experimental rule set.

[bbaldino]

I wondered about detekt's policy on rules: might it be interesting to have an initial version of this which was naive and only detected some cases? Or are rules only merged when they are 'complete'?

We haven’t had this case so far. I obviously don’t want to include rules that are buggy into detekt. In your case the rule just doesn’t flag all corner cases, which is okay. I hope that I understood you correctly.
In my opinion this rule should go into an experimental rule set.

Yeah that makes sense and is what I was wondering. I still have been a bit strapped for time, but I'll plan on seeing if I can focus on a smaller ruleset which can detect some cases but not give any false positives.

[schalkms]

@bbaldino great! Just pings us if you need help or assistance.

[schalkms]

Since this PR become inactive, I'm wondering if you @bbaldino still plan to work further on this PR. We are open to answer any questions you have.

[bbaldino]

Apologies for the radio silence...work got busy and I haven't had time to come back to this--and unfortunately don't think I'll be able to anytime soon. Feel free to close this if you like, I can reopen if I ever get back around to it.

[schalkms]

@bbaldino no worries.. Thanks for the fast reply. I'll put a help wanted tag on this PR in case somebody wants to pick it up and thus help out here.

[cortinico]

@schalkms I can pick this up, can you assign this PR to me?

[BraisGabin]

All yours ;) and thanks for the help!