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

v1.15 and type inference is still poor in Dart #26110

Closed
MikeMitterer opened this issue Mar 29, 2016 · 17 comments
Closed

v1.15 and type inference is still poor in Dart #26110

MikeMitterer opened this issue Mar 29, 2016 · 17 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

@MikeMitterer
Copy link

screenshot-1989

From time to time I try to declare variables right side only just to figure out if it finally works but it still fails! (After how many years of Dart??? 3? 4?)

In your styleguide you promote this kind of declaration (https://www.dartlang.org/effective-dart/style/#prefer-using-lowercamelcase-for-constant-names) which is cool if it would work. And yes - in some situations it works but it fails in other cases. Maybe you guys know all these cases but I don't.

There is always this feeling that type inference fails in Dart and that I have to define the types on both sides to make sure that Analyzer knows what I mean.

The same thing for Generics. final List<String> name = new List<String>() this is how a String-List looks in my code. I'm almost!!!! sure that for the assignment new List() is enough but I'm always a bit leery if the analyzer / autocompletion in IJ always knows what I mean so I use the long form.

Very annoying - Dart is such a nice language but here it really falls behind other languages like Scala.

@sgjesse sgjesse added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Mar 29, 2016
@zoechi
Copy link
Contributor

zoechi commented Mar 29, 2016

I think it's great that you report the issue. To be able to trust type inference is something I'm really looking forward to as well (also since a few years already ;-) ).

Scala is almost 10 years older and therefore had 10 more years to be refined.

There was lot of progress made the past months improving the analyzer.
I think instead of complaining it's better to just create an issue with a clear repo case that shows what's actually happening and what is expected.

@srawlins
Copy link
Member

To decode your example for others: the Analyzer complains about image2.classes = "thumb" because classes is supposed to be an Iterable, not a String. It should therefore complain about image.classes = "thumb" as well, but does not, because the Analyzer has not inferred image's type.

Example: https://dartpad.dartlang.org/634ca4a8a4e61650a56c

I think this is a duplicate of #18864 ("Add hint for incorrect assignment based on propagated types")

@srawlins
Copy link
Member

Or rather, maybe #17945, or #17800 or #9401. They're all similar. #9401 reveals you can launch the analyzer with --type-checks-for-inferred-types (undocumented?).

@eernstg
Copy link
Member

eernstg commented Mar 29, 2016

I think the real issue here is the tension between optional typing and mandatory typing.

When the spec says (version 1.11, p.16) that final v induces a getter with signature get v, and (p26, on getters) 'If no return type is specified, the return type of the getter is dynamic', it's clear that the type of image and hence image.classes is dynamic, so no diagnostic messages should be emitted. The point is that the types are optional, and you shouldn't be subject to any type related constraints if you haven't asked for it (using explicit type annotations).

In contrast, languages like Scala require complete typing, and they will only allow for omitting type annotations when the context allows the type annotations to be inferred. With that approach, image would have the type dom.ImageElement, and the treatment of image.classes should be identical to the treatment of image2.classes.

So we are really considering a request for a different style of type system, not just a set of bug fixes.

The analyzer tries to deliver such a different style of static analysis via hints, based on local data flow analysis. But the hints do not rely on an explicitly and completely specified type system---there is no explicit specification of exactly which kind of flow analysis it should use, and results produced using flow analysis may not in general correspond directly to any "normal" type system. For instance, a flow analysis could conclude that y is an int with int x = 5; final num y = x; whereas all mainstream type systems would give it type num, and a flow analysis might even (unsoundly from the declaration alone, or soundly by checking all assignments to y in its scope) assert that y were an int without the final.

Maybe a good resolution of this issue is the same thing as creating an explicitly and completely specified type system with inference?

@bwilkerson
Copy link
Member

Maybe a good resolution of this issue is the same thing as creating an explicitly and completely specified type system with inference?

Actually, that's exactly what strong mode is intended to be.

@MikeMitterer Have you tried enabling strong mode to see whether it does what you want? If not, create a file named .analysis_options in the root of your package that contains the following lines:

analyzer:
  strong-mode: true

It might do more than you like, though. Strong mode defines a sound type system, which has lots of other implications. See https://github.com/dart-lang/dev_compiler/blob/master/STRONG_MODE.md for more information.

@MikeMitterer
Copy link
Author

Thanks for the feedback! Yes @bwilkerson I've tried the strong mode a few month ago. I found it a bit too strong. It even warned me if "super" was the first call in the initializer list (should be the last call). The problem is that all my inherited classes follow this pattern ': super(var), all other vars {' I got x-pages full of warnings? errors? can't remember but it was way too much... Ok - I just turned it on again to show you what I mean:
screenshot-1994
On the left side you can see the "super"-errors I mentioned. The popup shows another problem: "listen" wants a void function but the closure is defined without a return type so Analyzer complains.

Dart has a syntactic limitation in this case: it is not possible to statically annotate the return type of a closure literal.

Hmmm - so no solution for this. I gona turn strong mode off again...

+1 @eernstg For a completely specified type system. Quite a while ago I asked in G+/Dartisans who's using type annotations: (https://goo.gl/XpdOPZ) 83% are using strong types. (13% said they use strong types mostly)

@bwilkerson
Copy link
Member

@vsmenon for the feedback on strong mode.

@vsmenon
Copy link
Member

vsmenon commented Mar 30, 2016

@MikeMitterer the popup in your example is complaining about the parameter type, not the return type. We could make our errors clearer in that regard.

Regarding super-at-end, if our tools automatically fixed that for you (e.g., an IDE-based autofix), would that work for you?

@leafpetersen
Copy link
Member

Note that with a .analysis_options file you can opt out of entire classes of warnings that you don't care about. @pq could probably tell you what to put to turn off the super warning.

@MikeMitterer
Copy link
Author

@vsmenon Really??? Which parameter type - I thought it is complaining that listen is declared as listen(void callback(...)...) but the callback is defined as closure and this closure has a dynamic return type...

Yes - the super-at-end thing should be either fixed by the IDE or even better the compiler should handle that. It looks strange to me it the last thing in the initializer-list is "super". In other languages it's either common or recommend to call the super-constructor first and then do the other stuff - should be the same in Dart too.

@eernstg
Copy link
Member

eernstg commented Mar 31, 2016

About the return types of function expressions (lambdas): It's often very
nearly as concise and convenient to use a named local function, and they do
allow for return type annotations:

.. xs.map((T t) => expr) ..

then becomes

S goodName(T t) => expr;
.. xs.map(goodName) ..

where the goodName may even me helpful when returning to this code again
some time later. ;-)

On Thu, Mar 31, 2016 at 2:01 AM, Leaf Petersen notifications@github.com
wrote:

Note that with a .analysis_options file you can opt out of entire classes
of warnings that you don't care about. @pq https://github.com/pq could
probably tell you what to put to turn off the super warning.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#26110 (comment)

Erik Ernst - Google Danmark ApS
Skt Petri Passage 5, 2 sal, 1165 København K, Denmark
CVR no. 28866984

@zoechi
Copy link
Contributor

zoechi commented Mar 31, 2016

In other languages it's either common or recommend to call the super-constructor first and then do the other stuff

I think this is only where super is called in the constructor body and there it's important that super() is executed before this is accessed.
The super() call in the initialization list ensures this already.

That super() should be last within the initialization list is a different thing.

@MikeMitterer
Copy link
Author

Hmmm - I know what super does... what I tried to explain is that it should be part of the compilation process to make sure that everything that is in the initializer list is called in the right order. To me this is the main reason why Dart has an initializer-list. (like in C++). This initializer-list makes sure that an object is properly setup if we call other functions in it's CTOR.

BTW: @eernstg Because we are talking about Analyzer. I've just changed the interface of one of my proxy-classes and I gave Analyzer a shot. After so many years of Dart - I still don't get it why these "warnings" and "hints" ARE warnings or hints. These are errors! If I call a function with a wrong type or a wrong number of positional arguments it's likely that my application crashes. If a call could crash the application it should (must...) be treated as error.

Maybe this is not well defined in the specs or whatever but as said I haven't seen any other major language that treats such things so liberal (or if so you can turn on a "strict mode"). (OK - forget JS - everything is possible in JS but that's a reason not to use JS!)

screenshot-1995

@eernstg
Copy link
Member

eernstg commented Mar 31, 2016

About hints/warnings/errors: You might prefer very strict checks
(everything which is a diagnostic message is also an error) or very
forgiving checks (everything which does not prevent the program from
running is not an error), but the difference is small on the implementation
side: It's trivial to take an implementation with forgiving checks and make
it more strict; it is a little bit more involved to make an implementation
more forgiving (for instance, the runtime value of an unknown class name
could be dynamic or it could be a runtime error when used in is
expressions, etc -- for Dart an unknown type UnknownType is considered to
be a malformed type, and .. is UnknownType throws at runtime).

It does matter, though. For Dart, it was considered important to let
programmers run their programs even during development (as long as they are
close enough to "fault free" to let them have a runtime semantics), and
hence, e.g., static subtype constraint violations and a wrong number of
arguments in method invocations are classified as warnings. After all, you
might not even encounter that location in the code during your experiments,
and you might learn something on the way to that point, even if you do
reach it.

The hints provide additional information about likely runtime type errors
based on a wider set of techniques than the ones given in the language
specification, e.g., flow analysis (which is not used for the type analysis
in the language specification, except for a few simple cases involving type
propagation where we know after evaluating x is T to true that x has
type T --- search for 'shows that' in the language specification to find
more info on this).

If you want to make sure that there are no warnings or hints, just check
the compiler output, possibly using '--fatal-warnings', '--fatal-hints',
etc. to treat them as errors.

In short, this design allows you to hunt down all these apparently faulty
locations in your code as early as you wish, and it generally allows you to
run your program even at a time where it has known faults. I think this
gets you most of what you want, except for the word 'error' itself. ;-)

On Thu, Mar 31, 2016 at 11:09 AM, Mike Mitterer notifications@github.com
wrote:

Hmmm - I know what super does... what I tried to explain is that it should
be part of the compilation process to make sure that everything that is in
the initializer list is called in the right order. To me this is the main
reason why Dart has an initializer-list. (like in C++). This
initializer-list makes sure that an object is properly setup if we call
other functions in it's CTOR.

BTW: @eernstg https://github.com/eernstg Because we are talking about
Analyzer. I've just changed the interface of one of my proxy-classes and I
gave Analyzer a shot. After so many years of Dart - I still don't get it
why these "warnings" and "hints" ARE warnings or hints. These are errors!
If I call a function with a wrong type or a wrong number of positional
arguments it's likely that my application crashes. If a call could crash
the application it should (must...) be treated as error.

Maybe this is not well defined in the specs or whatever but as said I
haven't seen any other major language that treats such things so liberal
(or if so you can turn on a "strict mode"). (OK - forget JS - everything is
possible in JS but that's a reason not to use JS!)

[image: screenshot-1995]
https://cloud.githubusercontent.com/assets/116654/14170786/c51dc562-f72f-11e5-9e02-eadeaf775e11.png


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#26110 (comment)

Erik Ernst - Google Danmark ApS
Skt Petri Passage 5, 2 sal, 1165 København K, Denmark
CVR no. 28866984

@vsmenon
Copy link
Member

vsmenon commented Mar 31, 2016

@MikeMitterer I was looking at the very end of the popup message. It says:

(ActivateDeviceAction) -> void is not of type (Action) -> void

Your closure expects an ActivateDeviceAction, but the compiler cannot prove it'll pass that in - it might pass in some other Action. One possibility is to make on a generic method so it can return a stream of something more specific than Action (from a typing perspective).

Regarding super, there is (was?) discussion in the style guide. Can't seem to find that, but the quoted text on why is in the linter:

https://github.com/dart-lang/linter/blob/master/lib/src/rules/super_goes_last.dart

@leafpetersen
Copy link
Member

Strong mode will allow a (Whatever) -> Something where a (Whatever) -> void was expected, btw.

@bwilkerson bwilkerson added analyzer-strong-mode P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Mar 31, 2016
@jmesserly
Copy link

I think all of this is addressed now in strong mode, let me know if there's something else we should file a specific issue for. (Covariant parameter overrides are covered under: #25578)

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. 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

9 participants