Skip to content

Overloads defined for None and object get inferred as Any #3757

Closed

Description

@benkuhn

(This came up while playing with descriptors, where the first parameter of __get__ can be either None or an arbitrary object and you want different return types in the two cases.)

Example reproduction using mypy --strict-optional on 0.520:

from typing import *

@overload
def a_or_1(obj: None) -> str:
    ...

@overload
def a_or_1(obj: object) -> int:
    # If you change the type of `obj` to e.g. AnyStr, the bug goes away
    ...

def a_or_1(obj):
    if obj is None:
        return 'a'
    return 1

reveal_type(a_or_1('a string'))   # correctly revealed as 'builtins.int'
reveal_type(a_or_1(None))         # revealed as type Any (should be 'builtins.str')

Activity

  1. ilinum commented on Jul 22, 2017

    @ilinum
    Collaborator

    Thanks for reporting the issue!

    This is a duplicate of #3256.

  2. benkuhn commented on Jul 22, 2017

    @benkuhn
    ContributorAuthor

    Hmm, really? Without much knowledge of the underlying machinery, that seems surprising to me--#3256 is only a problem without strict-optional, and this test case only makes sense with strict-optional enabled. Also, this seems to have to do with some kind of special-casing of object, and object isn't mentioned at all in #3256. #3256 seems like a design problem for how to cope with None causing type overlaps, whereas this seems like it should straightforwardly just work (the testOverloadWithNone test case in the current unit tests fails if you replace the int annotation with object).

    Sorry if there's some deeper reason I'm missing as to why they're actually the same! I'm interested in knowing because I was planning to try to fix this today :)

  3. benkuhn commented on Jul 22, 2017

    @benkuhn
    ContributorAuthor

    Hmm, ok, it appears to actually be intentional that None and object overlap in overloads. There's a test case testNoneMatchesObjectInOverload in check-optional.test, introduced in #1836 to solve #1766. The code that handles it is a special case in checkexpr.overload_arg_similarity:2653:

        if isinstance(actual, NoneTyp):
            if not experiments.STRICT_OPTIONAL:
                # NoneTyp matches anything if we're not doing strict Optional checking
                return 2
            else:
                # NoneType is a subtype of object
                if isinstance(formal, Instance) and formal.type.fullname() == "builtins.object":
                    return 2

    I made a branch changing the return 2 to return 1 (meaning the types are compatible using type promotions), which fixes this issue and passes all other tests. But maybe that's an abuse of the meaning of "similarity" there? Let me know if this, or something similar, seems like a good direction.

  4. benkuhn commented on Jul 23, 2017

    @benkuhn
    ContributorAuthor

    Hey @ilinum or anyone else, wondering whether you have feedback on whether this is truly a duplicate, or whether my proposal (or something like it) seems like a good resolution?

  5. ilinum commented on Jul 24, 2017

    @ilinum
    Collaborator

    Sorry, I was away for the weekend, so didn't see your comments until just now.

    Yep, you're right this is not a duplicate of the #3256. I missed the fact that your example fails with --strict-optional. Sorry!

    Your fix seems reasonable, I think it could work. It's probably worth sending a PR and seeing what other say.

  6. reopened this on Jul 24, 2017
  7. ilevkivskyi commented on May 20, 2018

    @ilevkivskyi
    Member

    Oops, wrong place, deleting the comment.

  8. Michael0x2a commented on Jun 15, 2018

    @Michael0x2a
    Collaborator

    Update: The original example still won't typecheck (by design), but we recently added code to master that special-cases and allows this kind of overload for specifically descriptors -- see #5093 for examples.

    So, I think we can probably close this now? Feel free to reopen if anybody disagrees.

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions