Migration tool: sync* method should return non nullable Iterable

[a14n]

On the following method:

Iterable<String> debugWordWrap(String message, int width, { String wrapIndent = '' }) sync* {

migration tool suggests:

Iterable<String >? debugWordWrap(String  message, int  width, { String  wrapIndent = '' }) sync* {

I don't think the return type should be nullable.

/cc @stereotype441

[stereotype441]

If you click on the inserted ?, in the lower right corner of the screen you should see a trace indicating why the tool inserted it. Does that give any clues?

[a14n]

The tool inserted it because of emtpy returns:

Iterable<String >? debugWordWrap(String  message, int  width, { String  wrapIndent = '' }) sync* {
  if (message.length < width || message.trimLeft()[0] == '#') {
    yield message;
    return;
  }

My point is that sync* methods should be special cased because the returned Iterable can never be null.

[lrhn]

Agree. An empty return in a sync* or async* method does not return null, like it does in non-generator functions, so it should not be taken as a hint that the return type should be nullable.

[stereotype441]

The tool inserted it because of emtpy returns:

Iterable<String >? debugWordWrap(String  message, int  width, { String  wrapIndent = '' }) sync* {
  if (message.length < width || message.trimLeft()[0] == '#') {
    yield message;
    return;
  }

Aha, got it. This should be pretty easy to fix; we just need the EdgeBuilder to ignore return statements when processing generator functions.

[srawlins]

Hey @a14n, a question occurs: Are you able to build the SDK frequently yourself? Do we need to publish the nnbd_migration package in order for you to see these fixes as we land them?

[a14n]

I usually wait for the Dart roll that occurs quite frequently.