Skip to content

Commit

Permalink
Add checker for relationship backref usage (#36)
Browse files Browse the repository at this point in the history
* Add checker for relationship backref usage

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix line length in relationship_backref_checker.py

Refactors line lengths in `flake8_sqlalchemy/checkers/relationship_backref_checker.py` to comply with the 88 character limit.

- Breaks long lines in docstrings for the `run` and `is_relationship_with_backref` methods to ensure compliance with the project's line length restriction.
- Maintains readability and code structure while adhering to the `.flake8` configuration.


---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/miketheman/flake8-sqlalchemy/pull/36?shareId=6c56ba85-5a9f-4269-b767-acc03d66e380).

* Document the SQA300 check for backref usage

Adds documentation for the `SQA300` check in the README.md file.

- Introduces a new section for `SQA300` to guide users on using `back_populates` instead of `backref` in SQLAlchemy relationships.
- Provides examples of incorrect usage with `backref` and correct usage with `back_populates`, following the existing documentation structure for other checks.


---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/miketheman/flake8-sqlalchemy/pull/36?shareId=a6468ad9-2d08-40d2-9bc2-8ee40950ccfd).

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
miketheman and pre-commit-ci[bot] authored Jun 15, 2024
1 parent d50f534 commit 1dda900
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 1 deletion.
28 changes: 28 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,34 @@ class Users(Base):
name = mapped_column(String, comment="User name: first, middle, last")
```

### `SQA300` - Use `back_populates` instead of `backref` in relationship

Encourages the use of `back_populates` instead of `backref` in SQLAlchemy relationships to ensure clarity and consistency in bidirectional relationships.

#### Bad

```python
class Parent(Base):
__tablename__ = "parent"
id = Column(Integer, primary_key=True)
children = relationship("Child", backref="parent")
```

#### Good

```python
class Parent(Base):
__tablename__ = "parent"
id = Column(Integer, primary_key=True)
children = relationship("Child", back_populates="parent")

class Child(Base):
__tablename__ = "child"
id = Column(Integer, primary_key=True)
parent_id = Column(Integer, ForeignKey("parent.id"))
parent = relationship("Parent", back_populates="children")
```

## License

This project is licensed under the terms of the MIT license.
Expand Down
2 changes: 2 additions & 0 deletions flake8_sqlalchemy/checkers/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from .column_comment import ColumnCommentChecker
from .import_alias import ImportAliasChecker
from .relationship_backref_checker import RelationshipBackrefChecker

__all__ = (
"ColumnCommentChecker",
"ImportAliasChecker",
"RelationshipBackrefChecker",
)
41 changes: 41 additions & 0 deletions flake8_sqlalchemy/checkers/relationship_backref_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""Checker for detecting `backref` usage in SQLAlchemy relationships."""

import ast
from typing import List

from ..issue import Issue
from ._base import Checker


class SQA300(Issue):
code = "SQA300"
message = f"{code} Use `back_populates` instead of `backref` in relationship"


class RelationshipBackrefChecker(Checker):
def run(self, node: ast.Call) -> List[Issue]:
"""
Checks if a relationship() call uses `backref` and suggests
`back_populates` instead.
"""
issues: List[Issue] = []

if self.is_relationship_with_backref(node):
issues.append(SQA300(node.lineno, node.col_offset))

return issues

@staticmethod
def is_relationship_with_backref(node: ast.Call) -> bool:
"""
Determines if the given node represents a relationship() call with a
`backref` argument.
"""
if RelationshipBackrefChecker.get_call_name(node) != "relationship":
return False

for keyword in node.keywords:
if keyword.arg == "backref":
return True

return False
7 changes: 6 additions & 1 deletion flake8_sqlalchemy/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,19 @@
import importlib.metadata
from typing import Any, Dict, Generator, List, Tuple, Type

from .checkers import ColumnCommentChecker, ImportAliasChecker
from .checkers import (
ColumnCommentChecker,
ImportAliasChecker,
RelationshipBackrefChecker,
)
from .issue import Issue


class Visitor(ast.NodeVisitor):
checkers: Dict[str, List[Any]] = {
"Call": [
ColumnCommentChecker(),
RelationshipBackrefChecker(),
],
"Import": [
ImportAliasChecker(),
Expand Down
30 changes: 30 additions & 0 deletions tests/checkers/test_relationship_backref_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
from textwrap import dedent


def test_relationship_backref_passes(helpers):
sample_code = """
from sqlalchemy.orm import relationship
class Parent:
children = relationship("Child", back_populates="parent")
class Child:
parent = relationship("Parent", back_populates="children")
"""
assert helpers.results(dedent(sample_code)) == set()


def test_relationship_backref_fails(helpers):
sample_code = """
from sqlalchemy.orm import relationship
class Parent:
children = relationship("Child", backref="parent")
class Child:
parent = relationship("Parent", backref="children")
"""
assert helpers.results(dedent(sample_code)) == {
"5:16 SQA300 Use `back_populates` instead of `backref` in relationship",
"8:14 SQA300 Use `back_populates` instead of `backref` in relationship",
}

0 comments on commit 1dda900

Please sign in to comment.