Detect Android and Gradle project types#325
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds centralized Gradle-versus-Android classification for Gradle build files using plugin, version-catalog, and Android DSL detection, then applies it when resolving directories containing both Gradle and Android manifests. ChangesGradle and Android classification
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/fosslight_dependency/run_dependency_scanner.py (1)
92-96: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueDrop the redundant
is_gradlebranch.classify_gradle_build_file()already falls back to(True, False)for every non-Android build file, so_GRADLE_BUILD_PATTERNSnever changes the outcome. If default-to-Gradle is intended, remove the unused check; otherwise the fallback needs to change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_dependency/run_dependency_scanner.py` around lines 92 - 96, Remove the redundant `is_gradle` calculation and conditional from `classify_gradle_build_file()`, returning `(True, False)` directly for non-Android build files if default-to-Gradle behavior is intended; otherwise update the fallback return to reflect the desired classification.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 565-578: Update run_gradle_task to call the module-level
parse_dependency_tree function directly instead of self.parse_dependency_tree,
passing the same dependency output and required arguments. Use the existing
parse_dependency_tree definition and its callers to preserve the expected
parsing behavior and avoid the AttributeError.
- Around line 517-525: Update _insert_plugins_block_after_buildscript to detect
both spaced and unspaced buildscript declarations, such as “buildscript {” and
“buildscript{”, before prepending the plugin block. Reuse or extend
_find_block_end with flexible matching, and only fall back to prepending when no
buildscript block is found.
In `@src/fosslight_dependency/run_dependency_scanner.py`:
- Around line 48-67: Update _ANDROID_BUILD_PATTERNS to match
classify_gradle_build_file’s normalized lowercase, whitespace-stripped input:
lowercase all camel-case markers and remove the redundant "android {" entry,
retaining only "android{".
---
Nitpick comments:
In `@src/fosslight_dependency/run_dependency_scanner.py`:
- Around line 92-96: Remove the redundant `is_gradle` calculation and
conditional from `classify_gradle_build_file()`, returning `(True, False)`
directly for non-Android build files if default-to-Gradle behavior is intended;
otherwise update the fallback return to reflect the desired classification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c5927ce4-79e2-452a-be91-2e449e4b5ca0
📒 Files selected for processing (2)
src/fosslight_dependency/_package_manager.pysrc/fosslight_dependency/run_dependency_scanner.py
| def parse_dependency_tree(self, f_name): | ||
| config = android_config if self.package_manager_name == const.ANDROID else gradle_config | ||
|
|
||
| try: | ||
| for stack, name in create_dep_stack(self, f_name, config): | ||
| self.total_dep_list.append(name) | ||
| if len(stack) == 0: | ||
| self.direct_dep_list.append(name) | ||
| else: | ||
| if stack[-1] not in self.relation_tree: | ||
| self.relation_tree[stack[-1]] = [] | ||
| self.relation_tree[stack[-1]].append(name) | ||
| except Exception as e: | ||
| logger.warning(f'Fail to parse gradle dependency tree:{e}') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether the module-level functions are bound to the class or overridden.
rg -nP '(parse_dependency_tree|create_dep_stack)' src/fosslight_dependency/_package_manager.py
echo '--- class-level rebinding? ---'
rg -nP 'PackageManager\.(parse_dependency_tree|create_dep_stack)\s*=' src/fosslight_dependency
echo '--- subclass method defs? ---'
rg -nP 'def\s+parse_dependency_tree\s*\(' src/fosslight_dependency
ast-grep outline src/fosslight_dependency/_package_manager.py --items allRepository: fosslight/fosslight_dependency_scanner
Length of output: 2833
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- PackageManager call site and class body around run_gradle_task ---'
sed -n '130,190p' src/fosslight_dependency/_package_manager.py
echo '--- class tail around parse_dependency_tree ---'
sed -n '520,585p' src/fosslight_dependency/_package_manager.py
echo '--- subclasses and whether they call run_gradle_task / override parse_dependency_tree ---'
rg -n -C 2 'class .*PackageManager|def run_gradle_task|def parse_dependency_tree' src/fosslight_dependency/package_manager src/fosslight_dependency/_package_manager.py
echo '--- any explicit binding of module-level functions to PackageManager or subclasses ---'
rg -n 'PackageManager\.(parse_dependency_tree|create_dep_stack)\s*=|self\.parse_dependency_tree|self\.create_dep_stack' src/fosslight_dependencyRepository: fosslight/fosslight_dependency_scanner
Length of output: 14365
Call parse_dependency_tree directly from run_gradle_task. In src/fosslight_dependency/_package_manager.py:156, self.parse_dependency_tree(dep_output) will raise AttributeError because parse_dependency_tree lives at module scope, not on PackageManager, unless it’s rebound or overridden in the subclass.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 577-577: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fosslight_dependency/_package_manager.py` around lines 565 - 578, Update
run_gradle_task to call the module-level parse_dependency_tree function directly
instead of self.parse_dependency_tree, passing the same dependency output and
required arguments. Use the existing parse_dependency_tree definition and its
callers to preserve the expected parsing behavior and avoid the AttributeError.
bdab4dc to
f248b02
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 450-494: The existing allDeps name check in add_allDeps_in_gradle
returns early and prevents dependency collection when a user-defined task
collides. Replace the injected task name and corresponding detection checks with
a scanner-specific name such as fosslightAllDeps, and update the task
registration and execution references, including run_gradle_task(),
consistently.
- Around line 291-293: Update the Gradle detection logic in the dependency
injection method so an existing `licenseReport {` block is considered configured
only when it contains the scanner-specific renderer and output path for
`dependency-license.json`; do not return success for generic/default settings.
When the block exists but lacks these settings, augment it in place rather than
adding a duplicate `dependency-license-report` plugin declaration, ensuring
`self.input_file_name` is generated.
- Around line 710-722: Update ensure_executable to determine executability with
os.access(filepath, os.X_OK) rather than checking whether any executable bit is
set in current_mode; retain the existing mode-preservation and chmod behavior
when the current user cannot execute the file.
- Around line 172-175: Update the Gradle command constructed in the
generateLicenseReport execution block of _run_gradle_process to include the
--no-parallel flag, ensuring multi-project license report generation runs
without parallel execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7ab26992-d28f-4d10-8d1b-7ffdf2af22d3
📒 Files selected for processing (2)
src/fosslight_dependency/_package_manager.pysrc/fosslight_dependency/run_dependency_scanner.py
| if 'dependency-license-report' in data or 'licenseReport {' in data: | ||
| logger.info('gradle-license-report plugin is already configured in build.gradle. Skip injection.') | ||
| return True |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not treat every licenseReport block as scanner-ready.
An existing block may use default renderers or paths, yet this returns success and skips injection of dependency-license.json. The subsequent task then cannot find self.input_file_name. Only skip when the scanner-specific renderer/output is already configured; otherwise augment the existing plugin configuration without adding a duplicate plugin declaration.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fosslight_dependency/_package_manager.py` around lines 291 - 293, Update
the Gradle detection logic in the dependency injection method so an existing
`licenseReport {` block is considered configured only when it contains the
scanner-specific renderer and output path for `dependency-license.json`; do not
return success for generic/default settings. When the block exists but lacks
these settings, augment it in place rather than adding a duplicate
`dependency-license-report` plugin declaration, ensuring `self.input_file_name`
is generated.
1e3b2a0 to
f43b087
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 813-824: Update get_gradle_cmd and its caller
collect_gradle_download_urls to accept input_dir, resolve gradlew/gradlew.bat
relative to that directory, and return the resolved wrapper path. Use that path
for ensure_executable and for restoring the original mode, ensuring wrapper
detection and cleanup target the same file executed with cwd=input_dir.
- Around line 580-630: The block scanner in _find_block_end must use an explicit
while-loop index so comment handling actually advances past skipped text. Update
both // and /* ... */ branches to move the index beyond the comment before
continuing, while preserving quote and brace-depth handling so
_insert_plugins_block_after_buildscript receives the true block boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bea95aee-3423-46a3-ba11-4b4ec193f0eb
📒 Files selected for processing (2)
src/fosslight_dependency/_package_manager.pysrc/fosslight_dependency/run_dependency_scanner.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/fosslight_dependency/run_dependency_scanner.py
| for idx in range(open_brace_pos, len(content)): | ||
| char = content[idx] | ||
| next_char = content[idx + 1] if idx + 1 < len(content) else '' | ||
|
|
||
| if quote_char is not None: | ||
| if escaped: | ||
| escaped = False | ||
| elif char == '\\': | ||
| escaped = True | ||
| elif char == quote_char: | ||
| quote_char = None | ||
| continue | ||
|
|
||
| if char == '/' and next_char == '/': | ||
| while idx < len(content) and content[idx] != '\n': | ||
| idx += 1 | ||
| continue | ||
|
|
||
| if char == '/' and next_char == '*': | ||
| idx += 2 | ||
| while idx + 1 < len(content) and not (content[idx] == '*' and content[idx + 1] == '/'): | ||
| idx += 1 | ||
| continue | ||
|
|
||
| if char in {"'", '"'}: | ||
| quote_char = char | ||
| continue | ||
|
|
||
| if char == '{': | ||
| brace_depth += 1 | ||
| elif char == '}': | ||
| brace_depth -= 1 | ||
| if brace_depth == 0: | ||
| return idx | ||
|
|
||
| return -1 | ||
|
|
||
|
|
||
| def _insert_plugins_block_after_buildscript(content, plugin_block): | ||
| if not plugin_block: | ||
| return content | ||
|
|
||
| buildscript_keywords = ['buildscript {', 'buildscript{'] | ||
| buildscript_end = -1 | ||
| for keyword in buildscript_keywords: | ||
| buildscript_end = _find_block_end(content, keyword) | ||
| if buildscript_end != -1: | ||
| break | ||
|
|
||
| if buildscript_end != -1: | ||
| return content[:buildscript_end + 1] + f'\n{plugin_block}\n' + content[buildscript_end + 1:] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Actually advance past comments while locating the block boundary.
Changing idx inside a for ... in range(...) loop does not advance its iterator. Braces inside // or /* ... */ comments are therefore counted and can make the plugin block get inserted inside a comment or buildscript body. Use an explicit while index and move it past each comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fosslight_dependency/_package_manager.py` around lines 580 - 630, The
block scanner in _find_block_end must use an explicit while-loop index so
comment handling actually advances past skipped text. Update both // and /* ...
*/ branches to move the index beyond the comment before continuing, while
preserving quote and brace-depth handling so
_insert_plugins_block_after_buildscript receives the true block boundary.
| if os.path.isfile('gradlew') or os.path.isfile('gradlew.bat'): | ||
| if platform.system() == const.WINDOWS: | ||
| cmd_gradle = "gradlew.bat" | ||
| else: | ||
| cmd_gradle = "./gradlew" | ||
| current_mode = change_file_mode(cmd_gradle) | ||
| return cmd_gradle, current_mode | ||
| current_mode, changed_mode = ensure_executable(cmd_gradle) | ||
| return cmd_gradle, current_mode, changed_mode | ||
|
|
||
|
|
||
| def collect_gradle_download_urls(input_dir, package_manager_name, app_name=None): | ||
| download_url_map = {} | ||
| cmd_gradle, current_mode = get_gradle_cmd() | ||
| cmd_gradle, current_mode, changed_mode = get_gradle_cmd() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Resolve and restore the wrapper relative to input_dir.
get_gradle_cmd() checks and modifies a wrapper in the process working directory, but the command executes with cwd=input_dir. When these differ, scanning can miss the wrapper or modify/restore the wrong file. Pass input_dir into the resolver and retain the resolved wrapper path for ensure_executable() and restoration.
Also applies to: 840-841
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/fosslight_dependency/_package_manager.py` around lines 813 - 824, Update
get_gradle_cmd and its caller collect_gradle_download_urls to accept input_dir,
resolve gradlew/gradlew.bat relative to that directory, and return the resolved
wrapper path. Use that path for ensure_executable and for restoring the original
mode, ensuring wrapper detection and cleanup target the same file executed with
cwd=input_dir.
2f8b789 to
3e7aa24
Compare
Signed-off-by: woocheol <jayden6659@gmail.com>
1 participant
Summary by CodeRabbit