From 04158ac8ea8e57c109d7df5f325b54121ad36b2d Mon Sep 17 00:00:00 2001 From: Escape0707 Date: Tue, 28 Jul 2026 17:52:12 +0900 Subject: [PATCH] ci: replace custom noqa markers with guard ignores (#39679) --- api/controllers/common/session.py | 2 +- api/controllers/console/app/workflow.py | 2 +- api/controllers/console/datasets/datasets.py | 4 +- api/controllers/console/datasets/external.py | 2 +- api/controllers/console/explore/trial.py | 2 +- api/fields/conversation_fields.py | 2 +- api/fields/dataset_fields.py | 2 +- api/fields/document_fields.py | 2 +- api/services/app_service.py | 4 +- .../commands/test_check_no_new_getattr.py | 48 ++++++++++++++++--- dify-agent/src/shellctl/__init__.py | 2 +- dify-agent/src/shellctl/shared/__init__.py | 2 +- scripts/ast_grep_guard.py | 6 +-- scripts/check_no_new_controller_sqlalchemy.py | 4 +- scripts/check_no_new_getattr.py | 4 +- scripts/lint_controller_sqlalchemy.py | 2 +- 16 files changed, 63 insertions(+), 27 deletions(-) diff --git a/api/controllers/common/session.py b/api/controllers/common/session.py index 24b1a8729d3..fdffa46b189 100644 --- a/api/controllers/common/session.py +++ b/api/controllers/common/session.py @@ -52,7 +52,7 @@ def with_session[T, **P, R]( session.commit() return result except Exception: - session.rollback() # noqa: no-new-controller-sqlalchemy decorator owns transaction rollback + session.rollback() # guard-ignore: no-new-controller-sqlalchemy -- decorator owns rollback raise with session_factory.create_session() as session: diff --git a/api/controllers/console/app/workflow.py b/api/controllers/console/app/workflow.py index 6025d02fe39..f934f440b0e 100644 --- a/api/controllers/console/app/workflow.py +++ b/api/controllers/console/app/workflow.py @@ -317,7 +317,7 @@ class _WorkflowResponseSource: self._session = session def __getattr__(self, name: str) -> object: - return getattr(self._workflow, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self._workflow, name) # guard-ignore: no-new-getattr -- delegates model fields @property def created_by_account(self) -> Account | None: diff --git a/api/controllers/console/datasets/datasets.py b/api/controllers/console/datasets/datasets.py index 2ec5a9fe103..19a6c4dc0dc 100644 --- a/api/controllers/console/datasets/datasets.py +++ b/api/controllers/console/datasets/datasets.py @@ -218,7 +218,7 @@ class _DatasetQueryResponseSource: return self.query.get_queries(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.query, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.query, name) # guard-ignore: no-new-getattr -- delegates model fields class DatasetQueryListResponse(ResponseModel): @@ -257,7 +257,7 @@ class _RelatedAppResponseSource: return self.app.mode_compatible_with_agent_with_session(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.app, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.app, name) # guard-ignore: no-new-getattr -- delegates model fields class RelatedAppListResponse(ResponseModel): diff --git a/api/controllers/console/datasets/external.py b/api/controllers/console/datasets/external.py index 94efe388561..55867e9cbbb 100644 --- a/api/controllers/console/datasets/external.py +++ b/api/controllers/console/datasets/external.py @@ -106,7 +106,7 @@ class ExternalKnowledgeApiResponseSource: return self.external_knowledge_api.get_dataset_bindings(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.external_knowledge_api, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.external_knowledge_api, name) # guard-ignore: no-new-getattr -- delegates model fields def external_knowledge_api_response( diff --git a/api/controllers/console/explore/trial.py b/api/controllers/console/explore/trial.py index 553e65202ce..d0823f178cb 100644 --- a/api/controllers/console/explore/trial.py +++ b/api/controllers/console/explore/trial.py @@ -404,7 +404,7 @@ class TrialWorkflowResponseSource: return self.workflow.get_tool_published(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.workflow, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.workflow, name) # guard-ignore: no-new-getattr -- delegates model fields register_schema_models( diff --git a/api/fields/conversation_fields.py b/api/fields/conversation_fields.py index 073305d2dd9..7612d09f58d 100644 --- a/api/fields/conversation_fields.py +++ b/api/fields/conversation_fields.py @@ -34,7 +34,7 @@ class _SessionResponseSource[SourceT]: self._session = session def __getattr__(self, name: str) -> object: - return getattr(self._source, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self._source, name) # guard-ignore: no-new-getattr -- delegates model fields class _FeedbackResponseSource(_SessionResponseSource[MessageFeedback]): diff --git a/api/fields/dataset_fields.py b/api/fields/dataset_fields.py index 4846aa9689c..c81fb79df3f 100644 --- a/api/fields/dataset_fields.py +++ b/api/fields/dataset_fields.py @@ -227,7 +227,7 @@ class DatasetDetailResponseSource: return self.dataset.get_total_available_documents(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.dataset, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.dataset, name) # guard-ignore: no-new-getattr -- delegates model fields def dataset_detail_response_source(dataset: Any, *, session: Session) -> DatasetDetailResponseSource: diff --git a/api/fields/document_fields.py b/api/fields/document_fields.py index aa3b4135ec6..16cc49541b7 100644 --- a/api/fields/document_fields.py +++ b/api/fields/document_fields.py @@ -90,7 +90,7 @@ class DocumentWithSession: return self.document.get_doc_metadata_details(session=self.session) def __getattr__(self, name: str) -> Any: - return getattr(self.document, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self.document, name) # guard-ignore: no-new-getattr -- delegates model fields def document_response(document: Document, *, session: Session) -> DocumentResponse: diff --git a/api/services/app_service.py b/api/services/app_service.py index a858163e2fb..6faa88eb114 100644 --- a/api/services/app_service.py +++ b/api/services/app_service.py @@ -114,7 +114,7 @@ class AppModelConfigResponseView: self._session = session def __getattr__(self, name: str) -> Any: - return getattr(self._app_model_config, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self._app_model_config, name) # guard-ignore: no-new-getattr -- delegates model fields @property def annotation_reply_dict(self) -> Any: @@ -129,7 +129,7 @@ class AppResponseView: self._session = session def __getattr__(self, name: str) -> Any: - return getattr(self._app, name) # noqa: no-new-getattr response adapter delegates model fields + return getattr(self._app, name) # guard-ignore: no-new-getattr -- delegates model fields @property def desc_or_prompt(self) -> str: diff --git a/api/tests/unit_tests/commands/test_check_no_new_getattr.py b/api/tests/unit_tests/commands/test_check_no_new_getattr.py index 4efcb37da92..a63569c706f 100644 --- a/api/tests/unit_tests/commands/test_check_no_new_getattr.py +++ b/api/tests/unit_tests/commands/test_check_no_new_getattr.py @@ -85,6 +85,42 @@ def main_branch_rev(repo: Path) -> str: return git(repo, "rev-parse", "main") +@pytest.mark.parametrize( + ("source_line", "rule_id"), + [ + ( + "value = getattr(module, name) # guard-ignore: no-new-getattr -- lazy export proxy", + "no-new-getattr", + ), + ( + "session.rollback() # guard-ignore: no-new-controller-sqlalchemy -- decorator owns rollback", + "no-new-controller-sqlalchemy", + ), + ], +) +def test_has_reasoned_guard_ignore_accepts_custom_rules(source_line: str, rule_id: str) -> None: + module = load_guard_module() + + assert module.has_reasoned_guard_ignore(source_line, rule_id) + + +@pytest.mark.parametrize( + ("source_line", "rule_id"), + [ + ("value = getattr(module, name) # noqa: no-new-getattr legacy marker", "no-new-getattr"), + ("value = getattr(module, name) # guard-ignore: no-new-getattr", "no-new-getattr"), + ( + "value = getattr(module, name) # guard-ignore: another-rule -- wrong rule", + "no-new-getattr", + ), + ], +) +def test_has_reasoned_guard_ignore_rejects_invalid_markers(source_line: str, rule_id: str) -> None: + module = load_guard_module() + + assert not module.has_reasoned_guard_ignore(source_line, rule_id) + + def test_resolve_ast_grep_command_prefers_ast_grep(monkeypatch: pytest.MonkeyPatch) -> None: module = load_guard_module() monkeypatch.setattr( @@ -776,7 +812,7 @@ def test_modified_hunk_with_increased_getattr_count_fails(tmp_path: Path) -> Non assert "net-new getattr" in result.stderr -def test_inline_noqa_suppression_with_explanatory_text_skips_added_getattr(tmp_path: Path) -> None: +def test_inline_guard_ignore_with_explanatory_text_skips_added_getattr(tmp_path: Path) -> None: init_repo(tmp_path) write_repo_file( tmp_path, @@ -795,20 +831,20 @@ def test_inline_noqa_suppression_with_explanatory_text_skips_added_getattr(tmp_p "pkg/existing.py", """ def read_value(obj): - return getattr(obj, "dynamic_name", None) # noqa: no-new-getattr needed for plugin-defined attributes + return getattr(obj, "dynamic_name", None) # guard-ignore: no-new-getattr -- plugin-defined attributes """, ) commit_all(tmp_path, "add suppressed getattr") result = run_script(tmp_path, "--base-rev", base_rev) - assert "no-new-getattr needed for plugin-defined attributes" in (tmp_path / "pkg/existing.py").read_text( + assert "guard-ignore: no-new-getattr -- plugin-defined attributes" in (tmp_path / "pkg/existing.py").read_text( encoding="utf-8" ) assert result.returncode == 0, stderr_lines(result) -def test_inline_noqa_without_explanatory_text_is_not_sufficient(tmp_path: Path) -> None: +def test_inline_guard_ignore_without_explanatory_text_is_not_sufficient(tmp_path: Path) -> None: init_repo(tmp_path) write_repo_file( tmp_path, @@ -827,10 +863,10 @@ def test_inline_noqa_without_explanatory_text_is_not_sufficient(tmp_path: Path) "pkg/existing.py", """ def read_value(obj): - return getattr(obj, "dynamic_name", None) # noqa: no-new-getattr + return getattr(obj, "dynamic_name", None) # guard-ignore: no-new-getattr """, ) - commit_all(tmp_path, "add bare noqa getattr") + commit_all(tmp_path, "add bare guard ignore getattr") result = run_script(tmp_path, "--base-rev", base_rev) diff --git a/dify-agent/src/shellctl/__init__.py b/dify-agent/src/shellctl/__init__.py index 3f2237dc690..cb294b365cf 100644 --- a/dify-agent/src/shellctl/__init__.py +++ b/dify-agent/src/shellctl/__init__.py @@ -115,7 +115,7 @@ def __getattr__(name: str) -> Any: if name not in _EXPORTS: raise AttributeError(f"module {__name__!r} has no attribute {name!r}") module = import_module(_EXPORTS[name]) - value = getattr(module, name) # noqa: no-new-getattr lazy export proxy + value = getattr(module, name) # guard-ignore: no-new-getattr -- lazy export proxy globals()[name] = value return value diff --git a/dify-agent/src/shellctl/shared/__init__.py b/dify-agent/src/shellctl/shared/__init__.py index 5f235b19d9f..83419e4e691 100644 --- a/dify-agent/src/shellctl/shared/__init__.py +++ b/dify-agent/src/shellctl/shared/__init__.py @@ -173,7 +173,7 @@ def __getattr__(name: str) -> Any: if name not in _EXPORTS: raise AttributeError(f"module {__name__!r} has no attribute {name!r}") module = import_module(_EXPORTS[name]) - value = getattr(module, name) # noqa: no-new-getattr lazy export proxy + value = getattr(module, name) # guard-ignore: no-new-getattr -- lazy export proxy globals()[name] = value return value diff --git a/scripts/ast_grep_guard.py b/scripts/ast_grep_guard.py index 3e46486f24e..9f67bfd6d08 100644 --- a/scripts/ast_grep_guard.py +++ b/scripts/ast_grep_guard.py @@ -245,13 +245,13 @@ def extract_meta_variables(raw_match: dict[str, Any]) -> dict[str, str]: return result -def has_reasoned_noqa(source_line: str, rule_id: str) -> bool: - pattern = re.compile(rf"# noqa: {re.escape(rule_id)}(?:\s+(?P\S.*))?\s*$") +def has_reasoned_guard_ignore(source_line: str, rule_id: str) -> bool: + pattern = re.compile(rf"# guard-ignore: {re.escape(rule_id)} -- (?P\S.*)\s*$") match = pattern.search(source_line) if not match: return False reason = match.group("reason") - return reason is not None and bool(reason.strip()) + return bool(reason.strip()) def collect_hunk_violations( diff --git a/scripts/check_no_new_controller_sqlalchemy.py b/scripts/check_no_new_controller_sqlalchemy.py index bf1bea336b7..3812ce8f762 100644 --- a/scripts/check_no_new_controller_sqlalchemy.py +++ b/scripts/check_no_new_controller_sqlalchemy.py @@ -10,7 +10,7 @@ from __future__ import annotations import re from pathlib import Path -from ast_grep_guard import Match, has_reasoned_noqa, rule_path, run_guard +from ast_grep_guard import Match, has_reasoned_guard_ignore, rule_path, run_guard RULE_ID = "no-new-controller-sqlalchemy" @@ -40,7 +40,7 @@ def is_flask_session_get(match: Match) -> bool: def is_suppressed(match: Match) -> bool: - return has_reasoned_noqa(match.source_line, RULE_ID) + return has_reasoned_guard_ignore(match.source_line, RULE_ID) def is_reportable_match(match: Match) -> bool: diff --git a/scripts/check_no_new_getattr.py b/scripts/check_no_new_getattr.py index e788395c62c..9c2248ecf2a 100644 --- a/scripts/check_no_new_getattr.py +++ b/scripts/check_no_new_getattr.py @@ -3,7 +3,7 @@ from __future__ import annotations -from ast_grep_guard import Match, has_reasoned_noqa, is_python_source_path, rule_path, run_guard +from ast_grep_guard import Match, has_reasoned_guard_ignore, is_python_source_path, rule_path, run_guard RULE_ID = "no-new-getattr" @@ -12,7 +12,7 @@ VIOLATION_MESSAGE = "no-new-getattr net-new getattr() in added code" def is_reportable_match(match: Match) -> bool: - return not has_reasoned_noqa(match.source_line, RULE_ID) + return not has_reasoned_guard_ignore(match.source_line, RULE_ID) def main() -> int: diff --git a/scripts/lint_controller_sqlalchemy.py b/scripts/lint_controller_sqlalchemy.py index 87c9582b17f..bef362a42b4 100644 --- a/scripts/lint_controller_sqlalchemy.py +++ b/scripts/lint_controller_sqlalchemy.py @@ -44,7 +44,7 @@ def parse_args() -> argparse.Namespace: help="Files or directories to scan. Defaults to api/controllers.", ) parser.add_argument("--include-allowed", action="store_true", help="Print allowed flush()/commit() findings.") - parser.add_argument("--include-suppressed", action="store_true", help="Print reasoned noqa suppressions.") + parser.add_argument("--include-suppressed", action="store_true", help="Print reasoned guard suppressions.") parser.add_argument("--summary-only", action="store_true", help="Print counts without per-finding details.") parser.add_argument("--json", action="store_true", help="Emit machine-readable JSON.") parser.add_argument(