`importlib.metadata.DeprecatedList` leaks `method_name` variable

[sobolevn]

BPO	46546
Nosy	@brettcannon, @jaraco, @sobolevn
PRs	bpo-46546: remove method_name var leak from importlib.DeprecatedList #30956

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2022-01-27.14:52:17.428>
created_at = <Date 2022-01-27.11:24:52.800>
labels = ['type-bug', 'library', '3.11']
title = '`importlib.metadata.DeprecatedList` leaks `method_name` variable'
updated_at = <Date 2022-01-27.14:52:17.427>
user = 'https://github.com/sobolevn'

bugs.python.org fields:

activity = <Date 2022-01-27.14:52:17.427>
actor = 'sobolevn'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-27.14:52:17.428>
closer = 'sobolevn'
components = ['Library (Lib)']
creation = <Date 2022-01-27.11:24:52.800>
creator = 'sobolevn'
dependencies = []
files = []
hgrepos = []
issue_num = 46546
keywords = ['patch']
message_count = 3.0
messages = ['411855', '411882', '411885']
nosy_count = 3.0
nosy_names = ['brett.cannon', 'jaraco', 'sobolevn']
pr_nums = ['30956']
priority = 'normal'
resolution = 'third party'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue46546'
versions = ['Python 3.11']

[sobolevn]

Right now in DeprecatedList there's a possibly unwated name leak of method_name here:

cpython/Lib/importlib/metadata/__init__.py

Lines 295 to 308 in 08c0ed2

	for method_name in [
	'__setitem__',
	'__delitem__',
	'append',
	'reverse',
	'extend',
	'pop',
	'remove',
	'__iadd__',
	'insert',
	'sort',
	]:
	locals()[method_name] = _wrap_deprecated_method(method_name)

for method_name in [
        '__setitem__',
        '__delitem__',
        'append',
        'reverse',
        'extend',
        'pop',
        'remove',
        '__iadd__',
        'insert',
        'sort',
    ]:
        locals()[method_name] = _wrap_deprecated_method(method_name)

Example:

>>> import importlib
>>> import importlib.metadata
>>> importlib.metadata.DeprecatedList.method_name
'sort'

Right now method_name is unused, undocumented, and untested.

My proposal is to add del method_name after the for loop, so it won't leak.

[jaraco]

Thanks Nikita for the report. I agree, it's inelegant that this property leaks. Because this code is synced with importlib_metadata, I'm not sure it's worth the effort of changing it here. Your change here implies a cherry-pick to the backport and updating its changelog there too.

I'd be more inclined to accept a change like this on importlib_metadata, which has a faster iteration and whose changes get naturally incorporated into CPython periodically.

What's the harm in leaving this attribute on a class that is itself standing in for deprecated behavior and slated for removal?

[sobolevn]

Thanks, Jason! I've submitted python/importlib_metadata#365

What's the harm in leaving this attribute on a class that is itself standing in for deprecated behavior and slated for removal?

I think it does not do much harm, but there are several things that it affects:

1. Typing, we have to annotate it in typeshed, or write a custom ignore rule for our test suite
2. It is listed in help():

 |  ----------------------------------------------------------------------
 |  Data and other attributes defined here:
 |  
 |  __hash__ = None
 |  
 |  method_name = 'sort'
 |  
 |  ----------------------------------------------------------------------

3. It is also in dir(importlib.metadata.DeprecatedList)

So, even if we remove this class in some time, we still should care about it until the removal :)

I am going to close this as "third party".