sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Improve migration tool to avoid invalid_null_aware_operator

Open stereotype441 opened this issue 3 years ago • 0 comments

I've just been looking at several examples in our internal codebase where the output of the migration tool gets flagged for additional review due to the invalid_null_aware_operator warning. There seem to be two common cases where the tool could do a better job:

  • The thing to the left of the null-aware operator is an explicitly typed parameter of a public method; in every case I've examined but one*, the correct migration would be to mark the parameter as nullable. (What the migration tool does instead is to leave the parameter as non-nullable unless it can find a specific instance of a nullable value being passed in as a parameter).
  • The thing to the left of the null-aware operator is an instance getter invocation or an instance method call (possibly null-aware), and the target of the invocation has already been confirmed to be non-nullable (because it's in code that has already been migrated). In every case I've examined, the correct migration would be to replace the null-aware operator (?.) with ..

*The one exception is a case where the parameter clearly is not meant to be nullable, because earlier in the same control flow path, the parameter is accessed using . instead of ?.. This is clearly a bug that requires human intervention; either both accesses should be ?. or both accesses should be ., and there's no way for the tool to tell which is correct.

I'm going to try to implement heuristics to choose the proper migration in these two cases, and I'll verify that in the exceptional case, the tool operates in a way that continues to flag the code for additional review.

stereotype441 avatar Aug 05 '22 16:08 stereotype441