Add an Error / Exception / Warning when contextlib.suppress() is entered with no specified exception(s) to suppress

[cooperlees]

BPO	46819
Nosy	@ncoghlan, @zware, @serhiy-storchaka, @1st1, @cooperlees

^{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-02-22.18:11:19.716>
created_at = <Date 2022-02-21.17:26:24.777>
labels = ['invalid', 'library', '3.11']
title = 'Add an Error / Exception / Warning when contextlib.suppress() is entered with no specified exception(s) to suppress'
updated_at = <Date 2022-02-22.18:30:01.753>
user = 'https://github.com/cooperlees'

bugs.python.org fields:

activity = <Date 2022-02-22.18:30:01.753>
actor = 'cooperlees'
assignee = 'none'
closed = True
closed_date = <Date 2022-02-22.18:11:19.716>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2022-02-21.17:26:24.777>
creator = 'cooperlees'
dependencies = []
files = []
hgrepos = []
issue_num = 46819
keywords = []
message_count = 10.0
messages = ['413663', '413667', '413681', '413724', '413728', '413731', '413732', '413733', '413736', '413737']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'zach.ware', 'serhiy.storchaka', 'yselivanov', 'cooperlees']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46819'
versions = ['Python 3.11']

[cooperlees]

Today if you enter a contextlib.suppress() context and specify no exceptions there is no error or warning (I didn't check pywarnings to be fair). Isn't this a useless context then? If not, please explain why and close.

If it is, I'd love to discuss possibly raising a new NoSupressionError or at least a warning to let people know they executing an unneeded context.

Example code that 3.11 does not error on:

cooper@home1:~$ python3.11
Python 3.11.0a5+ (main, Feb 21 2022, 08:52:10) [GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import contextlib
>>> with contextlib.suppress():
...   print("Foo")
...
Foo

This was reported to flake8-bugbear and if this is not accepted I may accept adding this to the linter. But feel this could be fixable in cpython itself.

[zware]

I'm -1 on this suggestion; consider the following:

exceptions_to_suppress = []
if some_condition:
    exceptions_to_suppress.append(ValueError)

with contextlib.suppress(*exceptions_to_suppress):
    do_a_thing()

This seems a reasonable case to support and would require quite some gymnastics to continue supporting while also warning on an empty set.

[cooperlees]

Totally agree with your example use case. There you have a chance for it being useful under certain conditions. In that example there is a passed argument. In my example there is no passed argument. Thus, I believe that this will generally always be developer error, again, unless I'm missing something here.

My main suggestion here is to just error/warn when no argument at all is passed to contextlib.suppress and this this context is never a chance of it being useful. If someone passes None or an empty Sequence (or anything non truthy) I propose we stay behaving the same as today.

Please feel free to edit the title if that's not clear enough etc.

[zware]

But suppress takes varargs; from suppress's perspective, there is no difference between suppress() and l=[];suppress(*l). There would be a difference at the AST level, but trying to find it within suppress to issue a warning seems unfeasible. Certainly possible for a linter, though :)

[serhiy-storchaka]

I concur with Zachary.

Note that suppress without arguments corresponds to "except" and isinstance() with empty tuple.

[cooperlees]

Ok thanks. Looks like the warning in flake8-bugbear is the right place then, unfortunately.

And just to be sure:

Note that suppress without arguments corresponds to "except" and isinstance() with empty tuple.

Are you saying that contextlib.suppress() should effectively except BaseException (cause this is not the behavior from my tests) and suppress all or suppress nothing? I believe the empty tuple makes it except nothing?

cooper@home1:~$ python3.11
Python 3.11.0a5+ (main, Feb 22 2022, 08:51:50) [GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import contextlib
>>> with contextlib.suppress():
...   raise ValueError("I raise ...")
...
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ValueError: I raise ...
>>>

[zware]

>>> try:
...     raise ValueError("I raise...")
... except ():
...     pass
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ValueError: I raise...

[serhiy-storchaka]

No, I say that

    with suppress():
        ...

is equivalent to

try:
    ...
except ():
    pass

or

try:
    ...
except BaseException as err:
    if not isinstance(err, ()):
        raise

If you want to suppress all exceptions (it is not very clever idea), use suppress(BaseException).

[cooperlees]

I would never want to do that ... I understand it's bad. Just questioning things before adding lints so we put things in the right places and more importantly making sure I understand.

Thanks for the discussion all.

[cooperlees]

FWIW - Will be looking to add to flake8-bugbear here: PyCQA/flake8-bugbear#222