RxJava2MissingCompositeDisposableClearDetector missing nested PsiStatement's checks?

[PPetka]

Hey, I have notice that in your RxJava2MissingCompositeDisposableClearDetector class you check only 1st level PsiStatments in methods? Is that desired behaviour?

for example if u have Java method like

public void foo(){
   compositeDisposable.clear() //this will be found
 
   if(true){
       compositeDisposable.clear() //this wont be found 
   }else{
       compositeDisposable.clear() //this wont be found
   }
}

Check wont find occurences in nested code blocks since they are not PsiExpressions they are PsiCodeBlock's
This code example is dumb but its about to point that any nested calls wont be found.

I am begginer in developing lint-rules and I am not sure if there are other types of nested blocks that are not conditioned and should be checked.

Greetings

[vanniktech]

You're right. That's not caught.

@Test fun clearInIfStatement() {
  lint()
    .files(rxJava2(), java("""
        |package foo;
        |
        |import io.reactivex.disposables.CompositeDisposable;
        |
        |class Example {
        |  CompositeDisposable cd;
        |
        |  public void foo() {
        |    if (true) {
        |      cd.clear();
        |    }
        |  }
        |}""".trimMargin()))
    .issues(ISSUE_MISSING_COMPOSITE_DISPOSABLE_CLEAR)
    .run()
    .expectClean()
}

fails

[PPetka]

Alright then. I have wrote a recurrent function that search for nested PsiExpression's

 private List<PsiExpression> getAllNestedPsiExpressions(PsiStatement statement) {
        List<PsiExpression> expressions = new ArrayList<>();
        PsiElement[] childList = statement.getChildren();
        for (PsiElement element : childList) {
            if (element instanceof PsiExpression) {
                expressions.add((PsiExpression) element);
            } else if (element instanceof PsiStatement) {
                PsiStatement innerStatement = (PsiStatement) element;
                expressions.addAll(getAllNestedPsiExpressions(innerStatement));
            } else if (element instanceof PsiCodeBlock) {
                PsiStatement[] statements = ((PsiCodeBlock) element).getStatements();
                for (PsiStatement psiStatement : statements) {
                    expressions.addAll(getAllNestedPsiExpressions(psiStatement));
                }
            }
        }
        return expressions;
    }

but i ll bet there must be a simpler solution. Also wondering if it is worth to check if clear() is called in conditions that may not be called like if() and then throw an appropriate error.

If u find a better way please let me know.

[vanniktech]

Feel free to take my test case and extend the detector with your function in a PR.