Sdk: flutter analyze does not detect returns from switch statement with all enum cases

[maks]

Given code like this:

enum MyEnum { Val1, Val2 }

...

  @override
  Widget buildListItem() {
    switch (item.type) {
      case MyEnum.Val1:
        return Container();
        break;
      case MyEnum.Val2:
       return Container();
        break;
    }
  }

flutter analyze complains:

Analyzing ZZZZZ-app...                                           

   info • This function has a return type of 'Widget', but doesn't end with a return statement • lib/ZZZZZ.dart:16:3 •
          missing_return

Now if I add a default case:

default:
        return Container(); // just to satisfy flutter analyzer
        break;

analyzer is happy.

Given that the VS Code plugin is able to warn me when I have missing cases in the switch statement based on the known values of the enum and then be ok once I have cases covering both values without a default case, I would expect that analyzer would be able to know this too?

[mraleph]

Note that item.type can be null, so you are indeed missing the return statement for that case.

It is a bit strange that you are not getting the same errors from flutter analyze and in the VS Code though - meaning that analyzer is most likely running with different analysis options.

I think this is a bug either for flutter analyze itself or for VS Code plugin /cc @DanTup

I am tentatively closing this.

[DanTup]

When I test this in VS Code, I do get the warning:

@maks if you're not seeing this, can you confirm your Flutter version and whether you can repro in a small sample file with this code?

import 'package:flutter/widgets.dart';

enum MyEnum { Val1, Val2 }

Widget buildListItem() {
  MyEnum item;
  switch (item) {
    case MyEnum.Val1:
      return Container();
      break;
    case MyEnum.Val2:
      return Container();
      break;
  }
}

If so, please file a bug at https://github.com/Dart-Code/Dart-Code and I'll take a look. Thanks!

[bwilkerson]

Also, just FYI, you don't need a break after a return. The list of statements following a case is required to either be empty, or to end with return, throw, rethrow, break or continue.

[DanTup]

Could/should it be marked as dead code? We'd grey it out in VS Code in that case.

[bwilkerson]

Yes. It looks like our dead code detector is failing in this case.

[maks]

@mraleph thank you for looking at this so quickly and yes sorry, you're completely right, I did forget about the null case, perhaps the new nullable-types work being done atm will help with this in the near future.

And my apologies for not being more specific in the original issue description, I didn't actually mean that the lint messages regarding the return were different in VSC vs the cli analyzer.
Rather what I was trying to say was that when I had the switch statement with only 1 clause in VSC:

import 'package:flutter/widgets.dart';

enum MyEnum { Val1, Val2 }

Widget buildListItem() {
  MyEnum item;
  switch (item) {
    case MyEnum.Val1:
      return Container();
      break;
  }
}

I would see a (analyzer?) error about it in VSC:

And then once I had added the second enum value case, that error went away but of course the return missing info message remained. So I think this is just a much more minor issue with that dart_missing_constant_In_switch not warning about missing the null case that Vyacheslav correctly spotted. And even better I guess once the nullability support lands, it will be possible to give the switch a non-nullable object of enum type and be confident all cases have been covered.

Also thanks @bwilkerson for pointing out not needing to have the breaks. Coming from Kotlin I do miss when expressions and not needing returns everywhere, but this will at least allow me to make my code a bit more succinct :-)

[nvma]

I would also like to contribute to the issue by pointing out that process exiting doesn't not considered to be a return.

  Future<Bootstrap> bootstrapApplication() async {
    final String baseUrl = config.getString("API_URL");
    try {
      final response = await client.get(baseUrl + "bootstrap");
      return Bootstrap.fromJson(json.decode(response.body));
    } catch (e) {
      print(e);
      exit(0);
    }
  }

I would argue that exiting a process could be considered a return, from the standpoint of the language server.

[maks]

@mraleph unless I'm missing something again I think it maybe need to reopen this as I just came back to this by adding an if before the swtich:

enum MyEnum { Val1, Val2 }

...

  @override
  Widget buildListItem(MyEnum anEnumValue) {
    if (anEnumValue == null) {
      return Container();
    }
    switch (anEnumValue) {
      case MyEnum.Val1:
        return Container();
      case MyEnum.Val2:
       return Container();
    }
  }

I still get the message:

This function has a return type of 'Widget', but doesn't end with a return statement.
Try adding a return statement, or changing the return type to 'void'.

The reason I added an if instead of a default is because I want to handle all the enum cases explicitly so that eveytime a new enum value is added the lint complains about it not being handled in the switch, which I dont get if there is a default clause.

Or is this something that will be fixed as soon as NNBD ships? If so then its just a temporary inconvience.

[mraleph]

I would expect this to be fixed when NNBD ships.

/cc @stereotype441

[QuintinWillison]

@mraleph and @stereotype441, does this therefore relate to #35065 then?

I'm a bit confused as, for me, if I look at what the tooling is telling me - in particular in the context of an equivalent chunk of Java code - then the result is more than a little bit surprising.

In my case I have:

enum OpState {
  NotStarted,
  InProgress,
  Succeeded,
  Failed,
}

String opStateDescription(OpState state, String action, String operating, String done) {
  switch (state) {
    case OpState.NotStarted: return action;
    case OpState.InProgress: return operating + '...';
    case OpState.Succeeded: return done;
    case OpState.Failed: return 'Failed to ' + action;
  }
}

And am getting:

This function has a return type of 'String', but doesn't end with a return statement.
Try adding a return statement, or changing the return type to 'void'.dart(missing_return)

from my IDE (VS Code).

Am I doing something stupid? I'm not sure why this issue is closed as I can't see a solution to the issue or an absolute reference to something that will solve it in future.

_For what it's worth_, the Dart language specification is more than a little bit impenetrable for me (as someone without a degree in mathematics!) so I'm finding my way by following my nose (based on knowledge of other languages), looking at examples, reading the Flutter&Dart higher level docs and relying on linter/analysis best practice defaults. So, as such, the particular curio that this issue relates to dents my confidence in adopting Dart as I'm struggling to understand the "why".

[maks]

@QuintinWillison your code is exactly the same as mine when I opened this issue and you're making the exact same mistake I did, which as Vyacheslav quite rightly pointed out to me: you are not allowing for the fact that your state can have a value of null.

Also I'm not sure why you consider this surprising compared to Java given Java's behaviour is very similiar and if anything more surprising.

Given that null-safety feature is being added to Dart literally as we speak, if anything this is just a temporary inconvenience.

[MoacirSchmidt]

add a default to switch