Skip to content

Comments

typing: set warn_unreachable#7613

Merged
bluetech merged 1 commit intopytest-dev:masterfrom
bluetech:typing-warn-unreachable
Aug 4, 2020
Merged

typing: set warn_unreachable#7613
bluetech merged 1 commit intopytest-dev:masterfrom
bluetech:typing-warn-unreachable

Conversation

@bluetech
Copy link
Member

@bluetech bluetech commented Aug 3, 2020

This makes mypy raise an error whenever it detects code which is statically unreachable, e.g.

x: int
if isinstance(x, str):
    ... # Statement is unreachable  [unreachable]

This is really neat and finds quite a few logic and typing bugs.

Sometimes the code is intentionally unreachable in terms of types, e.g. raising TypeError when a function is given an argument with a wrong type. In these cases a type: ignore[unreachable] is needed, but I think it's a nice code hint.

if (
p.kind is Parameter.POSITIONAL_OR_KEYWORD
or p.kind is Parameter.KEYWORD_ONLY
# TODO: Remove type ignore after https://github.com/python/typeshed/pull/4383.
Copy link
Member

@Zac-HD Zac-HD Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

FYI the . at end of line breaks the link for me. And perhaps we could add warn_unused_ignores = True to the config so that this will be caught by CI when we upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the . at end of line breaks the link for me.

Removed.

And perhaps we could add warn_unused_ignores = True to the config so that this will be caught by CI when we upgrade?

We can't use warn_unused_ignores because there are some warnings that only happen on some Python versions, or some platforms, etc. which will then trigger the warning on others.

What I've done up to now is that when I upgrade the mypy version we use, I set it temporarily and fix what it brings up.

This makes mypy raise an error whenever it detects code which is
statically unreachable, e.g.

    x: int
    if isinstance(x, str):
        ... # Statement is unreachable  [unreachable]

This is really neat and finds quite a few logic and typing bugs.

Sometimes the code is intentionally unreachable in terms of types, e.g.
raising TypeError when a function is given an argument with a wrong
type. In these cases a `type: ignore[unreachable]` is needed, but I
think it's a nice code hint.
@bluetech bluetech force-pushed the typing-warn-unreachable branch from 7c04128 to 9ab14c6 Compare August 4, 2020 07:00

pytest.xfail("internal reportrecorder tests need refactoring")
# (The silly condition is to fool mypy that the code below this is reachable)
if 1 + 1 == 2:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, # type: ignore[unreachable] doesn't work here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works, but then the code is considered unreachable, which I suspect might make the type checking less strict.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bluetech!

@bluetech bluetech merged commit 303030c into pytest-dev:master Aug 4, 2020
@bluetech bluetech deleted the typing-warn-unreachable branch August 24, 2020 15:06
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.

3 participants