Skip to content

feat: add file_utils module #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CristianAmbrosini wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add file_utils module #35

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions file_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""File utility helpers."""
import os


def read_file(path: str) -> str:
with open(path) as f:
return f.read()


def write_file(path: str, content: str) -> None:
with open(path, 'w') as f:
f.write(content)


def file_exists(path: str) -> bool:
return os.path.isfile(path)


def get_extension(path: str) -> str:
_, ext = os.path.splitext(path)
return ext.lstrip('.')


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

@sonar-review-dev18 sonar-review-dev18 Bot Apr 24, 2026 •
edited by CristianAmbrosini
Loading

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

for entry in os.listdir(directory):
full_path = os.path.join(directory, entry)
if os.path.isfile(full_path):

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

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

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

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

if extension is None or entry.endswith(f'.{extension}'):
files.append(full_path)
return sorted(files)
Loading