From 41e6c48160c6b31826f1e11c670f814ba50d88b5 Mon Sep 17 00:00:00 2001 From: Richard Xia Date: Tue, 17 May 2022 23:12:35 -0700 Subject: [PATCH] mypy plugin: More precisely detect when fields are required. The mypy plugin would previously incorrectly determine that a field was not required in a few scenarios where the field really is required. This specifically affected cases when the `Field()` function is used, where the plugin assumed that the first argument would always be `default`. This changes the code to examine each argument more closely, and it now properly handles several more scenarios where the default is explicitly named or when the default_factory named argument is used. --- changes/4086-richardxia.md | 1 + pydantic/mypy.py | 11 +++++++++-- tests/mypy/modules/fail_defaults.py | 17 +++++++++++++++++ tests/mypy/outputs/fail_defaults.txt | 4 ++++ tests/mypy/test_mypy.py | 2 ++ 5 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 changes/4086-richardxia.md create mode 100644 tests/mypy/modules/fail_defaults.py create mode 100644 tests/mypy/outputs/fail_defaults.txt diff --git a/changes/4086-richardxia.md b/changes/4086-richardxia.md new file mode 100644 index 0000000000..bf71bc36bc --- /dev/null +++ b/changes/4086-richardxia.md @@ -0,0 +1 @@ +Improve mypy plugin's ability to detect required fields. diff --git a/pydantic/mypy.py b/pydantic/mypy.py index d89fe04367..36e287b3ef 100644 --- a/pydantic/mypy.py +++ b/pydantic/mypy.py @@ -435,8 +435,15 @@ def get_is_required(cls: ClassDef, stmt: AssignmentStmt, lhs: NameExpr) -> bool: return True if isinstance(expr, CallExpr) and isinstance(expr.callee, RefExpr) and expr.callee.fullname == FIELD_FULLNAME: # The "default value" is a call to `Field`; at this point, the field is - # only required if default is Ellipsis (i.e., `field_name: Annotation = Field(...)`) - return len(expr.args) > 0 and expr.args[0].__class__ is EllipsisExpr + # only required if default is Ellipsis (i.e., `field_name: Annotation = Field(...)`) or if default_factory + # is specified. + for arg, name in zip(expr.args, expr.arg_names): + # If name is None, then this arg is the default because it is the only positonal argument. + if name is None or name == 'default': + return arg.__class__ is EllipsisExpr + if name == 'default_factory': + return False + return True # Only required if the "default value" is Ellipsis (i.e., `field_name: Annotation = ...`) return isinstance(expr, EllipsisExpr) diff --git a/tests/mypy/modules/fail_defaults.py b/tests/mypy/modules/fail_defaults.py new file mode 100644 index 0000000000..36e1d91c84 --- /dev/null +++ b/tests/mypy/modules/fail_defaults.py @@ -0,0 +1,17 @@ +from pydantic import BaseModel, Field + + +class Model(BaseModel): + # Required + undefined_default_no_args: int = Field() + undefined_default: int = Field(description='my desc') + positional_ellipsis_default: int = Field(...) + named_ellipsis_default: int = Field(default=...) + + # Not required + positional_default: int = Field(1) + named_default: int = Field(default=2) + named_default_factory: int = Field(default_factory=lambda: 3) + + +Model() diff --git a/tests/mypy/outputs/fail_defaults.txt b/tests/mypy/outputs/fail_defaults.txt new file mode 100644 index 0000000000..9c50dfd4d2 --- /dev/null +++ b/tests/mypy/outputs/fail_defaults.txt @@ -0,0 +1,4 @@ +17: error: Missing named argument "undefined_default_no_args" for "Model" [call-arg] +17: error: Missing named argument "undefined_default" for "Model" [call-arg] +17: error: Missing named argument "positional_ellipsis_default" for "Model" [call-arg] +17: error: Missing named argument "named_ellipsis_default" for "Model" [call-arg] diff --git a/tests/mypy/test_mypy.py b/tests/mypy/test_mypy.py index 4e6d173ae4..c976069e03 100644 --- a/tests/mypy/test_mypy.py +++ b/tests/mypy/test_mypy.py @@ -25,6 +25,7 @@ ('mypy-plugin.ini', 'plugin_fail.py', 'plugin-fail.txt'), ('mypy-plugin-strict.ini', 'plugin_success.py', 'plugin-success-strict.txt'), ('mypy-plugin-strict.ini', 'plugin_fail.py', 'plugin-fail-strict.txt'), + ('mypy-plugin-strict.ini', 'fail_defaults.py', 'fail_defaults.txt'), ('mypy-default.ini', 'success.py', None), ('mypy-default.ini', 'fail1.py', 'fail1.txt'), ('mypy-default.ini', 'fail2.py', 'fail2.txt'), @@ -40,6 +41,7 @@ ('pyproject-plugin.toml', 'plugin_fail.py', 'plugin-fail.txt'), ('pyproject-plugin-strict.toml', 'plugin_success.py', 'plugin-success-strict.txt'), ('pyproject-plugin-strict.toml', 'plugin_fail.py', 'plugin-fail-strict.txt'), + ('pyproject-plugin-strict.toml', 'fail_defaults.py', 'fail_defaults.txt'), ] executable_modules = list({fname[:-3] for _, fname, out_fname in cases if out_fname is None})