Skip to content

Enable strict optional for more test files (3) #15597

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jul 7, 2023
Merged

Enable strict optional for more test files (3) #15597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76d975d
Enable strict optional for more test files (3)
cdce8p Jul 5, 2023
43910ab
Merge remote-tracking branch 'upstream/master' into test-optional-3
cdce8p Jul 7, 2023
ee24b77
Update test helper
cdce8p Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions mypy/test/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import shutil
import sys
import time
from typing import Any, Callable, Collection, Iterable, Iterator, Pattern
from typing import Any, Callable, Iterable, Iterator, Pattern

# Exporting Suite as alias to TestCase for backwards compatibility
# TODO: avoid aliasing - import and subclass TestCase directly
Expand Down Expand Up @@ -317,10 +317,7 @@ def assert_type(typ: type, value: object) -> None:


def parse_options(
program_text: str,
testcase: DataDrivenTestCase,
incremental_step: int,
no_strict_optional_files: Collection[str] = (),
program_text: str, testcase: DataDrivenTestCase, incremental_step: int
) -> Options:
"""Parse comments like '# flags: --foo' in a test case."""
options = Options()
Expand All @@ -342,10 +339,6 @@ def parse_options(
else:
flag_list = []
options = Options()
# TODO: Enable strict optional in test cases by default (requires *many* test case changes)
if os.path.basename(testcase.file) in no_strict_optional_files:
options.strict_optional = False

options.error_summary = False
options.hide_error_codes = True
options.force_uppercase_builtins = True
Expand Down
7 changes: 1 addition & 6 deletions mypy/test/testfinegrained.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@
# Set to True to perform (somewhat expensive) checks for duplicate AST nodes after merge
CHECK_CONSISTENCY = False

# TODO: Enable strict optional in test cases by default. Remove files here, once test cases are updated
no_strict_optional_files = {"fine-grained.test", "fine-grained-suggest.test"}


class FineGrainedSuite(DataSuite):
files = find_test_files(
Expand Down Expand Up @@ -143,9 +140,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None:

def get_options(self, source: str, testcase: DataDrivenTestCase, build_cache: bool) -> Options:
# This handles things like '# flags: --foo'.
options = parse_options(
source, testcase, incremental_step=1, no_strict_optional_files=no_strict_optional_files
)
options = parse_options(source, testcase, incremental_step=1)
options.incremental = True
options.use_builtins_fixtures = True
options.show_traceback = True
Expand Down
2 changes: 1 addition & 1 deletion test-data/unit/fine-grained-suggest.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ optional2(10)
optional2('test')

def optional3(x: Optional[List[Any]]):
assert not x
assert x
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't quite understand this one!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tbh I didn't either. assert not x doesn't make much sense here. I only excludes the non-empty list. With strict optional, that results in an error for the next line where

    return x[0]  # E: Value of type "Optional[List[Any]]" is not indexable

To keep the intention, the assert should probably be assert not x and x is not None but I wasn't sure this would be better than just assert x.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
assert x
assert not x and x is not None

This would work as well, although as mentioned earlier, I'm not convinced it's better. Feel free to accept or dismiss it.

return x[0]

optional3(test)
Expand Down
46 changes: 26 additions & 20 deletions test-data/unit/fine-grained.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ main:3: error: "A" has no attribute "x"
[case testVariableTypeBecomesInvalid]
import m
def f() -> None:
a = None # type: m.A
a: m.A
[file m.py]
class A: pass
[file m.py.2]
Expand Down Expand Up @@ -1724,15 +1724,15 @@ f = 1
main:1: error: Module "a" has no attribute "f"

[case testDecoratedMethodRefresh]
from typing import Iterator, Callable, List
from typing import Iterator, Callable, List, Optional
from a import f
import a

def dec(f: Callable[['A'], Iterator[int]]) -> Callable[[int], int]: pass
def dec(f: Callable[['A'], Optional[Iterator[int]]]) -> Callable[[int], int]: pass

class A:
@dec
def f(self) -> Iterator[int]:
def f(self) -> Optional[Iterator[int]]:
self.x = a.g() # type: int
return None
[builtins fixtures/list.pyi]
Expand Down Expand Up @@ -4626,6 +4626,7 @@ class User:
==

[case testNoStrictOptionalModule]
# flags: --no-strict-optional
import a
a.y = a.x
[file a.py]
Expand All @@ -4643,9 +4644,10 @@ y: int
[out]
==
==
main:2: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "int")
main:3: error: Incompatible types in assignment (expression has type "Optional[str]", variable has type "int")

[case testNoStrictOptionalFunction]
# flags: --no-strict-optional
import a
from typing import Optional
def f() -> None:
Expand All @@ -4666,9 +4668,10 @@ def g(x: str) -> None:
[out]
==
==
main:5: error: Argument 1 to "g" has incompatible type "Optional[int]"; expected "str"
main:6: error: Argument 1 to "g" has incompatible type "Optional[int]"; expected "str"

[case testNoStrictOptionalMethod]
# flags: --no-strict-optional
import a
from typing import Optional
class C:
Expand All @@ -4693,7 +4696,7 @@ class B:
[out]
==
==
main:6: error: Argument 1 to "g" of "B" has incompatible type "Optional[int]"; expected "str"
main:7: error: Argument 1 to "g" of "B" has incompatible type "Optional[int]"; expected "str"

[case testStrictOptionalModule]
# flags: --strict-optional
Expand Down Expand Up @@ -5276,10 +5279,11 @@ class I(metaclass=ABCMeta):
@abstractmethod
def f(self) -> None: pass
[file b.py]
from typing import Optional
from z import I
class Foo(I):
pass
def x() -> Foo: return None
def x() -> Optional[Foo]: return None
[file z.py.2]
from abc import abstractmethod, ABCMeta
class I(metaclass=ABCMeta):
Expand All @@ -5301,10 +5305,11 @@ class I(metaclass=ABCMeta):
@abstractmethod
def f(self) -> None: pass
[file b.py]
from typing import Optional
from a import I
class Foo(I):
pass
def x() -> Foo: return None
def x() -> Optional[Foo]: return None
[file a.py.2]
from abc import abstractmethod, ABCMeta
class I(metaclass=ABCMeta):
Expand Down Expand Up @@ -7769,7 +7774,8 @@ from typing import List
import b
class A(b.B):
def meth(self) -> None:
self.x, *self.y = None, None # type: str, List[str]
self.x: str
self.y: List[str]
[file b.py]
from typing import List
class B:
Expand All @@ -7781,7 +7787,7 @@ class B:
[builtins fixtures/list.pyi]
[out]
==
main:5: error: Incompatible types in assignment (expression has type "List[str]", base class "B" defined the type as "List[int]")
main:6: error: Incompatible types in assignment (expression has type "List[str]", base class "B" defined the type as "List[int]")

[case testLiskovFineVariableCleanDefInMethodNested-only_when_nocache]
from b import B
Expand Down Expand Up @@ -9109,27 +9115,27 @@ import a
[file a.py]
# mypy: no-warn-no-return

from typing import List
def foo() -> List:
from typing import List, Optional
def foo() -> Optional[List]:
20

[file a.py.2]
# mypy: disallow-any-generics, no-warn-no-return

from typing import List
def foo() -> List:
from typing import List, Optional
def foo() -> Optional[List]:
20

[file a.py.3]
# mypy: no-warn-no-return

from typing import List
def foo() -> List:
from typing import List, Optional
def foo() -> Optional[List]:
20

[file a.py.4]
from typing import List
def foo() -> List:
from typing import List, Optional
def foo() -> Optional[List]:
20
[out]
==
Expand Down Expand Up @@ -9867,7 +9873,7 @@ x = 0 # Arbitrary change to trigger reprocessing
[builtins fixtures/dict.pyi]
[out]
==
a.py:5: note: Revealed type is "def (x: builtins.int) -> builtins.str"
a.py:5: note: Revealed type is "def (x: builtins.int) -> Union[builtins.str, None]"

[case testTypeVarTupleCached]
import a
Expand Down