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

Don't null check a mutable property #3353

Closed
BraisGabin opened this issue Jan 6, 2021 · 4 comments
Closed

Don't null check a mutable property #3353

BraisGabin opened this issue Jan 6, 2021 · 4 comments

Comments

@BraisGabin
Copy link
Member

Expected Behavior of the rule

non compliant:

class A(
    var foo: Asdf?
) {
  fun bar() {
    if (foo != null) {
      foo!!.toString()
      doThings(foo!!.value)
    }
  }
}

compliant:

class A(
    var foo: Asdf?
) {
  fun bar() {
    val foo = foo // or use let, or whatever you want
    if (foo != null) {
      foo.toString()
      doThings(foo.value)
    }
  }
}

Context

#3352 is the example that make me think about this rule. This is a pattern that I see often in the my project. Usually this errors happens when you translates code from Java to Kotlin.

Maybe we could check this kind of things too:
non compliant:

class A {
  fun getActivity(): Activity? = // whatever

  fun bar() {
    if (getActivity() != null) {
      getActivity()!!.doSomething()
    }
  }
}

compliant:

class A {
  fun getActivity(): Activity? = // whatever

  fun bar() {
    val activity = getActivity() // or ?.let or whatever you want
    if (activity != null) {
      .doSomething()
    }
  }
}

This pattern is kind of usual in Android because the framework give us this kind of APIs. But I don't think that this as an Android specific rule.

@cortinico
Copy link
Member

non compliant:

class A(
    var foo: Asdf?
) {
  fun bar() {
    if (foo != null) {
      foo!!.toString()
      doThings(foo!!.value)
    }
  }
}

I believe the root of this rule should be "don't null check a mutable property". The multiple !! (or whatever null checks) are needed as foo is a nullable mutable property, and therefore checking if it's not null won't suffice to smart cast it to a Asdf.

@BraisGabin BraisGabin changed the title Don't assert notNull after you check it Don't null check a mutable property Jan 11, 2021
@github-actions
Copy link

github-actions bot commented Nov 7, 2021

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@atulgpt
Copy link
Contributor

atulgpt commented Jan 18, 2023

Hi, @BraisGabin I think this issue got fixed by the above PR. right?

@BraisGabin
Copy link
Member Author

Nice spot! Closing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants