feat: batch renames, Python import rewrites and repairs of existing rule failures in sqb rename and sqb mv (#1229)

* feat: batch model renames, Python import rewrites and repairs of existing rule failures in sqb rename and sqb mv

sqb rename takes several model TARGET NEW_NAME pairs, or --from-file, and plans
and verifies them together like batch moves: one compile before, one of the
edited project after, all or nothing. Each renamed model gets its own
migrate_from, so every old name keeps a compatibility view. Column renames and
--cascade stay single and are refused in batch form; name reuse stays refused.

The scratch copy and the real write now apply changes in the same order: write
every change, then remove only the moved sources no change writes to, so a
chained move cannot lose a file in the real write while verification passes.

sqb mv rewrites project Python imports of the Python files it moves (direct,
aliased, grouped, from-package and relative imports) through a reusable
import-rewrite function, and refuses with the file and line when an import
cannot be rewritten safely: an unaliased import, a star import from the moved
module's package, a dynamic import naming it, or a mixed from-package import.

A project that already fails project rules can be refactored: existing rule
failures no longer block planning, and the edited project is compared with
them. New or worse rule failures, and every other error, still block. Text and
JSON output, including --dry-run, explain the failures before and after.

Projects failing at the input stage, such as a duplicate macro name, are
refused cleanly with their compile error, and --json prints an error document
on stdout when the project does not compile or the request is invalid.

* fix: plan SQL tests beside existing rule failures and block any growth in a rule failure

With rule failures in the project, the compiles of sqb rename and sqb mv
skipped SQL-test planning, so a planning error could pass verification.
They now plan SQL tests whenever the only errors are rule failures, as
sqb compile does; planning errors block.

A rule failure of the edited project is existing only with the same code,
place and message, or when only its numbers changed and none grew. Any
other change blocks, wherever it is reported, so a failure reported at a
sample file can no longer grow unnoticed. Nothing counts as resolved when
another error may have kept rules from running.

Python import rewriting also refuses importing a moved module by name from
a package whose __init__.py binds that name, or when the move renames the
module, and refuses a move while a project Python file is not UTF-8.

sqb rename with a target and no new name, or with nothing, is again a
usage error with exit code 2.

* test: regenerate refactor goldens for the rule baseline report

Author: kvlonge

Cargo.lock

@@ -5,8 +5,8 @@ use crate::argv::constants::{
     COMMAND_DEST, CONCURRENCY_COMMANDS, CONCURRENCY_ENV_VAR, CURSOR_COMMANDS, DBT_COMMAND,
     DBT_DEBUG_COMMAND, DBT_PASSTHROUGH_COMMANDS, DBT_PASSTHROUGH_SEPARATOR, DBT_VERBOSE_OPTION,
     DBT_VERBOSE_OPTIONS, DEBUG_OPTION, INT_CONVERTER, JANITOR_COMMAND, NO_COLOR_OPTION,
-    POSITIONAL_SELECT_COMMANDS, SCOPE_COMMAND, SCOPE_DEFAULT_PAGE_SIZE, SCOPE_GLOBAL_SUMMARY,
-    SCOPE_KIND_DEST, TEST_COMMAND,
+    POSITIONAL_SELECT_COMMANDS, RENAME_COMMAND, SCOPE_COMMAND, SCOPE_DEFAULT_PAGE_SIZE,
+    SCOPE_GLOBAL_SUMMARY, SCOPE_KIND_DEST, TEST_COMMAND,
 };
 use crate::argv::models::{Namespace, ParserSpec, Value};
 
@@ -96,6 +96,12 @@ impl Run<'_> {
                 ));
             }
         }
+        if command == RENAME_COMMAND
+            && let Some(message) = rename_error(&namespace)
+        {
+            let rename: &ParserSpec = subcommand_parser(parser, RENAME_COMMAND).unwrap_or(parser);
+            return Err(self.error(rename, &message));
+        }
         if command == SCOPE_COMMAND
             && let Some(message) = scope_error(parser, &namespace)
         {
@@ -263,3 +269,28 @@ fn integer(value: &Value) -> i128 {
         i128::MAX
     })
 }
+
+/// The usage error of a `sqb rename` without `--from-file` and without one full pair.
+fn rename_error(namespace: &Namespace) -> Option<String> {
+    if namespace.value("refactor_from_file") != Value::None
+        || namespace.value("refactor_new_name") != Value::None
+    {
+        return None;
+    }
+    let missing: &str = if namespace.value("refactor_target") == Value::None {
+        "TARGET, NEW_NAME"
+    } else {
+        "NEW_NAME"
+    };
+    Some(format!("the following arguments are required: {missing}"))
+}
+
+/// The parser of sub-command `name` below `parser`.
+fn subcommand_parser<'a>(parser: &'a ParserSpec, name: &str) -> Option<&'a ParserSpec> {
+    parser
+        .actions
+        .iter()
+        .flat_map(|action| &action.parsers)
+        .find(|(command, _)| command == name)
+        .map(|(_, spec)| spec)
+}

crates/sqlbuild-cli/src/argv/_helpers/normalization.rs

@@ -4891,18 +4891,37 @@
        {
         "kind": "store",
         "dest": "refactor_target",
-        "required": true,
+        "nargs": "?",
         "help": "<model> or <model>.<column>; the model: and column: prefixes are optional",
         "metavar": "TARGET",
         "group": 0
        },
        {
         "kind": "store",
         "dest": "refactor_new_name",
-        "required": true,
+        "nargs": "?",
         "metavar": "NEW_NAME",
         "group": 0
        },
