Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive with warn-unreachable and no-strict-optional (canonical) #16718

Closed
snmishra opened this issue Dec 30, 2023 · 5 comments
Closed
Labels
bug mypy got something wrong

Comments

@snmishra
Copy link

snmishra commented Dec 30, 2023

Bug Report

Tests with re.match or re.search give incorrect unreachable error when using mypy 1.8.0

To Reproduce

import re

test_str = "XYZ"

if re.match("ABC", test_str):
    print("match")
else:
    print("no match")

Expected Behavior

No mypy errors

Actual Behavior

> mypy test.py
test.py:10: error: Statement is unreachable  [unreachable]
        print("no match")
        ^~~~~~~~~~~~~~~~~
Found 1 error in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: 1.8.0
  • Mypy command-line flags:
  • Mypy configuration options from mypy.ini (and other config files):
python_version = "3.12"
check_untyped_defs = true
# We don't have stubs for all libraries
ignore_missing_imports = true
# We need some ignores for Pylance
follow_imports = "normal"
ignore_errors = false
show_error_codes = true
pretty = true
# Strictness
allow_redefinition = false
implicit_reexport = true
local_partial_types = true
no_implicit_optional = true
strict_equality = true
strict_optional = false
# Warnings
warn_no_return = true
warn_redundant_casts = true
warn_unreachable = true
warn_unused_configs = true
warn_unused_ignores = true
  • Python version used: 3.12
@snmishra snmishra added the bug mypy got something wrong label Dec 30, 2023
@hauntsaninja
Copy link
Collaborator

This is expected given that you're using strict_optional = false. Don't use it, it is evil

@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Dec 31, 2023
@snmishra
Copy link
Author

snmishra commented Jan 1, 2024

Thanks! strict_optional = False indeed is evil and doing a lot more than I had expected from the name. The documentation probably should have a fat warning that this is evil.

@HansAarneLiblik
Copy link

I don't think this should be closed. What change is the reason behind this new failure?

For us the failing code is strictly typed

def func(inp: str) -> str | None:
  if re.match(MY_REGEX, inp):
    return inp
    
  return None  # This errors as "unreachable"

We have a pretty big codebase, where A LOT of code is still using strict_optional = false, as the migration to strict_optional=true takes time.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 2, 2024

Oh you didn't mention this was a regression. Looks like the relevant change is #16566, so mypy correctly sees that Match is final and cannot have a falsey subclass.

I don't think there's any change to be made here. strict_optional = false tells mypy to not think about None, so mypy will not think about None.

If this is a problem for your existing codebase, I recommend using strict_optional = false only in the modules that need it, and disabling warn_unreachable = true in those modules. This can be accomplished in your mypy config file or by using comments at the top of those files.

@HansAarneLiblik
Copy link

HansAarneLiblik commented Jan 2, 2024

Match may be final, but re.match returns Match[str] | None and we're relying on the | None part

image

We're already using strict_optional=false ONLY on manually defined list of pre-existing files (migrating to true is in the works, but as I said this takes time)

image

We can ignore this error on the impacted functions, but IMO this is a regression not a bug in our "outdated" codebase

hauntsaninja added a commit to hauntsaninja/mypy that referenced this issue Jan 2, 2024
On multiple occasions, I've encountered folks using this, running into
issues and then being perplexed when they figure out what
`--no-strict-optional` actually does. Most recently in
python#16718 (comment)

This is a non-standard, dangerous option that should not be used in
modern typed Python. It's been five and a half years since it was the
default behaviour in mypy, so we should deemphasise and warn about its
existence.
hauntsaninja added a commit that referenced this issue Jan 2, 2024
On multiple occasions, I've encountered folks using this, running into
issues and then being perplexed when they figure out what
`--no-strict-optional` actually does. Most recently in
#16718 (comment)

This is a non-standard, dangerous option that should not be used in
modern typed Python. It's been five and a half years since it was the
default behaviour in mypy, so we should deemphasise and warn about its
existence.
@hauntsaninja hauntsaninja changed the title Incorrect unreachable error False positive with warn-unreachable and no-strict-optional (canonical) Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong
Projects
None yet
Development

No branches or pull requests

3 participants