Skip to content

Fix TypeError in _implements_name when __module__/__name__ is an unbound property - #385

Open
itzzdev09 wants to merge 2 commits into
zopefoundation:masterfrom
itzzdev09:fix-implements-name-property-crash
Open

itzzdev09 wants to merge 2 commits into
zopefoundation:masterfrom
itzzdev09:fix-implements-name-property-crash

Conversation

@itzzdev09

Copy link
Copy Markdown

Fixes #249.

>>> from zope.interface import Interface
>>> from inspect import getmembers
>>> getmembers(type(Interface))
Traceback (most recent call last):
  ...
  File ".../declarations.py", line 373, in _implements_name
    return (getattr(ob, '__module__', '?') or '?') + \
TypeError: unsupported operand type(s) for +: 'property' and 'str'

getObjectSpecification(ob) computes implementedBy(ob.__class__). Since ob here is InterfaceClass, ob.__class__ is _InterfaceMetaClass — the metaclass — so _implements_name ends up called with cls=_InterfaceMetaClass. That metaclass defines __module__ as a property, meant to be computed for its own instances (InterfaceClass/Interface objects). Fetched directly on the metaclass itself rather than through an instance, it stays the raw, unbound property object instead of a string:

>>> getattr(_InterfaceMetaClass, '__module__', '?')
<property object at 0x...>

_implements_name assumes this is always a string (or falls back to '?' for a missing/None value) and blows up trying to concatenate it. Since this is any object's __module__/__name__, not just this specific metaclass, the fix checks the type generally: fall back to '?' when the fetched value isn't a str, alongside the existing missing/None fallback.

Added a targeted unit test with a synthetic metaclass reproducing the exact mechanism (getattr on a metaclass whose __module__ is a property), plus the original getmembers(type(Interface)) repro from the issue, both added to the fallback test classes so they run against the Python implementation (and the C-accelerated one via the existing Test_implementedBy/Test_getObjectSpecification inheritance, when built). Both fail on main; full suite passes (1135, PURE_PYTHON=1 — no C toolchain available here to build/test the optimized path, but _implements_name/implementedByFallback is the only implementation of this logic and is what the C extension already delegates to for this path). flake8 clean.

…und property

Fixes zopefoundation#249.

getObjectSpecification(ob) computes implementedBy(ob.__class__), so
inspect.getmembers(type(Interface)) ends up calling _implements_name with
cls=_InterfaceMetaClass. That metaclass defines __module__ as a property
meant for its own instances; fetched on the metaclass itself it stays an
unbound property object rather than a string, and the '+' concatenation
in _implements_name raised TypeError.

Fall back to '?' when the fetched value isn't a str, same as the
existing fallback for a missing or None value.
@jensens

jensens commented Sep 22, 2026

Copy link
Copy Markdown
Member

Sorry, this is more complex as it seems. I also can not 100% tell if the probably correct side effects are behavior we want in zope.interface.

This needs review form others too.

Further I am not sure if you signed the Contributor License Agreement?

Summary:

  • Overall: Fix valid and verified locally;
  • minor changes needed before merge;
  • open question in 2 needs decision;
  • root cause in 3 should to be addressed, but IMO OK as separate PR;
  • rest optional.

I reviewed with AI Assist, see below.

Overview

  • Fix is correct. Both new tests fail on master and pass here, C extension and PURE_PYTHON=1. Full suite green in both modes (1371 tests). CI 69/69.
  • 1: missing CHANGES.rst entry.
  • 2: the fix changes import-time declaration state, undocumented and untested.
  • 3–4: root cause left in place, spec name — follow-up material.
  • 5–7: tests, style, one observation.

Before merge

1. No CHANGES.rst entry — every fix gets one under 8.7 (unreleased) with the issue link, see the existing entries.

2. The fix changes what import zope.interface leaves behind — _wire() L1171, implementedByFallback L509–L523

  • On master, classImplements(InterfaceClass, IInterface) in _wire() hits the inspect.getmembers(InterfaceClass) causes TypeError #249 TypeError inside ClassProvides(InterfaceClass, _InterfaceMetaClass); the broad except TypeError at L520 swallows it and puts InterfaceClass in BuiltinImplementationSpecifications, with __implemented__ and __providedBy__ already set but no __provides__.
  • With this PR the call completes. Verified, C and pure:
    • InterfaceClass in BuiltinImplementationSpecifications: True → False
    • InterfaceClass.__provides__: AttributeError → directlyProvides(InterfaceClass)
    • providedBy(InterfaceClass): TypeError → []
    • _InterfaceMetaClass gains __implemented__ / __providedBy__ / __provides__
    • Interface.__provides__ (any interface object): AttributeError → classImplements(InterfaceClass, IInterface)
  • This is the state every other class has, so it looks like the intended one. Unchanged, verified: providedBy(IFoo), IInterface.providedBy(IFoo), directlyProvidedBy(IFoo), alsoProvides(IFoo, IMark), pickling of interfaces and of providedBy(IFoo). The last bullet is caught by directlyProvidedBy L981, which already expects an Implements there.
  • Ask: one sentence in the changelog and PR description, and one assertion in the test (e.g. assertNotIn(InterfaceClass, BuiltinImplementationSpecifications) or assertIn('__provides__', InterfaceClass.__dict__)), so the import-time effect is stated and guarded.

Follow-up material, not for this PR

3. Root cause stays — L509–L523

  • The try wraps the whole ClassProvides construction, so any TypeError raised while computing implementedBy(metaclass) is taken as "attribute assignment failed, builtin type" and the class is misregistered silently. That is how inspect.getmembers(InterfaceClass) causes TypeError #249 hid at import for years. This PR removes one trigger, not the trap.
  • Narrowing the try to the two setattr calls, or building the ClassProvides outside it, would surface the next such error at import. Happy to open an issue.

4. implementedBy(_InterfaceMetaClass).__name__ is now '?._InterfaceMetaClass' — L380–L387

  • The real module name is not recoverable from the metaclass: _InterfaceMetaClass.__dict__['__module__'] is the property, nothing stores the string.
  • __name__ is the sort key for Implements (_compare), so a special case like the InterfaceBase one above it would be the way to get 'zope.interface.interface._InterfaceMetaClass'. Only one such metaclass exists → cosmetic.

Tests and style

5. test_module_or_name_is_unbound_property asserts nothing about the result — L582

  • assertEqual(self._callFUT(Meta).__name__, '?.Meta') would pin the behaviour.
  • The __name__ branch at L384–L385 has no trigger for classes (type.__name__ is always str) and no test. Either drop it or cover it.

6. test_getmembers_on_interface_metaclass never calls _callFUT() — L2143

  • Test_getObjectSpecificationFallback and Test_getObjectSpecification run the identical integration check; neither exercises the target function.
  • With the fix, getmembers(type(Interface)) no longer reaches implementedBy(_InterfaceMetaClass) at all: InterfaceClass.__provides__ exists since import and getObjectSpecification returns it. The test still discriminates (it fails on master through the broken import-time state), but the comment describes a path that does not run.
  • Suggestion: self._callFUT(type(Interface)) plus an assertion on the result here; keep the getmembers repro as an integration test with an accurate comment.

7. Two identical guard blocks — L380–L387

  • Same fetch → or '?' → isinstance check twice. One small helper or one expression, e.g. v if isinstance(v, str) and v else '?'.

Observation, no action: pickle.dumps(providedBy(InterfaceClass)) raises TypeError: module name must be a string (whichmodule hits the same property). Not a regression, the object could not be built before.


Transparency: AI-assisted review

  • Model: Claude Fable 5.1 (Anthropic), run through Claude Code.
  • Method: automated /code-review pass in a sub-agent (diff + surrounding source at 2714fde), then a verification pass in the main session.
  • Environment: Linux x86_64, CPython 3.14.3, C extension built in place and PURE_PYTHON=1. Baseline: master dd65a10.
  • Executed: new tests on master (4 errors, both modes) and on the PR (4 ok, both modes); full suite on the PR (both modes); the state and behaviour comparisons in 2 with a probe script on master and PR (both modes); '?._InterfaceMetaClass' and the pickling observation.
  • Code reading only: 3, 5–7.
  • Proposed changes not implemented or tested.
  • Read and approved by me before posting.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

inspect.getmembers(InterfaceClass) causes TypeError

2 participants