From 3f8472ca7bc24a5f5bcabde765e6cd49c61472a5 Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Thu, 24 Jul 2025 09:40:49 -0700 Subject: [PATCH] chore: move some rules from ruff -> pylint (#34292) --- .pre-commit-config.yaml | 3 +- pyproject.toml | 16 +++++++- superset/examples/bart_lines.py | 2 +- superset/examples/birth_names.py | 2 +- superset/examples/multiformat_time_series.py | 2 +- .../examples/supported_charts_dashboard.py | 2 +- superset/extensions/pylint.py | 37 ------------------- .../shared/migrate_viz/query_functions.py | 2 +- ...vert_metric_currencies_from_str_to_json.py | 2 +- .../db_engine_specs/base_engine_spec_tests.py | 6 +-- .../fixtures/dashboard_with_tabs.py | 2 +- .../sql_lab/permalink/api_tests.py | 2 +- .../databases/importers/v1/command_test.py | 2 +- .../commands/report/execute_test.py | 2 +- tests/unit_tests/db_engine_specs/test_base.py | 2 +- .../migrations/shared/catalogs_test.py | 2 +- tests/unit_tests/security/manager_test.py | 2 +- tests/unit_tests/sql_lab_test.py | 2 +- 18 files changed, 34 insertions(+), 56 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f1d2e032ac5..fa0793390ac 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -105,7 +105,8 @@ repos: - | TARGET_BRANCH=${GITHUB_BASE_REF:-master} git fetch origin "$TARGET_BRANCH" - files=$(git diff --name-only --diff-filter=ACM origin/"$TARGET_BRANCH"..HEAD | grep '^superset/.*\.py$' || true) + BASE=$(git merge-base origin/"$TARGET_BRANCH" HEAD) + files=$(git diff --name-only --diff-filter=ACM "$BASE"..HEAD | grep '^superset/.*\.py$' || true) if [ -n "$files" ]; then pylint --rcfile=.pylintrc --load-plugins=superset.extensions.pylint $files else diff --git a/pyproject.toml b/pyproject.toml index b8c5ebb0b8c..0608fbd3764 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -311,15 +311,16 @@ select = [ "Q", "S", "T", + "TID", "W", ] + ignore = [ "S101", "PT006", "T201", "N999", ] - extend-select = ["I"] # Allow fix for all enabled rules (when `--fix`) is provided. @@ -329,6 +330,16 @@ unfixable = [] # Allow unused variables when underscore-prefixed. dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" +[tool.ruff.lint.per-file-ignores] +"scripts/*" = ["TID251"] +"setup.py" = ["TID251"] +"superset/config.py" = ["TID251"] +"superset/cli/update.py" = ["TID251"] +"superset/key_value/types.py" = ["TID251"] +"superset/translations/utils.py" = ["TID251"] +"superset/extensions/__init__.py" = ["TID251"] +"superset/utils/json.py" = ["TID251"] + [tool.ruff.lint.isort] case-sensitive = false combine-as-imports = true @@ -345,6 +356,9 @@ section-order = [ "local-folder" ] +[tool.ruff.lint.flake8-tidy-imports] +banned-api = { json = { msg = "Use superset.utils.json instead" }, simplejson = { msg = "Use superset.utils.json instead" } } + [tool.ruff.format] # Like Black, use double quotes for strings. quote-style = "double" diff --git a/superset/examples/bart_lines.py b/superset/examples/bart_lines.py index 887d32c4736..7d7e71d59ff 100644 --- a/superset/examples/bart_lines.py +++ b/superset/examples/bart_lines.py @@ -23,7 +23,7 @@ from superset import db from superset.sql.parse import Table from superset.utils import json -from ..utils.database import get_example_database +from ..utils.database import get_example_database # noqa: TID252 from .helpers import get_table_connector_registry, read_example_data logger = logging.getLogger(__name__) diff --git a/superset/examples/birth_names.py b/superset/examples/birth_names.py index 619cdc4d601..6f2d9a5ebfd 100644 --- a/superset/examples/birth_names.py +++ b/superset/examples/birth_names.py @@ -31,7 +31,7 @@ from superset.sql.parse import Table from superset.utils import json from superset.utils.core import DatasourceType -from ..utils.database import get_example_database +from ..utils.database import get_example_database # noqa: TID252 from .helpers import ( get_slice_json, get_table_connector_registry, diff --git a/superset/examples/multiformat_time_series.py b/superset/examples/multiformat_time_series.py index 7063b0563bb..1ed46a76dba 100644 --- a/superset/examples/multiformat_time_series.py +++ b/superset/examples/multiformat_time_series.py @@ -25,7 +25,7 @@ from superset.models.slice import Slice from superset.sql.parse import Table from superset.utils.core import DatasourceType -from ..utils.database import get_example_database +from ..utils.database import get_example_database # noqa: TID252 from .helpers import ( get_slice_json, get_table_connector_registry, diff --git a/superset/examples/supported_charts_dashboard.py b/superset/examples/supported_charts_dashboard.py index 04d4e922517..a77fd515d48 100644 --- a/superset/examples/supported_charts_dashboard.py +++ b/superset/examples/supported_charts_dashboard.py @@ -28,7 +28,7 @@ from superset.sql.parse import Table from superset.utils import json from superset.utils.core import DatasourceType -from ..utils.database import get_example_database +from ..utils.database import get_example_database # noqa: TID252 from .helpers import ( get_slice_json, get_table_connector_registry, diff --git a/superset/extensions/pylint.py b/superset/extensions/pylint.py index 27875e2ce70..a4ce23fd358 100644 --- a/superset/extensions/pylint.py +++ b/superset/extensions/pylint.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import os from pathlib import Path from astroid import nodes @@ -22,41 +21,6 @@ from pylint.checkers import BaseChecker from pylint.lint import PyLinter -class JSONLibraryImportChecker(BaseChecker): - name = "disallowed-json-import" - priority = -1 - msgs = { - "C9001": ( - "Disallowed json import used, use superset.utils.json instead", - "disallowed-json-import", - "Used when a disallowed import is used in a specific file.", - ), - } - exclude_files = [ - "setup.py", - "superset/utils/json.py", - "superset/config.py", - "superset/cli/update.py", - "superset/key_value/types.py", - "superset/translations/utils.py", - "superset/extensions/__init__.py", - ] - path_strip_prefix = os.getcwd() + os.sep - - def visit_import(self, node: nodes.Import) -> None: - file = (node.root().file).replace(self.path_strip_prefix, "", 1) - if file not in self.exclude_files: - for module_name, _ in node.names: - if module_name in ["json", "simplejson"]: - self.add_message("disallowed-json-import", node=node) - - def visit_importfrom(self, node: nodes.ImportFrom) -> None: - file = (node.root().file).replace(self.path_strip_prefix, "", 1) - if file not in self.exclude_files: - if node.modname in ["json", "simplejson"]: - self.add_message("disallowed-json-import", node=node) - - class TransactionChecker(BaseChecker): name = "consider-using-transaction" msgs = { @@ -113,6 +77,5 @@ class SQLParsingLibraryImportChecker(BaseChecker): def register(linter: PyLinter) -> None: - linter.register_checker(JSONLibraryImportChecker(linter)) linter.register_checker(SQLParsingLibraryImportChecker(linter)) linter.register_checker(TransactionChecker(linter)) diff --git a/superset/migrations/shared/migrate_viz/query_functions.py b/superset/migrations/shared/migrate_viz/query_functions.py index 736a38c5772..83f35c2b09c 100644 --- a/superset/migrations/shared/migrate_viz/query_functions.py +++ b/superset/migrations/shared/migrate_viz/query_functions.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import json +import json # noqa: TID251 import math from enum import Enum from typing import Any, Dict, List, Optional, Union diff --git a/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py b/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py index 708e7c52dae..deb053c003f 100644 --- a/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py +++ b/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py @@ -22,7 +22,7 @@ Create Date: 2025-06-06 00:39:00.107746 """ -import json +import json # noqa: TID251 import logging from alembic import op diff --git a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py index 8dbc7de194a..7097e5b0971 100644 --- a/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py +++ b/tests/integration_tests/db_engine_specs/base_engine_spec_tests.py @@ -34,15 +34,15 @@ from superset.utils.database import get_example_database from tests.integration_tests.base_tests import SupersetTestCase from tests.integration_tests.test_app import app -from ..fixtures.birth_names_dashboard import ( +from ..fixtures.birth_names_dashboard import ( # noqa: TID252 load_birth_names_dashboard_with_slices, # noqa: F401 load_birth_names_data, # noqa: F401 ) -from ..fixtures.energy_dashboard import ( +from ..fixtures.energy_dashboard import ( # noqa: TID252 load_energy_table_data, # noqa: F401 load_energy_table_with_slice, # noqa: F401 ) -from ..fixtures.pyodbcRow import Row +from ..fixtures.pyodbcRow import Row # noqa: TID252 class SupersetTestCases(SupersetTestCase): diff --git a/tests/integration_tests/fixtures/dashboard_with_tabs.py b/tests/integration_tests/fixtures/dashboard_with_tabs.py index 9d86555fe3f..efe12fd95dc 100644 --- a/tests/integration_tests/fixtures/dashboard_with_tabs.py +++ b/tests/integration_tests/fixtures/dashboard_with_tabs.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import json +import json # noqa: TID251 import pytest diff --git a/tests/integration_tests/sql_lab/permalink/api_tests.py b/tests/integration_tests/sql_lab/permalink/api_tests.py index a67d78989f0..e11dfbc179b 100644 --- a/tests/integration_tests/sql_lab/permalink/api_tests.py +++ b/tests/integration_tests/sql_lab/permalink/api_tests.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -import json +import json # noqa: TID251 from collections.abc import Iterator from typing import Any from uuid import uuid3 diff --git a/tests/unit_tests/commands/databases/importers/v1/command_test.py b/tests/unit_tests/commands/databases/importers/v1/command_test.py index 14c1779aef4..529b43d7343 100644 --- a/tests/unit_tests/commands/databases/importers/v1/command_test.py +++ b/tests/unit_tests/commands/databases/importers/v1/command_test.py @@ -16,7 +16,7 @@ # under the License. import copy -import json +import json # noqa: TID251 from typing import Any from pytest_mock import MockerFixture diff --git a/tests/unit_tests/commands/report/execute_test.py b/tests/unit_tests/commands/report/execute_test.py index 0f245777297..642454de484 100644 --- a/tests/unit_tests/commands/report/execute_test.py +++ b/tests/unit_tests/commands/report/execute_test.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -import json +import json # noqa: TID251 from datetime import datetime from unittest.mock import patch from uuid import UUID diff --git a/tests/unit_tests/db_engine_specs/test_base.py b/tests/unit_tests/db_engine_specs/test_base.py index b9d58c5d864..44ddee0aa91 100644 --- a/tests/unit_tests/db_engine_specs/test_base.py +++ b/tests/unit_tests/db_engine_specs/test_base.py @@ -19,7 +19,7 @@ from __future__ import annotations -import json +import json # noqa: TID251 from textwrap import dedent from typing import Any diff --git a/tests/unit_tests/migrations/shared/catalogs_test.py b/tests/unit_tests/migrations/shared/catalogs_test.py index 29a4a6fb282..dd810cef392 100644 --- a/tests/unit_tests/migrations/shared/catalogs_test.py +++ b/tests/unit_tests/migrations/shared/catalogs_test.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -import json +import json # noqa: TID251 import pytest from pytest_mock import MockerFixture diff --git a/tests/unit_tests/security/manager_test.py b/tests/unit_tests/security/manager_test.py index 91c38940861..f2c958ee0d9 100644 --- a/tests/unit_tests/security/manager_test.py +++ b/tests/unit_tests/security/manager_test.py @@ -17,7 +17,7 @@ # pylint: disable=invalid-name, unused-argument, redefined-outer-name -import json +import json # noqa: TID251 import pytest from flask_appbuilder.security.sqla.models import Role, User diff --git a/tests/unit_tests/sql_lab_test.py b/tests/unit_tests/sql_lab_test.py index 8f1cfbc4d3d..60bc3c37104 100644 --- a/tests/unit_tests/sql_lab_test.py +++ b/tests/unit_tests/sql_lab_test.py @@ -16,7 +16,7 @@ # under the License. # pylint: disable=import-outside-toplevel, invalid-name, unused-argument, too-many-locals -import json +import json # noqa: TID251 from unittest import mock from uuid import UUID