Skip to content

Add --no_merge option for source scanner - #187

Merged
dd-jy merged 3 commits into
mainfrom
feat/no_merge
Jul 31, 2026
Merged

Add --no_merge option for source scanner#187
dd-jy merged 3 commits into
mainfrom
feat/no_merge

Conversation

@JustinWonjaePark

@JustinWonjaePark JustinWonjaePark commented Jul 3, 2026

Copy link
Copy Markdown
Contributor
  • New Features
    • Added --no_merge command-line and settings option to FOSSLight Scanner to prevent folder-level merging during source code analysis.
    • The option is passed to FOSSLight Source Scanner (as merge_by_folder) and to the container-based scan flow.
    • Added a description of the --no_merge option in the help message.
  • Tests
    • Updated test cases in test_cli.py and test__parse_setting.py to match the updated return signature of parsing functions.

Summary by CodeRabbit

  • New Features
    • Added a --no_merge option to keep source scan results file-based instead of merging by folder.
    • Added support for configuring this behavior through saved scan settings.
  • Bug Fixes
    • Invalid no_merge values now default safely.
  • Tests
    • Updated tests and fixtures for the new setting.
  • Chores
    • Updated the scanner dependency to a newer compatible version.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a no_merge boolean option for source analysis. It wires the option through settings, CLI handling, and scanner execution. It also updates the fosslight_source dependency requirement.

Changes

no_merge option support

Layer / File(s) Summary
Setting parsing for no_merge
src/fosslight_scanner/_parse_setting.py, tests/fixtures/setting.json, tests/test__parse_setting.py
parse_setting_json reads, validates, and returns no_merge. The fixture and parsing test include the new value.
CLI argument wiring for no_merge
src/fosslight_scanner/cli.py, src/fosslight_scanner/_help.py, tests/test_cli.py
set_args() resolves no_merge from CLI and settings. main() adds --no_merge and forwards the value to run_main().
Scanner execution using no_merge
src/fosslight_scanner/fosslight_scanner.py
run_scanner and run_main accept no_merge. The option controls API folder merging and Docker command arguments.
Source-analysis dependency update
pyproject.toml
The fosslight_source requirement changes to >=2.3.3,<3.0.0.

Estimated code review effort: 2 (Simple) | ~15 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant Settings
  participant Scanner
  participant SourceAnalysis
  CLI->>Settings: read no_merge
  Settings-->>CLI: return validated value
  CLI->>Scanner: pass no_merge
  Scanner->>SourceAnalysis: set merge behavior
Loading

Possibly related PRs

Suggested reviewers: soimkim, dd-jy

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adding the --no_merge option to the source scanner.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/no_merge

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JustinWonjaePark
JustinWonjaePark force-pushed the feat/no_merge branch 2 times, most recently from 55bdcf6 to d04ba59 Compare July 3, 2026 02:33
Signed-off-by: Park Wonjae <wonjae.park@lge.com>
@JustinWonjaePark JustinWonjaePark self-assigned this Jul 3, 2026
@JustinWonjaePark JustinWonjaePark added the enhancement [PR/Issue] New feature or request label Jul 3, 2026
@JustinWonjaePark
JustinWonjaePark requested review from dd-jy and soimkim July 3, 2026 03:48
@JustinWonjaePark
JustinWonjaePark marked this pull request as ready for review July 3, 2026 03:48

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/fosslight_scanner/fosslight_scanner.py (2)

267-268: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Build the Docker invocation as an argument list.

subprocess.run(command, ...) does not execute a shell. command already contains a shell-quoted full command, so on POSIX it is treated as one executable name. The Docker source-analysis path fails before it can apply --no_merge.

Use a list such as ["docker", "run", ..., "--no_merge"]. Add each optional argument as a separate list element. Do not apply shlex.quote when shell=False.

🤖 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_scanner/fosslight_scanner.py` around lines 267 - 268, Update
the Docker source-analysis command construction around the no_merge option to
use a list of separate arguments for subprocess.run instead of a shell-quoted
command string. Build the invocation beginning with the Docker executable and
append required and optional arguments, including --no_merge, as individual
elements; remove shlex.quote usage while retaining shell=False behavior.

374-374: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the unused unpacked values.

final_excel_dir and result_log are not used after init(out_dir). Ruff reports RUF059 for both bindings. Replace them with _.

🤖 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_scanner/fosslight_scanner.py` at line 374, Update the
init(out_dir) assignment to bind the unused final_excel_dir and result_log
return values to _ while preserving the success binding.

Source: Linters/SAST tools

🤖 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.

Outside diff comments:
In `@src/fosslight_scanner/fosslight_scanner.py`:
- Around line 267-268: Update the Docker source-analysis command construction
around the no_merge option to use a list of separate arguments for
subprocess.run instead of a shell-quoted command string. Build the invocation
beginning with the Docker executable and append required and optional arguments,
including --no_merge, as individual elements; remove shlex.quote usage while
retaining shell=False behavior.
- Line 374: Update the init(out_dir) assignment to bind the unused
final_excel_dir and result_log return values to _ while preserving the success
binding.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d90725fc-d30d-47e3-9d7b-509bfbd1ccbd

📥 Commits

Reviewing files that changed from the base of the PR and between 4e43a9e and e201372.

📒 Files selected for processing (2)
  • pyproject.toml
  • src/fosslight_scanner/fosslight_scanner.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml

@dd-jy
dd-jy merged commit 8c53f97 into main Jul 31, 2026
6 of 7 checks passed
@dd-jy
dd-jy deleted the feat/no_merge branch July 31, 2026 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement [PR/Issue] New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants