Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing generic type doesn't show a warning #26914

Closed
zoechi opened this issue Jul 19, 2016 · 7 comments
Closed

Missing generic type doesn't show a warning #26914

zoechi opened this issue Jul 19, 2016 · 7 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@zoechi
Copy link
Contributor

zoechi commented Jul 19, 2016

see comment in AddNewRow class:

abstract class DataItem<K, V> extends ItemBase<K, V> {
  bool collapsed;
}

class AddNewRow extends EventData {
  final DataItem item; // <===== error expected but I don't get one
  final Column column;

  AddNewRow(Object sender, this.item, this.column) : super(sender: sender);
}

here the analyzer complains for the same type

image

and if I add one I get another hint because AddnewRow.item isn't compatible

image

with implicit-dynamic: false

Dart VM version: 1.19.0-edge.69a8b11f6717d4cc8e55f9bc0123eac4f90db849 (Mon Jul 18 16:37:12 2016) on "linux_x64"
@zoechi zoechi changed the title Missing generic type doesn't show an error Missing generic type doesn't show a warning Jul 19, 2016
@bwilkerson bwilkerson added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-strong-mode P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 19, 2016
@jmesserly jmesserly added type-enhancement A request for a change that isn't a bug and removed type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 19, 2016
@jmesserly
Copy link

jmesserly commented Jul 19, 2016

yeah. This is intended behavior. @leafpetersen (and @munificent too?) thought we did not want to warn about missing generics in type annotations. My initial implementation actually did, though, because it felt more consistent to me. So this is good feedback. (Perhaps we should make this more clear in the documentation too: https://github.com/dart-lang/dev_compiler/blob/master/doc/STATIC_SAFETY.md#disable-implicit-dynamic)

I think the reasoning is more or less: DataItem as a type annotation is a lot less likely to cause a runtime failure in strong mode compared with new DataItem. And it felt a bit too harsh to take implicit type args away there.

I described this as a "no-implicit-generic-types" option here: #25573 (comment), but it wasn't clear to us if it was worth adding that flag. Also we may want to consider implicit-Object instead.

p.s. @zoechi HUGE thanks for trying this out!!! This feedback is immensely valuable, keep it coming :) (and feel free to disagree with us too, we definitely don't get these right the first time.)

@jmesserly
Copy link

duping against #26784 but definitely let's continue the discussion on this one

@jmesserly jmesserly added the closed-duplicate Closed in favor of an existing report label Jul 19, 2016
@leafpetersen
Copy link
Member

I'm not entirely sure which part is the bug here. It looks like you are maybe using a lint rule, and the lint is not kicking in for the use of DataItem in the class? Is that the issue? If so, this isn't strong mode related.

As @jmesserly says the strong-mode no-implicit-dynamic flag currently only warns on places where strong mode could have inferred a type (but failed). We may reconsider this based on feedback though.

@jmesserly
Copy link

yeah, let me know if I misinterpreted that. I used the context of your other bugs to assume this report was about a difference between the "lint" and the "no-implicit-dynamic" feature.

@zoechi
Copy link
Contributor Author

zoechi commented Jul 19, 2016

@leafpetersen it's possible, I'm still trying to understand all the implications of using no-implicit-dynamic

Currently I'm struggling to combine the comments here and the response I got in #26901 (comment) from @jmesserly

I'm not sure if it is now expected to get hints/warnings for missing type arguments or not from no-implicit-dynamic

@leafpetersen
Copy link
Member

So basically, when you use a generic class as a type, and leave off the generic arguments, --no-implicit-dynamic treats that as user intent: you are leaving off type arguments in a position where strong mode will never do inference, and so we treat this as just a shorthand way of explicitly writing dynamic. When you use a generic class as a constructor, strong mode treats that as a request for inference, and --no-implicit-dynamic makes a failure to infer a type there into an error.

Clearly this point is confusing, which suggests we may have either made the wrong choice on generic classes used as types, and/or picked the wrong name for the flag. I think at one point we were considering calling this --no-inferred-dynamic, which might (or might not) have made this point clearer.

@zoechi
Copy link
Contributor Author

zoechi commented Jul 19, 2016

Ouch, I have a sub-project where I forgot to remove - always_specify_types
Sorry for the confusion. I'll check again and report back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants