scaffold-project.py and family.py: scaffold-bad-id and family-bad-id name an id derived from --project or --family as derived and say to change that flag, so --project North_Wind no longer says --id 'north_wind' must match and sends the person to an --id the naming policy then refuses (#659)

scaffold-project.py

@@ -703,7 +703,7 @@ def _refuse_unholdable_manifest(args, values: dict, policy) -> None:
     """
     flags = dict(MANIFEST_FLAGS)
     for key, flag, given, source in (
-            ("PROJECT_ID", "--id", args.id, "--project lowercased"),
+            ("PROJECT_ID", "--id", args.id, ID_DERIVED_FROM),
             ("PROJECT_NAME", "--name", args.name, "--project"),
             ("ELECTED_BY", "--elected-by", args.elected_by,
              "GIT_AUTHOR_NAME or `git config user.name`")):
@@ -774,6 +774,58 @@ def _identity(args, project: str) -> tuple[str, str, str, str, str]:
     return project_id, display, elected_on, reference, elected_by
 
 
+#: Where `_identity` takes the id from when no `--id` is passed, as a
+#: sentence names it: in the label `_refuse_unholdable_manifest` gives a
+#: value no flag supplied, and in `scaffold-bad-id` (#659).
+ID_DERIVED_FROM = "--project lowercased"
+
+#: The rule a `--project` is held to, as `naming-unclassified` states it
+#: (`_check_one_name`). `scaffold-bad-id`'s derived remedy names it in the
+#: same words, so the person who follows it is not sent from one refusal to
+#: the other (#659).
+PROJECT_NAME_RULE = ("one CamelCase token with no hyphen, underscore, dot "
+                     "or space")
+
+#: How `scaffold-bad-id` is told to go on (#659), in the words
+#: `adopt-project.py plan` gives `adopt-bad-id` (#658). A `--id` the person
+#: passed is theirs to retype. An id `_identity` derived is `--project`
+#: lowercased, and `--project` also names the three repositories, which the
+#: naming policy reads itself: every form it holds takes only ASCII letters,
+#: digits and hyphens, so "pass an explicit lowercase --id" led straight to
+#: `naming-unclassified`, and so would "change --project" to `North-Wind`.
+#: The derived remedy names the rule.
+PASSED_ID_REMEDIATION = "Remediation: pass an explicit lowercase --id."
+DERIVED_ID_REMEDIATION = (
+    f"Remediation: change --project to {PROJECT_NAME_RULE}. The id is "
+    f"{ID_DERIVED_FROM}, and an explicit --id would not change --project, "
+    "which also names the assembly root and both legs.")
+
+
+def _refuse_bad_id(project_id: str, given: str | None) -> None:
+    """`scaffold-bad-id`: an id the project's validators would not take
+    (`PROJECT_ID_RE`), named by where it came from (#659), as
+    `adopt-project.py plan` names `adopt-bad-id` (#658).
+
+    `given` is `--id` as parsed. One the person passed is named as the flag
+    they typed, value as typed, and keeps its remedy. With none, or an empty
+    one, `_identity` derived the id (`args.id or project.lower()`), and
+    `--id 'north_wind' must match` named a value nobody had typed: the flag
+    is named as `_refuse_unholdable_manifest` names one, by where its value
+    came from, and an empty `--id ''` as empty, because it was passed."""
+    if PROJECT_ID_RE.match(project_id):
+        return
+    pattern = PROJECT_ID_RE.pattern
+    if given:
+        raise Refusal("scaffold-bad-id",
+                      f"--id {project_id!r} must match {pattern}",
+                      PASSED_ID_REMEDIATION)
+    unset = "not given" if given is None else "empty"
+    raise Refusal("scaffold-bad-id",
+                  f"--id ({unset}, so {ID_DERIVED_FROM}) is {project_id!r}, "
+                  f"which must match {pattern}",
+                  DERIVED_ID_REMEDIATION)
+
+
 #: How a refusal names an elector `_elector` did not get from `--elected-by`
 #: (#282), so the sentence says where the value came from and not a flag
 #: nobody typed: the words #255's `plan` uses for its own fallback.
@@ -1080,8 +1132,7 @@ def _check_one_name(policy, name: str, role: str, declared_pins: set,
         raise Refusal(
             "naming-unclassified",
             f"{name!r} matches no family in the naming policy. "
-            "`<Project>` is one CamelCase token with no hyphen, "
-            "underscore, dot or space.",
+            f"`<Project>` is {PROJECT_NAME_RULE}.",
             "Remediation: re-run with a --project value of that form.",
         )
     if not accepts_role(found, role):
@@ -1774,10 +1825,7 @@ def _scaffold(args) -> int:
     local, repositories, urls = _repository_urls(args, names)
     _refuse_an_unwritable_clone_url(local, urls["assembly"])
     policy = NamingPolicy.load(SHAPE_ROOT / "contracts" / "repository-naming.yaml")
-    if not PROJECT_ID_RE.match(project_id):
-        raise Refusal("scaffold-bad-id",
-                      f"--id {project_id!r} must match {PROJECT_ID_RE.pattern}",
-                      "Remediation: pass an explicit lowercase --id.")
+    _refuse_bad_id(project_id, args.id)
     # WHAT THE PROJECT DECLARES ABOUT ITSELF, before any name is judged.
     # With no `--pin`, a descendant-form name is a claim with no referent and
     # the DECLARED ROLE wins. With `--pin openGlass@<sha>`, `MedxGlass` IS a

scripts/family.py

@@ -585,6 +585,14 @@ def _member_url(repository: str, local_remote_dir: Path | None) -> str:
                 f"{repo_basename(repository)}.git"))
 
 
+#: The rule a `--family` is held to, as `naming-unclassified` states it
+#: (`_classify_family_name`). `family-bad-id`'s derived remedy names it in
+#: the same words, so the person who follows it is not sent from one
+#: refusal to the other (#659).
+FAMILY_NAME_RULE = ("one CamelCase token with no hyphen, underscore, dot "
+                    "or space")
+
+
 def _classify_family_name(policy: NamingPolicy, name: str) -> None:
     """The family name against the policy, in the form a family declares.
 
@@ -598,9 +606,9 @@ def _classify_family_name(policy: NamingPolicy, name: str) -> None:
         raise Refusal(
             "naming-unclassified",
             f"{name!r} matches no family in the naming policy",
-            "Remediation: --family takes one CamelCase token with no hyphen, "
-            "underscore, dot or space — the same rule an assembly root's name "
-            "follows, because a holder is spelled like one.")
+            f"Remediation: --family takes {FAMILY_NAME_RULE} — the same rule "
+            "an assembly root's name follows, because a holder is spelled "
+            "like one.")
     if found.family != "family":
         raise Refusal(
             "naming-not-a-family",
@@ -1502,6 +1510,53 @@ def _landing(args, family: str) -> tuple[Path | None, Path]:
     return folder, holder
 
 
+#: Where `init` takes the id from when no `--id` is passed, as a sentence
+#: names it: in the label `_manifest_flags` gives a value no flag supplied,
+#: and in `family-bad-id` (#659).
+FAMILY_ID_DERIVED_FROM = "--family lowercased"
+
+#: How `family-bad-id` is told to go on (#659), in the words the scaffold
+#: gives `scaffold-bad-id` and `adopt-project.py plan` `adopt-bad-id`
+#: (#658). A `--id` the person passed is theirs to retype. An id `init`
+#: derived is `--family` lowercased, and `--family` also names the holder
+#: and, unless `--work-dir` is given, its family folder (`_landing`); the
+#: name is the one `_classify_family_name` reads against a policy whose
+#: forms take only ASCII letters, digits and hyphens, so "pass an explicit
+#: lowercase --id" led straight to `naming-unclassified`, and so would
+#: "change --family" to `North-Wind`. The derived remedy names the rule.
+PASSED_FAMILY_ID_REMEDIATION = "Remediation: pass an explicit lowercase --id."
+DERIVED_FAMILY_ID_REMEDIATION = (
+    f"Remediation: change --family to {FAMILY_NAME_RULE}. The id is "
+    f"{FAMILY_ID_DERIVED_FROM}, and an explicit --id would not change "
+    "--family, which also names the holder and, unless --work-dir is given, "
+    "its family folder.")
+
+
+def _refuse_bad_family_id(family_id: str, given: str | None) -> None:
+    """`family-bad-id`: an id `validate-family.py` would not take
+    (`PROJECT_ID_RE`), named by where it came from (#659), as the scaffold
+    names `scaffold-bad-id`.
+
+    `given` is `--id` as parsed. One the person passed is named as the flag
+    they typed, value as typed, and keeps its remedy. With none, or an empty
+    one, the id is `--family` lowercased (`args.id or family.lower()`), and
+    `--id 'north_wind' must match` named a value nobody had typed: the flag
+    is named as `_manifest_flags` names one, by where its value came from,
+    and an empty `--id ''` as empty, because it was passed."""
+    if PROJECT_ID_RE.match(family_id):
+        return
+    pattern = PROJECT_ID_RE.pattern
+    if given:
+        raise Refusal("family-bad-id",
+                      f"--id {family_id!r} must match {pattern}",
+                      PASSED_FAMILY_ID_REMEDIATION)
+    unset = "not given" if given is None else "empty"
+    raise Refusal("family-bad-id",
+                  f"--id ({unset}, so {FAMILY_ID_DERIVED_FROM}) is "
+                  f"{family_id!r}, which must match {pattern}",
+                  DERIVED_FAMILY_ID_REMEDIATION)
+
+
 def _checked_family_identity(args) -> tuple[str, str, str]:
     """`(family, family_id, topic)` — every name check `init` makes before
     it decides where anything lands.
@@ -1523,10 +1578,7 @@ def _checked_family_identity(args) -> tuple[str, str, str]:
     refuse_bad_branch("family-bad-tracking-branch", "--tracking-branch",
                       args.tracking_branch)
     family_id = checked_value("--id", args.id or family.lower())
-    if not PROJECT_ID_RE.match(family_id):
-        raise Refusal("family-bad-id",
-                      f"--id {family_id!r} must match {PROJECT_ID_RE.pattern}",
-                      "Remediation: pass an explicit lowercase --id.")
+    _refuse_bad_family_id(family_id, args.id)
     policy = NamingPolicy.load(NAMING_POLICY)
     _classify_family_name(policy, family)
     topic = policy.topic_for(family_id)
@@ -2332,7 +2384,7 @@ def _manifest_flags(args) -> dict:
     from, so that a refusal does not point at a flag nobody typed."""
     flags = dict(MANIFEST_FLAGS)
     for key, flag, given, source in (
-            ("FAMILY_ID", "--id", args.id, "--family lowercased"),
+            ("FAMILY_ID", "--id", args.id, FAMILY_ID_DERIVED_FROM),
             ("FAMILY_NAME", "--name", args.name, "--family"),
             ("CREATED_BY", "--created-by", args.created_by,
              "GIT_AUTHOR_NAME or `git config user.name`"),

tests/test_derived_id_refusals.py

@@ -0,0 +1,174 @@
+# SPDX-License-Identifier: Apache-2.0
+"""`scaffold-bad-id` and `family-bad-id` name an id by where it came from,
+and send a derived one to the flag it came from (#659).
+
+WHAT WAS WRONG (#459's L2-56, and its label half in comment 6198174555).
+`scaffold-project.py` fills `--id` in as `--project` lowercased when none is
+passed, and `family.py init` as `--family` lowercased. Both then refused that
+id as `--id 'north_wind' must match ^[a-z0-9][a-z0-9-]*$`, a flag nobody had
+passed, with "Remediation: pass an explicit lowercase --id." And that remedy
+led straight to a second refusal: the naming policy reads `--project`
+(`--family`) itself, every form it holds takes only ASCII letters, digits
+and hyphens, so `--id north-wind` met `naming-unclassified: 'North_Wind'
+matches no family in the naming policy`.
+
+THE CURE, in the words `adopt-project.py plan` uses for `adopt-bad-id`
+(#658): a derived id's flag is named by where its value came from, `--id
+(not given, so --project lowercased)`, the label each tool's manifest
+refusals already give it, and its remedy is to change the flag it came from,
+which also names what the run makes. A `--id` the person passed keeps
+today's sentence and remedy, value as typed, even when it equals the derived
+one: what is read is whether the flag was passed, not what its value is. An
+empty `--id ''` derives the id as no `--id` does (`args.id or ...`), and is
+named as empty, because it was passed.
+
+Every run is a `--dry-run` into `--local-remote-dir`, which never calls `gh`.
+"""
+
+from __future__ import annotations
+
+import sys
+
+import pytest
+
+from conftest import ORG, REPO, SCAFFOLD, run_script
+
+sys.path.insert(0, str(REPO / "scripts"))
+from repo_shape import PROJECT_ID_RE, SAFE_ARG_RE  # noqa: E402
+
+FAMILY = REPO / "scripts" / "family.py"
+PATTERN = PROJECT_ID_RE.pattern
+
+#: The rule each tool's `naming-unclassified` holds a name to, in its own
+#: words: the scaffold's and `init`'s are the same.
+NAME_RULE = "one CamelCase token with no hyphen, underscore, dot or space"
+
+#: Per tool: the command, the flag naming whoever creates it, the flag the
+#: id is derived from and what else that flag names, and the finding's code.
+TOOLS = {
+    "scaffold": {"command": (SCAFFOLD,), "who": "--elected-by",
+                 "flag": "--project", "code": "scaffold-bad-id",
+                 "names": "the assembly root and both legs"},
+    "family": {"command": (FAMILY, "init"), "who": "--created-by",
+               "flag": "--family", "code": "family-bad-id",
+               "names": "the holder and, unless --work-dir is given, its "
+                        "family folder"},
+}
+PASSED_REMEDY = "Remediation: pass an explicit lowercase --id."
+
+
+def derived(tool: str, unset: str) -> str:
+    """How `tool` names a derived id's flag: `--id` given no value
+    (`unset`), so the flag the id is derived from, lowercased."""
+    return f"--id ({unset}, so {TOOLS[tool]['flag']} lowercased)"
+
+
+def derived_remedy(tool: str) -> str:
+    """The remedy `tool` gives for an id derived from its flag: change the
+    flag, to a name the naming policy takes."""
+    flag = TOOLS[tool]["flag"]
+    return (f"Remediation: change {flag} to {NAME_RULE}. The id is {flag} "
+            f"lowercased, and an explicit --id would not change {flag}, "
+            f"which also names {TOOLS[tool]['names']}.")
+
+
+#: Every character a name may hold on a command line (`SAFE_ARG_RE`, which
+#: `checked_value` asks of `--project` and `--family` before the id is
+#: derived from it) that leaves the name, lowercased, no id
+#: (`PROJECT_ID_RE`): the characters a DERIVED id is refused for.
+REFUSED_IN_AN_ID = tuple(
+    c for c in map(chr, range(0x21, 0x7F))
+    if SAFE_ARG_RE.fullmatch(f"North{c}Wind")
+    and not PROJECT_ID_RE.match(f"North{c}Wind".lower()))
+
+
+def run_tool(tmp_path, tool: str, name: str, *extra: str):
+    """One dry run of `tool` for `name`, into a local remote that never
+    exists: every row here is refused before anything is made."""
+    remotes = tmp_path / "remotes"
+    result = run_script(*TOOLS[tool]["command"], "--org", ORG,
+                        TOOLS[tool]["flag"], name,
+                        TOOLS[tool]["who"], "Test Human", "--dry-run",
+                        "--local-remote-dir", str(remotes),
+                        "--work-dir", str(tmp_path / "work"), *extra)
+    assert result.returncode == 2, result.stdout + result.stderr
+    assert not remotes.exists()
+    return result.stderr
+
+
+def refusal(tool: str, what: str, remedy: str) -> str:
+    return f"REFUSED {TOOLS[tool]['code']}: {what}\n{remedy}\n"
+
+
+def assert_named_as_passed(tool: str, stderr: str, value: str) -> None:
+    """A passed `--id` gets none of a derived id's words, neither its label
+    ("not given") nor its remedy ("change --project"), and keeps today's
+    sentence and remedy, value as typed. The derived words are asked first,
+    so a run that gives a passed id either of them fails on that word."""
+    assert "not given" not in stderr
+    assert f"change {TOOLS[tool]['flag']}" not in stderr
+    assert refusal(tool, f"--id {value!r} must match {PATTERN}",
+                   PASSED_REMEDY) in stderr
+
+
+@pytest.mark.parametrize("tool", TOOLS)
+def test_a_derived_id_is_named_by_where_it_came_from_and_sent_to_that_flag(
+        tmp_path, tool):
+    stderr = run_tool(tmp_path, tool, "North_Wind")
+    assert refusal(
+        tool,
+        f"{derived(tool, 'not given')} is 'north_wind', which must match "
+        f"{PATTERN}",
+        derived_remedy(tool)) in stderr
+    assert "--id 'north_wind'" not in stderr
+    assert PASSED_REMEDY not in stderr
+
+
+@pytest.mark.parametrize("tool", TOOLS)
+def test_an_empty_id_is_named_as_empty_and_not_as_not_given(tmp_path, tool):
+    """`--id ''` was passed, so "not given" would be false of it; the id is
+    still the derived one, and the remedy is still the flag's."""
+    stderr = run_tool(tmp_path, tool, "North_Wind", "--id=")
+    assert refusal(
+        tool,
+        f"{derived(tool, 'empty')} is 'north_wind', which must match "
+        f"{PATTERN}",
+        derived_remedy(tool)) in stderr
+    assert "not given" not in stderr
+
+
+@pytest.mark.parametrize("tool", TOOLS)
+def test_a_passed_id_equal_to_the_derived_one_is_still_named_as_passed(
+        tmp_path, tool):
+    """Passed means typed, not "differs from the name lowercased"."""
+    stderr = run_tool(tmp_path, tool, "North_Wind", "--id", "north_wind")
+    assert_named_as_passed(tool, stderr, "north_wind")
+
+
+@pytest.mark.parametrize("tool", TOOLS)
+def test_a_passed_bad_id_keeps_its_sentence_and_its_remedy(tmp_path, tool):
+    stderr = run_tool(tmp_path, tool, "NorthWind", "--id", "Foo_Bar")
+    assert_named_as_passed(tool, stderr, "Foo_Bar")
+
+
+def test_the_characters_a_derived_id_is_refused_for():
+    """The rows below are every one of them; an empty list would pass
+    vacuously, and a new one is a name the remedy has to be true of."""
+    assert REFUSED_IN_AN_ID == ("+", ".", "/", "@", "_", "~")
+
+
+@pytest.mark.parametrize("tool", TOOLS)
+@pytest.mark.parametrize("char", REFUSED_IN_AN_ID)
+def test_a_name_whose_derived_id_is_refused_is_refused_by_the_naming_policy(
+        tmp_path, tool, char):
+    """Why a derived id's remedy is its flag's and not `--id`'s: the naming
+    policy reads the name itself and refuses every one whose lowercase is no
+    id, so a `--id` that is an id only meets that refusal. Run with one, so
+    this is the run the old remedy led to. And the refusal states the rule
+    the derived remedy names, in the same words."""
+    name = f"North{char}Wind"
+    stderr = run_tool(tmp_path, tool, name, "--id", "north-wind")
+    assert (f"REFUSED naming-unclassified: {name!r} matches no family in the "
+            "naming policy") in stderr
+    assert NAME_RULE in stderr
+    assert TOOLS[tool]["code"] not in stderr