fix(templates): validate cookiecutter project_name as python identifier (#263)
* fix(templates): add pre_gen_project hook validating project_name identifier * fix(templates): use valid identifier defaults in cookiecutter.json * test(templates): add regression tests for project_name identifier validation * fix(tests): resolve fullstack package name via backend ctx --------- Co-authored-by: opencode-agent <agent@opencode.local>
This commit is contained in:
parent
06899b3c01
commit
e4f9313641
7 changed files with 253 additions and 7 deletions
|
|
@ -1,5 +1,5 @@
|
||||||
{
|
{
|
||||||
"project_name": "my-project",
|
"project_name": "my_project",
|
||||||
"project_type": "backend",
|
"project_type": "backend",
|
||||||
"description": "Project description",
|
"description": "Project description",
|
||||||
"use_auth": ["no", "yes"],
|
"use_auth": ["no", "yes"],
|
||||||
|
|
|
||||||
35
.opencode/templates/backend/hooks/pre_gen_project.py
Normal file
35
.opencode/templates/backend/hooks/pre_gen_project.py
Normal file
|
|
@ -0,0 +1,35 @@
|
||||||
|
"""Pre-generation hook for the backend cookiecutter template.
|
||||||
|
|
||||||
|
Validates that ``project_name`` is a valid Python identifier so the
|
||||||
|
generated package dir + imports (``from <project_name>.X import Y``) do
|
||||||
|
not raise ``SyntaxError``. Hyphens, dots, spaces and leading digits are
|
||||||
|
rejected with a hint to use an underscore-separated name instead.
|
||||||
|
|
||||||
|
See issue #262: default ``my-project`` used to render as
|
||||||
|
``from my-project.config.settings import settings`` which is a
|
||||||
|
``SyntaxError`` (``-`` is not allowed in a Python identifier).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
|
||||||
|
_VALID_IDENTIFIER = re.compile(r"^[a-z][a-z0-9_]*$")
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> None:
|
||||||
|
project_name = "{{ cookiecutter.project_name }}"
|
||||||
|
if not _VALID_IDENTIFIER.fullmatch(project_name):
|
||||||
|
sys.exit(
|
||||||
|
f"Invalid project_name: {project_name!r}\n"
|
||||||
|
"project_name must be a valid Python identifier matching "
|
||||||
|
"^[a-z][a-z0-9_]*$ (lowercase, no hyphens/dots/spaces, "
|
||||||
|
"no leading digit).\n"
|
||||||
|
f"Use 'my_project' instead of 'my-project' (or 'my.project', "
|
||||||
|
"'my project')."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
{
|
{
|
||||||
"project_name": "my-cli",
|
"project_name": "my_cli",
|
||||||
"project_type": "cli",
|
"project_type": "cli",
|
||||||
"description": "CLI tool description",
|
"description": "CLI tool description",
|
||||||
"use_auth": ["no", "yes"],
|
"use_auth": ["no", "yes"],
|
||||||
|
|
|
||||||
34
.opencode/templates/cli/hooks/pre_gen_project.py
Normal file
34
.opencode/templates/cli/hooks/pre_gen_project.py
Normal file
|
|
@ -0,0 +1,34 @@
|
||||||
|
"""Pre-generation hook for the cli cookiecutter template.
|
||||||
|
|
||||||
|
Validates that ``project_name`` is a valid Python identifier so the
|
||||||
|
generated package dir + imports (``from <project_name>.X import Y``) do
|
||||||
|
not raise ``SyntaxError``. Hyphens, dots, spaces and leading digits are
|
||||||
|
rejected with a hint to use an underscore-separated name instead.
|
||||||
|
|
||||||
|
See issue #262: default ``my-cli`` used to render as
|
||||||
|
``from my-cli.core import app`` which is a ``SyntaxError`` (``-`` is not
|
||||||
|
allowed in a Python identifier).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
|
||||||
|
_VALID_IDENTIFIER = re.compile(r"^[a-z][a-z0-9_]*$")
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> None:
|
||||||
|
project_name = "{{ cookiecutter.project_name }}"
|
||||||
|
if not _VALID_IDENTIFIER.fullmatch(project_name):
|
||||||
|
sys.exit(
|
||||||
|
f"Invalid project_name: {project_name!r}\n"
|
||||||
|
"project_name must be a valid Python identifier matching "
|
||||||
|
"^[a-z][a-z0-9_]*$ (lowercase, no hyphens/dots/spaces, "
|
||||||
|
"no leading digit).\n"
|
||||||
|
f"Use 'my_cli' instead of 'my-cli' (or 'my.cli', 'my cli')."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
|
|
@ -1,5 +1,5 @@
|
||||||
{
|
{
|
||||||
"project_name": "my-fullstack",
|
"project_name": "my_fullstack",
|
||||||
"project_type": "fullstack",
|
"project_type": "fullstack",
|
||||||
"description": "Fullstack project description",
|
"description": "Fullstack project description",
|
||||||
"use_auth": ["no", "yes"],
|
"use_auth": ["no", "yes"],
|
||||||
|
|
|
||||||
35
.opencode/templates/fullstack/hooks/pre_gen_project.py
Normal file
35
.opencode/templates/fullstack/hooks/pre_gen_project.py
Normal file
|
|
@ -0,0 +1,35 @@
|
||||||
|
"""Pre-generation hook for the fullstack cookiecutter template.
|
||||||
|
|
||||||
|
Validates that ``project_name`` is a valid Python identifier so the
|
||||||
|
generated package dir + imports (``from <project_name>.X import Y``) do
|
||||||
|
not raise ``SyntaxError``. Hyphens, dots, spaces and leading digits are
|
||||||
|
rejected with a hint to use an underscore-separated name instead.
|
||||||
|
|
||||||
|
See issue #262: default ``my-fullstack`` used to render as
|
||||||
|
``from my-fullstack.config.settings import settings`` which is a
|
||||||
|
``SyntaxError`` (``-`` is not allowed in a Python identifier).
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
import sys
|
||||||
|
|
||||||
|
_VALID_IDENTIFIER = re.compile(r"^[a-z][a-z0-9_]*$")
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> None:
|
||||||
|
project_name = "{{ cookiecutter.project_name }}"
|
||||||
|
if not _VALID_IDENTIFIER.fullmatch(project_name):
|
||||||
|
sys.exit(
|
||||||
|
f"Invalid project_name: {project_name!r}\n"
|
||||||
|
"project_name must be a valid Python identifier matching "
|
||||||
|
"^[a-z][a-z0-9_]*$ (lowercase, no hyphens/dots/spaces, "
|
||||||
|
"no leading digit).\n"
|
||||||
|
f"Use 'my_fullstack' instead of 'my-fullstack' (or 'my.fullstack', "
|
||||||
|
"'my fullstack')."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
main()
|
||||||
|
|
@ -14,12 +14,15 @@ Covers:
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import ast
|
||||||
import importlib.util
|
import importlib.util
|
||||||
import json
|
import json
|
||||||
|
import re
|
||||||
import sys
|
import sys
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from cookiecutter.exceptions import FailedHookException
|
||||||
from cookiecutter.main import cookiecutter
|
from cookiecutter.main import cookiecutter
|
||||||
|
|
||||||
REPO_ROOT = Path(__file__).resolve().parent.parent
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
||||||
|
|
@ -53,15 +56,15 @@ def render(tmp_path, template_name, extra_context):
|
||||||
@pytest.mark.parametrize(
|
@pytest.mark.parametrize(
|
||||||
"template_name, extra_context",
|
"template_name, extra_context",
|
||||||
[
|
[
|
||||||
("backend", {"project_name": "demo-be"}),
|
("backend", {"project_name": "demo_be"}),
|
||||||
("cli", {"project_name": "demo-cli"}),
|
("cli", {"project_name": "demo_cli"}),
|
||||||
("fullstack", {"project_name": "demo-fs"}),
|
("fullstack", {"project_name": "demo_fs"}),
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
def test_template_dir_exists(render):
|
def test_template_dir_exists(render):
|
||||||
"""Each template renders to a project dir of the given name."""
|
"""Each template renders to a project dir of the given name."""
|
||||||
assert render.is_dir()
|
assert render.is_dir()
|
||||||
assert render.name in {"demo-be", "demo-cli", "demo-fs"}
|
assert render.name in {"demo_be", "demo_cli", "demo_fs"}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("template_name", ["backend", "cli", "fullstack"])
|
@pytest.mark.parametrize("template_name", ["backend", "cli", "fullstack"])
|
||||||
|
|
@ -824,3 +827,142 @@ def test_main_imports_setup_logging_unconditionally(render):
|
||||||
# ``setup_logging()`` call without a preceding import in the same file.
|
# ``setup_logging()`` call without a preceding import in the same file.
|
||||||
import_lines = [ln for ln in main.splitlines() if "import" in ln and "setup_logging" in ln]
|
import_lines = [ln for ln in main.splitlines() if "import" in ln and "setup_logging" in ln]
|
||||||
assert import_lines, "setup_logging not imported in main.py"
|
assert import_lines, "setup_logging not imported in main.py"
|
||||||
|
|
||||||
|
|
||||||
|
# ── issue #262: project_name must be a valid Python identifier ──────────────
|
||||||
|
|
||||||
|
# Regex mirrored in every ``pre_gen_project.py`` hook (kept here as a single
|
||||||
|
# source of truth for the test, validated against the actual hook files).
|
||||||
|
_VALID_PROJECT_NAME = re.compile(r"^[a-z][a-z0-9_]*$")
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"template_name, bad_name",
|
||||||
|
[
|
||||||
|
("backend", "my-project"), # hyphen
|
||||||
|
("backend", "my.project"), # dot
|
||||||
|
("backend", "my project"), # space
|
||||||
|
("backend", "My_Project"), # uppercase
|
||||||
|
("backend", "123abc"), # leading digit
|
||||||
|
("fullstack", "my-fullstack"),
|
||||||
|
("fullstack", "My_Fullstack"),
|
||||||
|
("cli", "my-cli"),
|
||||||
|
("cli", "123cli"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_pre_gen_hook_rejects_invalid_project_name(tmp_path, template_name, bad_name):
|
||||||
|
"""pre_gen_project.py exits non-zero for invalid project_name.
|
||||||
|
|
||||||
|
Regression guard for issue #262: hyphens/dots/spaces/uppercase/leading
|
||||||
|
digits in ``project_name`` would render as ``from my-project.X import Y``
|
||||||
|
which is a ``SyntaxError`` in the generated project. The hook must abort
|
||||||
|
cookiecutter with ``FailedHookException`` BEFORE any file is rendered.
|
||||||
|
"""
|
||||||
|
with pytest.raises(FailedHookException):
|
||||||
|
cookiecutter(
|
||||||
|
template=str(TEMPLATES_DIR / template_name),
|
||||||
|
no_input=True,
|
||||||
|
output_dir=str(tmp_path),
|
||||||
|
extra_context={"project_name": bad_name},
|
||||||
|
)
|
||||||
|
# nothing rendered
|
||||||
|
assert not (tmp_path / bad_name).exists()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"template_name, good_name",
|
||||||
|
[
|
||||||
|
("backend", "my_project"),
|
||||||
|
("fullstack", "my_fullstack"),
|
||||||
|
("cli", "my_cli"),
|
||||||
|
("backend", "be"),
|
||||||
|
("cli", "cl"),
|
||||||
|
("fullstack", "fs"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_pre_gen_hook_accepts_valid_project_name(tmp_path, template_name, good_name):
|
||||||
|
"""Valid identifiers pass the hook and render a project."""
|
||||||
|
out = cookiecutter(
|
||||||
|
template=str(TEMPLATES_DIR / template_name),
|
||||||
|
no_input=True,
|
||||||
|
output_dir=str(tmp_path),
|
||||||
|
extra_context={"project_name": good_name},
|
||||||
|
)
|
||||||
|
assert Path(out).is_dir()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("template_name", ["backend", "cli", "fullstack"])
|
||||||
|
def test_cookiecutter_json_default_is_valid_identifier(template_name):
|
||||||
|
"""The default ``project_name`` in cookiecutter.json passes the hook regex.
|
||||||
|
|
||||||
|
Issue #262: defaults must NOT contain hyphens (``my-project``) — they
|
||||||
|
are used as-is in paths/imports and would break the no-input flow.
|
||||||
|
"""
|
||||||
|
cfg = json.loads((TEMPLATES_DIR / template_name / "cookiecutter.json").read_text())
|
||||||
|
default = cfg["project_name"]
|
||||||
|
assert _VALID_PROJECT_NAME.fullmatch(default), (
|
||||||
|
f"default project_name {default!r} in {template_name}/cookiecutter.json "
|
||||||
|
f"is not a valid Python identifier (^[a-z][a-z0-9_]*$)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"template_name, extra_context",
|
||||||
|
[
|
||||||
|
("backend", {"project_name": "demo_be", "use_db": "yes", "use_auth": "yes"}),
|
||||||
|
("cli", {"project_name": "demo_cli"}),
|
||||||
|
("fullstack", {"project_name": "demo_fs", "use_db": "yes", "use_auth": "yes"}),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_generated_imports_are_parseable(render, template_name, extra_context):
|
||||||
|
"""Every generated ``.py`` parses with ``ast.parse`` (no SyntaxError).
|
||||||
|
|
||||||
|
Issue #262: hyphens in ``project_name`` produced ``from my-project.X
|
||||||
|
import Y`` which is a ``SyntaxError``. With the pre_gen hook the name
|
||||||
|
is already a valid identifier, so all generated imports must parse.
|
||||||
|
"""
|
||||||
|
pkg = extra_context["project_name"]
|
||||||
|
offenders: list[str] = []
|
||||||
|
for py in render.rglob("*.py"):
|
||||||
|
text = py.read_text()
|
||||||
|
try:
|
||||||
|
ast.parse(text)
|
||||||
|
except SyntaxError as exc:
|
||||||
|
offenders.append(f"{py.relative_to(render)}: {exc}")
|
||||||
|
assert not offenders, f"SyntaxError in generated files (pkg={pkg}):\n" + "\n".join(offenders)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"template_name, extra_context",
|
||||||
|
[
|
||||||
|
("backend", {"project_name": "demo_be"}),
|
||||||
|
("cli", {"project_name": "demo_cli"}),
|
||||||
|
("fullstack", {"project_name": "demo_fs"}),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_resolve_package_name_matches_directory(render, template_name, extra_context):
|
||||||
|
"""``_resolve_package_name`` from project-status matches the package dir.
|
||||||
|
|
||||||
|
Issue #262: with a valid identifier ``project_name``, the directory name
|
||||||
|
equals the package name, and ``_resolve_package_name`` (which normalizes
|
||||||
|
``[project].name`` via PEP 503) returns the same value. This is the
|
||||||
|
contract that ``project-status`` relies on to detect BACKEND/FULLSTACK/
|
||||||
|
CLI without false FAIL/WARN.
|
||||||
|
"""
|
||||||
|
pkg = extra_context["project_name"]
|
||||||
|
ctx = ps.RepoCtx(root=render, config=ps.load_config(render))
|
||||||
|
# fullstack: pyproject.toml lives under ``backend/`` (see _api_dirs_for
|
||||||
|
# in project-status.py), so resolve against a backend-rooted context.
|
||||||
|
if template_name == "fullstack":
|
||||||
|
ctx = ps.RepoCtx(root=render / "backend", config=ps.load_config(render / "backend"))
|
||||||
|
resolved = ps._resolve_package_name(ctx)
|
||||||
|
assert resolved is not None, "_resolve_package_name returned None"
|
||||||
|
assert resolved == pkg, (
|
||||||
|
f"_resolve_package_name()={resolved!r} != directory name {pkg!r}; "
|
||||||
|
f"project-status would look for src/{resolved}/ but the real dir is src/{pkg}/"
|
||||||
|
)
|
||||||
|
# fullstack: the package dir lives under backend/src/
|
||||||
|
if template_name == "fullstack":
|
||||||
|
assert (render / "backend" / "src" / pkg).is_dir()
|
||||||
|
else:
|
||||||
|
assert (render / "src" / pkg).is_dir()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue