Migrate from setup.py and setup.cfg to pyproject.toml

src/django_migration_linter/sql_analyser/__init__.py

@@ -0,0 +1,8 @@
+from __future__ import annotations
+
+from .base import BaseAnalyser  # noqa
+from .mysql import MySqlAnalyser  # noqa
+from .postgresql import PostgresqlAnalyser  # noqa
+from .sqlite import SqliteAnalyser  # noqa
+
+from .analyser import analyse_sql_statements, get_sql_analyser_class  # noqa isort:skip

src/django_migration_linter/sql_analyser/analyser.py

@@ -0,0 +1,70 @@
+from __future__ import annotations
+
+import logging
+from typing import TYPE_CHECKING, Iterable, Type
+
+if TYPE_CHECKING:
+    from sql_analyser.base import Issue
+
+from django_migration_linter.sql_analyser import (
+    BaseAnalyser,
+    MySqlAnalyser,
+    PostgresqlAnalyser,
+    SqliteAnalyser,
+)
+
+logger = logging.getLogger("django_migration_linter")
+
+ANALYSER_STRING_MAPPING: dict[str, Type[BaseAnalyser]] = {
+    "sqlite": SqliteAnalyser,
+    "mysql": MySqlAnalyser,
+    "postgresql": PostgresqlAnalyser,
+}
+
+
+def get_sql_analyser_class(
+    database_vendor: str, analyser_string: str | None = None
+) -> Type[BaseAnalyser]:
+    if analyser_string:
+        return get_sql_analyser_from_string(analyser_string)
+    return get_sql_analyser_class_from_db_vendor(database_vendor)
+
+
+def get_sql_analyser_from_string(analyser_string: str) -> Type[BaseAnalyser]:
+    if analyser_string not in ANALYSER_STRING_MAPPING:
+        raise ValueError(
+            "Unknown SQL analyser '{}'. Known values: '{}'".format(
+                analyser_string,
+                "','".join(ANALYSER_STRING_MAPPING.keys()),
+            )
+        )
+    return ANALYSER_STRING_MAPPING[analyser_string]
+
+
+def get_sql_analyser_class_from_db_vendor(database_vendor: str) -> Type[BaseAnalyser]:
+    sql_analyser_class: Type[BaseAnalyser]
+    if "mysql" in database_vendor:
+        sql_analyser_class = MySqlAnalyser
+    elif "postgre" in database_vendor:
+        sql_analyser_class = PostgresqlAnalyser
+    elif "sqlite" in database_vendor:
+        sql_analyser_class = SqliteAnalyser
+    else:
+        raise ValueError(
+            "Unsupported database vendor '{}'. Try specifying an SQL analyser.".format(
+                database_vendor
+            )
+        )
+
+    logger.debug("Chosen SQL analyser class: %s", sql_analyser_class)
+    return sql_analyser_class
+
+
+def analyse_sql_statements(
+    sql_analyser_class: Type[BaseAnalyser],
+    sql_statements: list[str],
+    exclude_migration_tests: Iterable[str] | None = None,
+) -> tuple[list[Issue], list[Issue], list[Issue]]:
+    sql_analyser = sql_analyser_class(exclude_migration_tests)
+    sql_analyser.analyse(sql_statements)
+    return sql_analyser.errors, sql_analyser.ignored, sql_analyser.warnings

src/django_migration_linter/sql_analyser/base.py

