fix(init): sanitize default package name derived from directory name - #10975
fix(init): sanitize default package name derived from directory name#10975Socialpranker wants to merge 3 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider handling the edge case where the sanitized name becomes empty after replacing invalid characters and stripping hyphens, as this could happen for directory names containing only disallowed characters.
- The
_sanitize_package_namedocstring currently describes the allowed character set for names but the canonicalization step produces only lowercase letters, numbers, and hyphens; clarifying that distinction would make the helper’s behavior easier to understand at a glance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider handling the edge case where the sanitized name becomes empty after replacing invalid characters and stripping hyphens, as this could happen for directory names containing only disallowed characters.
- The `_sanitize_package_name` docstring currently describes the allowed character set for names but the canonicalization step produces only lowercase letters, numbers, and hyphens; clarifying that distinction would make the helper’s behavior easier to understand at a glance.
## Individual Comments
### Comment 1
<location path="src/poetry/console/commands/init.py" line_range="131" />
<code_context>
name = self.option("name")
if not name:
- name = project_path.name.lower()
+ name = self._sanitize_package_name(project_path.name)
if is_interactive:
</code_context>
<issue_to_address>
**issue:** Consider handling the case where sanitization produces an empty package name.
If `_sanitize_package_name` returns an empty string (e.g., when the directory name consists only of stripped characters), subsequent logic will operate on an invalid/empty project name. Please add a fallback for this case, such as raising an error or defaulting to a safe default name.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
The |
|
On the second point from the review — the docstring — you were right, and it was wrong rather than merely unclear. Fixed in a93ff3e. It claimed the return value conforms to the PyPA name format, "only ASCII letters, numbers, period, underscore and hyphen".
The last row is the other half of it: the fallback added in 5818d36 deliberately returns a value that does not conform to the format, which the old docstring flatly contradicted. The rewrite states both — that the conforming path is narrower than the format allows, and that the fallback is the one exception, by design. One thing worth flagging for a human reviewer, since it looks like an oversight and isn't: a directory named
|
`poetry init`/`poetry new` used the current directory's name verbatim as the default package name, which can contain spaces or other characters that are invalid per the PyPA name format (only ASCII letters, numbers, period, underscore and hyphen are allowed). This produced a `pyproject.toml` with an invalid `[project] name`, e.g. "my project with spaces" instead of "my-project-with-spaces". Sanitize the derived name the same way `packaging.utils.canonicalize_name` normalizes existing valid names, after first replacing any disallowed characters (including whitespace) with a hyphen. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A directory whose name holds no ASCII alphanumerics — any non-Latin script, for instance — sanitized to an empty string, so `poetry init` offered an empty package name. Before this branch such a directory yielded its lowercased name, so this was a regression introduced here. Fall back to the lowercased directory name whenever sanitization leaves nothing behind.
The docstring promised the full PyPA name format, but canonicalize_name collapses period and underscore into hyphens and lowercases, so the return value is narrower. The fallback for names without ASCII alphanumerics is also outside the format by design.
a93ff3e to
208ff4f
Compare
|
Checking back — did the docstring fix address what you were looking for, or is there another round needed? |
2 participants
Summary
Closes #10974.
poetry init/poetry new(which shares the same code path) used the current directory's name verbatim as the default package name. Per the PyPA name normalization spec, a valid name may only contain ASCII letters, numbers, period, underscore and hyphen — so a directory likemy project with spacesproduced apyproject.tomlwith an invalidname = "my project with spaces"instead ofname = "my-project-with-spaces".This adds a small
InitCommand._sanitize_package_namehelper that replaces any disallowed character (including whitespace) with a hyphen, then reusespackaging.utils.canonicalize_name(already a dependency, already used elsewhere in this file for normalizing dependency names) to collapse/lowercase the result the same way it normalizes any other valid package name.This only changes the default value derived from the directory name; an explicit
--name/interactive value is left as-is, same as before.Test plan
test_sanitize_package_name— parametrized unit test covering the reported case plus a few edge cases (repeated separators, mixed./_, leading/trailing separators).test_noninteractive_sanitizes_default_name_from_directory— reproduces the exact scenario from the issue (poetry initrun non-interactively in a directory namedmy project with spaces), asserts the generatedpyproject.tomlhas a validname.pytest tests/console/commands/test_init.py- 73 passedpytest tests/console/commands/test_new.py- 24 passed (unaffected,NewCommandshares this code path)ruff check/ruff format --check- cleanmypy src/poetry/console/commands/init.py- no issuesAI disclosure
This PR was written with the help of Claude Code (Claude Sonnet 5). All code was reviewed and tested by me before submission.