Skip to content

#3182. Add test for augmenting function-typed variable#3558

Open
sgrekhov wants to merge 2 commits intodart-lang:masterfrom
sgrekhov:co19-3182-68
Open

#3182. Add test for augmenting function-typed variable#3558
sgrekhov wants to merge 2 commits intodart-lang:masterfrom
sgrekhov:co19-3182-68

Conversation

@sgrekhov
Copy link
Copy Markdown
Contributor

No description provided.

@sgrekhov sgrekhov requested a review from eernstg January 22, 2026 13:40
Copy link
Copy Markdown
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the fine-grained level, this is all fine.

However, I'm worried about the depth of unfolding which is done in these tests. A large part of the testing is concerned with a situation where the tools should report an error if two specific type annotations (which may be explicit or implicit) denote the same type or not.

This probably amounts to an invocation of DartType.isSameType, or whatever the name is.

My worries are based on the suspicion that we have lots of other tests diving into the details of that isSameType query, and in the context of augmentations we don't have to test this query, we just have to make sure it will be discovered if the isSameType query is omitted by accident from the implementation of augmentations.

With this in mind, do you think all these variants of different function types will actually test anything which is specific for augmentations? Otherwise I think we should just have a few different cases where the point is that the types are not the same, and then we'd rely on other (older) tests to handle all the cases of isSameType.

@sgrekhov
Copy link
Copy Markdown
Contributor Author

It’s a hard question for me. I don’t have a strong opinion here. I agree that these tests also test static types of function-type literals. However, I checked and found that we don’t have detailed tests for the static type of function literals - only a few tests with a copyright year of 2011. I’m sure those old tests are incomplete, and these new augmentation tests partially fill this gap for now.

Also, some of the tests in this PR check omitted types of function literal parameters and return types. With the augmentation feature, this is probably the easiest way to verify that the static type of Function() is dynamic Function(), not void Function() or Object? Function(). expectStaticType() doesn’t work in this case, so we have to use something like:

Function() foo = () {};
Never Function() bar = throw 42;

main() {
  if (1 > 2) {
    foo().whatever;
    bar().whatever; // But Never also works
  }
}

In the case of augmentations for Function() foo = ..., only augment dynamic Function() foo... should work. So I’d prefer to preserve these tests. At least there are only 13 tests in this PR, not 130, and it won’t create excessive load on try jobs even if these tests also test something that is already covered by other tests.

@sgrekhov sgrekhov requested a review from eernstg January 23, 2026 09:27
@eernstg
Copy link
Copy Markdown
Member

eernstg commented Jan 26, 2026

I think we'll have to mark this PR as blocked until we have a decision in dart-lang/language#4621.

@sgrekhov sgrekhov added the status-blocked Blocked from making progress by another (referenced) issue label Jan 27, 2026
@sgrekhov
Copy link
Copy Markdown
Contributor Author

Blocked by dart-lang/language#4621

TODO (sgrekhov): add tests when function is augmented by async function and vice versa.

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

Labels

status-blocked Blocked from making progress by another (referenced) issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants