stubtest: adjust symtable logic

[hauntsaninja]

Fixes python/typeshed#11318

[AlexWaygood]

Nice! Not 100% sure about relying on __module__ so much (see my comment below), but the tweak to the symtable logic is definitely an improvement

[AlexWaygood]

It looks like this PR causes stubtest to start emitting 5 new errors when run on typeshed's stdlib stubs:

error: json.encoder.encode_basestring is not present in stub
Stub: in file stdlib\json\encoder.pyi
MISSING
Runtime:
<built-in function encode_basestring>

error: json.encoder.encode_basestring_ascii is not present in stub
Stub: in file stdlib\json\encoder.pyi
MISSING
Runtime:
<built-in function encode_basestring_ascii>

error: ssl.socket_error is not present in stub
Stub: in file stdlib\ssl.pyi
MISSING
Runtime:
<class 'OSError'>

error: uuid.bytes_ is not present in stub
Stub: in file stdlib\uuid.pyi
MISSING
Runtime:
<class 'bytes'>

error: uuid.int_ is not present in stub
Stub: in file stdlib\uuid.pyi
MISSING
Runtime:
<class 'int'>

It looks like these are all functions and classes that originate from other modules but are aliased in these modules using assignments (meaning they pass the sym.is_assigned() check). None of them really look like public-API things that I'd want stubtest to start complaining about if they were missing from the stub. Since the __module__ heuristic is actually pretty reliable for functions and classes, we could possibly do that first for obj if obj is callable, to avoid these new false positives. Something like this:

def _belongs_to_runtime(r: types.ModuleType, attr: str) -> bool:
    """Heuristics to determine whether a name originates from another module."""
    obj = getattr(r, attr)
    if isinstance(obj, types.ModuleType):
        return False

    obj_mod = getattr(obj, "__module__", None)

    symbols = _module_symbol_table(r)
    if symbols is not None:
        # if symtable says we got this from another module, return False
        for sym in symbols.get_symbols():
            if sym.is_imported() and attr == sym.get_name():
                return False

        # for functions and classes, the `__module__` attribute is very reliable;
        # try that next
        if callable(obj) and isinstance(obj_mod, str):
            return bool(obj_mod == r.__name__)

        # if symtable says that we assigned this symbol in the module,
        # return True
        for sym in symbols.get_symbols():
            if sym.is_assigned() and attr == sym.get_name():
                return True

    # The __module__ attribute is pretty unreliable
    # for anything except functions and classes,
    # but it's maybe the best clue we've got at this point
    if isinstance(obj_mod, str):
        return bool(obj_mod == r.__name__)

    return True

[hauntsaninja]

Hm, thanks for running that and apologies that I didn't!

However, I think I'm at a different conclusion... socket_error is public API. The json.encoder also looks desirable... encode_basestring_ascii is the API users should use in this module, not the py_encode_basestring_ascii that typeshed has. The uuid things are not desirable, but if you changed the variable names I wouldn't be able to tell.

[AlexWaygood]

Good points -- I didn't look at those new errors closely enough. I'm persuaded, thanks! A couple optional nits below, but this LGTM:

[hauntsaninja]

I realised we can just lookup the symtable directly :-)

[hauntsaninja]

Thanks for the reviews! (and also for inventing this symtable approach in the first place)

[AlexWaygood]

Nice!