Skip to content

feat: add file_utils module - #35

Open
CristianAmbrosini wants to merge 1 commit into
mainfrom
cristian/test-telemetry-1777032670
Open

feat: add file_utils module#35
CristianAmbrosini wants to merge 1 commit into
mainfrom
cristian/test-telemetry-1777032670

Conversation

@CristianAmbrosini

Copy link
Copy Markdown
Owner

Summary

  • Adds file_utils.py with helpers: read_file, write_file, file_exists, get_extension, list_files

Test plan

  • Bot triggers review
  • Telemetry events emitted

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonar-review-dev18

sonar-review-dev18 Bot commented Apr 24, 2026

Copy link
Copy Markdown

Summary

Introduces a new file_utils.py module with five common file operation helpers: read/write text files, check file existence, extract file extensions, and list files in a directory.

The functions use straightforward standard library calls (os module) with basic type hints. Each helper is minimal and focused on a single responsibility—no dependencies or complex logic.

What reviewers should know

Review focus areas:

  • No error handling: Functions don't catch or raise exceptions (e.g., FileNotFoundError, PermissionError). This is intentional simplicity, but reviewers should confirm this aligns with the project's expected behavior.
  • Text-only I/O: read_file and write_file use text mode by design—will fail on binary files without modification.
  • Extension handling: get_extension strips the leading dot (returns "txt" not ".txt"). The list_files extension filter expects the same format.
  • Path handling: list_files returns full paths and sorts results. Parent directories are not created by write_file.

Start with list_files—it's the most complex with the optional parameter logic. The rest are straightforward wrappers.


  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-cloud-dev18

Copy link
Copy Markdown

SonarQube reviewer guide

Review in SonarQube

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sonar-review-dev18 sonar-review-dev18 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.

The module is simple and mostly straightforward, but there is one bug in list_files and a logic duplication issue worth fixing before merge.

SonarQube Cloud quality gate is failing — coverage on new code is 0% against an 80% threshold. See the reviewer guide above for details.

🗣️ Give feedback

Comment thread file_utils.py
files = []
for entry in os.listdir(directory):
full_path = os.path.join(directory, entry)
if os.path.isfile(full_path):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: If a caller passes extension with a leading dot (e.g. list_files(dir, '.txt')), the check becomes entry.endswith('..txt'), which will never match anything. The function silently returns an empty list with no indication of the mistake.

Given that get_extension already strips the leading dot, normalizing here is safer than expecting callers to know the convention:

Suggested change
if os.path.isfile(full_path):
if extension is None or entry.endswith(f'.{extension.lstrip(".")}'):
  • Mark as noise

Comment thread file_utils.py
files = []
for entry in os.listdir(directory):
full_path = os.path.join(directory, entry)
if os.path.isfile(full_path):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Logic duplication: list_files reimplements extension matching via str.endswith(f'.{extension}'), while get_extension uses os.path.splitext for the same concept. These will diverge if extension handling changes (e.g. case normalization, compound extensions like .tar.gz). list_files should delegate to get_extension to stay consistent:

if extension is None or get_extension(entry) == extension.lstrip('.'):
  • Mark as noise

Comment thread file_utils.py


def list_files(directory: str, extension: str = None) -> list[str]:
files = []

@sonar-review-dev18 sonar-review-dev18 Bot Apr 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing test coverage: list_files has three distinct branches — no extension filter, extension filter matching, and extension filter not matching — plus the os.path.isfile guard that silently skips subdirectories. None of these paths are tested. A unit test using pytest's tmp_path fixture would catch regressions in the filtering logic, including the leading-dot bug noted above.

  • Mark as noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant