diff --git a/AGENTS.md b/AGENTS.md index 87545e66..9a166543 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,11 +116,11 @@ scripts/bootstrap.py`. README's "What setup.sh does" carries the exact commands. ## Adopting an existing repository -`adopt-project.py` converts a repository that already exists, IN PLACE: it keeps -its name, which `--project` must be, its identity and its full history and -becomes the assembly root, and the two legs are new repositories extracted with -`git filter-repo` (a hard dependency: `execute` refuses without it, printing the -install command). Seven steps, in order, and step 4 is the one you must not skip. +`adopt-project.py` converts an existing repository IN PLACE: it keeps its name, +which `--project` must be, its identity and its full history and becomes the +assembly root; its two legs are new repositories extracted with `git filter-repo` +(`execute` refuses without it). `--org` defaults to the org of an `org/repo` or a +url `origin`; a path `origin`, or none, needs it. Seven steps; never skip step 4. ```sh ./adopt-project.py plan --source --project diff --git a/adopt-project.py b/adopt-project.py index 792a0f56..732055e9 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -4696,18 +4696,85 @@ def _refuse_a_tracking_branch_the_remote_lacks(urls: dict, tracking: str, #: `North-Wind`, which `naming-unclassified` then refused. PROJECT_NAME_RULE = ("one CamelCase token with no hyphen, underscore, dot or " "space") +#: What a refusal says first where `--project` is the source's own name, +#: adopted in place, before `_rename_first`'s way out: the rule the new name +#: is held to. The derived id's remedy says it (L2-54), and so does +#: `_check_names`' refusal of that name (L2-89 on #457). +OWN_NAME_RULE = ("--project is the source's own name, which must be " + f"{PROJECT_NAME_RULE}. ") +#: What a passed `--id` the project's validators would not take is told +#: (#902): `PROJECT_ID_RE` in words. "Pass an explicit lowercase --id" was +#: said to `--id foo_bar`, which is both, and whose fault is the underscore +#: (L2-82 on #459). The scaffold's `scaffold-bad-id` and `family.py init`'s +#: `family-bad-id` say the same sentence (#911). +PASSED_ID_REMEDIATION = ("Remediation: pass an --id of lowercase letters, " + "digits and hyphens, starting with a letter or " + "digit.") +#: And what `check` and `execute` say of a plan's `id:` the pattern refuses +#: (`plan-bad-id`, #902), in the same words. +PLAN_ID_REMEDIATION = ( + "Remediation: set `id:` in the plan to lowercase letters, digits and " + "hyphens, starting with a letter or digit, or re-run `plan` with an --id " + "of that form. The GitHub topic is derived from it.") +#: What `plan-bad-id` says of an id derived from `legs.assembly` (#902, L2-88 +#: on #457), as `plan` words the id it derives from `--project` (L2-54): the +#: name it comes from, and the rule the naming policy holds that name to. +DERIVED_PLAN_ID_REMEDIATION = ( + f"Remediation: change `legs.assembly` in the plan to {PROJECT_NAME_RULE}, " + "or re-run `plan` with a --project of that form. The id is " + "legs.assembly lowercased, and an `id:` would leave `legs.assembly`, the " + "assembly root's name, as it is. The GitHub topic is derived from the id.") +#: What `check` and `execute` say of a plan value the plan's reader read as a +#: number or a boolean where `execute` takes text (#902, L1-K3 on #457): +#: `_not_text` says what was read, and this how to write what was meant. +NOT_TEXT_REMEDIATION = ( + "Remediation: write each value named between double quotes in the plan, " + "as typed (`\"007\"`, `\"true\"`), so that the plan's reader takes it as " + "text, and run `check` again; or re-run `plan`, which writes every value " + "so that it is read back as typed.") + + +def _a_number_or_boolean(value) -> bool: + """Whether the plan's reader read `value` as a number or a boolean, which + `execute` would turn into text with `str`: `007` into `7`, `true` into + `True` and `1.50` into `1.5` (#902, L1-K3 on #457).""" + return isinstance(value, (bool, int, float)) + + +def _not_text(key: str, value) -> str: + """A sentence about the plan's `key:`, which the plan's reader read as + `value`, a number or a boolean, where `execute` takes text: what was + read, and the text `execute` would take in its place (#902). The kind + is the one `_the_loader_made` names, as #181's misread rule names it.""" + return (f"`{key}:` was read out of the plan as {_the_loader_made(value)}, " + f"not as text, so `execute` would take it as the text " + f"{_worded(str(value))}") def _refuse_bad_id(project_id: str, project: str, given: set, - source: Source) -> None: + source: Source | None, naming: NamingPolicy, + pins: set[str]) -> None: """`plan`'s refusal of an id the project's validators would not take (`PROJECT_ID_RE`), in the words of where it came from. `given` is `flags_given`. A `--id` the person passed, `''` among them (L2-53), is - named as the flag they typed, with the value as typed, and the remedy is - another `--id`. One `plan` derived from `--project` is named by - `_flag_named` from `FLAG_DEFAULTS`, as the tracking branch and the - elector it fills in are (#529): `--id 'north_wind' must match` named a - flag nobody had passed (#601). The finding's code is the same for both. + named as the flag they typed, with the value as typed, and is told what + an id may hold (`PASSED_ID_REMEDIATION`, #902). One `plan` derived from + `--project` is named by `_flag_named` from `FLAG_DEFAULTS`, as the + tracking branch and the elector it fills in are (#529): `--id + 'north_wind' must match` named a flag nobody had passed (#601). The + finding's code is the same for both. + + A REFUSED NAME SPEAKS BEFORE A PASSED ID (#902, L2-90 on #457). `--project + North_Wind --id north_wind` was refused for its id, and typing the + remedy's `--id north-wind` then met `naming-unclassified`, which no + `--id` mends. So a passed id is refused only once the names `--project` + makes are ones the naming policy holds: `_check_names` is asked of them + first, with `pins`, the products `--pin` declares, which are all it + reads, so it answers here as it answers after the pins' digests are + read. It is handed `source` only where `--project` is the source's own + name, as below, since the names are not yet held to the source + (`_refuse_another_repository`). The scaffold and `family.py init` ask + their names first too (#911). A DERIVED ID IS MENDED IN `--project`, NOT WITH `--id` (L2-54). It is refused only for a `--project` holding a character no id takes, and the @@ -4722,17 +4789,19 @@ def _refuse_bad_id(project_id: str, project: str, given: set, if PROJECT_ID_RE.match(project_id): return flag, pattern = _flag_named("id", given), PROJECT_ID_RE.pattern + repository = None if source is None else source.repository + in_place = repository is not None and _is_the_source( + repository, repository.partition("/")[0], project) if "id" in given: + _check_names(naming, _names(project), pins, + source if in_place else None) raise Refusal("adopt-bad-id", f"{flag} {project_id!r} must match {pattern}", - "Remediation: pass an explicit lowercase --id.") + PASSED_ID_REMEDIATION) derived = FLAG_DEFAULTS["id"] - repository = source.repository - if repository is not None and _is_the_source( - repository, repository.partition("/")[0], project): - remedy = _rename_first(repository, f"the id is {derived}, and " - "--project is the source's own name, which " - f"must be {PROJECT_NAME_RULE}. ") + if in_place: + remedy = _rename_first(repository, + f"the id is {derived}, and {OWN_NAME_RULE}") else: remedy = (f"Remediation: change --project to {PROJECT_NAME_RULE}. " f"The id is {derived}, and an explicit --id would not " @@ -4743,6 +4812,18 @@ def _refuse_bad_id(project_id: str, project: str, given: set, remedy) +def _names_held(naming: NamingPolicy, project: str, pins: set[str]) -> bool: + """Whether the naming policy holds the three names `--project` makes, as + `_check_names` asks it of `pins`, the products `--pin` declares (#902). + `_checked_flag_inputs` refuses a passed `--id` that is no id only where + it does; beside a name it refuses, the naming refusal speaks first.""" + try: + _check_names(naming, _names(project), pins) + except Refusal: + return False + return True + + #: What `plan` says after `checked_value`'s remedy when the value refused is #: the source's own branch, which nobody passed (#797, L1-71's first case on #: #457). The alphabet alone said nothing of the way past it, which works. @@ -4790,8 +4871,10 @@ def _checked_plan_inputs(args, source: Source, # passed, so `--id "$ID"` with `ID` unset planned the derived id, and an # empty elector became `git config user.name` though `--help` says "not # blank". Each is a value typed, held to its flag's own rule below and - # refused there by name. (`_planned_org` still reads `--org ''` as no - # `--org`, as it did.) + # refused there by name. So is `--org ''` since #902 (L2-86 on #457): + # this set counted it passed while `_planned_org` read it as no `--org`, + # so one refusal named `--org is '~x'`, a value nobody typed, and + # another said `--org` was not given. flags_given = {key for key in FLAG_DEFAULTS if getattr(args, key) is not None} products = set(declared_pin_products(args)) @@ -4812,11 +4895,16 @@ def _checked_plan_inputs(args, source: Source, # Whether `--id` was passed is read BEFORE it is filled in: a refusal of # the id names where it came from (#601), and says what to change. An # `org/repo` source was asked it before it was cloned, and passes here. + # A passed one that is no id reaches here only beside a name the naming + # policy refuses, which is refused first (#902), as `_check_names` asks + # it below, with the source. args.id = args.id if "id" in flags_given else args.project.lower() - _refuse_bad_id(args.id, args.project, flags_given, source) + _refuse_bad_id(args.id, args.project, flags_given, source, naming, + products) # The organisation, given or read from the source's `origin`, and named - # by which of the two it is should it be refused (#292, #427). - org_given = bool(args.org) + # by which of the two it is should it be refused (#292, #427). Given is + # on the command line, `''` included (#902), as `flags_given` reads it. + org_given = args.org is not None args.org = _planned_org(args.org, source) # IN PLACE, THE SOURCE'S OWN NAME (#499), as soon as both halves are # known and before any pin's digest is read: `execute` acts on the @@ -4824,8 +4912,17 @@ def _checked_plan_inputs(args, source: Source, # An `org/repo` source was asked before it was cloned # (`_refuse_a_clone_of_another_repository`) and passes here again. _refuse_another_repository(args, source, naming, products, org_given) - args.elected_on = args.elected_on or _dt.date.today().isoformat() - args.reference = args.reference or default_reference(args.elected_on) + # A DATE OR A REFERENCE PASSED EMPTY IS A VALUE (#902, L2-87 on #457): + # read by its truth, `--elected-on ''` recorded today's date and + # `--reference ''` the default reference, exit 0, where `--tracking-branch + # ''` is refused. Each is read by `is not None`, as `--id` and + # `--elected-by` are (L2-53): an empty date was refused with the flags + # (`_checked_flag_inputs`), and an empty reference is the project's own + # validator's to refuse (`_refuse_unholdable_values`). + args.elected_on = (args.elected_on if args.elected_on is not None + else _dt.date.today().isoformat()) + args.reference = (args.reference if args.reference is not None + else default_reference(args.elected_on)) # A value `plan` filled in by itself is named by where it came from, as # #181 names one, and not as a flag the person never passed. A passed # `--elected-by` was asked with the other flags' values already. @@ -4868,7 +4965,7 @@ def _refuse_unwritable_flag_values(fields) -> None: "the flag or correct that. " + WRITTEN_AS_TEXT) -def _checked_flag_inputs(args) -> None: +def _checked_flag_inputs(args, naming: NamingPolicy) -> None: """The refusals of a passed flag's own value, which `plan` makes from its flags ALONE, in the order `_checked_plan_inputs` asked them, before anything exists (#903): no work directory is made, no `git` reads the @@ -4895,8 +4992,11 @@ def _checked_flag_inputs(args) -> None: `--project` makes, with a remedy about the source; an empty `--elected-by`, which is a value `unwritable_values` holds and the manifest validator refuses (`adopt-manifest-invalid`, in - `_refuse_unholdable_values`); and a missing `--path-policy` - (`yaml-unreadable`). + `_refuse_unholdable_values`), and an empty `--reference` with it + (#902); and a missing `--path-policy` (`yaml-unreadable`). A passed + `--id` that is no id is here only where `naming` holds the names + `--project` makes (`_names_held`, #902): beside a name it refuses, the + naming refusal speaks first, and its remedy needs the source. """ args.project = checked_value("--project", args.project) # A `--tracking-branch` that was passed: PASSED IS ON THE COMMAND LINE, @@ -4943,30 +5043,37 @@ def _checked_flag_inputs(args) -> None: # given none. One derived from `--project` is refused where the source's # own name is known, for its remedy names that: right after `--source` # is read, for an `org/repo` one, and after git has read the `origin` - # of a local one. - if args.id is not None: - _refuse_bad_id(args.id, args.project, {"id"}, None) - # An `--org` that was passed, as `_planned_org` holds it. One that was + # of a local one. AND ONLY WHERE THE NAMING POLICY HOLDS THE NAMES + # `--project` MAKES (#902): a refused name speaks before a passed id, + # and its remedy names the source where `--project` is the source's own + # name, so a passed id beside such a name is refused where the source + # is known, as the derived one is (`_refuse_bad_id`). + if args.id is not None and _names_held(naming, args.project, + set(products)): + _refuse_bad_id(args.id, args.project, {"id"}, None, naming, + set(products)) + # An `--org` that was passed, as `_planned_org` holds it, `''` included + # (#902): an empty one is a value passed, refused by name. One that was # not is the organisation of the source's name, read there. - if args.org: + if args.org is not None: checked_value(_flag_named("org", {"org"}), args.org) # Read before the plan is written, whether or not it is what chooses the # reference: a date the tools cannot parse would reach `execute` as a # manifest field instead of as a question. One nobody passed is today's, - # which `_checked_plan_inputs` fills in. - if args.elected_on: + # which `_checked_plan_inputs` fills in; `''` was passed (#902). + if args.elected_on is not None: election_date(args.elected_on) # Before the plan is written, in the words `check` would use (#255): a # value `execute` writes into a file must be text one can hold. Of the # values that were passed alone: an elector `plan` reads from `git # config user.name` is asked in `_checked_plan_inputs`, after the # source, and a reference it chooses is text it holds. An `--elected-by - # ''` is a value typed and is text one can hold, so it passes here and - # is refused by the manifest validator, after the source - # (`adopt-manifest-invalid`); a `--reference ''` is none. + # ''` and a `--reference ''` are values typed (#902) and text one can + # hold, so each passes here and is refused by the manifest validator, + # after the source (`adopt-manifest-invalid`). passed = [(_flag_named(key, {key}), value, ROOT_MANIFEST_AND_README) for key, value in ((ELECTED_BY_KEY, args.elected_by), - (REFERENCE_KEY, args.reference or None)) + (REFERENCE_KEY, args.reference)) if value is not None] _refuse_unwritable_flag_values(passed) @@ -5000,8 +5107,14 @@ def _planned_org(org: str | None, source: Source) -> str: `--elected-by` is named when `git config user.name` filled it in, and the remedy says first to pass `--org`. The refusal and its exit code stay: `plan` never puts such a value on a command line. + + AN `--org ''` IS A VALUE PASSED (#902, L2-86 on #457). Read by its truth, + it was taken for no `--org`, so the organisation of the source's + `origin` was planned, and named as `--org is '~x'` by one refusal and as + `--org` not given by another. It is held to `checked_value` as passed, + which refuses it by name, as it refuses `--tracking-branch ''`. """ - if org: + if org is not None: return checked_value("--org", org) read = (source.repository or "/").split("/")[0] if not read: @@ -5685,7 +5798,11 @@ def cmd_plan(args) -> int: # is made, git reads nothing of the source and no `gh` runs before # them. A passed `--tracking-branch` runs `git check-ref-format` in a # throwaway repository first, and never touches the source. - _checked_flag_inputs(args) + # The naming policy is read first, a file of this checkout: a passed + # `--id` is refused with the flags only where the names `--project` + # makes are ones it holds (#902). + naming = NamingPolicy.load(NAMING_POLICY) + _checked_flag_inputs(args, naming) # What `--source` names, next, before the work directory is made # (#796): every refusal of it that needs none -- an empty value, a path # that is no repository's top level, a name `gh` cannot clone -- is @@ -5693,7 +5810,6 @@ def cmd_plan(args) -> int: # main a file ended `plan` in a traceback after the `--work-dir` it was # given had been made, and that directory stayed. named = _source_spec(args.source) - naming = NamingPolicy.load(NAMING_POLICY) # An `org/repo` source names its repository before it is cloned, so # `--org` and `--project` are held to it here (#903). A local source # names it by its `origin`, which `_checked_plan_inputs` reads. @@ -6148,15 +6264,38 @@ def project_id(self) -> str: command line and a written manifest, and a topic that does not match `contracts/repository-naming.yaml` is one `validate-repository-naming.py` will refuse in the project's own gate, long after the run that set it. + + EACH REFUSAL NAMES WHAT THE PLAN HOLDS (#902). With no `id:`, the id + is `legs.assembly` lowercased, and it is named so (`DERIVED_ID`), with + `legs.assembly` as the remedy: the refusal said "`id:` is + 'north_wind'" of a plan with no `id:`, and sent the person to `--id`, + though only a character no id takes makes a derived id bad, and the + naming policy refuses that in `legs.assembly` whatever `id:` says + (L2-88 on #457). An `id:` the plan's reader read as a number or a + boolean (`id: 007`) was `str` of what was read, `7`, which the + pattern takes: it is refused by what was read (L1-K3). And an `id:` + the pattern refuses is told what an id may hold, as `plan` tells a + passed `--id` (`PASSED_ID_REMEDIATION`). """ - project_id = str(self.get("id", self.names()["assembly"].lower())) + set_id, pattern = self.get("id"), PROJECT_ID_RE.pattern + if set_id is None: + project_id = self.names()["assembly"].lower() + if not PROJECT_ID_RE.match(project_id): + raise Refusal( + "plan-bad-id", + f"{self.path}: {DERIVED_ID} is {project_id!r}, which " + f"must match {pattern}", DERIVED_PLAN_ID_REMEDIATION) + return project_id + if _a_number_or_boolean(set_id): + raise Refusal("plan-bad-id", + f"{self.path}: {_not_text('id', set_id)}", + NOT_TEXT_REMEDIATION) + project_id = str(set_id) if not PROJECT_ID_RE.match(project_id): raise Refusal( "plan-bad-id", f"{self.path}: `id:` is {project_id!r} and must match " - f"{PROJECT_ID_RE.pattern}", - "Remediation: fix `id:` in the plan, or re-run `plan` with an " - "explicit lowercase --id. The GitHub topic is derived from it.") + f"{pattern}", PLAN_ID_REMEDIATION) return project_id def get(self, key, default=None): @@ -7782,7 +7921,7 @@ def _elected_on_findings(plan: Plan) -> list[str]: except Refusal as refused: if refused.code != ELECTION_DATE_MALFORMED: raise - chosen = ("" if plan.get("reference") else + chosen = ("" if plan.get("reference") is not None else ", and, the plan naming no `reference:`, chooses the " "reference by it") return [f"FINDING {ELECTION_DATE_MALFORMED}: the plan's `elected_on:` " @@ -8591,8 +8730,17 @@ def _branch_name_refusal(plan: Plan, key: str, default: str, the refusal `check` reports, or None: `checked_value`'s, `execute`'s own, for a name outside its alphabet, and otherwise git's verdict under `code`. The value is `execute`'s (`_checked_plan_values`): `default` - where the key is absent, and `str` of what the file holds.""" + where the key is absent, and `str` of what the file holds. + + A NUMBER OR A BOOLEAN IS NO BRANCH NAME (#902, L1-K3 on #457). `str` of + it is one: `tracking_branch: 007` was the branch `7`, `true` was `True` + and `1.50` was `1.5`, and `adopt_branch:` the same, so a plan holding + any of them checked ok and was adopted onto that branch without a word. + It is refused under the key's own code, saying what was read + (`_not_text`), and is not asked of git.""" value = plan.get(key, default) + if _a_number_or_boolean(value): + return Refusal(code, _not_text(key, value), NOT_TEXT_REMEDIATION) try: checked_value(key, value) refuse_bad_branch(code, key, value) @@ -11393,8 +11541,7 @@ def cmd_check(args) -> int: # `execute` stops at `plan-not-the-source` and never asks the names, and # a refusal of the plan's name is then about a name that is not the # source's: `_own_name_remediation` has said what to do. - renamed = (_rename_first(source.repository) - if source.repository is not None and not another else None) + in_place = source.repository if not another else None try: _check_names(naming, names, pins, None if another else source) for role, name in names.items(): @@ -11405,9 +11552,11 @@ def cmd_check(args) -> int: findings.append(f"FINDING {exc.code}: {exc.detail}") # Only the rename-first remedy is printed, once after the findings: # it is the one `check` was missing. `--project takes ...` is not, - # and `check` never printed it (#716). - if renamed is not None and exc.remediation == renamed: - remediation.append(renamed) + # and `check` never printed it (#716). It says the rule its new name + # follows first, as `_check_names` says it (#902). + if in_place is not None and exc.remediation == _name_remedy( + exc.code, in_place): + remediation.append(exc.remediation) else: # WHAT THE DISK OR GITHUB WILL NOT HOLD (#863): the names, the # organisation and the topic, `execute`'s one list, which it refuses @@ -12832,7 +12981,11 @@ def _checked_plan_values(plan: Plan) -> tuple[str, str, str, str]: Split out of `cmd_execute` for #138, in the order the refusals came in. A branch name `checked_value` refuses is `check`'s finding too, in the - same words (`_branch_name_refusal`, #520); one inside its alphabet that + same words (`_branch_name_refusal`, #520), and so is a branch the plan's + reader read as a number or a boolean, which is asked first + (`_checked_branch_value`, #902), so that `tracking_branch: -007` is + `plan-bad-tracking-branch` in `check` and in `execute`, and not + `unsafe-value` of the `-7` nobody typed; one inside its alphabet that git will not create is refused with `check`'s other findings, by `_refuse_what_check_finds`, and before any leg exists as well. """ @@ -12842,12 +12995,28 @@ def _checked_plan_values(plan: Plan) -> tuple[str, str, str, str]: _refuse_bad_leg_paths("plan-bad-leg-path", spec_leg, code_leg) spec_path = checked_value(*spec_leg) code_path = checked_value(*code_leg) - branch = checked_value("adopt_branch", plan.get("adopt_branch", - ADOPT_BRANCH)) - tracking = checked_value(TRACKING_KEY, plan.get(TRACKING_KEY, "main")) + # Each branch asks the number-or-boolean question before `checked_value`, + # as `check` does (`_branch_name_refusal`, #902), in `BRANCH_KEYS`' order: + # a `tracking_branch: -007` is `plan-bad-tracking-branch` in both, not + # `unsafe-value` of the `-7` nobody typed. + branch, tracking = [_checked_branch_value(plan, *key) + for key in BRANCH_KEYS] return spec_path, code_path, branch, tracking +def _checked_branch_value(plan: Plan, key: str, default: str, + code: str) -> str: + """The plan's branch `key:` as `execute` hands it to git, or the refusal + `check` finds it under (`_branch_name_refusal`, #520): a number or a + boolean is refused under `code`, saying what was read, and any other + value is `checked_value`'s. `default` is what `execute` reads where the + key is absent.""" + value = plan.get(key, default) + if _a_number_or_boolean(value): + raise Refusal(code, _not_text(key, value), NOT_TEXT_REMEDIATION) + return checked_value(key, value) + + def _plan_org(plan: Plan) -> str: """The plan's `org:` as `execute` puts it into each repository's name and URL: `str()` of whatever the file holds, which is the text @@ -14228,9 +14397,15 @@ def _template_table(plan: Plan, names, repositories, urls, spec_path, # A plan that omits `reference:` — one written by hand, or by a tool # older than this rule — is resolved from ITS OWN `elected_on`, not # from the day `execute` happens to run. The plan carries the human's - # act; the calendar of the machine running the split does not. + # act; the calendar of the machine running the split does not. An + # EMPTY `reference:` is a value, not one omitted (#902): read by its + # truth, `reference: ""` was written as the default without a word, + # and `plan --reference ''` planned it. The project's own validator + # refuses it ("drop the key or name the document"), and `check`, + # `execute` and `plan` ask the validator of this table. "REFERENCE": str(plan.get(REFERENCE_KEY) - or default_reference(elected_on)), + if plan.get(REFERENCE_KEY) is not None + else default_reference(elected_on)), "ELECTED_BY": str(plan.get(ELECTED_BY_KEY, "")), "ELECTED_ON": elected_on, "TRACKING_BRANCH": tracking, @@ -15087,34 +15262,62 @@ def _check_names(policy: NamingPolicy, names: dict[str, str], that renaming it on GitHub comes first (`_rename_first`), never that `--project` takes one CamelCase token. A leg's name is the plan's own to edit, and keeps the remediation it had. + + THE RULE IS SAID IN PLACE TOO, of the new name (#902, L2-89 on #457). + The way out was `_rename_first`'s alone, which says to rename the source + and give its new name as `--project`, and said no rule for that name, + where the same refusal of a name that is not the source's says + "--project takes one CamelCase token with no hyphen, underscore, dot or + space". Both say the rule now, from one table (`_name_remedy`). """ for role, name in names.items(): found = policy.classify(name, role, pins) - renamed = (_rename_first(source.repository) - if role == "assembly" and source is not None - and source.repository is not None else None) + in_place = (source.repository if role == "assembly" + and source is not None else None) if found is None: raise Refusal( - "naming-unclassified", + UNCLASSIFIED, f"{name!r} matches no family in the naming policy", - renamed or - f"Remediation: --project takes {PROJECT_NAME_RULE}.") + _name_remedy(UNCLASSIFIED, in_place)) if not accepts_role(found, role): raise Refusal( - "naming-role-mismatch", + ROLE_MISMATCH, f"{name!r} classifies as {found.family}" + (f"/{found.role}" if found.role else "") + f", not as the {role!r} form of a project leg " f"({found.reason})", - renamed or - "Remediation: --project takes one CamelCase token. Two forms " - "beyond the bare one MAY be the assembly root: a declared " - "descendant (declare the pin with `--pin " - "open@<40 hex commit>` and it classifies as one, " - "2026-09-02), and an `open` " - "neutral product, which may elect the shape and be its own " - "root (2026-09-05). Neither may be the spec or code leg, and " - "an `-Install` may be no leg at all.") + _name_remedy(ROLE_MISMATCH, in_place)) + + +#: `_check_names`' two refusals. +UNCLASSIFIED = "naming-unclassified" +ROLE_MISMATCH = "naming-role-mismatch" +#: What each of them says the assembly root's name may be, by its code: the +#: rule `naming-unclassified` gives (`PROJECT_NAME_RULE`), and the forms an +#: assembly root may take beyond it, which `naming-role-mismatch` gives. +NAME_RULES = { + UNCLASSIFIED: f"{PROJECT_NAME_RULE}.", + ROLE_MISMATCH: ( + "one CamelCase token. Two forms beyond the bare one MAY be the " + "assembly root: a declared descendant (declare the pin with `--pin " + "open@<40 hex commit>` and it classifies as one, " + "2026-09-02), and an `open` neutral product, which may " + "elect the shape and be its own root (2026-09-05). Neither may be the " + "spec or code leg, and an `-Install` may be no leg at all."), +} + + +def _name_remedy(code: str, repository: str | None) -> str: + """What `_check_names`' refusal `code` says to do: `--project` takes the + rule (`NAME_RULES`), or, where `repository` is the source adopted in + place, whose own name `--project` is, that rule as the one its new name + must follow, then `_rename_first`'s way out (#902). `check` asks it + again to know the remedy it prints (#716).""" + rule = NAME_RULES[code] + if repository is None: + return f"Remediation: --project takes {rule}" + return _rename_first(repository, "--project is the source's own name, " + f"which must be {rule} ") # --------------------------------------------------------------------------- @@ -15246,13 +15449,20 @@ def _own_name_remediation(repository: str, naming: NamingPolicy, refuses that name, `_rename_first`. `repository` is the one the source is held to (`_held_repository`), as text: where the live source names none, the plan's record of it, and `str(None)` is no name (#716). `done` - is what the refusing command left as it was, said first.""" + is what the refusing command left as it was, said first. + + Where the naming policy refuses the source's own name, the rule the new + name must follow is said before the way out (#902, L2-89 on #457), the + rule `_check_names` gives for that refusal (`NAME_RULES`): the way out + renames the source, and said nothing of the name to give it.""" org, _, name = repository.partition("/") try: _check_names(naming, _names(name), pins) except Refusal as exc: + rule = NAME_RULES.get(exc.code) return _rename_first(repository, f"{done}The source's own name is no " - f"remedy either: {exc.detail}. ") + f"remedy either: {exc.detail}. " + + (f"The name must be {rule} " if rule else "")) return (f"Remediation: {done}Run `plan` again with `--project " f"{_shown(name)} --org {_shown(org)}`, the source's own name, " "which `plan` writes into the plan's `org:`, `project:` and " @@ -15309,13 +15519,17 @@ def _refuse_a_clone_of_another_repository(args, repository: str, named = Source(Path(SOURCE_READONLY), repository, "", "", cloned=True) # An `--id` that was passed was asked with the flags. The one derived # from `--project` is asked first, as `_checked_plan_inputs` asks it. - if args.id is None: - _refuse_bad_id(args.project.lower(), args.project, set(), named) + # One that was passed and is no id reaches here only beside a name the + # naming policy refuses (`_checked_flag_inputs`), which `_refuse_bad_id` + # refuses first, in the words it has once the source is known (#902). + products = set(declared_pin_products(args)) + passed = args.id is not None + _refuse_bad_id(args.id if passed else args.project.lower(), args.project, + {"id"} if passed else set(), named, naming, products) asked = argparse.Namespace(org=_planned_org(args.org, named), project=args.project) - _refuse_another_repository(asked, named, naming, - set(declared_pin_products(args)), - bool(args.org)) + _refuse_another_repository(asked, named, naming, products, + args.org is not None) def _last_two_directories(path: str) -> str | None: diff --git a/tests/test_adopt_check_plan_name.py b/tests/test_adopt_check_plan_name.py index 8e0c6339..2d2aff1c 100644 --- a/tests/test_adopt_check_plan_name.py +++ b/tests/test_adopt_check_plan_name.py @@ -53,8 +53,8 @@ from conftest import ADOPT, git, run_script from test_adopt_project_is_the_source import ( # noqa: F401 - a fixture - PLAN, PLAN_NOT_THE_SOURCE, RENAME_FIRST, RETYPE, adopter, check, edited, - findings, plan, refused_before_any_remote, source_of) + PLAN, PLAN_NOT_THE_SOURCE, RETYPE, adopter, check, edited, findings, + in_place_remedy, plan, refused_before_any_remote, source_of) from test_adopt_relative_submodule_url import ( HTTPS_ORIGIN, SIBLING, THING, planned as planned_with_submodule, source_with as source_with_submodule) @@ -140,7 +140,7 @@ def test_check_and_execute_say_the_same_remedy_of_the_sources_own_name( assert [line.split(":")[0] for line in findings(checked.stderr)] == [ f"FINDING {code}"], checked.stderr assert refusal.code == code - assert refusal.remediation == f"Remediation: {repository} {RENAME_FIRST}" + assert refusal.remediation == in_place_remedy(repository, code) # Said as `execute` says it, as a line of its own, after the findings. assert checked.stderr.splitlines().count(refusal.remediation) == 1, \ checked.stderr diff --git a/tests/test_adopt_manifest_values.py b/tests/test_adopt_manifest_values.py index f79beb5c..6feab907 100644 --- a/tests/test_adopt_manifest_values.py +++ b/tests/test_adopt_manifest_values.py @@ -443,9 +443,10 @@ def test_the_derived_ids_words_are_the_tables(adopter, monkeypatch): an edit to `FLAG_DEFAULTS["id"]` would not have reached it. Its label and its remedy are both read from the table.""" monkeypatch.setitem(adopter.FLAG_DEFAULTS, "id", "a stand-in") + naming = adopter.NamingPolicy.load(adopter.NAMING_POLICY) with pytest.raises(adopter.Refusal) as refused: adopter._refuse_bad_id("north_wind", "North_Wind", set(), - Namespace(repository=None)) + Namespace(repository=None), naming, set()) assert refused.value.detail.startswith( "--id (not given, so a stand-in) is 'north_wind'"), refused.value assert "The id is a stand-in, and" in refused.value.remediation diff --git a/tests/test_adopt_plan.py b/tests/test_adopt_plan.py index 79a1d810..43467b0b 100644 --- a/tests/test_adopt_plan.py +++ b/tests/test_adopt_plan.py @@ -373,8 +373,11 @@ def test_a_project_name_that_is_not_a_lowercase_id_is_refused(source_repo, #: `repo_shape.PROJECT_ID_RE`, as `adopt-bad-id` prints it. ID_PATTERN = "^[a-z0-9][a-z0-9-]*$" -#: What a passed `--id` is told to do: pass another. -PASS_AN_ID = "Remediation: pass an explicit lowercase --id." +#: What a passed `--id` is told to do: pass another, of what an id may hold +#: (#902), in the scaffold's and `family.py init`'s words (#911). "Pass an +#: explicit lowercase --id" was said to `--id foo_bar`, which is both. +PASS_AN_ID = ("Remediation: pass an --id of lowercase letters, digits and " + "hyphens, starting with a letter or digit.") #: How `adopt-bad-id` names an id `plan` derived from `--project`: #: `_flag_named` over `FLAG_DEFAULTS`, as the tracking branch and the #: elector are named (#529). @@ -413,16 +416,16 @@ def test_a_passed_id_that_is_not_a_lowercase_id_is_named_as_passed( assert not plan.exists() -@pytest.mark.parametrize("project", ["Northwind", "North_Wind"]) -def test_an_empty_id_is_refused_as_the_id_passed(source_repo, tmp_path, - project): +def test_an_empty_id_is_refused_as_the_id_passed(source_repo, tmp_path): """L2-53: `--id ''` was read as not passed, so a script's `--id "$ID"` with `ID` unset planned `id: northwind`, exit 0, and beside `--project North_Wind` was refused as an id derived from it, "(--id was not given)" (rv601's P4). It is a value typed, held to the id's rule as any other - and refused by name, as `--tracking-branch ''` is since #479.""" + and refused by name, as `--tracking-branch ''` is since #479. (Beside + `--project North_Wind` the name is refused first since #902: + `tests/test_adopt_values_as_typed.py`.)""" plan = tmp_path / "plan.yaml" - result = write_plan(source_repo, plan, project=project, + result = write_plan(source_repo, plan, project="Northwind", extra=("--id", "")) assert bad_id(result) == [ f"REFUSED adopt-bad-id: --id '' must match {ID_PATTERN}", @@ -479,17 +482,22 @@ def test_a_derived_ids_remedy_names_the_rule_naming_unclassified_does( f"Remediation: change --project to {NAME_RULE}. "), derived -def test_a_passed_id_equal_to_the_derived_one_is_still_named_as_passed( +def test_a_passed_id_equal_to_the_derived_one_meets_the_name_first( source_repo, tmp_path): """Whether the id was passed is what the person did, not whether it equals `--project` lowercased: `--id north_wind` beside `--project - North_Wind` is a flag typed, so its refusal names the flag.""" + North_Wind` is a flag typed, and was refused as one, "pass an explicit + lowercase --id", whose retype met `naming-unclassified` (L2-90 on #457). + Since #902 the name the naming policy refuses speaks first, and no id + refusal is said; nor is the derived id's.""" plan = tmp_path / "plan.yaml" result = write_plan(source_repo, plan, project="North_Wind", extra=("--id", "north_wind")) assert bad_id(result) == [ - f"REFUSED adopt-bad-id: --id 'north_wind' must match {ID_PATTERN}", - PASS_AN_ID], result.stderr + "REFUSED naming-unclassified: 'North_Wind' matches no family in the " + "naming policy", f"Remediation: --project takes {NAME_RULE}."], \ + result.stderr + assert "adopt-bad-id" not in result.stderr assert not plan.exists() diff --git a/tests/test_adopt_plan_flag_refusals_first.py b/tests/test_adopt_plan_flag_refusals_first.py index c4098297..a03eaed7 100644 --- a/tests/test_adopt_plan_flag_refusals_first.py +++ b/tests/test_adopt_plan_flag_refusals_first.py @@ -211,8 +211,13 @@ def refused_first(harness: Harness, result, code: str, said: str, id="id"), pytest.param(("--id", ""), "adopt-bad-id", "--id", id="id-empty"), pytest.param(("--org=-evil",), "unsafe-value", "--org", id="org"), + # A flag passed empty is a value passed (#902): refused by name here. + pytest.param(("--org", ""), "unsafe-value", "--org is ''", + id="org-empty"), pytest.param(("--elected-on", "2026-9-2"), "election-date-malformed", "--elected-on", id="elected-on"), + pytest.param(("--elected-on", ""), "election-date-malformed", + "--elected-on ''", id="elected-on-empty"), pytest.param(("--reference", "ref\nx"), "adopt-unwritable-value", "--reference", id="reference"), pytest.param(("--elected-by", "a\nb"), "adopt-unwritable-value", @@ -423,18 +428,15 @@ def test_control_the_sources_own_name_still_reaches_the_clone( assert calls[0].startswith(f"repo clone {cloned} "), calls -def test_control_an_empty_org_is_none_given_and_reaches_the_clone(harness): - """`--org ''` is read as no `--org` (`_planned_org`), so it is the - organisation of `--source` and no value refused for its text: `plan` - goes on to `gh repo clone`, which the stub refuses, once. Passes on - main as well.""" +def test_an_empty_org_is_a_value_passed_and_never_reaches_the_clone( + harness): + """`--org ''` was read as no `--org` (`_planned_org`), so it was the + organisation of `--source`, and `plan` went on to `gh repo clone`, + while `flags_given` counted it passed (L2-86 on #457). Since #902 it is + a value passed, refused by name with the other flags, and no `gh` + runs.""" result = harness.plan(REPOSITORY, "--org", "", work=True) - assert result.returncode == 2, result.stdout + result.stderr - assert result.stderr.startswith("REFUSED source-unclonable: "), \ - result.stderr - calls = harness.gh_calls() - assert len(calls) == 1, calls - assert calls[0].startswith(f"repo clone {REPOSITORY} "), calls + refused_first(harness, result, "unsafe-value", "--org is ''", True) def test_control_a_local_source_is_held_to_its_name_after_git_reads_it( diff --git a/tests/test_adopt_plan_yaml_quoting.py b/tests/test_adopt_plan_yaml_quoting.py index a1a29102..63ec9d68 100644 --- a/tests/test_adopt_plan_yaml_quoting.py +++ b/tests/test_adopt_plan_yaml_quoting.py @@ -1106,9 +1106,11 @@ def test_the_misread_rule_names_an_id_it_derived_from_the_assembly( #: named the branch by what was READ, with its kind, and then the text #: `execute` writes (review181's N2); the validator refused the id as `id is #: 7`. Since #259 the line is quoted and `7` reads back as the text `7`, so -#: nothing names any of them, as on main: `execute` writes `7` for a plan -#: that says `007`. A date or a visibility read as not text is still -#: refused (`LATE_DATES`, `NOT_TEXT_INVALID`), and a pin's product is #273's +#: neither rule named any of them, and `execute` wrote `7` for a plan that +#: says `007` (L1-K3 on #457). Since #902 `check` refuses each by what was +#: read: the branch as `plan-bad-tracking-branch`, the id as `plan-bad-id`. +#: A date or a visibility read as not text is still refused +#: (`LATE_DATES`, `NOT_TEXT_INVALID`), and a pin's product is #273's #: (`test_a_pin_whose_product_is_not_text_is_273s_finding_alone`). The #: source has the branch as `execute` takes it, `str` of what was read #: (#479). @@ -1118,31 +1120,45 @@ def test_the_misread_rule_names_an_id_it_derived_from_the_assembly( (TRACKING_KEY, "1.50", "the number 1.5", "1.5"), ("id", "007", SEVEN_READ, "7"), ] +#: The ones #181's misread rule is still asked about: an id read as not +#: text is `plan-bad-id` since #902, refused before any rule is asked. +MISREAD_AS_NOT_TEXT = [row for row in READ_AS_NOT_TEXT if row[0] != "id"] @pytest.mark.parametrize("key, spelled, read_as, written", READ_AS_NOT_TEXT, ids=ids_of(READ_AS_NOT_TEXT, 0, 1)) -def test_check_passes_a_plan_value_read_as_not_text_as_main_does( +def test_check_refuses_a_plan_value_read_as_not_text_by_what_was_read( answered_copy, tmp_path, key, spelled, read_as, written): """`check` named each of these, the branch as #181's misread and the id in the validator's words, while the template wrote its line bare. Since #259 the line is quoted, the text `execute` writes reads back, and - neither rule has a finding: `check` passes the plan, as main does, and - `execute` writes `str` of what was read, `7` for `007` (a residual this - change shares with main, in its description). `read_as` is the - misread rule's answer for a bare line, asked in the test below.""" + neither rule had a finding: `check` passed the plan and `execute` wrote + `str` of what was read, `7` for `007` (L1-K3 on #457). Since #902 each + is refused by what the plan's reader made of it, and the text `execute` + would have taken: the branch is a finding under its key's code, and the + id is `plan-bad-id`, which `check` and `execute` refuse an id as.""" source, text = answered_copy holding(source, key, written) plan = tmp_path / PLAN_FILE carrying(plan, text, key, spelled) result = checked(plan) - assert result.returncode == 0, result.stdout + result.stderr - assert PLAN_OK in result.stdout, result.stdout - assert ANY_FINDING not in result.stderr, result.stderr + said = (f"`{key}:` was read out of the plan as {read_as}, not as text, " + f"so `execute` would take it as the text `{written}`") + assert PLAN_OK not in result.stdout, result.stdout + if key == "id": + assert result.returncode == 2, result.stdout + result.stderr + assert result.stderr.startswith( + f"REFUSED plan-bad-id: {plan}: {said}\n"), result.stderr + else: + assert result.returncode == 1, result.stdout + result.stderr + assert [line for line in result.stderr.splitlines() + if line.startswith(ANY_FINDING)] == [ + f"FINDING plan-bad-tracking-branch: {said}"], result.stderr -@pytest.mark.parametrize("key, spelled, read_as, written", READ_AS_NOT_TEXT, - ids=ids_of(READ_AS_NOT_TEXT, 0, 1)) +@pytest.mark.parametrize("key, spelled, read_as, written", + MISREAD_AS_NOT_TEXT, + ids=ids_of(MISREAD_AS_NOT_TEXT, 0, 1)) def test_the_misread_rule_names_a_plan_value_that_is_not_text_by_what_was_read( adopter, answered, bare_template, tmp_path, key, spelled, read_as, written): diff --git a/tests/test_adopt_project_is_the_source.py b/tests/test_adopt_project_is_the_source.py index b1921f81..36745b1a 100644 --- a/tests/test_adopt_project_is_the_source.py +++ b/tests/test_adopt_project_is_the_source.py @@ -66,6 +66,30 @@ #: before the repository and `RENAME_FIRST` (#658). DERIVED_OWN_NAME = ("the id is --project lowercased, and --project is the " f"source's own name, which must be {NAME_RULE}.") +#: What a naming refusal of the source's own name says before the repository +#: and `RENAME_FIRST`, in place, by its code (#902, L2-89 on #457): the rule +#: the new name must follow, the one the same refusal gives `--project` for +#: a name that is not the source's. +OWN_NAME_SAID = { + "naming-unclassified": ("--project is the source's own name, which must " + f"be {NAME_RULE}."), + "naming-role-mismatch": ( + "--project is the source's own name, which must be one CamelCase " + "token. Two forms beyond the bare one MAY be the assembly root: a " + "declared descendant (declare the pin with `--pin open@<40 " + "hex commit>` and it classifies as one, 2026-09-02), and an " + "`open` neutral product, which may elect the shape and be " + "its own root (2026-09-05). Neither may be the spec or code leg, and " + "an `-Install` may be no leg at all."), +} + + +def in_place_remedy(repository: str, code: str) -> str: + """The remediation of naming refusal `code` of `repository`'s own name, + adopted in place: the rule, then renaming first.""" + return f"Remediation: {OWN_NAME_SAID[code]} {repository} {RENAME_FIRST}" + + #: A source small enough that `plan` asks no question. TREE = {"README.md": "# here\n", "src/app.py": "print(1)\n", "specs/001/spec.md": "# Spec\n"} @@ -170,8 +194,8 @@ def test_plan_refuses_a_name_that_is_not_the_sources(tmp_path, repository, else: assert remediation.endswith( "The source's own name is no remedy either: 'billing-service' " - "matches no family in the naming policy. acme/billing-service " - f"{RENAME_FIRST}"), remediation + f"matches no family in the naming policy. The name must be " + f"{NAME_RULE}. acme/billing-service {RENAME_FIRST}"), remediation assert RETYPE not in refused.stderr assert not (tmp_path / PLAN).exists() @@ -193,11 +217,37 @@ def test_the_sources_own_name_the_policy_refuses_is_renamed_first( assert refused.returncode == 2, refused.stdout + refused.stderr detail, remediation = refused.stderr.splitlines()[:2] assert detail.startswith(f"REFUSED {code}: "), refused.stderr - assert remediation == f"Remediation: {repository} {RENAME_FIRST}" + assert remediation == in_place_remedy(repository, code) assert RETYPE not in refused.stderr assert not (tmp_path / PLAN).exists() +def test_not_the_source_gives_the_rule_of_the_names_own_refusal(tmp_path): + """#902, L2-89: `--project ToolsInstall` over a source `acme/Tools-Install` + is not the source, and the source's own name is refused by + `naming-role-mismatch`, an install and no project leg, not by + `naming-unclassified`. The rule it gives the new name is that code's, the + one `plan` gives `--project` for it (`NAME_RULES`), and not `no hyphen, + underscore, dot or space`, which `Tools-Install` as an install form + would be told wrongly.""" + source = source_of(tmp_path, "acme/Tools-Install") + + refused = plan(tmp_path, source, "--project", "ToolsInstall") + + assert refused.returncode == 2, refused.stdout + refused.stderr + detail, remediation = refused.stderr.splitlines()[:2] + assert detail.startswith( + f"REFUSED {NOT_THE_SOURCE}: --project ToolsInstall, {OF_ORIGIN}, " + "names acme/ToolsInstall, and the source is acme/Tools-Install: "), \ + detail + rule = OWN_NAME_SAID["naming-role-mismatch"].split("which must be ", 1)[1] + assert remediation.endswith( + f"The name must be {rule} acme/Tools-Install {RENAME_FIRST}"), \ + remediation + assert f"The name must be {NAME_RULE}" not in remediation + assert not (tmp_path / PLAN).exists() + + def test_a_name_in_another_case_is_the_source(tmp_path): """GitHub resolves `acme/atlas` and `ACME/Atlas` to one repository, so they are the source, and `plan` and `check` pass.""" @@ -452,8 +502,8 @@ def test_a_plan_older_than_its_sources_origin_is_caught_by_check_and_execute( "in-place adoption keeps the source's own name, and `execute` acts " "on the repository `org:` and `legs.assembly:` name"] rename = (" The source's own name is no remedy either: 'billing-service' " - f"matches no family in the naming policy. acme/billing-service " - f"{RENAME_FIRST}") + f"matches no family in the naming policy. The name must be " + f"{NAME_RULE}. acme/billing-service {RENAME_FIRST}") assert f"Remediation:{rename}\n" in checked.stderr, checked.stderr assert refusal.code == PLAN_NOT_THE_SOURCE assert refusal.remediation == ( @@ -484,8 +534,8 @@ def test_execute_says_rename_first_of_the_sources_own_name(adopter, tmp_path, tmp_path / PLAN, tmp_path / "work") assert refusal.code == "naming-unclassified" - assert refusal.remediation == \ - f"Remediation: acme/billing-service {RENAME_FIRST}" + assert refusal.remediation == in_place_remedy("acme/billing-service", + "naming-unclassified") def test_a_source_with_no_origin_executes_past_the_comparison( diff --git a/tests/test_adopt_values_as_typed.py b/tests/test_adopt_values_as_typed.py new file mode 100644 index 00000000..bf4985e8 --- /dev/null +++ b/tests/test_adopt_values_as_typed.py @@ -0,0 +1,498 @@ +# SPDX-License-Identifier: Apache-2.0 +"""`plan` and `check` take a value as it was typed, or refuse it by name (#902). + +Lane 3 grouped seven lines of #457 into #902, each measured at `5017830` and +again on main before this change: + +- L1-K3. A hand-edited plan's bare `id: 007`, `tracking_branch: true` or + `adopt_branch: 007`, which the plan's reader reads as a number or a boolean, + was `str` of what was read: `check` said `plan ok`, `execute` said + `adoption verified`, and `project.yaml` said `id: "7"`, or both legs and the + split were on the branch `True`. `check` now refuses an `id:` read so, as + `plan-bad-id`, and finds a branch read so under the key's own code, saying + what was read; `execute` refuses both before any leg exists, in the same + words (`tracking_branch: -007` too: `execute` asked `checked_value` first, + so it said `unsafe-value` of the `-7` nobody typed where `check` said + `plan-bad-tracking-branch`). +- L2-86 and L2-87. `--org ''`, `--elected-on ''` and `--reference ''` were read + as not passed: the organisation of the source's `origin` was planned and + named once as `--org is '~x'`, today's date was recorded, and the default + reference. Each is a value passed now, refused by name by its own rule, as + `--tracking-branch ''` is. +- L2-88. `check`'s `plan-bad-id` of a plan with no `id:` said "`id:` is + 'north_wind'" and sent the person to `--id`. It names the id as derived + from `legs.assembly`, and that as the remedy. +- L2-89. In place, `naming-unclassified` and `adopt-not-the-source` gave the + way out, renaming the source, and no rule for the new name. Both say it. +- L2-90 and L2-82 (adopt's half of #459's). A passed `--id` that is no id was + refused before a `--project` the naming policy refuses, and was told "pass + an explicit lowercase --id" though `foo_bar` is both. The naming refusal + speaks first, and the id's remedy says what an id may hold, in the words + the scaffold and `family.py init` use (#911). + +Nothing here reaches GitHub: every source is local, `check` is run against a +plan whose names it asks `gh repo view` about only once they pass (the +session's guard answers, `tests/gh_guard.py`), and `execute` runs under +`--local-remote-dir`. +""" + +from __future__ import annotations + +import re +import shutil +import sys +from pathlib import Path + +import pytest + +from conftest import ADOPT, git, make_source_repo, run_script, write_plan +from test_adopt_plan_flag_refusals_first import Harness +from test_adopt_project_is_the_source import (NAME_RULE, RENAME_FIRST, TREE, + findings, in_place_remedy, + source_of) + +#: `repo_shape.PROJECT_ID_RE`, as the refusals print it. +PATTERN = "^[a-z0-9][a-z0-9-]*$" +#: What a passed `--id` that is no id is told (#902), in the scaffold's and +#: `family.py init`'s words (#911). +WHAT_AN_ID_HOLDS = ("Remediation: pass an --id of lowercase letters, digits " + "and hyphens, starting with a letter or digit.") +#: And a plan's `id:` the pattern refuses. +PLAN_ID_HOLDS = ( + "Remediation: set `id:` in the plan to lowercase letters, digits and " + "hyphens, starting with a letter or digit, or re-run `plan` with an --id " + "of that form. The GitHub topic is derived from it.") +#: What a plan with no `id:` is told: the name its id comes from. +LEGS_ASSEMBLY = ( + f"Remediation: change `legs.assembly` in the plan to {NAME_RULE}, or " + "re-run `plan` with a --project of that form. The id is legs.assembly " + "lowercased, and an `id:` would leave `legs.assembly`, the assembly " + "root's name, as it is. The GitHub topic is derived from the id.") +#: What a value read as a number or a boolean is told. +QUOTE_IT = ( + "Remediation: write each value named between double quotes in the plan, " + 'as typed (`"007"`, `"true"`), so that the plan\'s reader takes it as ' + "text, and run `check` again; or re-run `plan`, which writes every value " + "so that it is read back as typed.") +#: `checked_value`'s words for an empty value, as `--tracking-branch ''` +#: has them. +EMPTY_ORG = ("REFUSED unsafe-value: --org is '', which is not a value this " + "tool will put on a `git` or `gh` command line") + +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + + +def first_two(result) -> list[str]: + """The refusal and its remedy, the first two lines on stderr, once the + run exited 2.""" + assert result.returncode == 2, result.stdout + result.stderr + return result.stderr.splitlines()[:2] + + +def planned(tmp_path: Path, *flags: str, branches: tuple = ()) -> Path: + """A plan of a small source with no `origin`, written by `plan` with + `flags`, and the source given `branches` besides `main` first.""" + source = make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + for branch in branches: + git("branch", branch, cwd=source) + out = tmp_path / "adoption-plan.yaml" + result = write_plan(source, out, extra=flags) + assert result.returncode == 0, result.stdout + result.stderr + return out + + +def set_line(plan: Path, key: str, value: str | None) -> None: + """The plan's top-level `key:` line set to `value`, or deleted where it + is None, as a hand edit makes it, LF as `plan` wrote it.""" + text = plan.read_text(encoding="utf-8") + pattern = re.compile(rf"^{re.escape(key)}: .*\n", re.M) + assert len(pattern.findall(text)) == 1, text + text = pattern.sub("" if value is None else f"{key}: {value}\n", text) + with plan.open("w", encoding="utf-8", newline="\n") as handle: + handle.write(text) + + +def check(plan: Path): + return run_script(ADOPT, "check", "--plan", str(plan)) + + +def execute(plan: Path, base: Path): + return run_script(ADOPT, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work")) + + +def not_text(key: str, read: str, text: str) -> str: + return (f"`{key}:` was read out of the plan as {read}, not as text, so " + f"`execute` would take it as the text `{text}`") + + +# --- L1-K3: a bare value the plan's reader reads as a number or a boolean --- + +@pytest.mark.parametrize("key, typed, branch, read, text", [ + ("tracking_branch", "007", "7", "the number 7", "7"), + ("tracking_branch", "true", "True", "the boolean true", "True"), + ("tracking_branch", "1.50", "1.5", "the number 1.5", "1.5"), + ("adopt_branch", "007", None, "the number 7", "7"), + ("adopt_branch", "true", None, "the boolean true", "True"), +], ids=["tracking-007", "tracking-true", "tracking-1.50", "split-007", + "split-true"]) +def test_check_finds_a_branch_read_as_a_number_or_a_boolean( + tmp_path, key, typed, branch, read, text): + """L1-K3 and L1-15: with a branch `7` in the source, `tracking_branch: + 007` checked ok and was adopted onto `7`. It is a finding under the key's + own code, saying what the plan's reader made of it and the text + `execute` would have taken, and no other finding names it.""" + plan = planned(tmp_path, branches=(branch,) if branch else ()) + set_line(plan, key, typed) + + checked = check(plan) + + assert checked.returncode == 1, checked.stdout + checked.stderr + code = key.replace("_", "-") + assert findings(checked.stderr) == [ + f"FINDING plan-bad-{code}: {not_text(key, read, text)}"], \ + checked.stderr + assert QUOTE_IT in checked.stderr.splitlines(), checked.stderr + + +@pytest.mark.parametrize("typed, read, text", [ + ("007", "the number 7", "7"), + ("true", "the boolean true", "True"), + ("2026", "the number 2026", "2026"), +], ids=["007", "true", "2026"]) +def test_check_refuses_an_id_read_as_a_number_or_a_boolean(tmp_path, typed, + read, text): + """L1-K3: `id: 007` was the id `7`, which the pattern takes, so `check` + said `plan ok` and `execute` wrote `id: "7"`. It is `plan-bad-id`, where + `check` and `execute` refuse an id, saying what was read; `2026`, whose + text is the same, is refused too, because what was typed is not known.""" + plan = planned(tmp_path) + set_line(plan, "id", typed) + + assert first_two(check(plan)) == [ + f"REFUSED plan-bad-id: {plan}: {not_text('id', read, text)}", + QUOTE_IT] + + +@pytest.mark.parametrize("key, quoted, branches", [ + ("id", '"007"', ()), + ("tracking_branch", '"007"', ("007",)), + ("adopt_branch", '"true"', ()), +], ids=["id", "tracking-branch", "split-branch"]) +def test_the_same_values_quoted_check_ok(tmp_path, key, quoted, branches): + """Control: the remedy, typed. Between double quotes each is text, as + typed, and `check` says `plan ok`.""" + plan = planned(tmp_path, branches=branches) + set_line(plan, key, quoted) + + checked = check(plan) + + assert checked.returncode == 0, checked.stdout + checked.stderr + assert "plan ok" in checked.stdout + + +@needs_filter_repo +@pytest.mark.parametrize("key, typed, branch, code", [ + ("id", "007", None, "plan-bad-id"), + ("tracking_branch", "true", "True", "plan-bad-tracking-branch"), + ("adopt_branch", "007", None, "plan-bad-adopt-branch"), +], ids=["id", "tracking-branch", "split-branch"]) +def test_execute_refuses_each_before_any_leg_exists(tmp_path, key, typed, + branch, code): + """L1-K3: `execute` of each ended `adoption verified`, exit 0. It refuses + under `check`'s code before a leg repository is made or anything is + pushed: the remote directory holds no leg, and the source no split.""" + plan = planned(tmp_path, branches=(branch,) if branch else ()) + set_line(plan, key, typed) + + done = execute(plan, tmp_path) + + assert done.returncode == 2, done.stdout + done.stderr + assert done.stderr.startswith(f"REFUSED {code}: "), done.stderr + assert "adoption verified" not in done.stdout + remotes = tmp_path / "remotes" + assert not any(remotes.glob("*.git")), list(remotes.iterdir()) + branches = git("branch", "--list", cwd=tmp_path / "Thing").stdout + assert "adopt/three-repo-shape" not in branches + assert not re.search(r"^\s*7$", branches, re.M), branches + + +@needs_filter_repo +@pytest.mark.parametrize("key, code", [ + ("tracking_branch", "plan-bad-tracking-branch"), + ("adopt_branch", "plan-bad-adopt-branch"), +], ids=["tracking-branch", "split-branch"]) +def test_execute_refuses_a_negative_number_in_checks_words(tmp_path, key, + code): + """L1-K3: `tracking_branch: -007` is the number -7, and `-7` is a value + `checked_value` refuses, so `execute`, which asked it first, said + `unsafe-value` of a `-7` nobody typed while `check` said what was read + (#520: the two refuse a branch name in the same words). It asks the + number-or-boolean question first, as `check` does, and refuses under the + key's own code with `check`'s sentence and remedy, before any leg + exists.""" + plan = planned(tmp_path) + set_line(plan, key, "-007") + sentence = not_text(key, "the number -7", "-7") + + checked = check(plan) + done = execute(plan, tmp_path) + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [f"FINDING {code}: {sentence}"], \ + checked.stderr + assert first_two(done) == [f"REFUSED {code}: {sentence}", QUOTE_IT] + assert "unsafe-value" not in done.stderr + assert not any((tmp_path / "remotes").glob("*.git")) + + +# --- L2-86 and L2-87: a flag passed empty is a value ------------------------ + +@pytest.mark.parametrize("origin", [ + "https://example.invalid/~x/Thing.git", + "https://github.com/testorg/Atlas.git", + None, +], ids=["origin-org-the-validator-refuses", "another-repository", + "no-origin"]) +def test_an_empty_org_is_refused_as_passed(tmp_path, origin): + """L2-86: `--org ''` was counted passed for its label and read as not + given for its value, so the organisation of the source's `origin` was + planned: one refusal said `--org is '~x'`, which nobody typed, and + `adopt-not-the-source` said `--org` was not given; with no `origin` it + was `adopt-no-org`. It is a value passed, refused by name as + `--tracking-branch ''` is, before the source's own organisation is + read.""" + source = make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + if origin is not None: + git("remote", "add", "origin", origin, cwd=source) + out = tmp_path / "adoption-plan.yaml" + + refused = write_plan(source, out, project="Thing", org="") + + assert first_two(refused)[0] == EMPTY_ORG, refused.stderr + assert "not given" not in refused.stderr, refused.stderr + assert not out.exists() + + +def test_an_empty_election_date_is_refused_by_name(tmp_path): + """L2-87: `--elected-on ''` recorded today's date, exit 0. It is the + date passed, and `election_date` refuses it by name.""" + source = make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + out = tmp_path / "adoption-plan.yaml" + + refused = write_plan(source, out, extra=("--elected-on", "")) + + assert first_two(refused)[0] == ( + "REFUSED election-date-malformed: --elected-on '' is not a " + "YYYY-MM-DD date (and a plan file's `elected_on:` carries the same " + "form)"), refused.stderr + assert not out.exists() + + +def test_an_empty_reference_is_refused_in_the_validators_words(tmp_path): + """L2-87: `--reference ''` recorded the default reference, exit 0. It is + the reference passed, and the project's own validator refuses it, named + as the flag, as the scaffold's is (#911).""" + source = make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + out = tmp_path / "adoption-plan.yaml" + + refused = write_plan(source, out, extra=("--reference", "")) + + assert first_two(refused)[0] == ( + "REFUSED adopt-manifest-invalid: --reference is '', which the " + "project's own validate-manifest.py refuses: manifest-reference: " + "reference is ''; drop the key or name the document the election " + "followed."), refused.stderr + assert not out.exists() + + +def test_check_finds_an_empty_reference_in_the_plan(tmp_path): + """And `reference: ""` in a plan is that value too: `execute` wrote the + default reference in its place without a word. `check` asks the + validator of it, by the plan's key.""" + plan = planned(tmp_path) + set_line(plan, "reference", '""') + + checked = check(plan) + + assert checked.returncode == 1, checked.stdout + checked.stderr + assert findings(checked.stderr) == [ + "FINDING plan-manifest-invalid: reference is '', which the project's " + "own validate-manifest.py refuses: manifest-reference: reference is " + "''; drop the key or name the document the election followed."], \ + checked.stderr + + +@pytest.mark.parametrize("reference, chosen", [ + (None, True), ('""', False)], ids=["no-reference", "empty-reference"]) +def test_a_bad_date_says_it_chooses_the_reference_only_where_none_is_set( + tmp_path, reference, chosen): + """With `reference: ""` a value, the date chooses no reference: #294's + finding of a bad `elected_on:` says it does only where the plan sets + none, as `execute` now reads it.""" + plan = planned(tmp_path) + set_line(plan, "elected_on", '"someday"') + set_line(plan, "reference", reference) + + checked = check(plan) + + assert checked.returncode == 1, checked.stdout + checked.stderr + said = [line for line in findings(checked.stderr) + if line.startswith("FINDING election-date-malformed: ")] + assert len(said) == 1, checked.stderr + assert said[0].endswith("chooses the reference by it") == chosen, said + + +# --- L2-88: a plan with no `id:` --------------------------------------------- + +def test_a_plan_with_no_id_names_its_id_as_derived_from_legs_assembly( + tmp_path): + """L2-88: rv699's plan, its `id:` deleted and its `legs.assembly` edited + to `North_Wind`. The refusal named "`id:`", which the plan has none of, + and sent the person to `--id`; it names the id as `DERIVED_ID` does and + sends them to `legs.assembly`.""" + plan = planned(tmp_path, "--project", "NorthWind", "--id", "northwind") + set_line(plan, "id", None) + text = plan.read_text(encoding="utf-8") + with plan.open("w", encoding="utf-8", newline="\n") as handle: + handle.write(text.replace("\n assembly: NorthWind\n", + "\n assembly: North_Wind\n", 1)) + + assert first_two(check(plan)) == [ + f"REFUSED plan-bad-id: {plan}: id (not set in the plan, so " + f"legs.assembly lowercased) is 'north_wind', which must match " + f"{PATTERN}", LEGS_ASSEMBLY] + + +@pytest.mark.parametrize("typed", ["foo_bar", '"North Wind"']) +def test_a_plans_id_the_pattern_refuses_is_told_what_an_id_may_hold( + tmp_path, typed): + """L2-82's sentence, for the plan's own key: `id: foo_bar` was told to + re-run `plan` "with an explicit lowercase --id", and it is both.""" + plan = planned(tmp_path) + set_line(plan, "id", typed) + held = typed.strip('"') + + assert first_two(check(plan)) == [ + f"REFUSED plan-bad-id: {plan}: `id:` is {held!r} and must match " + f"{PATTERN}", PLAN_ID_HOLDS] + + +# --- L2-89, L2-90 and L2-82: the name before a passed id --------------------- + +def test_in_place_the_naming_refusal_says_the_rule_before_renaming(tmp_path): + """L2-89: the source's own name, `North_Wind`, is refused in place with + `_rename_first`'s way out, which said no rule for the new name. It says + the rule first, as the derived id's refusal does (L2-54).""" + source = source_of(tmp_path, "testorg/North_Wind") + + refused = run_script(ADOPT, "plan", "--source", str(source), + "--project", "North_Wind", "--id", "north-wind", + "--elected-by", "Test Human", "--out", + str(tmp_path / "plan.yaml")) + + assert first_two(refused) == [ + "REFUSED naming-unclassified: 'North_Wind' matches no family in the " + "naming policy", + in_place_remedy("testorg/North_Wind", "naming-unclassified")] + + +def test_not_the_source_says_the_rule_its_own_name_breaks(tmp_path): + """L2-89: `--project NorthWind` over `testorg/North_Wind` is + `adopt-not-the-source`, and the source's own name is no remedy; the way + out said no rule for the new name, and says it now.""" + source = source_of(tmp_path, "testorg/North_Wind") + + refused = run_script(ADOPT, "plan", "--source", str(source), + "--project", "NorthWind", "--id", "north-wind", + "--elected-by", "Test Human", "--out", + str(tmp_path / "plan.yaml")) + + remedy = first_two(refused)[1] + assert remedy == ( + "Remediation: no plan was written. The source's own name is no " + "remedy either: 'North_Wind' matches no family in the naming policy. " + f"The name must be {NAME_RULE}. testorg/North_Wind {RENAME_FIRST}"), \ + refused.stderr + + +NOT_IN_PLACE_RULE = f"Remediation: --project takes {NAME_RULE}." + + +@pytest.mark.parametrize("origin, project, passed, code, remedy", [ + (None, "North_Wind", "north_wind", "naming-unclassified", + NOT_IN_PLACE_RULE), + (None, "North_Wind", "", "naming-unclassified", NOT_IN_PLACE_RULE), + ("testorg/Atlas", "North_Wind", "north_wind", "naming-unclassified", + NOT_IN_PLACE_RULE), + ("testorg/North_Wind", "North_Wind", "north_wind", "naming-unclassified", + in_place_remedy("testorg/North_Wind", "naming-unclassified")), + (None, "Tools-Install", "tools_install", "naming-role-mismatch", + "Remediation: --project takes one CamelCase token. Two forms beyond " + "the bare one MAY be the assembly root"), +], ids=["no-origin", "empty-id", "another-repository", "in-place", + "not-a-project-leg"]) +def test_a_refused_name_speaks_before_a_passed_id(tmp_path, origin, project, + passed, code, remedy): + """L2-90: `--project North_Wind --id north_wind` was told "pass an + explicit lowercase --id", and its retype, `--id north-wind`, then met + `naming-unclassified`, which no `--id` mends. The naming refusal speaks + first, as the scaffold's and `family.py init`'s do (#911), in the words + it has where it speaks later: in place, the source's own name is + renamed first; another repository's name is the person's to retype.""" + source = source_of(tmp_path, origin) + + refused = run_script(ADOPT, "plan", "--source", str(source), + "--project", project, "--org", "testorg", + f"--id={passed}", "--elected-by", "Test Human", + "--out", str(tmp_path / "plan.yaml")) + + detail, said = first_two(refused) + assert detail.startswith(f"REFUSED {code}: {project!r} "), refused.stderr + assert said.startswith(remedy), refused.stderr + assert "adopt-bad-id" not in refused.stderr + + +@pytest.mark.parametrize("passed", ["foo_bar", "Foo-Bar", "-north", + "north wind", ""]) +def test_a_passed_id_is_told_what_an_id_may_hold(tmp_path, passed): + """L2-82, adopt's half: `--project NorthWind --id foo_bar` was told + "pass an explicit lowercase --id", though it is explicit and lowercase: + its fault is the underscore. The remedy says what an id may hold, in + the scaffold's words (#911); the name passes, so the id is refused.""" + source = make_source_repo(tmp_path / "Thing", tree=TREE, edits=()) + out = tmp_path / "adoption-plan.yaml" + + refused = write_plan(source, out, extra=(f"--id={passed}",)) + + assert first_two(refused) == [ + f"REFUSED adopt-bad-id: --id {passed!r} must match {PATTERN}", + WHAT_AN_ID_HOLDS] + assert not out.exists() + + +@pytest.mark.skipif(sys.platform == "win32", + reason="the `gh` stub is a `#!/bin/sh` script") +def test_an_org_repo_sources_own_name_speaks_before_a_passed_id_uncloned( + tmp_path): + """L2-90 with an `org/repo` source, which `plan` clones: since #903 a + passed `--id` is refused with the flags, before the clone, and the + naming refusal after it. A passed id beside a name the policy refuses + is left to where the source is known, and that is before the clone for + an `org/repo` one: the name is refused, in place, and `gh` never runs.""" + harness = Harness(tmp_path) + + refused = harness.plan( + "testorg/North_Wind", "--id", "north_wind", work=True, + planned=("--project", "North_Wind", "--org", "testorg", + "--elected-by", "Test Human", "--elected-on", "2026-09-02")) + + assert first_two(refused) == [ + "REFUSED naming-unclassified: 'North_Wind' matches no family in the " + "naming policy", + in_place_remedy("testorg/North_Wind", "naming-unclassified")] + assert harness.gh_calls() == [] + assert not harness.work.exists()