proposal: `avoid_non_nullable_valued_map_containskey_before_access`

[srawlins]

avoid_non_nullable_valued_map_containskey_before_access

I challenge anyone to think up a better name!

Description

The pattern to avoid is calling map.containsKey(x) in an if expression on a Map with a non-nullable value type, then repeat the access with map[x]!, inside the if's body.

Details

When migrating pre-null safe code to null safety, I see this pattern a lot:

void f(Map<String, int> m) {
  if (m.containsKey('key')) {
    var value = m['key'];
    // ...
  }
  // ...
}

Instead of migrating this to use ! after m['key'], many users like this migration:

void f(Map<String, int> m) {
  var value = m['key'];
  if (value != null) {
    // ...
  }
  // ...
}

It's shorter and executes the map access once, and avoids a !.

Kind

Style?

Good Examples

Nullable value Map is OK:

void f(Map<String, int?> m) {
  if (m.containsKey('key')) {
    var value = m['key'];
    // ...
  }
  // ...
}

Bad Examples

void f(Map<String, int> m) {
  if (m.containsKey('key')) {
    var value = m['key'];
    // ...
  }
  // ...
}

[bwilkerson]

I challenge anyone to think up a better name!

😄

I like the proposed style. In essence, what it's doing is making use of the fact that the index operator on Map is nullable for two reasons (the value at a given key might be null if the value type is nullable, and the value returned might be null if there is no value associated with the given key) but that only the latter applies when the value type is non-nullable. So while a nullable-valued map still needs to be checked in order to distinguish between the two cases, a non-nullable-valued map doesn't need the disambiguation.

As you mentioned, the proposed style has the nice property that it optimizes both accessing the value (by performing a single lookup rather than two) and checking whether the value is null.

I wonder whether that property might not be a good way to name the lint; after all, it seems like the primary value proposition. Perhaps something like optimize_non_nullable_map_value_access?