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

Fuzzy arrow / dynamic as bottom is a hint, but should be a warning #31874

Closed
mit-mit opened this Issue Jan 12, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@mit-mit
Member

mit-mit commented Jan 12, 2018

We stumbled over the grpc-dart repo not passing uses_dynamic_as_bottom by coincidence despite that repo running analysis on travis. The issue is that we currently report this as just a hint, which doesn't fail the build:

$ dartanalyzer lib test
Analyzing lib, test...
  hint • A function of type '(Socket) → void' can't be assigned to a variable of type '(dynamic) → FutureOr<void>' at lib/src/client/connection.dart:108:22 • uses_dynamic_as_bottom

Shouldn't this at least be a warning?

@mit-mit

This comment has been minimized.

Show comment
Hide comment
@mit-mit
Member

mit-mit commented Jan 12, 2018

@vsmenon

This comment has been minimized.

Show comment
Hide comment
@vsmenon

vsmenon Jan 12, 2018

Member

This will eventually become an error. Not sure it's worth making it a warning as an intermediate step.

Member

vsmenon commented Jan 12, 2018

This will eventually become an error. Not sure it's worth making it a warning as an intermediate step.

@leafpetersen

This comment has been minimized.

Show comment
Hide comment
@leafpetersen

leafpetersen Jan 12, 2018

Member

It might be worth making this a warning, if we can do so without breaking the internal builds (we're currently working on eliminating the hint internally). Making it a warning earlier might help us avoid accidental regressions. I'll look into it.

In the meantime if you want to protect against this, you can put the following in your analysis options file:

analyzer:
  errors:
    uses_dynamic_as_bottom: warning
Member

leafpetersen commented Jan 12, 2018

It might be worth making this a warning, if we can do so without breaking the internal builds (we're currently working on eliminating the hint internally). Making it a warning earlier might help us avoid accidental regressions. I'll look into it.

In the meantime if you want to protect against this, you can put the following in your analysis options file:

analyzer:
  errors:
    uses_dynamic_as_bottom: warning
@leafpetersen

This comment has been minimized.

Show comment
Hide comment
@leafpetersen

leafpetersen Jan 12, 2018

Member

@srawlins if we promoted this from hint to warning, could we keep it from breaking builds internally?

Member

leafpetersen commented Jan 12, 2018

@srawlins if we promoted this from hint to warning, could we keep it from breaking builds internally?

@srawlins

This comment has been minimized.

Show comment
Hide comment
@srawlins

srawlins Jan 12, 2018

Member

It won't break anything internally.

Member

srawlins commented Jan 12, 2018

It won't break anything internally.

@leafpetersen

This comment has been minimized.

Show comment
Hide comment
@leafpetersen

leafpetersen Jan 16, 2018

Member

I have a CL for this, but have to land a CL making our tests clean first. Mostly done, hoping to land today.

Member

leafpetersen commented Jan 16, 2018

I have a CL for this, but have to land a CL making our tests clean first. Mostly done, hoping to land today.

@leafpetersen

This comment has been minimized.

Show comment
Hide comment
Member

leafpetersen commented Jan 16, 2018

@whesse whesse closed this in fff71ee Jan 17, 2018

@mit-mit mit-mit added this to the 2.0-alpha1 milestone Jan 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment