Validate package name in create

[kunalKumar-13]

Closes #72.

plonecli create addon collective.new-testcase generated a package that could not be installed — each dotted part becomes a Python package directory, so a dash produced an invalid module name and buildout failed later with an opaque EntryPoint error.

create now rejects the name up front:

Error: Invalid value for NAME: 'collective.new-testcase' is not a valid package name:
'new-testcase' is not a valid Python identifier. Try 'collective.new_testcase'.

The underscore suggestion only appears when it would actually produce a valid name, so collective.2foo gets the error without a misleading hint.

Went with rejecting rather than normalising, per the issue title — silently rewriting the name the user typed seemed worse than telling them. Happy to switch to normalisation if you'd rather have that.

One thing worth flagging: NAME can be a target path, not just a bare name — the existing tests pass things like /tmp/.../my.addon. Validation therefore only checks the last path component. I only found this because the existing suite caught it.

14 tests added. Full suite: 230 passed, 16 skipped. ruff check and ruff format --check clean on both files.

[Copilot]

🟡 Changes recommended

The new validation’s error message is inaccurate for keyword-based failures (and awkward for multiple invalid parts), which is user-facing and should be corrected before release.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds upfront validation for plonecli create package names to prevent generating projects whose dotted components can’t become importable Python package directories (e.g., components containing -, starting with digits, or being Python keywords), addressing issue #72.

Changes:

• Add _validate_package_name() and invoke it at the start of the create command.
• Provide a targeted underscore suggestion only when it results in a valid dotted name.
• Add a dedicated test module covering valid/invalid names, path inputs, and suggestion behavior; update CHANGES.md.

File summaries

File	Description
plonecli/cli.py	Introduces and wires in package-name validation for create, including improved CLI error feedback.
tests/test_package_name_validation.py	Adds unit tests for name/path validation and messaging behavior.
CHANGES.md	Documents the behavior change for the upcoming release.

Review details

Suppressed comments (1)

plonecli/cli.py:118

• The error message says the offending parts "is not a valid Python identifier", but bad also includes reserved keywords (e.g. class), which are valid identifiers. Also, if multiple parts are invalid, the singular wording becomes grammatically incorrect; consider building the message with proper pluralization and explicit keyword mention.

    raise click.BadParameter(
        f"{name!r} is not a valid package name: "
        f"{', '.join(repr(p) for p in bad)} is not a valid Python identifier.{hint}",
        param_hint="NAME",
    )

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[kunalKumar-13]

Good catch, fixed in 2a39588. The message now distinguishes the two cases: when every offending part is a keyword it says 'class' is a Python keyword rather than calling it an invalid identifier, which was misleading because class is a valid identifier and only reserved. The docstring says "a dash, a leading digit or a reserved word" now, and there's a test pinning the wording.

Same commit also fixes something bigger that the scaffolding job was catching: NAME is the output directory, not always a package name. backend_addon asks a package_name question defaulting to dst_path.name, but zope-setup asks project_name, where a hyphen is ordinary — plonecli create zope-setup my-project is in the README and this PR was rejecting it. The validator now checks the value that actually becomes the package name (an explicit -d package_name= when given, the directory otherwise) and only for templates that declare such a question, read from their copier.yml rather than a hardcoded list.

evals/scaffolding --quick went from 21 failures to 0.