#11772 Correct deferred annotations

[glyph]

Scope and purpose

Fixes #11772
Fixes #11753

Add a few words about why this PR is needed and what is its scope.
If the associate ticket(s) fully explain the need you can just refer to it/them.

Add any comments about trade-offs (if any) made in this PR and the reasoning behind them.

Add mentions of things that are not covered here and are planed to be done in separate PRs.

Contributor Checklist:

This process applies to all pull requests - no matter how small.
Have a look at our developer documentation before submitting your Pull Request.

Below is a non-exhaustive list (as a reminder):

• The title of the PR should describe the changes and starts with the associated issue number, like “#9782 Remove twisted.news. #1234 Brief description”.
• A release notes news fragment file was create in src/twisted/newsfragments/ (see: Release notes fragments docs.)
• The automated tests were updated.
• Once all checks are green, request a review by leaving a comment that contains exactly the string please review.
  Our bot will trigger the review process, by applying the pending review label
  and requesting a review from the Twisted dev team.

[glyph]

Note that this is stacked on top of #11767 since it requires some newer mypy features.

[glyph]

It's also stacked on top of #11754 but it supersedes that PR since there are a lot of follow-on changes from there

[glyph]

please review

[glyph]

The test failure is a result of #11843 and should be addressed by its merge.

[glyph]

please review

[glyph]

'comment' reviews really should not take it out of the review queue :)

[glyph]

The test failure is a result of #11843 and should be addressed by its merge.

And, indeed, it was.

[adiroiban]

'comment' reviews really should not take it out of the review queue :)

True... but this is how GitHub works ... if you request a review from a team, as soon as a team member makes a comment on the PR, the team is removed.

This is one reason why we have the "needs-review" label, to have better control.

In the future I will try to manually re-request a review from the team.

And in the future, maybe the robot can do this for us... if a only a "comment" review is added, re-add the team in the list of reviewers.

[ihaywood3]

this is a big PR but I can't see any problems with it. I note that two core developers have already reviewed most of the changes.
My only question is about _resolver.py - I assume these changes are to please mypy. Maybe a comment in the code or in the newsfragment?

[glyph]

this is a big PR but I can't see any problems with it. I note that two core developers have already reviewed most of the changes.

I will interpret this as an "approve" review, make the changes and land.

[glyph]

My only question is about _resolver.py - I assume these changes are to please mypy. Maybe a comment in the code or in the newsfragment?

Ah, reviewing this now I can see why you might be confused, but the change doesn't modify behavior (and thus doesn't belong in the news fragment) and this style isn't sufficiently interesting in its own context to warrant an explanation. The change could perhaps use one though, so I'll explain here:

Deferreds are an unusual kind of object because their type changes when you call methods on them. Technically speaking, this is impossible for a type-checker like Mypy to actually validate. Like, consider this:

def mutate(d: Deferred[int]) -> Deferred[str]:
    d.addCallback(str)
    return d
...

x: Deferred[int] = deferredIntReturner()
mutate(x)
mutate(x)

There's no way that mypy could see that the second call to mutate is invalid. It thinks that x is, and forever remains, Deferred[int]. If we were designing Deferred today, it would almost certainly return a new value from addCallback for this reason.

However, idiomatically, it's reasonable to work this way and callers sharing Deferred objects is not a problem that practically arises very often. So addCallback((int) -> str) can literally return self but be annotated to say it returns Deferred[str], and everyone can participate in this fiction.

In order to make the fiction of enforced-single-use-Deferred and Mypy's implementation match up, we have to use the return values of the addCallback family of methods rather than simply call methods repeatedly on the same object. The change to resolver.py is adopting this style to allow for type checking where previously none was happening because we were getting implicit Anys sneaking in to various places in the signature. More correct annotations now result in the need to actually make this check properly.