Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Hugo van Kemenade <[email protected]>
  • Loading branch information
iritkatriel and hugovk authored Nov 16, 2024
1 parent 48b787b commit 7ec364a
Showing 1 changed file with 25 additions and 34 deletions.
59 changes: 25 additions & 34 deletions peps/pep-0765.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ Status: Draft
Type: Standards Track
Created: 15-Nov-2024
Python-Version: 3.14
Post-History: 09-Nov-2024

.. highlight:: rst
Post-History: `09-Nov-2024 <https://discuss.python.org/t/an-analysis-of-return-in-finally-in-the-wild/70633>`__,

Abstract
========
Expand All @@ -26,14 +24,15 @@ Motivation

The semantics of ``return``, ``break`` and ``continue`` in a
finally block are surprising for many developers.
The documentation [1]_ mentions that
The :ref:`documentation <python:tut-cleanup>` mentions that:

- If the ``finally`` clause executes a ``break``, ``continue``
or ``return`` statement, exceptions are not re-raised.

1. If the ``finally`` clause executes a ``break``, ``continue``
or ``return`` statement, exceptions are not re-raised.
2. If a ``finally`` clause includes a ``return`` statement, the
returned value will be the one from the ``finally`` clause’s
``return`` statement, not the value from the ``try`` clause’s
``return`` statement.
- If a ``finally`` clause includes a ``return`` statement, the
returned value will be the one from the ``finally`` clause’s
``return`` statement, not the value from the ``try`` clause’s
``return`` statement.

Both of these behaviours cause confusion, but the first is
particularly dangerous because a swallowed exception is more
Expand All @@ -44,15 +43,19 @@ In 2019, :pep:`601` proposed to change Python to emit a
``SyntaxError``. It was rejected in favour of viewing this
as a programming style issue, to be handled by linters and :pep:`8`.
Indeed, :pep:`8` now recommends not to use control flow statements
in a ``finally`` block, and linters such as pylint [2]_, ruff [3]_
and flake8-bugbear [4]_ flag them as a problem.
in a ``finally`` block, and linters such as
`Pylint <https://pylint.readthedocs.io/en/stable/>`__,
`Ruff <https://docs.astral.sh/ruff/>`__ and
`flake8-bugbear <https://github.com/PyCQA/flake8-bugbear>`__
flag them as a problem.

Rationale
=========

A recent analysis of real world code [5]_ shows that:
A recent `analysis of real world code
<https://github.com/iritkatriel/finally/blob/main/README.md>`_ shows that:

- These features are rare (2 per million LOC in the top 8000 PyPI
- These features are rare (2 per million LOC in the top 8,000 PyPI
packages, 4 per million LOC in a random selection of packages).
This could be thanks to the linters that flag this pattern.
- Most of the usages are incorrect, and introduce unintended
Expand All @@ -63,7 +66,8 @@ A recent analysis of real world code [5]_ shows that:
This new data indicates that it would benefit Python's users if
Python itself moved them away from this harmful feature.

One of the arguments brought up [6]_ in the discussion about :pep:`601`
`One of the arguments brought up
<https://discuss.python.org/t/pep-601-forbid-return-break-continue-breaking-out-of-finally/2239/24>`__
was that language features should be orthogonal, and combine without
context-based restrictions. However, in the meantime :pep:`654` has
been implemented, and it forbids ``return``, ``break`` and ``continue``
Expand All @@ -86,6 +90,7 @@ of it.
This includes the following examples:

.. code-block::
:class: bad
def f():
try:
Expand All @@ -97,11 +102,12 @@ This includes the following examples:
try:
...
finally:
break # (or continue)
break # (or continue)
But excludes these:

.. code-block::
:class: good
try:
...
Expand All @@ -113,7 +119,7 @@ But excludes these:
...
finally:
for x in o:
break # (or continue)
break # (or continue)
CPython will emit a ``SyntaxWarning`` in version 3.14, and we leave
Expand Down Expand Up @@ -142,26 +148,11 @@ How to Teach This

The change will be documented in the language spec and in the
What's New documentation. The ``SyntaxWarning`` will alert users
that their code needs to change. The empirical evidence [5]_
that their code needs to change. The `empirical evidence
<https://github.com/iritkatriel/finally/blob/main/README.md>`__
shows that the changes necessary are typically quite
straightforward.

References
==========

.. [1] https://docs.python.org/3/tutorial/errors.html#defining-clean-up-actions
.. [2] https://pylint.readthedocs.io/en/stable/
.. [3] https://docs.astral.sh/ruff/
.. [4] https://github.com/PyCQA/flake8-bugbear
.. [5] https://github.com/iritkatriel/finally/blob/main/README.md
.. [6] https://discuss.python.org/t/pep-601-forbid-return-break-continue-breaking-out-of-finally/2239/24
Copyright
=========

Expand Down

0 comments on commit 7ec364a

Please sign in to comment.