Skip to content

Will need runtime check to cast to type (WebElement) -> dynamic #25018

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

Closed
zoechi opened this issue Nov 21, 2015 · 11 comments
Closed

Will need runtime check to cast to type (WebElement) -> dynamic #25018

zoechi opened this issue Nov 21, 2015 · 11 comments
Assignees
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on

Comments

@zoechi
Copy link
Contributor

zoechi commented Nov 21, 2015

  return driver
      .findElements(buttonsSelector)
      .asyncMap((WebElement e) async => {'button': e, 'text': await e.text})
             // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      .where((Map m) => m['text'] == text)
      .map((Map m) => m['button'])
      .first as WebElement;

Warning:(589, 17) (WebElement e) async => {'button' : e, 'text' : await e.text} ((WebElement) → Future<Map<dynamic, dynamic>>) will need runtime check to cast to type (WebElement) → dynamic

I don't get this warning here

final List<WebElement> cells =
      await (await driver
              .findElements(firstColumnSelector)
              .asyncMap(
                  (WebElement e) async => {'element': e, 'text': await e.text})
              .where((Map item) => item['text'] == taskTitle))
          .map((Map item) => item['element'])
          .toList() as List<WebElement>;

strong-mode is enabled

I think either the first shouldn't produce a warning or the second should.
If the warning is correct, is a typedef the only way to silence this?

@leafpetersen
Copy link
Member

As best I can see from the example, neither of those should produce a warning. I can't actually get the error to reproduce with some small test examples using analyzer version 1.14.0-dev.1.0. Do you have a small reproduction case? If not, can you tell me any more about the code? What's the return type of the function, and what is the return type of the call to findElements? If you declare the lambda as a local function instead (to give it an explicit type) do you still get the error?

@zoechi
Copy link
Contributor Author

zoechi commented Nov 24, 2015

I tried to create a simple project to reproduce it.
https://github.com/bwu-dart-playground/bug_repo/blob/master/sdk25018/test/e03_editing_test.dart#L79
I get a different warning now though. Now it looks like #24507

@zoechi
Copy link
Contributor Author

zoechi commented Nov 24, 2015

I just saw, that I forgot to add the .analysis_options file (with strong-mode enabled`). Adding it didn't change anything though.

@leafpetersen
Copy link
Member

Thanks, that's very helpful! I think this is a name confusion. It looks to me like you are defining a class named WebElement in that test file, and then trying to use that in a context that is expecting a wd.WebElement . If I change the anonymous callback in your example to have wd.WebElement as the argument type then the error goes away.

@bwilkerson The error message here is very confusing, and for once I don't think it's strong mode's fault. :) Even with the regular analyzer this is the error message I see:

warning] The argument type '(WebElement) → Future' cannot be assigned to the parameter type '(WebElement) → dynamic' (/Users/leafp/tmp/repo/bug_repo-master/sdk25018/test/e03_editing_test.dart, line 31, col 17)
1 warning found.

It would be awesome if we could disambiguate the two WebElement names in the error message.

Related to #23492

@zoechi
Copy link
Contributor Author

zoechi commented Nov 24, 2015

Sorry, I made a mistake when I tried to build the repo case.

WebElement should look something like

class WebElement implements wd.WebElement {
  Future<String> text;

  noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}

but with this change I don't get any warning.
Back to square one :-/
I'll try to find a better repo.

@leafpetersen
Copy link
Member

Ah - in that case, this is a strong mode error, and it's correct (but it's confusing because of the lack of name disambiguation).

In strong mode, function types are contra-variant on their arguments.

For WebElement -> Future<...> to be a subtype of wd.WebElement -> dynamic we need to have that wd.WebElement is a subtype of WebElement, which is the reverse of what we actually have.

You either need to make the lambda take a wd.WebElement and cast it to WebElement (if you're sure that all of the elements are WebElements), or else you need to refine the type of the iterable over which you are mapping (which probably requires generic methods to do properly).

@zoechi
Copy link
Contributor Author

zoechi commented Nov 24, 2015

Thanks a lot!

@zoechi
Copy link
Contributor Author

zoechi commented Nov 24, 2015

I agree, some kind of disambiguation would be great and if it is only some random code as prefix that differs, to indicate the types are different.

I use quite the same code in 5 locations but only in this one I typed the driver argument with the base class type wd.WebDriver instead of ExtendedWebDriver. That was the mistake. Actually this is why I like strong mode and lints.

Thanks again for your support.

@leafpetersen
Copy link
Member

No worries, thanks for trying things out. I'll close this bug: @bwilkerson is there an existing bug that covers the name disambiguation, or should I open one?

@bwilkerson
Copy link
Member

I'm not aware of any issue. I know that we do have code to perform the disambiguation (ErrorReporter.reportTypeErrorForNode), so it must just not be getting used in these cases.

@leafpetersen
Copy link
Member

Submitted #25036 for the name disambiguation.

@kevmoo kevmoo added P2 A bug or feature request we're likely to work on and removed Priority-Medium labels Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

5 participants