feat: move several models in one sqb mv and match carried declaration layouts (#1206)

* fix: give a carried declaration the shape of its destination role

When sqb mv carries a scoped declaration to a wider owner, the file now
matches the layout of the role folder it lands in instead of always being
written directly in it. An empty role keeps the source's group folder, a
grouped role keeps the source's group, and a direct role drops it. A file
without a group that must join a grouped role is refused with a message to
move it into a group first, rather than writing a layout that verification
rejects for mixing direct and grouped files.

Placement relocation now accepts several resource moves at once.

* feat: move several models in one verified sqb mv

sqb mv now takes several TARGET DESTINATION pairs, or --from-file with one
pair per line (tab or space separated, # comments and blank lines allowed).
The project compiles once before planning and the edited project once
after. Every move is planned together: declarations and unit tests are
placed for the state after all moves, a model may take a file or name
another move leaves, and a batch that moves a model twice, sends two models
to one file or gives them one name is refused. Nothing is written unless
the whole batch verifies; --dry-run prints the combined plan, and JSON
output lists the moves. A single move plans and prints as before.

* fix: refuse a move that sends two declaration files to one path

Two carried declaration files could share one target, either from two moved models' same-named local files or from grouped files flattened into a direct role. Planning now refuses this up front, naming both files, instead of leaving it to a confusing verification compile error.

Author: kvlonge

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

@@ -4991,19 +4991,38 @@
        {
         "kind": "store",
         "dest": "refactor_target",
-        "required": true,
+        "nargs": "?",
         "help": "<model>; the model: prefix is optional",
         "metavar": "TARGET",
         "group": 0
        },
        {
         "kind": "store",
         "dest": "refactor_destination",
-        "required": true,
+        "nargs": "?",
         "help": "new .sql path or folder/",
         "metavar": "DESTINATION",
         "group": 0
        },
+       {
+        "kind": "store",
+        "dest": "refactor_more_moves",
+        "nargs": "*",
+        "default": [],
+        "help": "further TARGET DESTINATION pairs, moved together with the first",
+        "metavar": "MORE",
+        "group": 0
+       },
+       {
+        "kind": "store",
+        "options": [
+         "--from-file"
+        ],
+        "dest": "refactor_from_file",
+        "help": "read TARGET DESTINATION pairs from FILE, one per line; # starts a comment",
+        "metavar": "FILE",
+        "group": 1
+       },
        {
         "kind": "store_true",
         "options": [

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

@@ -63,7 +63,7 @@ pub(crate) const INT_CONVERTER: &str = "int";
 pub(crate) const PATH_CONVERTER: &str = "Path";
 
 /// Every `CliNamespace` attribute with a default, in declaration order.
-pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 174] = [
+pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 176] = [
     ("command", NamespaceDefault::None),
     ("project_dir", NamespaceDefault::None),
     ("dbt_project_dir", NamespaceDefault::None),
@@ -252,5 +252,7 @@ pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 174] = [
     ("refactor_target", NamespaceDefault::None),
     ("refactor_new_name", NamespaceDefault::None),
     ("refactor_destination", NamespaceDefault::None),
+    ("refactor_more_moves", NamespaceDefault::EmptyList),
+    ("refactor_from_file", NamespaceDefault::None),
     ("refactor_cascade", NamespaceDefault::Bool(false)),
 ];

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

@@ -504,11 +504,13 @@ pub(crate) const JANITOR_ROUTE: Route = routed(
     REQUEST_KEYWORD_SHAPE,
 );
 
-const REFACTOR_FIELDS: [Field; 9] = [
+const REFACTOR_FIELDS: [Field; 11] = [
     field("command", Source::Text("command")),
     attr("target", "refactor_target"),
     attr("new_name", "refactor_new_name"),
     attr("destination", "refactor_destination"),
+    field("more_moves", Source::TupleOf("refactor_more_moves")),
+    attr("from_file", "refactor_from_file"),
     PROJECT_DIR,
     attr("cascade", "refactor_cascade"),
     same("dry_run"),

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

@@ -21,10 +21,11 @@ pub fn command_dispatch(facts: &CommandFacts) -> Dispatch {
     let checked =
         |route: &'static Route, failure: Option<DispatchFailure>| Dispatch::Run(route, failure);
     match command {
-        "rename" | "mv" => checked(
+        "rename" => checked(
             &REFACTOR_ROUTE,
             (!facts.refactor_target).then(|| unavailable_handler(command, "C950")),
         ),
+        "mv" => run(&REFACTOR_ROUTE),
         "contract" => checked(
             &CONTRACT_ROUTE,
             (!facts.contract_ready).then(|| {

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

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

crates/sqlbuild-python/src/bindings/_helpers/records/constants.rs

@@ -652,9 +652,11 @@ const REFACTOR_COMMAND_REQUEST: RecordSpec = RecordSpec {
     doc: "CLI inputs for one `sqb rename` or `sqb mv` invocation.",
     fields: &[
         FieldSpec::required("command"),
-        FieldSpec::required("target"),
+        FieldSpec::new("target", FieldDefault::None),
         FieldSpec::new("new_name", FieldDefault::None),
         FieldSpec::new("destination", FieldDefault::None),
+        FieldSpec::new("more_moves", FieldDefault::EmptyTuple),
+        FieldSpec::new("from_file", FieldDefault::None),
         FieldSpec::new("project_dir", FieldDefault::None),
         FieldSpec::new("cascade", FieldDefault::False),
         FieldSpec::new("dry_run", FieldDefault::False),

crates/sqlbuild-python/src/bindings/_helpers/refactor/refactor_command.rs

@@ -1,7 +1,7 @@
 //! `sqb rename` and `sqb mv`: plan from compiler facts, verify in a scratch copy, then write.
 
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::{BTreeSet, HashMap};
 use std::path::{Path, PathBuf};
 use std::time::Instant;
 
@@ -15,11 +15,12 @@ use sqlbuild_analysis::lineage_command::models::LineageKey;
 use sqlbuild_core::cli_text::main::format_count_noun::format_count_noun;
 use sqlbuild_core::cli_text::models::CliStyle;
 use sqlbuild_refactor::refactor_command::main::layer_move_suggestion::layer_move_suggestion;
+use sqlbuild_refactor::refactor_command::main::move_pairs::move_pairs;
 use sqlbuild_refactor::refactor_command::main::refactor_request_for_target::refactor_request_for_target;
 use sqlbuild_refactor::refactor_command::main::render_refactor_json::render_refactor_json;
 use sqlbuild_refactor::refactor_command::main::render_refactor_text::render_refactor_text;
 use sqlbuild_refactor::refactor_command::models::{
-    LayerMove, RefactorDiagnostic, RefactorStatus, RefactorTargetRequest,
+    LayerMove, MoveArguments, RefactorDiagnostic, RefactorStatus, RefactorTargetRequest,
 };
 use sqlbuild_refactor::refactoring::errors::{RefactorError, RefactorErrorKind};
 use sqlbuild_refactor::refactoring::main::commit_refactor_plan::commit_refactor_plan;
@@ -49,6 +50,7 @@ const STAGING_PREFIX: &str = "sqb-refactor-";
 const MOVE_COMMAND: &str = "mv";
 const EDIT_ERROR_CODE: &str = "C957";
 const WRITE_ERROR_CODE: &str = "C958";
+const MOVE_FILE_ERROR_CODE: &str = "C954";
 
 fn imported<'py>(py: Python<'py>, module: &str, name: &str) -> PyResult<Bound<'py, PyAny>> {
     py.import(module)?.getattr(name)
@@ -212,28 +214,88 @@ fn target_lookup(all_keys: &Bound<'_, PyAny>) -> PyResult<HashMap<String, Lineag
     Ok(names)
 }
 
-fn target_request(request: &Bound<'_, PyAny>) -> PyResult<RefactorTargetRequest> {
-    Ok(RefactorTargetRequest {
-        is_move: native_field(request, "command")?
-            .str()?
-            .extract::<String>()?
-            == MOVE_COMMAND,
-        target: native_field(request, "target")?.extract()?,
-        new_name: native_field(request, "new_name")?.extract()?,
-        destination: native_field(request, "destination")?.extract()?,
-        cascade: native_field(request, "cascade")?.extract()?,
-    })
+fn move_command(request: &Bound<'_, PyAny>) -> PyResult<bool> {
+    Ok(native_field(request, "command")?
+        .str()?
+        .extract::<String>()?
+        == MOVE_COMMAND)
+}
+
+/// The target of each refactoring the command asks for: every move pair, or one rename.
+fn target_requests(
+    py: Python<'_>,
+    request: &Bound<'_, PyAny>,
+) -> PyResult<Vec<RefactorTargetRequest>> {
+    let target: Option<String> = native_field(request, "target")?.extract()?;
+    let destination: Option<String> = native_field(request, "destination")?.extract()?;
+    if !move_command(request)? {
+        return Ok(vec![RefactorTargetRequest {
+            is_move: false,
+            target: target.unwrap_or_default(),
+            new_name: native_field(request, "new_name")?.extract()?,
+            destination,
+            cascade: native_field(request, "cascade")?.extract()?,
+        }]);
+    }
+    let more: Vec<String> = native_field(request, "more_moves")?.extract()?;
+    let from_file: Option<String> = native_field(request, "from_file")?.extract()?;
+    let contents: Option<String> = match &from_file {
+        None => None,
+        Some(path) => Some(std::fs::read_to_string(path).map_err(|error| {
+            user_error(
+                py,
+                (
+                    format!("cannot read move file {path}: {error}"),
+                    MOVE_FILE_ERROR_CODE,
+                    None,
+                ),
+            )
+        })?),
+    };
+    let mut words: Vec<&str> = target
+        .iter()
+        .chain(&destination)
+        .map(String::as_str)
+        .collect();
+    words.extend(more.iter().map(String::as_str));
+    let arguments: MoveArguments<'_> = MoveArguments {
+        words,
+        file: from_file.as_deref().zip(contents.as_deref()),
+    };
+    let pairs: Vec<(String, String)> =
+        move_pairs(&arguments).map_err(|error| refactor_error(py, error))?;
+    Ok(pairs
+        .into_iter()
+        .map(|(target, destination)| RefactorTargetRequest {
+            is_move: true,
+            target,
+            new_name: None,
+            destination: Some(destination),
+            cascade: false,
+        })
+        .collect())
 }
 
 fn plan(
     py: Python<'_>,
     project: &Bound<'_, PyAny>,
-    request: &RefactorRequest,
+    requests: &[RefactorRequest],
 ) -> PyResult<RefactorPlan> {
-    let facts: RefactorFacts = refactor_facts(project, request)?;
+    let facts: RefactorFacts = refactor_facts(project, requests)?;
+    let moving: BTreeSet<&str> = requests
+        .iter()
+        .filter(|_| requests.len() > 1)
+        .map(|request| request.model_name.as_str())
+        .collect();
+    let vacated: BTreeSet<String> = facts
+        .models
+        .iter()
+        .filter(|model| moving.contains(model.name.as_str()))
+        .map(|model| model.path.clone())
+        .collect();
     let host_error: RefCell<Option<PyErr>> = RefCell::new(None);
-    let host = |model: &str, destination: &str| -> Result<DeclarationPlacement, RefactorError> {
-        declaration_placement(project, (model, destination)).map_err(|error| {
+    let host = |moves: &[(String, String)]| -> Result<DeclarationPlacement, RefactorError> {
+        declaration_placement(project, moves, &vacated).map_err(|error| {
             host_error.replace(Some(error));
             RefactorError::internal("the declaration-move host failed")
         })
@@ -245,7 +307,7 @@ fn plan(
         })
     };
     let planned: Result<RefactorPlan, RefactorError> =
-        plan_refactor(&facts, request, &host, &test_host);
+        plan_refactor(&facts, requests, &host, &test_host);
     if let Some(error) = host_error.into_inner() {
         return Err(error);
     }
@@ -301,6 +363,7 @@ fn verify(
 }
 
 fn run(py: Python<'_>, command: &Command<'_>, status: &Status<'_>) -> PyResult<Option<Outcome>> {
+    let targets: Vec<RefactorTargetRequest> = target_requests(py, &command.request)?;
     let started: Instant = Instant::now();
     status.start("Compiling project...")?;
     let before: Compiled<'_> = compile(py, &command.project_dir, false)?;
@@ -318,11 +381,13 @@ fn run(py: Python<'_>, command: &Command<'_>, status: &Status<'_>) -> PyResult<O
         "all_keys",
     )?)?;
     let lookup = |name: &str| names.get(name).cloned();
-    let request: RefactorRequest =
-        refactor_request_for_target(&target_request(&command.request)?, &lookup)
-            .map_err(|error| refactor_error(py, error))?;
+    let requests: Vec<RefactorRequest> = targets
+        .iter()
+        .map(|target| refactor_request_for_target(target, &lookup))
+        .collect::<Result<_, _>>()
+        .map_err(|error| refactor_error(py, error))?;
     status.start("Planning edits...")?;
-    let planned: RefactorPlan = plan(py, &before.project, &request)?;
+    let planned: RefactorPlan = plan(py, &before.project, &requests)?;
     status.complete(&format!("Planned edits to {}.", files(&planned)))?;
     if !planned.blocking.is_empty() || !planned.manual.is_empty() {
         status.error("Refused: some references cannot be rewritten safely.")?;