Repository: quora/pyanalyze
Pull request #637 (2023-05-27 03:41:51)

Add missing_generic_parameters


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 1931259..0b04d80 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Add new error code `missing_generic_parameters` (off by
+  default) that flags missing parameters to generic types
+  such as `list` (#637)
 - Add new error code `reveal_type` for `reveal_type()`
   and similar functions, which previously emitted
   `inference_failure` (#636)
diff --git a/pyanalyze/error_code.py b/pyanalyze/error_code.py
index 6cf574d..5351b9f 100644
--- a/pyanalyze/error_code.py
+++ b/pyanalyze/error_code.py
@@ -105,6 +105,7 @@ class ErrorCode(enum.Enum):
     invalid_override_decorator = 85
     override_does_not_override = 86
     reveal_type = 87
+    missing_generic_parameters = 88
 
 
 # Allow testing unannotated functions without too much fuss
@@ -129,6 +130,7 @@ DISABLED_BY_DEFAULT = {
     # TODO: turn this on
     ErrorCode.implicit_reexport,
     ErrorCode.incompatible_override,
+    ErrorCode.missing_generic_parameters,
 }
 
 ERROR_DESCRIPTION = {
@@ -234,6 +236,7 @@ ERROR_DESCRIPTION = {
     ),
     ErrorCode.invalid_override_decorator: "@override decorator in invalid location",
     ErrorCode.override_does_not_override: "Method does not override any base method",
+    ErrorCode.missing_generic_parameters: "Missing type parameters for generic type",
 }
 
 
diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py
index 0b8009a..99a72d7 100644
--- a/pyanalyze/name_check_visitor.py
+++ b/pyanalyze/name_check_visitor.py
@@ -24,7 +24,7 @@ import sys
 import traceback
 import types
 from argparse import ArgumentParser
-from dataclasses import dataclass
+from dataclasses import InitVar, dataclass
 from itertools import chain
 from pathlib import Path
 from typing import (
@@ -50,7 +50,7 @@ from typing import (
 import asynq
 import qcore
 import typeshed_client
-from typing_extensions import Annotated, Protocol
+from typing_extensions import Annotated, Protocol, get_args, get_origin
 
 from . import attributes, format_strings, importer, node_visitor, type_evaluation
 from .analysis_lib import get_attribute_path
@@ -186,6 +186,7 @@ from .value import (
     replace_known_sequence_value,
     SequenceValue,
     set_self,
+    stringify_object,
     SubclassValue,
     TypedValue,
     TypeVarValue,
@@ -2161,7 +2162,45 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
 
     def _visit_annotation(self, node: ast.AST) -> Value:
         with qcore.override(self, "in_annotation", True):
-            return self.visit(node)
+            val = self.visit(node)
+            self.check_for_missing_generic_params(node, val)
+            return val
+
+    def check_for_missing_generic_params(self, node: ast.AST, value: Value) -> None:
+        if not isinstance(value, KnownValue):
+            return
+        val = value.val
+        if not safe_isinstance(val, type):
+            args = get_args(val)
+            if args:
+                return
+            val = get_origin(val)
+            if not safe_isinstance(val, type):
+                return
+            if val is tuple and value.val is not tuple:
+                # tuple[()]
+                return
+        if GenericAlias is not None and isinstance(val, GenericAlias):
+            return
+        generic_params = self.arg_spec_cache.get_type_parameters(val)
+        if not generic_params:
+            return
+        if sys.version_info < (3, 8) and val is InitVar:
+            # On 3.6 and 3.7, InitVar[type] just returns InitVar
+            return
+        # On 3.8, ast.Index has no lineno
+        if not hasattr(node, "lineno"):
+            if isinstance(node, ast.Index):
+                # static analysis: ignore[undefined_attribute]
+                node = node.value
+            else:
+                # Slice or ExtSlice, shouldn't happen
+                return
+        self.show_error(
+            node,
+            f"Missing type parameters for generic type {stringify_object(value.val)}",
+            error_code=ErrorCode.missing_generic_parameters,
+        )
 
     def _value_of_annotation_type(
         self,
@@ -2970,7 +3009,14 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
     def _visit_display_read(
         self, node: Union[ast.Set, ast.List, ast.Tuple], typ: type
     ) -> Value:
-        elts = [self.visit(elt) for elt in node.elts]
+        if typ is tuple and self.in_annotation:
+            elts = []
+            for elt in node.elts:
+                val = self.visit(elt)
+                self.check_for_missing_generic_params(elt, val)
+                elts.append(val)
+        else:
+            elts = [self.visit(elt) for elt in node.elts]
         return self._maybe_make_sequence(typ, elts, node, elt_nodes=node.elts)
 
     def _maybe_make_sequence(
@@ -3269,6 +3315,8 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
         if self.in_annotation and isinstance(op, ast.BitOr):
             # Accept PEP 604 (int | None) in annotations
             if isinstance(left, KnownValue) and isinstance(right, KnownValue):
+                self.check_for_missing_generic_params(left_node, left)
+                self.check_for_missing_generic_params(right_node, right)
                 return KnownValue(Union[left.val, right.val])
             else:
                 self._show_error_if_checking(
@@ -4378,6 +4426,7 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
         root_composite = self.composite_from_node(node.value)
         index_composite = self.composite_from_node(node.slice)
         index = index_composite.value
+        self.check_for_missing_generic_params(node.slice, index)
         if (
             root_composite.varname is not None
             and isinstance(index, KnownValue)
@@ -5258,7 +5307,7 @@ class NameCheckVisitor(node_visitor.ReplacingNodeVisitor):
 
     @classmethod
     def prepare_constructor_kwargs(
-        cls, kwargs: Mapping[str, Any], extra_options: Sequence[ConfigOption] = ()
+        cls, kwargs: Mapping[str, Any], extra_options: Sequence[ConfigOption[Any]] = ()
     ) -> Mapping[str, Any]:
         kwargs = dict(kwargs)
         instances = [*extra_options]
@@ -5425,7 +5474,7 @@ def build_stacked_scopes(
     return StackedScopes(module_vars, module, simplification_limit=simplification_limit)
 
 
-def _get_task_cls(fn: object) -> Type[asynq.FutureBase]:
+def _get_task_cls(fn: object) -> "Type[asynq.FutureBase[Any]]":
     """Returns the task class for an async function."""
 
     if hasattr(fn, "task_cls"):
diff --git a/pyanalyze/options.py b/pyanalyze/options.py
index f063bc6..e8b8e52 100644
--- a/pyanalyze/options.py
+++ b/pyanalyze/options.py
@@ -277,13 +277,13 @@ class PyObjectSequenceOption(ConcatenatedOption[T]):
 
 @dataclass
 class Options:
-    options: Mapping[str, Sequence[ConfigOption]]
+    options: Mapping[str, Sequence[ConfigOption[Any]]]
     module_path: ModulePath = ()
 
     @classmethod
     def from_option_list(
         cls,
-        instances: Sequence[ConfigOption] = (),
+        instances: Sequence[ConfigOption[Any]] = (),
         config_file_path: Optional[Path] = None,
     ) -> "Options":
         if config_file_path:
@@ -354,7 +354,7 @@ def add_arguments(parser: argparse.ArgumentParser) -> None:
 
 def parse_config_file(
     path: Path, *, priority: int = 0, seen_paths: Collection[Path] = frozenset()
-) -> Iterable[ConfigOption]:
+) -> Iterable[ConfigOption[Any]]:
     try:
         path = path.resolve(strict=True)
     except FileNotFoundError:
@@ -378,7 +378,7 @@ def _parse_config_section(
     path: Path,
     priority: int,
     seen_paths: Collection[Path],
-) -> Iterable[ConfigOption]:
+) -> Iterable[ConfigOption[Any]]:
     if "module" in section:
         if module_path == ():
             raise InvalidConfigOption(
diff --git a/pyproject.toml b/pyproject.toml
index 4ac6f55..5e03ca8 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -37,6 +37,7 @@ value_always_true = true
 suggested_parameter_type = true
 suggested_return_type = true
 incompatible_override = true
+missing_generic_parameters = true
 
 [[tool.pyanalyze.overrides]]
 module = "pyanalyze.typevar"
diff --git a/setup.py b/setup.py
index 7a5bf9c..d6e84a8 100644
--- a/setup.py
+++ b/setup.py
@@ -39,6 +39,7 @@ if __name__ == "__main__":
             "codemod",
             "tomli>=1.1.0",
         ],
+        extras_require={"tests": ["pytest", "mypy_extensions", "attrs"]},
         # These are useful for unit tests of pyanalyze extensions
         # outside the package.
         package_data={"pyanalyze": package_data},
diff --git a/tox.ini b/tox.ini
index 7185deb..3d27ba7 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,13 +1,11 @@
 [tox]
 envlist =
-    py37,py38,py39,py310,py311,black,flake8
+    py37,py38,py39,py310,py311,py312,black,flake8
 skip_missing_interpreters = True
 
 [testenv]
 deps =
-    pytest
-    mypy_extensions
-    attrs
+    .[tests]
 
 commands =
     pytest pyanalyze/


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index c3e1453..2dd0b41 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -3,11 +3,11 @@ name: main
 on:
   push:
     branches: [master]
-    tags: ['*']
+    tags: ["*"]
   pull_request:
     paths-ignore:
-    - .gitignore
-    - LICENSE
+      - .gitignore
+      - LICENSE
 
 jobs:
   build:
@@ -16,36 +16,36 @@ jobs:
       fail-fast: false
       matrix:
         include:
-        - name: py37
-          python: '3.7'
-          toxenv: py37
-        - name: py38
-          python: '3.8'
-          toxenv: py38
-        - name: py39
-          python: '3.9'
-          toxenv: py39
-        - name: py310
-          python: '3.10'
-          toxenv: py310
-        - name: py311
-          python: '3.11'
-          toxenv: py311
-        - name: black
-          python: '3.11'
-          toxenv: black
-        - name: flake8
-          python: '3.11'
-          toxenv: flake8
+          - name: py37
+            python: "3.7"
+            toxenv: py37
+          - name: py38
+            python: "3.8"
+            toxenv: py38
+          - name: py39
+            python: "3.9"
+            toxenv: py39
+          - name: py310
+            python: "3.10"
+            toxenv: py310
+          - name: py311
+            python: "3.11"
+            toxenv: py311
+          - name: black
+            python: "3.11"
+            toxenv: black
+          - name: flake8
+            python: "3.11"
+            toxenv: flake8
 
     steps:
-    - uses: actions/checkout@v3
-    - uses: actions/setup-python@v4
-      with:
-        python-version: ${{ matrix.python }}
-    - name: install tox
-      run: pip install --upgrade setuptools tox==4.0.16
-    - name: setup tox environment
-      run: tox -e ${{ matrix.toxenv }} --notest
-    - name: test
-      run: tox -e ${{ matrix.toxenv }}
+      - uses: actions/checkout@v3
+      - uses: actions/setup-python@v4
+        with:
+          python-version: ${{ matrix.python }}
+      - name: install tox
+        run: pip install --upgrade setuptools tox==4.0.16
+      - name: setup tox environment
+        run: tox -e ${{ matrix.toxenv }} --notest
+      - name: test
+        run: tox -e ${{ matrix.toxenv }}
diff --git a/pyanalyze/test_annotations.py b/pyanalyze/test_annotations.py
index 378f073..d0f8c59 100644
--- a/pyanalyze/test_annotations.py
+++ b/pyanalyze/test_annotations.py
@@ -78,7 +78,9 @@ class TestAnnotations(TestNameCheckVisitorBase):
     def test_generic(self):
         from typing import Any, List
 
-        def capybara(x: List[int], y: List, z: List[Any]) -> None:
+        def capybara(
+            x: List[int], y: List, z: List[Any]  # E: missing_generic_parameters
+        ) -> None:
             assert_is_value(x, GenericValue(list, [TypedValue(int)]))
             assert_is_value(y, TypedValue(list))
             assert_is_value(z, GenericValue(list, [AnyValue(AnySource.explicit)]))
@@ -1831,3 +1833,37 @@ class TestUnpack(TestNameCheckVisitorBase):
                     tuple, [(False, TypedValue(int)), (True, TypedValue(str))]
                 ),
             )
+
+
+class TestMissinGenericParameters(TestNameCheckVisitorBase):
+    @assert_passes()
+    def test(self):
+        from typing import List, Set, Dict
+
+        def capybara(
+            x: list,  # E: missing_generic_parameters
+            y: List,  # E: missing_generic_parameters
+            z: List[int],
+            a: Set[list],  # E: missing_generic_parameters
+            b: Dict[str, list],  # E: missing_generic_parameters
+        ) -> set:  # E: missing_generic_parameters
+            return {1}
+
+    @skip_before((3, 9))
+    @assert_passes()
+    def test_with_pep_585(self):
+        def capybara(
+            a: set[list],  # E: missing_generic_parameters
+            b: dict[str, list],  # E: missing_generic_parameters
+        ) -> None:
+            pass
+
+    @skip_before((3, 10))
+    @assert_passes()
+    def test_union_or(self):
+        def capybara(
+            x: list | int,  # E: missing_generic_parameters
+            y: str | list,  # E: missing_generic_parameters
+            z: float | bool | set,  # E: missing_generic_parameters
+        ) -> None:
+            pass
diff --git a/pyanalyze/test_attributes.py b/pyanalyze/test_attributes.py
index 006e1ec..6ba75e5 100644
--- a/pyanalyze/test_attributes.py
+++ b/pyanalyze/test_attributes.py
@@ -215,12 +215,14 @@ class TestAttributes(TestNameCheckVisitorBase):
 
     @assert_passes()
     def test_typeshed(self):
-        def capybara(c: staticmethod):
+        # missing_generic_parameters is a bit questionable here, but the
+        # class really is defined as generic in typeshed.
+        def capybara(c: staticmethod):  # E: missing_generic_parameters
             assert_is_value(c.__isabstractmethod__, TypedValue(bool))
 
     @assert_passes()
     def test_no_attribute_for_typeshed_class():
-        def capybara(c: staticmethod):
+        def capybara(c: staticmethod):  # E: missing_generic_parameters
             c.no_such_attribute  # E: undefined_attribute
 
     @assert_passes()
diff --git a/pyanalyze/test_implementation.py b/pyanalyze/test_implementation.py
index 97ed351..4bcce17 100644
--- a/pyanalyze/test_implementation.py
+++ b/pyanalyze/test_implementation.py
@@ -679,7 +679,12 @@ class TestGenericMutators(TestNameCheckVisitorBase):
             b: str
             c: NotRequired[str]
 
-        def capybara(td: TD, s: str, d: Dict[str, int], untyped: dict):
+        def capybara(
+            td: TD,
+            s: str,
+            d: Dict[str, int],
+            untyped: dict,  # E: missing_generic_parameters
+        ):
             assert_is_value(td.get("a"), TypedValue(int))
             assert_is_value(td.get("c"), TypedValue(str) | KnownValue(None))
             assert_is_value(td.get("c", 1), TypedValue(str) | KnownValue(1))
@@ -1148,7 +1153,7 @@ class TestDictGetItem(TestNameCheckVisitorBase):
             dct: Dict[str, int],
             rev: ReversedDict[str, int],
             nd: NormalDict[int, str],
-            untyped: dict,
+            untyped: dict,  # E: missing_generic_parameters
         ):
             d = {1: 2}
             assert_is_value(d[1], KnownValue(2))
@@ -1165,7 +1170,7 @@ class TestDictGetItem(TestNameCheckVisitorBase):
     def test_type_as_key(self):
         from typing import Type
 
-        def capybara(d: dict, t: Type[int]):
+        def capybara(d: dict, t: Type[int]):  # E: missing_generic_parameters
             d[int]
             d[t]
 
@@ -1237,7 +1242,7 @@ class TestDictSetItem(TestNameCheckVisitorBase):
     def test_bad_key_type(self):
         from typing import Dict
 
-        def capybara(untyped: dict) -> None:
+        def capybara(untyped: dict) -> None:  # E: missing_generic_parameters
             dct: Dict[str, int] = {}
             dct[1] = 1  # E: incompatible_argument
 
diff --git a/pyanalyze/test_stacked_scopes.py b/pyanalyze/test_stacked_scopes.py
index 5ae38da..ca83e71 100644
--- a/pyanalyze/test_stacked_scopes.py
+++ b/pyanalyze/test_stacked_scopes.py
@@ -932,7 +932,7 @@ class TestConstraints(TestNameCheckVisitorBase):
                 # Don't widen the type to A.
                 assert_is_value(x, TypedValue(B))
 
-        def kerodon(cond1, cond2, val, lst: list):
+        def kerodon(cond1, cond2, val, lst: list):  # E: missing_generic_parameters
             if cond1:
                 x = int(val)
             elif cond2: