Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

[Rule change] avoid-non-null-assertion: Maybe ignore if(some.field != null) some.field! #720

Open
fzyzcjy opened this issue Mar 1, 2022 · 7 comments
Assignees
Labels
area-rules type: enhancement New feature or request

Comments

@fzyzcjy
Copy link
Contributor

fzyzcjy commented Mar 1, 2022

What rule do you want to change?
avoid-non-null-assertion

How will the change be implemented? (New option, new default behavior, etc.)?
will fill out more details if interested

Please provide some example code that this change will affect:

What does the rule currently do for this code?

What will the rule do after it's changed?

Are you willing to submit a pull request to implement this change?

@incendial
Copy link
Member

Agree, that looks doable

@incendial incendial added area-rules type: enhancement New feature or request labels Mar 1, 2022
@roman-petrov
Copy link
Contributor

@incendial - if you decide to make avoid-non-null-assertion more permissive, can you please make it optional? I'm a bit fan of strict restrictions and currently do not see any problems with strict avoid-non-null-assertion on our codebase.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Mar 1, 2022

@roman-petrov

Agree and that sounds not hard to implement.

By the way, I am curious how do you deal with the following cases?

  • if(some.field != null) some.field! (especially when that field is a final primitive field)
  • SomeInheritedWidget.of(context)!.something (when the inherited widget really should be there, otherwise the programmer is making a big mistake, and we should throw instead of silently fallback)

@roman-petrov
Copy link
Contributor

@incendial

By the way, I am curious how do you deal with the following cases?

  • if(some.field != null) some.field! (especially when that field is a final primitive field)
  • SomeInheritedWidget.of(context)!.something (when the inherited widget really should be there, otherwise the programmer is making a big mistake, and we should throw instead of silently fallback)

At the moment in these cases our code becomes a bit verbose like

final value = some.field;
if (value != null) {
...
}

and that's ok for us.

A few month ago in "Flutter Dev Podcast" channel I participated in some discussions with Vyacheslav Egorov about such cases and as I understood:

  • When we do if(some.field != null) some.field! we do not have 100% protection from NPE, null safety still can be violated in this case.
  • Dart language team might decide to implement some language feature that makes code less verbose for such cases, something like
if ((final value = some.field) != null) {
...
}

This discussed language feature also might be related to this topic: Enable promotion for private final fields and fields on non-escaping private classes

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Mar 1, 2022

@roman-petrov Thank you for the elaboration!

So I wonder how did you solve the second case, i.e.: SomeInheritedWidget.of(context)!.something (when the inherited widget really should be there, otherwise the programmer is making a big mistake, and we should throw instead of silently fallback)

@roman-petrov
Copy link
Contributor

So I wonder how did you solve the second case, i.e.: SomeInheritedWidget.of(context)!.something (when the inherited widget really should be there, otherwise the programmer is making a big mistake, and we should throw instead of silently fallback)

About the second case: we do not use inherited widgets much. For example, some built-in Flutter inherited widgets return non nullable values (like Theme.of(context)).

But if I had to - I would use the same approach as I described above like

final widget = SomeInheritedWidget.of(context);
if (widget == null) { 
   throw ...
}

I don't think I would use exactly the same code I listed, but the concept will not change - check nullable before doing something. Without exceptions:)

I understand that some people will dislike such approaches, but everybody can have his own opinion about how to make things better :)

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented Mar 1, 2022

@roman-petrov Thanks for explaining your approach! I will consider about it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-rules type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants