Sdk: Every edit makes an analyzer warning appear, restarting and it isn't shown anymore

Created on 19 Jul 2016 · 14Comments · Source: dart-lang/sdk

  void fadeIn(int milliseconds) {
    style.display = 'block';
    style.transition = 'opacity ${milliseconds}ms ease-in';
    new Future<Null>(() {
      style.opacity = '1';
    });
    onMouseLeave.first.then/*<dom.MouseEvent>*/((e) {
      fadeOut(_options.fadeSpeed);
    });
  }

Just adding a space after ...Speed); shows this error and only goes away when I "Reanalyze Dart Sources" or "Restart Dart Analyzis Server" (in WebStorm "Dart Analysis" window).

P1 analyzer-strong-mode area-analyzer type-bug

Source

zoechi

All 14 comments

analyzer:
  strong-mode:
    implicit-casts: false
    implicit-dynamic: false
  exclude:
  - lib/remote_model/**

linter:
  rules:
    - always_declare_return_types
#    - always_specify_types
    - annotate_overrides
#    - avoid_as
    - avoid_init_to_null
    - avoid_empty_else
    - avoid_return_types_on_setters
    - await_only_futures
    - camel_case_types
    - comment_references
    - constant_identifier_names
    - control_flow_in_finally
    - empty_constructor_bodies
    - hash_and_equals
    - implementation_imports
    - iterable_contains_unrelated_type
    - library_names
    - library_prefixes
    - non_constant_identifier_names
    - one_member_abstracts
    - overridden_fields
    - package_api_docs
    - package_prefixed_library_names
    - prefer_is_not_empty
#    - public_member_api_docs
    - slash_for_doc_comments
#    - sort_constructors_first
    - sort_unnamed_constructors_first
    - super_goes_last
    - test_types_in_equals
    - throw_in_finally
    - type_annotate_public_apis
    - type_init_formals
#    - unnecessary_brace_in_string_interp
    - unnecessary_getters_setters # https://groups.google.com/a/dartlang.org/forum/#!topic/reviews/Q8D-mKoUVQA
    - unrelated_type_equality_checks
    - package_names

zoechi on 19 Jul 2016

@leafpetersen Can you confirm that this error is appropriate in strong mode? Either way, adding a space should not toggle whether it appears.

bwilkerson on 19 Jul 2016

I started by adding /*<dom.MouseEvent>*/ to get rid of the error that I got after disabling implicit-dynamic, but the error only vanished after I restarted the analyzer, but then came back after any edit.

zoechi on 19 Jul 2016

@leafpetersen Can you confirm that this error is appropriate in strong mode?

yes, the error is appropriate before adding type args (clarification: only when using no-implicit-dynamic, as appears to be the case here).

I probably would fix it with <Object> though. The <S> is the return type of the future (the parameter type is the T in Future<T>, i.e. given Future<T> the signature of then is <S>(T) -> (S | Future<S>)).

Either way, adding a space should not toggle whether it appears.

yeah. I haven't seen it come up in quite a while, but our technique for injecting generic method comments might not be triggering correct invalidation. That looks like the most likely possibility because the generic method comment isn't highlighted correctly. If it was, you'd see dom.MouseEvent colored like a type, not like a comment body.

One way I've seen it happen is if you write /*dom.MouseEvent*/ then go back and add the <, the invalidation doesn't trigger because Analyzer thinks you simply edited a comment. It doesn't know the comment became semantically meaningful.

That's my best guess about what's going on here.

jmesserly on 19 Jul 2016

How far off are we from generic method syntax in the VM? Then we can get rid of these hacky comments. @floitschG any updates on that front?

jmesserly on 19 Jul 2016

Just to clarify - that error is a strong mode error (when implicit-any is false), but it should never be shown for that code. Once the /*dom.MouseEvent*/ has been added, that error should never show up again. I believe that @zoechi is seeing this error _after_ having added the type argument, that it is showing up on edits in other parts of the code (outside of the comment). Is that correct? If so, I think this is different than the issue we've seen before with incorrect invalidation when editing inside of comments.

leafpetersen on 19 Jul 2016

It is definitely an edit outside of the comment. An edit anywhere in the same file results in reappearing of these warnings (I have lots of similar code in this file)

zoechi on 19 Jul 2016

I can reproduce this. With current bleeding edge, running the analyzer on the following code initially produces no errors, but if I put the cursor between "hello") and ; and enter a space, I get the "missing type arguments" error:

import 'dart:async';

void fadeIn(int milliseconds) {
  Future<int> f;
  f.then/*<String>*/((e) {print("hello");});
}

leafpetersen on 19 Jul 2016

Two guesses:

something how about scanner/parser injects the token stream + whatever incremental parsing Analyzer does on edits results in corrupted AST
something in generic method inference works incorrectly when re-run, perhaps clobbering the explicit type (but I kind of doubt that, it seems like it would show up a lot more often?)

jmesserly on 19 Jul 2016

@scheglov Can you look into the first possibility above ("something how about scanner/parser injects the token stream + whatever incremental parsing Analyzer does on edits results in corrupted AST")?

bwilkerson on 19 Jul 2016

We just tested out using the real generic method syntax instead of the comment syntax (i.e. setting enableGenericMethods to true) and that seems to make the issue go away, which is suggestive that it might be related to an interaction between incremental parsing and our editing of the token stream to inject the comment syntax into the "real" token stream.

leafpetersen on 19 Jul 2016

https://codereview.chromium.org/2164833003