@@ -0,0 +1,232 @@
+from __future__ import annotations
+
+import logging
+import re
+from copy import deepcopy
+from dataclasses import dataclass
+from enum import Enum
+from typing import Callable, Iterable
+
+logger = logging.getLogger("django_migration_linter")
+
+
+def find_check_from_code(checks: list[Check], code: str) -> Check | None:
+    return next((c for c in checks if c.code == code), None)
+
+
+def update_migration_checks(
+    base_checks: list[Check], specific_checks: list[Check]
+) -> list[Check]:
+    new_checks = deepcopy(base_checks)
+    for override_check in specific_checks:
+        migration_check = find_check_from_code(new_checks, override_check.code)
+
+        if migration_check is None or not override_check.code:
+            migration_check = deepcopy(override_check)
+            new_checks.append(migration_check)
+
+        for field in override_check.__dataclass_fields__:
+            setattr(migration_check, field, getattr(override_check, field))
+    return new_checks
+
+
+def has_not_null_column(sql_statements: list[str], **kwargs) -> bool:
+    # TODO: improve to detect that the same column is concerned
+    ends_with_default = False
+    for sql in sql_statements:
+        if "SET DEFAULT" in sql:
+            ends_with_default = True
+        elif "DROP DEFAULT" in sql:
+            ends_with_default = False
+    return (
+        any(
+            re.search("(?<!DROP )NOT NULL", sql)
+            and not (sql.startswith("CREATE TABLE") or sql.startswith("CREATE INDEX"))
+            for sql in sql_statements
+        )
+        and ends_with_default is False
+    )
+
+
+def has_add_unique(sql_statements: list[str], **kwargs) -> bool:
+    regex_result = None
+    for sql in sql_statements:
+        regex_result = re.search(
+            "ALTER TABLE (.*) ADD CONSTRAINT .* UNIQUE", sql
+        ) or re.search('CREATE UNIQUE INDEX .* ON (".*?")', sql)
+        if regex_result:
+            break
+    if not regex_result:
+        return False
+
+    concerned_table = regex_result.group(1)
+    table_is_added_in_transaction = any(
+        sql.startswith(f"CREATE TABLE {concerned_table}") for sql in sql_statements
+    )
+    return not table_is_added_in_transaction
+
+
+class CheckMode(Enum):
+    """
+    Defines whether the Check.fn gets a str or a list[str] as first parameter.
+    """
+
+    ONE_LINER = 1
+    TRANSACTION = 2
+
+
+class CheckType(Enum):
+    ERROR = 1
+    WARNING = 2
+
+
+@dataclass
+class Check:
+    """
+    Represents a check that will be done against SQL statement(s).
+    """
+
+    code: str
+    fn: Callable
+    message: str
+    mode: CheckMode
+    type: CheckType
+
+
+@dataclass
+class Issue:
+    code: str
+    message: str
+    table: str | None = None
+    column: str | None = None
+
+
+class BaseAnalyser:
+    base_migration_checks: list[Check] = [
+        Check(
+            code="RENAME_TABLE",
+            fn=lambda sql, **kw: re.search("RENAME TABLE", sql)
+            or re.search("ALTER TABLE .* RENAME TO", sql),
+            message="RENAMING tables",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="NOT_NULL",
+            fn=has_not_null_column,
+            message="NOT NULL constraint on columns",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="DROP_COLUMN",
+            fn=lambda sql, **kw: re.search("DROP COLUMN", sql),
+            message="DROPPING columns",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="DROP_TABLE",
+            fn=lambda sql, **kw: sql.startswith("DROP TABLE"),
+            message="DROPPING table",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="RENAME_COLUMN",
+            fn=lambda sql, **kw: re.search("ALTER TABLE .* CHANGE", sql)
+            or re.search("ALTER TABLE .* RENAME COLUMN", sql),
+            message="RENAMING columns",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="ALTER_COLUMN",
+            fn=lambda sql, **kw: re.search("ALTER TABLE .* ALTER COLUMN .* TYPE", sql),
+            message=(
+                "ALTERING columns (Could be backward compatible. "
+                "You may ignore this migration.)"
+            ),
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="ADD_UNIQUE",
+            fn=has_add_unique,
+            message="ADDING unique constraint",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.ERROR,
+        ),
+    ]
+
+    migration_checks: list[Check] = []
+
+    def __init__(self, exclude_migration_tests: Iterable[str] | None):
+        self.exclude_migration_tests: Iterable[str] = exclude_migration_tests or []
+        self.errors: list[Issue] = []
+        self.warnings: list[Issue] = []
+        self.ignored: list[Issue] = []
+        self.migration_checks = update_migration_checks(
+            self.base_migration_checks, self.migration_checks
+        )
+
+    def analyse(self, sql_statements: list[str]) -> None:
+        for statement in sql_statements:
+            for test in self.one_line_migration_checks:
+                self._check_sql(test, sql=statement)
+
+        for test in self.transaction_migration_checks:
+            self._check_sql(test, sql=sql_statements)
+
+    @property
+    def one_line_migration_checks(self) -> Iterable[Check]:
+        return (c for c in self.migration_checks if c.mode == CheckMode.ONE_LINER)
+
+    @property
+    def transaction_migration_checks(self) -> Iterable[Check]:
+        return (c for c in self.migration_checks if c.mode == CheckMode.TRANSACTION)
+
+    def _check_sql(self, check: Check, sql: list[str] | str) -> None:
+        if check.fn(sql, errors=self.errors):
+            if check.code in self.exclude_migration_tests:
+                action = "IGNORED"
+                list_to_add = self.ignored
+            elif check.type == CheckType.WARNING:
+                action = "WARNING"
+                list_to_add = self.warnings
+            else:
+                action = "ERROR"
+                list_to_add = self.errors
+            logger.debug("Testing %s -- %s", sql, action)
+            issue = self.build_issue(migration_check=check, sql_statement=sql)
+            list_to_add.append(issue)
+        else:
+            logger.debug("Testing %s -- PASSED", sql)
+
+    def build_issue(
+        self, migration_check: Check, sql_statement: list[str] | str
+    ) -> Issue:
+        table = self.detect_table(sql_statement)
+        col = self.detect_column(sql_statement)
+        return Issue(
+            code=migration_check.code,
+            message=migration_check.message,
+            table=table,
+            column=col,
+        )
+
+    @staticmethod
+    def detect_table(sql: list[str] | str) -> str | None:
+        if isinstance(sql, str):
+            regex_result = re.search("TABLE [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+        return None
+
+    @staticmethod
+    def detect_column(sql: list[str] | str) -> str | None:
+        if isinstance(sql, str):
+            regex_result = re.search("COLUMN [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+        return None

src/django_migration_linter/sql_analyser/mysql.py

@@ -0,0 +1,31 @@
+from __future__ import annotations
+
+import re
+
+from .base import BaseAnalyser, Check, CheckMode, CheckType
+
+
+class MySqlAnalyser(BaseAnalyser):
+    migration_checks: list[Check] = [
+        Check(
+            code="ALTER_COLUMN",
+            fn=lambda sql, **kw: re.search("ALTER TABLE .* MODIFY .* (?!NULL);?$", sql),
+            message=(
+                "ALTERING columns (Could be backward compatible. "
+                "You may ignore this migration.)"
+            ),
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+    ]
+
+    @staticmethod
+    def detect_column(sql: list[str] | str) -> str | None:
+        if isinstance(sql, str):
+            regex_result = re.search("COLUMN [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+            regex_result = re.search("MODIFY [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+        return None

src/django_migration_linter/sql_analyser/postgresql.py

@@ -0,0 +1,91 @@
+from __future__ import annotations
+
+import re
+
+from .base import BaseAnalyser, Check, CheckMode, CheckType
+
+
+def has_create_index_in_transaction(sql_statements: list[str], **kwargs) -> bool:
+    """Return if a migration opens a transaction, acquires EXCLUSIVE lock, then indexes.
+
+    Any locks that are obtained after a transaction is opened will not be released
+    until that transaction either commits or rolls back.
+
+    Accordingly, it's extremely important that a migration avoid any long-running
+    queries while exclusive locks are held.
+
+    Building an index is generally a long-running operation. A standard index
+    creation (that is, avoiding use of `CONCURRENTLY`) can be safe to run on
+    tables which are small in size and/or read-heavy. (`CREATE INDEX` only
+    prevents writes to a table; reads are allowed during index creation).
+
+    This check is a stricter version of `CREATE_INDEX` -- if a team wishes
+    to build indices nonconcurrently, it's imperative to be mindful of locks.
+    """
+    if not (sql_statements and sql_statements[0].startswith("BEGIN")):
+        return False
+
+    for i, sql in enumerate(sql_statements):
+        # If any statements acquire an exclusive lock, complain about index creation
+        # later.
+        # Nearly every single `ALTER TABLE` command requires an exclusive lock:
+        #     https://www.postgresql.org/docs/current/sql-altertable.html
+        # (Most common example is `ALTER TABLE... ADD COLUMN`, then later
+        # `CREATE INDEX`)
+        if sql.startswith("ALTER TABLE"):
+            return has_create_index(sql_statements[i + 1 :], ignore_concurrently=False)
+    return False
+
+
+def has_create_index(
+    sql_statements: list[str], ignore_concurrently: bool = True, **kwargs
+) -> bool:
+    regex_result = None
+    for sql in sql_statements:
+        regex_result = re.search(r"CREATE (UNIQUE )?INDEX.*ON (.*) \(", sql)
+        if ignore_concurrently and re.search("INDEX CONCURRENTLY", sql):
+            regex_result = None
+        if regex_result:
+            break
+    if not regex_result:
+        return False
+
+    concerned_table = regex_result.group(2)
+    table_is_added_in_transaction = any(
+        sql.startswith(f"CREATE TABLE {concerned_table}") for sql in sql_statements
+    )
+    return not table_is_added_in_transaction
+
+
+class PostgresqlAnalyser(BaseAnalyser):
+    migration_checks: list[Check] = [
+        Check(
+            code="CREATE_INDEX",
+            fn=has_create_index,
+            message="CREATE INDEX locks table",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.WARNING,
+        ),
+        Check(
+            code="CREATE_INDEX_EXCLUSIVE",
+            fn=has_create_index_in_transaction,
+            message="CREATE INDEX prolongs transaction, delaying lock release",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.WARNING,
+        ),
+        Check(
+            code="DROP_INDEX",
+            fn=lambda sql, **kw: re.search("DROP INDEX", sql)
+            and not re.search("INDEX CONCURRENTLY", sql),
+            message="DROP INDEX locks table",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.WARNING,
+        ),
+        Check(
+            code="REINDEX",
+            fn=lambda sql, **kw: sql.startswith("REINDEX"),
+            message="REINDEX locks table",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.WARNING,
+        ),
+    ]

src/django_migration_linter/sql_analyser/sqlite.py

@@ -0,0 +1,56 @@
+from __future__ import annotations
+
+import re
+
+from .base import BaseAnalyser, Check, CheckMode, CheckType
+
+
+class SqliteAnalyser(BaseAnalyser):
+    migration_checks: list[Check] = [
+        Check(
+            code="RENAME_TABLE",
+            fn=lambda sql, **kw: re.search("ALTER TABLE .* RENAME TO", sql)
+            and "__old" not in sql
+            and "new__" not in sql,
+            message="RENAMING tables",
+            mode=CheckMode.ONE_LINER,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="DROP_TABLE",
+            # TODO: improve to detect that the table names overlap
+            fn=lambda sql_statements, **kw: any(
+                sql.startswith("DROP TABLE") for sql in sql_statements
+            )
+            and not any(sql.startswith("CREATE TABLE") for sql in sql_statements),
+            message="DROPPING table",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.ERROR,
+        ),
+        Check(
+            code="NOT_NULL",
+            fn=lambda sql_statements, **kw: any(
+                re.search("NOT NULL(?! PRIMARY)(?! DEFAULT)", sql)
+                for sql in sql_statements
+            )
+            and any(
+                re.search("ALTER TABLE .* RENAME TO", sql)
+                and ("__old" in sql or "new__" in sql)
+                for sql in sql_statements
+            ),
+            message="NOT NULL constraint on columns",
+            mode=CheckMode.TRANSACTION,
+            type=CheckType.ERROR,
+        ),
+    ]
+
+    @staticmethod
+    def detect_table(sql: list[str] | str) -> str | None:
+        if isinstance(sql, str):
+            regex_result = re.search("TABLE [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+            regex_result = re.search("ON [`\"'](.*?)[`\"']", sql, re.IGNORECASE)
+            if regex_result:
+                return regex_result.group(1)
+        return None