+       {
+        "kind": "store",
+        "dest": "refactor_more_moves",
+        "nargs": "*",
+        "default": [],
+        "help": "further model TARGET NEW_NAME pairs, renamed together with the first",
+        "metavar": "MORE",
+        "group": 0
+       },
+       {
+        "kind": "store",
+        "options": [
+         "--from-file"
+        ],
+        "dest": "refactor_from_file",
+        "help": "read model TARGET NEW_NAME pairs from FILE, one per line; # starts a comment",
+        "metavar": "FILE",
+        "group": 1
+       },
        {
         "kind": "store_true",
         "options": [

crates/sqlbuild-cli/src/argv/command_table.json

@@ -51,6 +51,7 @@ pub(crate) const POSITIONAL_SELECT_COMMANDS: [&str; 8] = [
 ];
 pub(crate) const CONCURRENCY_COMMANDS: [&str; 5] = ["audit", "build", "load", "seed", "test"];
 pub(crate) const CURSOR_COMMANDS: [&str; 3] = ["build", "load", "plan"];
+pub(crate) const RENAME_COMMAND: &str = "rename";
 pub(crate) const SCOPE_COMMAND: &str = "scope";
 pub(crate) const TEST_COMMAND: &str = "test";
 pub(crate) const JANITOR_COMMAND: &str = "janitor";

crates/sqlbuild-cli/src/argv/constants.rs

@@ -88,6 +88,18 @@ fn given_help_version_or_rejected_arguments_when_parsing_then_exits_after_writin
             expected_exit: 2,
             expected_last_write: "sqb rules: error[C900]: the following arguments are required: rules_command\n\n",
         },
+        ExitTestCase {
+            description: "a rename target without a new name stays a usage error",
+            argv: &["rename", "stg_orders"],
+            expected_exit: 2,
+            expected_last_write: "sqb rename: error[C900]: the following arguments are required: NEW_NAME\n\n",
+        },
+        ExitTestCase {
+            description: "a rename without a target or a file stays a usage error",
+            argv: &["rename"],
+            expected_exit: 2,
+            expected_last_write: "sqb rename: error[C900]: the following arguments are required: TARGET, NEW_NAME\n\n",
+        },
     ];
 
     for test_case in test_cases {

crates/sqlbuild-cli/src/argv/tests/test_parse_cli_arguments.rs

@@ -9,7 +9,6 @@ use crate::dispatch::constants::{
     LINEAGE_ROUTE, LOAD_ROUTE, PLAN_ROUTE, PLAYGROUND_ROUTE, QUERY_ROUTE, RECALL_COMMAND_NAME,
     RECALL_ROUTE, REFACTOR_ROUTE, RULES_ROUTE, SCOPE_ROUTE, SEED_ROUTE, SKILLS_ROUTE, TEST_ROUTE,
 };
-use crate::dispatch::main::unavailable_handler::unavailable_handler;
 use crate::dispatch::models::{CommandFacts, Dispatch, DispatchFailure, Route};
 
 /// The route `facts` select, checked in the order the Python dispatcher checked them.
@@ -21,11 +20,7 @@ pub fn command_dispatch(facts: &CommandFacts) -> Dispatch {
     let checked =
         |route: &'static Route, failure: Option<DispatchFailure>| Dispatch::Run(route, failure);
     match command {
-        "rename" => checked(
-            &REFACTOR_ROUTE,
-            (!facts.refactor_target).then(|| unavailable_handler(command, "C950")),
-        ),
-        "mv" => run(&REFACTOR_ROUTE),
+        "rename" | "mv" => run(&REFACTOR_ROUTE),
         "contract" => checked(
             &CONTRACT_ROUTE,
             (!facts.contract_ready).then(|| {

crates/sqlbuild-cli/src/dispatch/main/command_dispatch.rs

@@ -97,7 +97,6 @@ pub struct CommandFacts {
     pub command: Option<String>,
     pub subcommand: Option<String>,
     pub json: bool,
-    pub refactor_target: bool,
     pub from_target: bool,
     pub contract_ready: bool,
     pub format_diff: bool,

crates/sqlbuild-cli/src/dispatch/models.rs

@@ -7,7 +7,6 @@ pub(super) fn facts(command: &str, subcommand: Option<&str>) -> CommandFacts {
     CommandFacts {
         command: Some(command.to_owned()),
         subcommand: subcommand.map(str::to_owned),
-        refactor_target: true,
         from_target: true,
         contract_ready: true,
         ..CommandFacts::default()

crates/sqlbuild-cli/src/dispatch/tests/helpers.rs

@@ -89,22 +89,13 @@ fn given_parsed_command_when_dispatching_then_selects_python_dispatcher_route()
             expected_dispatch: runs(&SCENARIO_CAPTURE_ROUTE),
         },
         CommandDispatchTestCase {
-            description: "rename without a target fails",
-            facts: CommandFacts {
-                refactor_target: false,
-                ..facts("rename", None)
-            },
-            expected_dispatch: Dispatch::Run(
-                &REFACTOR_ROUTE,
-                Some(failure("rename command handler is unavailable", "C950")),
-            ),
+            description: "rename reads its pairs itself, so they may come only from a file",
+            facts: facts("rename", None),
+            expected_dispatch: runs(&REFACTOR_ROUTE),
         },
         CommandDispatchTestCase {
-            description: "mv reads its pairs itself, so it may come only from a file",
-            facts: CommandFacts {
-                refactor_target: false,
-                ..facts("mv", None)
-            },
+            description: "mv reads its pairs itself, so they may come only from a file",
+            facts: facts("mv", None),
             expected_dispatch: runs(&REFACTOR_ROUTE),
         },
         CommandDispatchTestCase {