bito-code-review[bot] commented on code in PR #36433:
URL: https://github.com/apache/superset/pull/36433#discussion_r2590612353
##########
scripts/check-dependencies.py:
##########
@@ -0,0 +1,284 @@
+#!/usr/bin/env python3
+"""
+Script to check for unused dependencies in Superset frontend
+"""
+
+import json
+import os
+import subprocess
+import sys
+from pathlib import Path
+from typing import Dict, List, Set, Tuple
+import re
+
+class DependencyChecker:
+ def __init__(self, base_path: str = "."):
+ self.base_path = Path(base_path)
+ self.package_json_path = self.base_path / "package.json"
+ self.package_lock_path = self.base_path / "package-lock.json"
+ self.src_paths = [
+ self.base_path / "src",
+ self.base_path / "packages",
+ self.base_path / "plugins",
+ ]
+
+ def load_package_json(self) -> Dict:
+ """Load and return package.json content"""
+ with open(self.package_json_path, 'r') as f:
+ return json.load(f)
+
+ def get_all_dependencies(self) -> Dict[str, str]:
+ """Get all dependencies from package.json"""
+ package_data = self.load_package_json()
+ deps = {}
+
+ for dep_type in ['dependencies', 'devDependencies',
'peerDependencies']:
+ if dep_type in package_data:
+ deps.update(package_data[dep_type])
+
+ return deps
+
+ def search_for_imports(self, package_name: str) -> Tuple[bool, List[str]]:
+ """Search for imports of a package in the codebase"""
+ patterns = [
+ f"from ['\"]{{}}['\"]",
+ f"require\\(['\"]{{}}['\"]",
+ f"import.*['\"]{{}}['\"]",
+ f"from ['\"]{{}}/.+['\"]", # Subpath imports
+ f"require\\(['\"]{{}}/.+['\"]", # Subpath requires
+ ]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete Import Detection</b></div>
<div id="fix">
The script misses dynamic imports like import('package'), which are common
in the frontend for lazy loading. This could lead to false positives for unused
dependencies.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
patterns = [
f"from ['\"]{{}}['\"]",
f"require\\(['\"]{{}}['\"]",
f"import.*['\"]{{}}['\"]",
f"from ['\"]{{}}/.+['\"]", # Subpath imports
f"require\\(['\"]{{}}/.+['\"]", # Subpath requires
f"import\\(['\"]{{}}['\"]", # Dynamic imports
f"import\\(['\"]{{}}/.+['\"]", # Dynamic subpath imports
]
````
</div>
</details>
</div>
<small><i>Code Review Run #b0d8a2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
scripts/check-dependencies.py:
##########
@@ -0,0 +1,284 @@
+#!/usr/bin/env python3
+"""
+Script to check for unused dependencies in Superset frontend
+"""
+
+import json
+import os
+import subprocess
+import sys
+from pathlib import Path
+from typing import Dict, List, Set, Tuple
+import re
+
+class DependencyChecker:
+ def __init__(self, base_path: str = "."):
+ self.base_path = Path(base_path)
+ self.package_json_path = self.base_path / "package.json"
+ self.package_lock_path = self.base_path / "package-lock.json"
+ self.src_paths = [
+ self.base_path / "src",
+ self.base_path / "packages",
+ self.base_path / "plugins",
+ ]
+
+ def load_package_json(self) -> Dict:
+ """Load and return package.json content"""
+ with open(self.package_json_path, 'r') as f:
+ return json.load(f)
+
+ def get_all_dependencies(self) -> Dict[str, str]:
+ """Get all dependencies from package.json"""
+ package_data = self.load_package_json()
+ deps = {}
+
+ for dep_type in ['dependencies', 'devDependencies',
'peerDependencies']:
+ if dep_type in package_data:
+ deps.update(package_data[dep_type])
+
+ return deps
+
+ def search_for_imports(self, package_name: str) -> Tuple[bool, List[str]]:
+ """Search for imports of a package in the codebase"""
+ patterns = [
+ f"from ['\"]{{}}['\"]",
+ f"require\\(['\"]{{}}['\"]",
+ f"import.*['\"]{{}}['\"]",
+ f"from ['\"]{{}}/.+['\"]", # Subpath imports
+ f"require\\(['\"]{{}}/.+['\"]", # Subpath requires
+ ]
+
+ found_files = []
+
+ for src_path in self.src_paths:
+ if not src_path.exists():
+ continue
+
+ for pattern in patterns:
+ search_pattern = pattern.format(re.escape(package_name))
+ try:
+ # Use ripgrep for faster searching
+ result = subprocess.run(
+ ["rg", "-l", search_pattern, str(src_path)],
+ capture_output=True,
+ text=True
+ )
+ if result.returncode == 0 and result.stdout:
+ files = result.stdout.strip().split('\n')
+ found_files.extend(files)
+ except FileNotFoundError:
+ # Fallback to grep if ripgrep not available
+ result = subprocess.run(
+ ["grep", "-r", "-l", "-E", search_pattern,
str(src_path)],
+ capture_output=True,
+ text=True
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Subprocess security and check argument issues</b></div>
<div id="fix">
Subprocess call to `grep` lacks explicit `check` argument (PLW1510) and has
security concerns (S603, S607). Add `check=False` and use full path for
executables.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
result = subprocess.run(
["grep", "-r", "-l", "-E", search_pattern,
str(src_path)],
capture_output=True,
text=True,
check=False,
````
</div>
</details>
</div>
<small><i>Code Review Run #b0d8a2</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
