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

Apparent regression in always_specify_types with typedef #3275

Closed
stuartmorgan opened this issue Mar 8, 2022 · 4 comments · Fixed by #3279
Closed

Apparent regression in always_specify_types with typedef #3275

stuartmorgan opened this issue Mar 8, 2022 · 4 comments · Fixed by #3279
Assignees

Comments

@stuartmorgan
Copy link

stuartmorgan commented Mar 8, 2022

Describe the issue
#2692 seems to have (partially?) regressed recently on Flutter master. We have an auto-roller that rolls flutter/flutter into flutter/packages, and it started failing sometime in the last week or so (I still need to pinpoint the exact regression point) due to new issues with the analyze step we run in CI (example failure).

The failures are:

   info - lib/src/dart/binary.dart:354:12 - Specify type annotations. - always_specify_types
   info - lib/src/dart/binary.dart:401:16 - Specify type annotations. - always_specify_types
   info - lib/src/dart/text.dart:2200:32 - Specify type annotations. - always_specify_types
   info - lib/src/dart/text.dart:2223:33 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/content.dart:198:66 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/runtime.dart:555:25 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/runtime.dart:666:16 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/runtime.dart:668:16 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/runtime.dart:955:29 - Specify type annotations. - always_specify_types
   info - lib/src/flutter/runtime.dart:997:12 - Specify type annotations. - always_specify_types

An example failing line is:

final DynamicMap results = DynamicMap();

where the failure shows as being the DynamicMap() part. But DynamicMap is

typedef DynamicMap = Map<String, Object?>;

Interestingly it doesn't have an issue with the declaration of results, just the constructor call.

Other failures are DynamicMap.fromEntries and DynamicList.generate (where DynamicList is a similar typedef for List<Object?>).

To Reproduce

Minimal example:

typedef Foo = Map<String, Object>;
final Foo foo = Foo();

Expected behavior
No always_specify_types lint.

Additional context
Changing the second line to

final Foo foo = Map<String, Object>();

makes the lint message go away, so it's definitely the typedef.

@stuartmorgan
Copy link
Author

It looks like

That puts the regression range for the engine between 6576bd428ae1f9556924d06211b77a540b5a3d5a and 1875f0ad3f54c8cc12ffbe20c05d54a01781e564, which points to flutter/engine#31864

https://dart.googlesource.com/sdk.git/+/1cdfaa85878a42444e026b823fdb356c6fd35612 sounds suspicious in that very small range. @stereotype441 @scheglov

@stereotype441
Copy link
Member

That commit does indeed look suspicious! Especially considering the suspicious nature of its author 😃
@pq and I are looking now.

@stereotype441
Copy link
Member

Ok, I think what has happened is that in the example:

typedef Foo = Map<String, Object>;
final Foo foo = Foo();

when visiting the NamedType for the Foo Foo(), the resolver is no longer recording that the type Map<String, Object> came from expanding the type alias Foo (this should be recorded in DartType.alias). As a consequence, the linter thinks that the type being referenced was Map (which requires type arguments), and so since no type arguments were provided, it issues an error.

I'll continue the investigation from here.

@pq
Copy link
Member

pq commented Mar 8, 2022

Thanks for the great repro @stuartmorgan! This case was previously untested. (Test added w/ #3277.)

stereotype441 added a commit that referenced this issue Mar 8, 2022
In some circumstances, the analyzer doesn't reliably set
DartType.alias to point to the type alias that led to a type.  (This
is the ultimate cause of #3275).  I'm currently researching whether
it's possible to improve the analyzer behavior in this regard.  In the
meantime, we can fix #3275 by changing the linter to grab the type
alias element directly from NamedType.name.staticElement.

Fixes #3275.
@pq pq closed this as completed in #3279 Mar 8, 2022
pq pushed a commit that referenced this issue Mar 8, 2022
In some circumstances, the analyzer doesn't reliably set
DartType.alias to point to the type alias that led to a type.  (This
is the ultimate cause of #3275).  I'm currently researching whether
it's possible to improve the analyzer behavior in this regard.  In the
meantime, we can fix #3275 by changing the linter to grab the type
alias element directly from NamedType.name.staticElement.

Fixes #3275.
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Aug 23, 2023
…ng/linter#3279)

In some circumstances, the analyzer doesn't reliably set
DartType.alias to point to the type alias that led to a type.  (This
is the ultimate cause of dart-lang/linter#3275).  I'm currently researching whether
it's possible to improve the analyzer behavior in this regard.  In the
meantime, we can fix dart-lang/linter#3275 by changing the linter to grab the type
alias element directly from NamedType.name.staticElement.

Fixes dart-lang/linter#3275.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants