File list and settings file extensions#208
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughVersion bumped to 1.53.0 and ChangesFiles-from CLI feature
Sequence DiagramsequenceDiagram
participant CLI as Scan CLI
participant Validator as Arg Validator
participant Loader as load_files_from_file
participant Scanner as scanner.scan_files_with_options
CLI->>Validator: parse args (files/files_from/dir/stdin/dep/wfp)
Validator->>Validator: ensure exactly one input mode
Validator-->>CLI: validation passed or help/error
CLI->>Loader: read files_from filepath
Loader->>Loader: open file, read lines, trim, skip blanks/comments
Loader-->>CLI: file_list (or empty)
CLI->>Scanner: scanner.scan_files_with_options(file_list)
Scanner-->>CLI: scan results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
![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
🤖 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/scanoss/cli.py`:
- Around line 2359-2366: The file-list parsing currently opens files with
open(..., encoding='utf-8') and only catches (OSError, IOError), so a
UnicodeDecodeError will crash the CLI; update the error handling around the
open/for loop to also catch UnicodeDecodeError (e.g., except (OSError, IOError,
UnicodeDecodeError) as e) and call print_stderr with the same formatted message
using filepath and error e, or alternatively open the file with errors='replace'
to avoid decoding exceptions; ensure the code paths that append to files and
call print_stderr remain unchanged.
- Around line 1565-1568: The current conflict check uses "if args.files and
args.files_from" which treats an empty --files list as false and misses the
conflict; update the condition to explicitly detect a provided-but-empty --files
(e.g., treat args.files as a provided value when it's not None) by changing the
check to something like "if args.files_from and args.files is not None" or "if
args.files_from and (args.files is not None and len(args.files) >= 0)" so that
when args.files was supplied (even as an empty list) and args.files_from is set,
the branch runs; adjust the block around print_stderr, parser.parse_args and
sys.exit accordingly to ensure the error triggers for empty --files inputs.
- Around line 1714-1717: When load_files_from_file(args.files_from) returns no
usable entries, the code currently calls sys.exit(1) silently; update the block
around load_files_from_file and the subsequent check so that if file_list is
falsy you print a clear user-facing error to stderr (or use existing logger)
mentioning the provided args.files_from and that no valid file paths were found
(handle empty/comment-only files), then exit with code 1; keep the call to
scanner.scan_files_with_options(file_list, args.dep, scanner.winnowing.file_map)
unchanged when file_list is valid.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c42a109-ebe7-42a9-a749-41f812f8aa35
📒 Files selected for processing (3)
src/scanoss/__init__.pysrc/scanoss/cli.pytests/cli-test.py
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/scanoss/cli.py (2)
2366-2367:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle Unicode decode failures in file-list parsing.
Opening with
encoding='utf-8'can raiseUnicodeDecodeErrorwhen the file contains invalid UTF-8 sequences. The current exception handler catchesRuntimeError, which is unrelated to file decoding errors. This will cause the CLI to crash with an unhandled exception instead of printing the graceful error message on line 2367.🔧 Suggested fix
- except (OSError, IOError, RuntimeError) as e: + except (OSError, IOError, UnicodeError) as e: print_stderr(f'ERROR: Failed to read input file; {filepath}: {e}')🤖 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/scanoss/cli.py` around lines 2366 - 2367, The file-list parsing error handler currently catches (OSError, IOError, RuntimeError) but misses UnicodeDecodeError, so add UnicodeDecodeError to the exception tuple (e.g., except (OSError, IOError, UnicodeDecodeError) as e) in the block that reads the input file (the code around the file-list parsing / read logic and the except that calls print_stderr with filepath) so decoding failures produce the same graceful error print instead of an unhandled crash; remove the unrelated RuntimeError if desired.
1565-1568:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
--files/--files-fromconflict detection for empty--filesinput.The condition
if args.files and args.files_from:treats an empty--fileslist as falsy and misses the conflict. When a user provides--fileswithout values,args.filesbecomes[], which is falsy, so both modes may be accepted together.🔧 Suggested fix
- if args.files and args.files_from: + if args.files is not None and args.files_from is not None: print_stderr('Please specify only one of --files or --files-from') parser.parse_args([args.subparser, '-h']) sys.exit(1)🤖 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/scanoss/cli.py` around lines 1565 - 1568, Change the conflict check to detect the presence of the --files flag even when it yielded an empty list: replace the current condition using truthiness with an explicit None check, e.g. use "if args.files is not None and args.files_from is not None" (referencing args.files and args.files_from in the block that currently calls print_stderr and parser.parse_args) so an empty [] for args.files is treated as a provided value and the mutual-exclusion error is triggered.
🧹 Nitpick comments (1)
src/scanoss/cli.py (1)
2342-2368: 💤 Low valueConsider returning
Noneon read failure to distinguish from empty input.When the file cannot be read, the function prints an error and returns an empty list. The caller then prints a second, generic error message ("No valid file paths found"). This produces two error messages for a single failure, which may confuse users into thinking the file was read successfully but contained no valid paths.
Consider returning
Noneon exception (matching theOptional[list]return type hint on line 1714) and updating the caller to distinguish between read failure and empty/comment-only input.♻️ Proposed fix
except (OSError, IOError, UnicodeError) as e: print_stderr(f'ERROR: Failed to read input file; {filepath}: {e}') + return None - return files + return files if files or filepath else NoneThen update caller to handle
None:elif args.files_from: file_list: Optional[list] = load_files_from_file(args.files_from) - if not file_list: + if file_list is None: + sys.exit(1) + if not file_list: print_stderr(f'ERROR: No valid file paths found in: {args.files_from}') sys.exit(1)🤖 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/scanoss/cli.py` around lines 2342 - 2368, The function load_files_from_file should return None on read failure instead of an empty list so callers can distinguish a read error from a legitimately empty or comment-only file; modify load_files_from_file to catch exceptions and return None (and update any type hints to Optional[list]) and keep printing the detailed error via print_stderr, then update the caller(s) that call load_files_from_file to check for None (treat as read failure and emit a single, specific error message) versus an empty list (treat as no valid paths) so you don't produce two error messages for one failure.
🤖 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.
Duplicate comments:
In `@src/scanoss/cli.py`:
- Around line 2366-2367: The file-list parsing error handler currently catches
(OSError, IOError, RuntimeError) but misses UnicodeDecodeError, so add
UnicodeDecodeError to the exception tuple (e.g., except (OSError, IOError,
UnicodeDecodeError) as e) in the block that reads the input file (the code
around the file-list parsing / read logic and the except that calls print_stderr
with filepath) so decoding failures produce the same graceful error print
instead of an unhandled crash; remove the unrelated RuntimeError if desired.
- Around line 1565-1568: Change the conflict check to detect the presence of the
--files flag even when it yielded an empty list: replace the current condition
using truthiness with an explicit None check, e.g. use "if args.files is not
None and args.files_from is not None" (referencing args.files and
args.files_from in the block that currently calls print_stderr and
parser.parse_args) so an empty [] for args.files is treated as a provided value
and the mutual-exclusion error is triggered.
---
Nitpick comments:
In `@src/scanoss/cli.py`:
- Around line 2342-2368: The function load_files_from_file should return None on
read failure instead of an empty list so callers can distinguish a read error
from a legitimately empty or comment-only file; modify load_files_from_file to
catch exceptions and return None (and update any type hints to Optional[list])
and keep printing the detailed error via print_stderr, then update the caller(s)
that call load_files_from_file to check for None (treat as read failure and emit
a single, specific error message) versus an empty list (treat as no valid paths)
so you don't produce two error messages for one failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3512a71c-8365-47d4-bb9c-f2384fd704de
📒 Files selected for processing (2)
CHANGELOG.mdsrc/scanoss/cli.py
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
2 participants
Summary by CodeRabbit
New Features
--files-from/-ffflag to scan a newline-delimited list of file paths from a text file--filesand--files-fromcannot be combinedTests
Chores