GeezerVison

Copy link

Copy Markdown

Contributor

Byron requested changes Jul 1, 2021

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values. From context, this behavior is clearly
not the goal; a bare return statement should be used here (which
has the same effect except for the `value` attribute of the
StopIteration exception). The code had used a bare return prior to
82b131c (gitpython-developers#1282), when `return` was changed to `return iter([])`.
That was part of a change that added numerous type annotations. It
looks like it was either a mistake, or possibly an attempt to work
around an old bug in a static type checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.

Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values. From context, this behavior is clearly
not the goal; a bare return statement should be used here (which
has the same effect except for the `value` attribute of the
StopIteration exception). The code had used a bare return prior to
82b131c (gitpython-developers#1282), when `return` was changed to `return iter([])`.
That was part of a change that added numerous type annotations. It
looks like it was either a mistake, or possibly an attempt to work
around an old bug in a static type checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.

Returning an explicit value from a generator function causes that
value to be bound to the `value` attribute of the StopIteration
exception. This is available as the result of "yield from" when it
is used as an expression; or by explicitly catching StopIteration,
binding the StopIteration exception to a variable, and accessing
the attribute. This feature of generators is rarely used.

The `return iter([])` statement in Submodule.iter_items uses this
feature, causing the resulting StopIteration exception object to
have a `value` attribute that refers to a separate second iterator
that also yields no values (gitpython-developers#1779).

From context, this behavior is clearly not the goal; a bare return
statement should be used here (which has the same effect except for
the `value` attribute of the StopIteration exception). The code had
used a bare return prior to 82b131c (gitpython-developers#1282), when `return` was
changed to `return iter([])`. That was part of a change that added
numerous type annotations. It looks like it was either a mistake,
or possibly an attempt to work around an old bug in a static type
checker.

This commit extends the test_iter_items_from_invalid_hash test to
assert that the `value` attribute of the StopIteration is its usual
default value of None. This commit only extends the test; it does
not fix the bug.