diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f71b656..450ea6a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,7 +8,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: ['3.8', '3.9', '3.10', '3.11', '3.12'] + python-version: ['3.9', '3.10', '3.11', '3.12'] steps: - uses: actions/checkout@v4 diff --git a/README.md b/README.md index 5640712..a71cc2e 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,68 @@ class Model(BaseModel): foo = 1 # Will error at runtime ``` +### `PYD003` - *Unecessary Field call to specify a default value* + +Raise an error if the [`Field`](https://docs.pydantic.dev/latest/api/fields/#pydantic.fields.Field) function +is used only to specify a default value. + +```python +class Model(BaseModel): + foo: int = Field(default=1) +``` + +Instead, consider specifying the default value directly: + +```python +class Model(BaseModel): + foo: int = 1 +``` + +### `PYD004` - *Default argument specified in annotated* + +Raise an error if the `default` argument of the [`Field`](https://docs.pydantic.dev/latest/api/fields/#pydantic.fields.Field) function is used together with [`Annotated`](https://docs.python.org/3/library/typing.html#typing.Annotated). + +```python +class Model(BaseModel): + foo: Annotated[int, Field(default=1, description="desc")] +``` + +To make type checkers aware of the default value, consider specifying the default value directly: + +```python +class Model(BaseModel): + foo: Annotated[int, Field(description="desc")] = 1 +``` + +### `PYD005` - *Field name overrides annotation* + +Raise an error if the field name clashes with the annotation. + +```python +from datetime import date + +class Model(BaseModel): + date: date | None = None +``` + +Because of how Python [evaluates](https://docs.python.org/3/reference/simple_stmts.html#annassign) +annotated assignments, unexpected issues can happen when using an annotation name that clashes with a field +name. Pydantic will try its best to warn you about such issues, but can fail in complex scenarios (and the +issue may even be silently ignored). + +Instead, consider, using an [alias](https://docs.pydantic.dev/latest/concepts/fields/#field-aliases) or referencing your type under a different name: + +```python +from datetime import date + +date_ = date + +class Model(BaseModel): + date_aliased: date | None = Field(default=None, alias="date") + # or + date: date_ | None = None +``` + ### `PYD010` - *Usage of `__pydantic_config__`* Raise an error if a Pydantic configuration is set with [`__pydantic_config__`](https://docs.pydantic.dev/dev/concepts/config/#configuration-with-dataclass-from-the-standard-library-or-typeddict). diff --git a/pyproject.toml b/pyproject.toml index c837950..c9c46ee 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,13 +10,12 @@ readme = "README.md" authors = [ {name = "Victorien", email = "contact@vctrn.dev"} ] -requires-python = ">=3.8" +requires-python = ">=3.9" classifiers = [ "Development Status :: 4 - Beta", "Operating System :: OS Independent", "Programming Language :: Python :: 3", "Programming Language :: Python :: 3 :: Only", - "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", @@ -54,7 +53,7 @@ where = ["src"] [tool.ruff] line-length = 120 src = ["src"] -target-version = "py38" +target-version = "py39" [tool.ruff.lint] preview = true diff --git a/src/flake8_pydantic/_utils.py b/src/flake8_pydantic/_utils.py index 751bb6f..edeba4f 100644 --- a/src/flake8_pydantic/_utils.py +++ b/src/flake8_pydantic/_utils.py @@ -79,7 +79,7 @@ def _has_pydantic_decorator(node: ast.ClassDef) -> bool: for stmt in node.body: if isinstance(stmt, ast.FunctionDef): decorator_names = get_decorator_names(stmt.decorator_list) - if PYDANTIC_DECORATORS.intersection(decorator_names): + if PYDANTIC_DECORATORS & decorator_names: return True return False @@ -91,12 +91,6 @@ def _has_pydantic_method(node: ast.ClassDef) -> bool: return False -def is_dataclass(node: ast.ClassDef) -> bool: - """Determine if a class is a dataclass.""" - - return bool({"dataclass", "pydantic_dataclass"}.intersection(get_decorator_names(node.decorator_list))) - - def is_pydantic_model(node: ast.ClassDef, include_root_model: bool = True) -> bool: """Determine if a class definition is a Pydantic model. @@ -119,3 +113,44 @@ def is_pydantic_model(node: ast.ClassDef, include_root_model: bool = True) -> bo or _has_pydantic_decorator(node) or _has_pydantic_method(node) ) + + +def is_dataclass(node: ast.ClassDef) -> bool: + """Determine if a class is a dataclass.""" + + return bool({"dataclass", "pydantic_dataclass"} & get_decorator_names(node.decorator_list)) + + +def is_function(node: ast.Call, function_name: str) -> bool: + return ( + isinstance(node.func, ast.Name) + and node.func.id == function_name + or isinstance(node.func, ast.Attribute) + and node.func.attr == function_name + ) + + +def is_name(node: ast.expr, name: str) -> bool: + return isinstance(node, ast.Name) and node.id == name or isinstance(node, ast.Attribute) and node.attr == name + + +def extract_annotations(node: ast.expr) -> set[str]: + annotations: set[str] = set() + + if isinstance(node, ast.Name): + # foo: date = ... + annotations.add(node.id) + if isinstance(node, ast.BinOp): + # foo: date | None = ... + annotations |= extract_annotations(node.left) + annotations |= extract_annotations(node.right) + if isinstance(node, ast.Subscript): + # foo: dict[str, date] + # foo: Annotated[list[date], ...] + if isinstance(node.slice, ast.Tuple): + for elt in node.slice.elts: + annotations |= extract_annotations(elt) + if isinstance(node.slice, ast.Name): + annotations.add(node.slice.id) + + return annotations diff --git a/src/flake8_pydantic/errors.py b/src/flake8_pydantic/errors.py index 33d3b69..9f94d8a 100644 --- a/src/flake8_pydantic/errors.py +++ b/src/flake8_pydantic/errors.py @@ -33,6 +33,21 @@ class PYD002(Error): message = "Non-annotated attribute inside Pydantic model" +class PYD003(Error): + error_code = "PYD003" + message = "Unecessary Field call to specify a default value" + + +class PYD004(Error): + error_code = "PYD004" + message = "Default argument specified in annotated" + + +class PYD005(Error): + error_code = "PYD005" + message = "Field name overrides annotation" + + class PYD010(Error): error_code = "PYD010" message = "Usage of __pydantic_config__" diff --git a/src/flake8_pydantic/plugin.py b/src/flake8_pydantic/plugin.py index 700d609..707ae3e 100644 --- a/src/flake8_pydantic/plugin.py +++ b/src/flake8_pydantic/plugin.py @@ -1,8 +1,9 @@ from __future__ import annotations import ast +from collections.abc import Iterator from importlib.metadata import version -from typing import Any, Iterator +from typing import Any from .visitor import Visitor diff --git a/src/flake8_pydantic/visitor.py b/src/flake8_pydantic/visitor.py index d5b1f77..64480ab 100644 --- a/src/flake8_pydantic/visitor.py +++ b/src/flake8_pydantic/visitor.py @@ -5,8 +5,8 @@ from typing import Literal from ._compat import TypeAlias -from ._utils import is_dataclass, is_pydantic_model -from .errors import PYD001, PYD002, PYD010, Error +from ._utils import extract_annotations, is_dataclass, is_function, is_name, is_pydantic_model +from .errors import PYD001, PYD002, PYD003, PYD004, PYD005, PYD010, Error ClassType: TypeAlias = Literal["pydantic_model", "dataclass", "other_class"] @@ -35,10 +35,7 @@ def _check_pyd_001(self, node: ast.AnnAssign) -> None: if ( self.current_class in {"pydantic_model", "dataclass"} and isinstance(node.value, ast.Call) - and ( - (isinstance(node.value.func, ast.Name) and node.value.func.id == "Field") - or (isinstance(node.value.func, ast.Attribute) and node.value.func.attr == "Field") - ) + and is_function(node.value, "Field") and len(node.value.args) >= 1 ): self.errors.append(PYD001.from_node(node)) @@ -55,6 +52,49 @@ def _check_pyd_002(self, node: ast.ClassDef) -> None: for assignment in invalid_assignments: self.errors.append(PYD002.from_node(assignment)) + def _check_pyd_003(self, node: ast.AnnAssign) -> None: + if ( + self.current_class in {"pydantic_model", "dataclass"} + and isinstance(node.value, ast.Call) + and is_function(node.value, "Field") + and len(node.value.keywords) == 1 + and node.value.keywords[0].arg == "default" + ): + self.errors.append(PYD003.from_node(node)) + + def _check_pyd_004(self, node: ast.AnnAssign) -> None: + if ( + self.current_class in {"pydantic_model", "dataclass"} + and isinstance(node.annotation, ast.Subscript) + and is_name(node.annotation.value, "Annotated") + and isinstance(node.annotation.slice, ast.Tuple) + ): + field_call = next( + ( + elt + for elt in node.annotation.slice.elts + if isinstance(elt, ast.Call) + and is_function(elt, "Field") + and any(k.arg == "default" for k in elt.keywords) + ), + None, + ) + if field_call is not None: + self.errors.append(PYD004.from_node(node)) + + def _check_pyd_005(self, node: ast.ClassDef) -> None: + if self.current_class in {"pydantic_model", "dataclass"}: + previous_targets: set[str] = set() + + for stmt in node.body: + if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name): + # TODO only add before if AnnAssign? + # the following seems to work: + # date: date + previous_targets.add(stmt.target.id) + if previous_targets & extract_annotations(stmt.annotation): + self.errors.append(PYD005.from_node(stmt)) + def _check_pyd_010(self, node: ast.ClassDef) -> None: if self.current_class == "other_class": for stmt in node.body: @@ -74,10 +114,13 @@ def _check_pyd_010(self, node: ast.ClassDef) -> None: def visit_ClassDef(self, node: ast.ClassDef) -> None: self.enter_class(node) self._check_pyd_002(node) + self._check_pyd_005(node) self._check_pyd_010(node) self.generic_visit(node) self.leave_class() def visit_AnnAssign(self, node: ast.AnnAssign) -> None: self._check_pyd_001(node) + self._check_pyd_003(node) + self._check_pyd_004(node) self.generic_visit(node) diff --git a/tests/rules/test_pyd001.py b/tests/rules/test_pyd001.py index d9645d4..d664cab 100644 --- a/tests/rules/test_pyd001.py +++ b/tests/rules/test_pyd001.py @@ -20,7 +20,7 @@ class Model: PYD001_OK = """ class Model(BaseModel): - a: int = Field(default=1) + a: int = Field(default=1, description="") """ diff --git a/tests/rules/test_pyd003.py b/tests/rules/test_pyd003.py new file mode 100644 index 0000000..71e611f --- /dev/null +++ b/tests/rules/test_pyd003.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +import ast + +import pytest + +from flake8_pydantic.errors import PYD003, Error +from flake8_pydantic.visitor import Visitor + +PYD003_NOT_OK = """ +class Model(BaseModel): + a: int = Field(default=1) +""" + +PYD003_OK = """ +class Model(BaseModel): + a: int = Field(default=1, description="") +""" + + +@pytest.mark.parametrize( + ["source", "expected"], + [ + (PYD003_NOT_OK, [PYD003(3, 4)]), + (PYD003_OK, []), + ], +) +def test_pyd003(source: str, expected: list[Error]) -> None: + module = ast.parse(source) + visitor = Visitor() + visitor.visit(module) + + assert visitor.errors == expected diff --git a/tests/rules/test_pyd004.py b/tests/rules/test_pyd004.py new file mode 100644 index 0000000..972e381 --- /dev/null +++ b/tests/rules/test_pyd004.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +import ast + +import pytest + +from flake8_pydantic.errors import PYD004, Error +from flake8_pydantic.visitor import Visitor + +PYD004_1 = """ +class Model(BaseModel): + a: Annotated[int, Field(default=1, description="")] +""" + +PYD004_2 = """ +class Model(BaseModel): + a: Annotated[int, Unrelated(), Field(default=1)] +""" + + +@pytest.mark.parametrize( + ["source", "expected"], + [ + (PYD004_1, [PYD004(3, 4)]), + (PYD004_2, [PYD004(3, 4)]), + ], +) +def test_pyd004(source: str, expected: list[Error]) -> None: + module = ast.parse(source) + visitor = Visitor() + visitor.visit(module) + + assert visitor.errors == expected diff --git a/tests/rules/test_pyd005.py b/tests/rules/test_pyd005.py new file mode 100644 index 0000000..ca22195 --- /dev/null +++ b/tests/rules/test_pyd005.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +import ast + +import pytest + +from flake8_pydantic.errors import PYD005, Error +from flake8_pydantic.visitor import Visitor + +PYD005_1 = """ +class Model(BaseModel): + date: date +""" + +PYD005_2 = """ +class Model(BaseModel): + date: dict[str, date] +""" + +PYD005_3 = """ +class Model(BaseModel): + date: Annotated[list[date], ...] +""" + +PYD005_4 = """ +class Model(BaseModel): + date: int = 1 + foo: date +""" + +PYD005_5 = """ +class Model(BaseModel): + date: Union[date, None] = None +""" + +PYD005_6 = """ +class Model(BaseModel): + date: int | date | None = None +""" + +# OK: + +PYD005_7 = """ +class Model(BaseModel): + foo: date | None = None + date: int +""" + + +@pytest.mark.parametrize( + ["source", "expected"], + [ + (PYD005_1, [PYD005(3, 4)]), + (PYD005_2, [PYD005(3, 4)]), + (PYD005_3, [PYD005(3, 4)]), + (PYD005_4, [PYD005(4, 4)]), + (PYD005_5, [PYD005(3, 4)]), + (PYD005_6, [PYD005(3, 4)]), + (PYD005_7, []), + ], +) +def test_pyd005(source: str, expected: list[Error]) -> None: + module = ast.parse(source) + visitor = Visitor() + visitor.visit(module) + + assert visitor.errors == expected diff --git a/tox.ini b/tox.ini index 66f5113..08e82f5 100644 --- a/tox.ini +++ b/tox.ini @@ -1,11 +1,10 @@ [tox] requires = tox>4 -envlist = py3{8,9,10,11,12} +envlist = py3{9,10,11,12} [gh-actions] python = - 3.8: py38 3.9: py39 3.10: py310 3.11: py311