From fa630b54f8229deef1a2d9085fc48ce734435ee8 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 00:18:19 +0100 Subject: [PATCH 1/2] fix(rename): suggest the one-step sqb mv command when a rename crosses layers --- .../sqlbuild/references/docs/cli/rename.md | 27 +- .../cli/commands/_helpers/refactor/command.py | 9 +- .../commands/_helpers/refactor/layer_move.py | 93 +++++++ .../cli/commands/_helpers/refactor/output.py | 24 ++ src/sqlbuild/cli/commands/constants.py | 23 ++ src/sqlbuild/cli/commands/models.py | 10 + .../cli/commands/main/refactor/_test_types.py | 28 +++ .../cli/commands/main/refactor/helpers.py | 24 ++ .../refactor/test_rename_layer_refusals.py | 236 ++++++++++++++++++ .../commands/_helpers/refactor/_test_types.py | 10 + .../cli/commands/_helpers/refactor/helpers.py | 37 +++ .../_helpers/refactor/test_layer_move.py | 93 +++++++ website/src/content/docs/docs/cli/rename.mdx | 26 +- 13 files changed, 637 insertions(+), 3 deletions(-) create mode 100644 src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py create mode 100644 tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py create mode 100644 tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md index a5abad2a50..7eff5fada5 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md @@ -9,6 +9,7 @@ Online: https://sqlbuild.com/docs/cli/rename/ ## Contents - Usage +- Rename or move - What changes - History - Refusals @@ -39,7 +40,20 @@ sqb rename stg_orders.amount revenue --cascade Targets are written the way [`sqb lineage`](lineage.md) reads them: a bare name is a model, and `.` is one of its columns. The `model:` and `column:` prefixes are optional, so `sqb rename model:stg_orders stg_order_lines` and `sqb rename column:stg_orders.amount revenue` still work. A target that names a source, seed, or function is refused, because those are not renamed by these commands. -A model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` takes a new file path, which also sets the model name, or a folder ending in `/`, which keeps it. +## Rename or move + +`sqb rename` changes names and never moves files: a model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` changes where a model lives and always takes a path: a new `.sql` file path, which also sets the model name, or a folder ending in `/`, which keeps it. Neither command picks a folder for you, because a folder can change the schema, materialization, tags, and declaration visibility the model gets from path defaults. + +```bash +# Name only: the file stays in models/orders/intermediate/clean/ +sqb rename orders__int_clean__order_lines orders__int_clean__order_items + +# Name and a specific folder, in one verified step +sqb mv orders__int_clean__order_lines models/orders/intermediate/enriched/orders__int_enriched__order_lines.sql + +# Folder only: the name stays +sqb mv orders__int_clean__order_lines models/orders/intermediate/clean/web/ +``` ## What changes @@ -89,6 +103,17 @@ The command refuses, and writes nothing, when: - a downstream model's `SELECT *` would silently rename its output, without `--cascade`; - Python code or Python node SQL strings name the model or column. +A rename whose new name belongs to another layer's folder, under the layer-folder rules `SQBRPROJECT102` and `SQBRPROJECT105`, fails verification because `sqb rename` keeps the file where it is. When those rules are the only failures, the output says where the model belongs and prints the one-step [`sqb mv`](#rename-or-move) command, with that layer's folder and the current sub-folders filled in: + +```text +orders__int_enriched__order_lines__web belongs under models/orders/intermediate/enriched/ + Rename and move in one step: + sqb mv orders__int_clean__order_lines__web models/orders/intermediate/enriched/web/orders__int_enriched__order_lines__web.sql + (change the folder if you want it somewhere else) +``` + +When the current path has no layer folder to mirror, the command uses a `/` placeholder for you to fill in. + Each location is listed with its file, line, and column, with the fix. Edit those locations, or pass the model or column to the macro as an argument, and run the command again. The command never applies part of a rename. References produced by a macro are only possible when a project turns off explicit references (`[references] enforce_explicit = false`); such projects get the refusal list for them. diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/command.py b/src/sqlbuild/cli/commands/_helpers/refactor/command.py index 8286b058bf..f9f5c71f48 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/command.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/command.py @@ -10,6 +10,7 @@ from typing import TextIO from sqlbuild.cli.commands._helpers.refactor.compile_facts import compile_for_refactor +from sqlbuild.cli.commands._helpers.refactor.layer_move import layer_move_suggestion from sqlbuild.cli.commands._helpers.refactor.output import ( render_refactor_json, render_refactor_text, @@ -157,7 +158,13 @@ def _finish( render_refactor_json(plan=plan, status=status, diagnostics=diagnostics) if request.json_output else render_refactor_text( - plan=plan, status=status, diagnostics=diagnostics, use_color=use_color + plan=plan, + status=status, + diagnostics=diagnostics, + use_color=use_color, + layer_move=layer_move_suggestion( + plan=plan, diagnostics=diagnostics, project_dir=request.project_dir + ), ) ) _ = sys.stdout.write(output) diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py b/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py new file mode 100644 index 0000000000..a5ca6a1ae2 --- /dev/null +++ b/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py @@ -0,0 +1,93 @@ +"""Suggest the one-step `sqb mv` when a rename fails only on the layer-folder rules.""" + +from __future__ import annotations + +import shlex +from pathlib import Path, PurePosixPath + +from sqlbuild.cli.commands.constants import ( + CURRENT_LAYER_FOLDERS, + FOLDER_PLACEHOLDER, + LAYER_FOLDER_RULE_CODES, + LAYER_FOLDERS, + MODEL_NAME_LAYER_INDEX, + MODEL_NAME_LAYER_SEPARATOR, + PROJECT_DIR_OPTION, + SQL_FILE_SUFFIX, +) +from sqlbuild.cli.commands.models import LayerMoveSuggestion +from sqlbuild.compiler.compile.models import CompilerDiagnostic +from sqlbuild.compiler.refactoring.models import FileChange, RefactorPlan +from sqlbuild.compiler.refactoring.types import RefactorOperation + + +def layer_move_suggestion( + *, + plan: RefactorPlan, + diagnostics: tuple[CompilerDiagnostic, ...], + project_dir: Path | None, +) -> LayerMoveSuggestion | None: + """Return the `sqb mv` that renames and moves in one step, or None for any other failure.""" + + renamed: FileChange | None = next((change for change in plan.changes if change.moved), None) + if ( + plan.request.operation != RefactorOperation.RENAME_MODEL + or renamed is None + or not diagnostics + or any( + diagnostic.code not in LAYER_FOLDER_RULE_CODES + or diagnostic.path is None + or diagnostic.path.as_posix() != renamed.path + for diagnostic in diagnostics + ) + ): + return None + new_name: str = plan.request.new_name + name_parts: list[str] = new_name.split(MODEL_NAME_LAYER_SEPARATOR) + layer_folder: tuple[str, ...] | None = ( + LAYER_FOLDERS.get(name_parts[MODEL_NAME_LAYER_INDEX]) + if len(name_parts) > MODEL_NAME_LAYER_INDEX + else None + ) + if layer_folder is None: + return None + mirrored: tuple[tuple[str, ...], tuple[str, ...]] | None = _mirrored_folder( + original_path=renamed.original_path, layer_folder=layer_folder + ) + folders: tuple[str, ...] = ( + (*mirrored[0], *mirrored[1]) if mirrored is not None else (FOLDER_PLACEHOLDER,) + ) + arguments: list[str] = [ + "sqb", + "mv", + plan.request.model_name, + "/".join((*folders, f"{new_name}{SQL_FILE_SUFFIX}")), + ] + if project_dir is not None: + arguments.extend((PROJECT_DIR_OPTION, str(project_dir))) + return LayerMoveSuggestion( + new_name=new_name, + folder="/".join(mirrored[0]) if mirrored is not None else None, + layer_folder="/".join(layer_folder), + command=shlex.join(arguments), + ) + + +def _mirrored_folder( + *, original_path: str, layer_folder: tuple[str, ...] +) -> tuple[tuple[str, ...], tuple[str, ...]] | None: + """Return the new layer folder in place of the current one, and the sub-path kept below it.""" + + parent: tuple[str, ...] = PurePosixPath(original_path).parent.parts + for start in range(1, len(parent)): + current: tuple[str, ...] | None = next( + ( + candidate + for candidate in CURRENT_LAYER_FOLDERS + if parent[start : start + len(candidate)] == candidate + ), + None, + ) + if current is not None: + return (*parent[:start], *layer_folder), parent[start + len(current) :] + return None diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/output.py b/src/sqlbuild/cli/commands/_helpers/refactor/output.py index 74ae474591..9eaa243893 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/output.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/output.py @@ -4,6 +4,8 @@ import json +from sqlbuild.cli.commands.constants import FOLDER_PLACEHOLDER +from sqlbuild.cli.commands.models import LayerMoveSuggestion from sqlbuild.compiler.compile.models import CompilerDiagnostic from sqlbuild.compiler.refactoring.constants import OPERATION_TITLES from sqlbuild.compiler.refactoring.models import ( @@ -31,6 +33,7 @@ def render_refactor_text( status: RefactorStatus, diagnostics: tuple[CompilerDiagnostic, ...], use_color: bool, + layer_move: LayerMoveSuggestion | None = None, ) -> str: """Render the edit tree, manual locations, and one terminal status line.""" @@ -50,6 +53,8 @@ def render_refactor_text( if diagnostics: lines.append(style.error_strong("Compile errors in the edited project")) lines.extend(_diagnostic_lines(style=style, diagnostics=diagnostics)) + if layer_move is not None: + lines.extend(_layer_move_lines(style=style, layer_move=layer_move)) lines.append(_status_line(style=style, plan=plan, status=status)) return "\n".join(lines) + "\n" @@ -197,6 +202,25 @@ def _diagnostic_lines(*, style: CliStyle, diagnostics: tuple[CompilerDiagnostic, ] +def _layer_move_lines(*, style: CliStyle, layer_move: LayerMoveSuggestion) -> list[str]: + folder: str = ( + f"under {layer_move.folder}/" + if layer_move.folder is not None + else f"under a layer folder named {layer_move.layer_folder}/" + ) + note: str = ( + "(change the folder if you want it somewhere else)" + if layer_move.folder is not None + else f"(replace {FOLDER_PLACEHOLDER}/ with the folder the model should live in)" + ) + return [ + f"{style.object_name(layer_move.new_name)} belongs {folder}", + " Rename and move in one step:", + f" {style.command(layer_move.command)}", + f" {style.muted(note)}", + ] + + def _status_line(*, style: CliStyle, plan: RefactorPlan, status: RefactorStatus) -> str: files: str = format_count_noun(count=len(plan.changes), singular="file") if status == RefactorStatus.APPLIED: diff --git a/src/sqlbuild/cli/commands/constants.py b/src/sqlbuild/cli/commands/constants.py index a7392d5798..c40b1c2339 100644 --- a/src/sqlbuild/cli/commands/constants.py +++ b/src/sqlbuild/cli/commands/constants.py @@ -177,3 +177,26 @@ CliCommand.SKILLS: None, } ) +LAYER_FOLDER_RULE_CODES: frozenset[str] = frozenset({"SQBRPROJECT102", "SQBRPROJECT105"}) +MODEL_NAME_LAYER_SEPARATOR: str = "__" +MODEL_NAME_LAYER_INDEX: int = 1 +LAYER_FOLDERS: MappingProxyType[str, tuple[str, ...]] = MappingProxyType( + { + "stg": ("staging",), + "stg_v": ("staging",), + "int_clean": ("intermediate", "clean"), + "int_enriched": ("intermediate", "enriched"), + "int_v": ("intermediate", "enriched"), + "mart": ("mart",), + "mart_v": ("mart",), + } +) +CURRENT_LAYER_FOLDERS: tuple[tuple[str, ...], ...] = ( + ("intermediate", "clean"), + ("intermediate", "enriched"), + ("intermediate",), + ("staging",), + ("mart",), +) +FOLDER_PLACEHOLDER: str = "" +PROJECT_DIR_OPTION: str = "--project-dir" diff --git a/src/sqlbuild/cli/commands/models.py b/src/sqlbuild/cli/commands/models.py index 9f359355e3..d96c4da5b9 100644 --- a/src/sqlbuild/cli/commands/models.py +++ b/src/sqlbuild/cli/commands/models.py @@ -1100,6 +1100,16 @@ class RefactorCommandRequest: no_color: bool = False +@dataclass(frozen=True) +class LayerMoveSuggestion: + """The one-step `sqb mv` for a rename refused only by the layer-folder rules.""" + + new_name: str + folder: str | None + layer_folder: str + command: str + + @dataclass(frozen=True) class RefactorCompile: """One offline compile for a refactoring: its facts, or the errors that stopped it.""" diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/_test_types.py b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/_test_types.py index 910f4adea4..a5116cd00c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/_test_types.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/_test_types.py @@ -104,3 +104,31 @@ class PendingOriginJanitorE2ETestCase: description: str janitor_args: tuple[str, ...] expected_order_ids: tuple[int, ...] + + +@dataclass(frozen=True) +class LayerMoveRefusalE2ETestCase: + description: str + project_files: dict[str, str] + old_name: str + new_name: str + expected_hint: tuple[str, ...] + expected_moved_file: str + + +@dataclass(frozen=True) +class LayerMovePlaceholderE2ETestCase: + description: str + project_files: dict[str, str] + old_name: str + new_name: str + expected_hint: tuple[str, ...] + + +@dataclass(frozen=True) +class OtherRenameRefusalE2ETestCase: + description: str + project_files: dict[str, str] + old_name: str + new_name: str + expected_codes: tuple[str, ...] diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/helpers.py b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/helpers.py index 7cc11f4498..ab827cba70 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/helpers.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/helpers.py @@ -4,12 +4,15 @@ import base64 import json +import re +import shlex import subprocess from collections.abc import Mapping from pathlib import Path from typing import Any from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import ( + REPO_ROOT, execute_duckdb, prepare_inline_project, query_duckdb, @@ -17,6 +20,9 @@ ) DATABASE_FILE: str = "orders.duckdb" +_LAYER_MOVE_HINT: re.Pattern[str] = re.compile( + r"^.*\n Rename and move in one step:\n.*\n.*$", re.MULTILINE +) SCHEMA: str = "analytics" @@ -403,3 +409,21 @@ def order_ids(*, project_dir: Path, name: str) -> tuple[int, ...]: sql=f"SELECT order_id FROM {SCHEMA}.{name} ORDER BY order_id", ) ) + + +def layer_move_hint(stdout: str) -> tuple[str, ...]: + """Return the layer-folder hint lines around the one-step command; empty when absent.""" + + return tuple("\n".join(_LAYER_MOVE_HINT.findall(stdout)).splitlines()) + + +def run_printed_command(command: str) -> subprocess.CompletedProcess[str]: + """Run a command line exactly as the CLI printed it.""" + + return subprocess.run( + ["uv", "run", *shlex.split(command)], + cwd=REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py new file mode 100644 index 0000000000..aa843fe0fd --- /dev/null +++ b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py @@ -0,0 +1,236 @@ +"""CLI e2e coverage for the one-step `sqb mv` hint when `sqb rename` crosses layer folders.""" + +from __future__ import annotations + +import shlex +import shutil +import subprocess +from pathlib import Path + +import pytest + +from tests.e2e.src.sqlbuild.cli.commands.main.refactor._test_types import ( + LayerMovePlaceholderE2ETestCase, + LayerMoveRefusalE2ETestCase, + OtherRenameRefusalE2ETestCase, +) +from tests.e2e.src.sqlbuild.cli.commands.main.refactor.helpers import ( + layer_move_hint, + project_text, + run_printed_command, + sqb, +) +from tests.e2e.src.sqlbuild.cli.commands.shared.helpers import prepare_inline_project + +_LAYER_RULES: str = 'select = ["SQBRPROJECT102", "SQBRPROJECT105"]' +_PROJECT_TOML: str = ( + 'name = "orders"\nadapter = "duckdb"\n\n[connection]\ndatabase = "orders.duckdb"\n\n' + f"[rules]\n{_LAYER_RULES}\n" +) +_CLEAN_WEB_PATH: str = ( + "models/orders/intermediate/clean/web/orders__int_clean__order_lines__web.sql" +) +_UNLAYERED_WEB_PATH: str = "models/orders/web/order_lines_web.sql" +_MART_PATH: str = "models/orders/mart/orders__mart__order_lines.sql" +_ENRICHED_NAME: str = "orders__int_enriched__order_lines__web" +_STAGING: str = ( + 'MODEL (\n description "Staged order lines",\n materialized table,\n);\n\n' + "SELECT 1 AS order_line_id, 10 AS customer_id\n" +) +_WEB_LINES: str = ( + 'MODEL (\n description "Web order lines",\n materialized table,\n);\n\n' + 'SELECT order_line_id, customer_id\nFROM __ref("orders__stg__order_lines")\n' +) +_MART: str = ( + 'MODEL (\n description "Order lines",\n materialized table,\n);\n\n' + 'SELECT order_line_id, customer_id\nFROM __ref("{upstream}")\n' +) +_LAYERED_FILES: dict[str, str] = { + "sqlbuild_project.toml": _PROJECT_TOML, + "models/orders/staging/orders__stg__order_lines.sql": _STAGING, + _CLEAN_WEB_PATH: _WEB_LINES, + _MART_PATH: _MART.format(upstream="orders__int_clean__order_lines__web"), +} +_UNLAYERED_FILES: dict[str, str] = { + "sqlbuild_project.toml": _PROJECT_TOML, + "models/orders/staging/orders__stg__order_lines.sql": _STAGING, + _UNLAYERED_WEB_PATH: _WEB_LINES, + _MART_PATH: _MART.format(upstream="order_lines_web"), +} + + +@pytest.mark.parametrize( + "test_case", + [ + LayerMoveRefusalE2ETestCase( + description="int_clean to int_enriched mirrors the web sub-folder", + project_files=_LAYERED_FILES, + old_name="orders__int_clean__order_lines__web", + new_name=_ENRICHED_NAME, + expected_hint=( + f"{_ENRICHED_NAME} belongs under models/orders/intermediate/enriched/", + " Rename and move in one step:", + " sqb mv orders__int_clean__order_lines__web " + f"models/orders/intermediate/enriched/web/{_ENRICHED_NAME}.sql " + "--project-dir {project_dir}", + " (change the folder if you want it somewhere else)", + ), + expected_moved_file=f"models/orders/intermediate/enriched/web/{_ENRICHED_NAME}.sql", + ), + ], + ids=lambda case: case.description, +) +def test_given_rename_into_another_layer_when_refused_then_printed_mv_renames_and_moves( + test_case: LayerMoveRefusalE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.project_files + ) + intended_dir: Path = prepare_inline_project( + tmp_path=tmp_path, + project_name="intended", + repo_files={ + **test_case.project_files, + "sqlbuild_project.toml": _PROJECT_TOML.replace(_LAYER_RULES, "select = []"), + }, + ) + before: dict[str, str] = project_text(project_dir) + + refused: subprocess.CompletedProcess[str] = sqb( + project_dir, "rename", test_case.old_name, test_case.new_name + ) + after_refusal: dict[str, str] = project_text(project_dir) + hint: tuple[str, ...] = layer_move_hint(refused.stdout) + moved: subprocess.CompletedProcess[str] = run_printed_command(hint[2].strip()) + intended_rename: subprocess.CompletedProcess[str] = sqb( + intended_dir, "rename", test_case.old_name, test_case.new_name + ) + (intended_dir / test_case.expected_moved_file).parent.mkdir(parents=True) + _ = shutil.move( + intended_dir / Path(_CLEAN_WEB_PATH).with_name(f"{test_case.new_name}.sql"), + intended_dir / test_case.expected_moved_file, + ) + compiled: subprocess.CompletedProcess[str] = sqb(project_dir, "compile") + + assert ( + refused.returncode, + after_refusal == before, + hint, + moved.returncode, + intended_rename.returncode, + compiled.returncode, + ) == ( + 1, + True, + tuple( + line.replace("{project_dir}", shlex.quote(str(project_dir))) + for line in test_case.expected_hint + ), + 0, + 0, + 0, + ), (refused.stdout, moved.stdout, moved.stderr, compiled.stdout) + assert {**project_text(project_dir), "sqlbuild_project.toml": ""} == { + **project_text(intended_dir), + "sqlbuild_project.toml": "", + } + + +@pytest.mark.parametrize( + "test_case", + [ + LayerMovePlaceholderE2ETestCase( + description="no current layer folder to mirror", + project_files=_UNLAYERED_FILES, + old_name="order_lines_web", + new_name=_ENRICHED_NAME, + expected_hint=( + f"{_ENRICHED_NAME} belongs under a layer folder named intermediate/enriched/", + " Rename and move in one step:", + f" sqb mv order_lines_web '/{_ENRICHED_NAME}.sql' " + "--project-dir {project_dir}", + " (replace / with the folder the model should live in)", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_rename_into_another_layer_without_a_layer_folder_when_refused_then_hint_has_placeholder( + test_case: LayerMovePlaceholderE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.project_files + ) + before: dict[str, str] = project_text(project_dir) + + refused: subprocess.CompletedProcess[str] = sqb( + project_dir, "rename", test_case.old_name, test_case.new_name + ) + + assert ( + refused.returncode, + project_text(project_dir) == before, + layer_move_hint(refused.stdout), + ) == ( + 1, + True, + tuple( + line.replace("{project_dir}", shlex.quote(str(project_dir))) + for line in test_case.expected_hint + ), + ), refused.stdout + + +@pytest.mark.parametrize( + "test_case", + [ + OtherRenameRefusalE2ETestCase( + description="a name outside the layer grammar fails another rule", + project_files={ + **_LAYERED_FILES, + "sqlbuild_project.toml": _PROJECT_TOML.replace( + _LAYER_RULES, + 'select = ["SQBRPROJECT102", "SQBRPROJECT104", "SQBRPROJECT105"]', + ), + }, + old_name="orders__int_clean__order_lines__web", + new_name="order_lines_web", + expected_codes=("SQBRPROJECT104",), + ), + OtherRenameRefusalE2ETestCase( + description="a layer move that also fails another rule", + project_files={ + **_LAYERED_FILES, + "sqlbuild_project.toml": _PROJECT_TOML.replace( + _LAYER_RULES, + 'select = ["SQBRPROJECT102", "SQBRPROJECT105", "SQBRMODEL103"]', + ), + }, + old_name="orders__int_clean__order_lines__web", + new_name="orders__int_v__order_lines__web", + expected_codes=("SQBRMODEL103", "SQBRPROJECT105"), + ), + ], + ids=lambda case: case.description, +) +def test_given_rename_refused_for_another_reason_when_renaming_then_no_mv_hint_is_printed( + test_case: OtherRenameRefusalE2ETestCase, tmp_path: Path +) -> None: + project_dir: Path = prepare_inline_project( + tmp_path=tmp_path, project_name="orders", repo_files=test_case.project_files + ) + + refused: subprocess.CompletedProcess[str] = sqb( + project_dir, "rename", test_case.old_name, test_case.new_name + ) + + assert ( + refused.returncode, + layer_move_hint(refused.stdout), + tuple(f"── {code} " in refused.stdout for code in test_case.expected_codes), + refused.stdout.rstrip().endswith("Compiled: failed; no files changed"), + ) == (1, (), (True,) * len(test_case.expected_codes), True), refused.stdout + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py index 3be56f3915..8bb9cb6be1 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py @@ -34,3 +34,13 @@ class RefactorTargetErrorTestCase: cascade: bool expected_message: str expected_help: str | None + + +@dataclass(frozen=True) +class LayerMoveSuggestionTestCase: + description: str + old_path: str + new_name: str + codes: tuple[str, ...] + expected_command: str | None + expected_folder: str | None diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/helpers.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/helpers.py index bbd38d2a64..9f9a39a231 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/helpers.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/helpers.py @@ -2,6 +2,10 @@ from __future__ import annotations +from pathlib import Path, PurePosixPath + +from sqlbuild.compiler.compile.models import CompilerDiagnostic +from sqlbuild.compiler.compile.types import DiagnosticPhase, DiagnosticSeverity from sqlbuild.compiler.refactoring.models import FileChange, RefactorPlan, RefactorRequest from sqlbuild.compiler.refactoring.types import RefactorOperation @@ -23,3 +27,36 @@ def build_move_plan(*, changed_files: int) -> RefactorPlan: for index in range(changed_files) ), ) + + +def build_rename_plan(*, old_path: str, new_name: str) -> RefactorPlan: + """Return a model rename plan whose file keeps its folder.""" + + return RefactorPlan( + request=RefactorRequest( + operation=RefactorOperation.RENAME_MODEL, + model_name=PurePosixPath(old_path).stem, + new_name=new_name, + ), + changes=( + FileChange( + path=PurePosixPath(old_path).with_name(f"{new_name}.sql").as_posix(), + original_path=old_path, + ), + ), + ) + + +def rule_diagnostics(*, codes: tuple[str, ...], path: str) -> tuple[CompilerDiagnostic, ...]: + """Return one compile error per rule code, reported on `path`.""" + + return tuple( + CompilerDiagnostic( + phase=DiagnosticPhase.COMPILE, + severity=DiagnosticSeverity.ERROR, + code=code, + message=f"{code} failed", + path=Path(path), + ) + for code in codes + ) diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py new file mode 100644 index 0000000000..25fa7cfa67 --- /dev/null +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py @@ -0,0 +1,93 @@ +"""The one-step `sqb mv` suggested when a model rename fails only on the layer-folder rules.""" + +from __future__ import annotations + +import pytest + +from sqlbuild.cli.commands._helpers.refactor.layer_move import layer_move_suggestion +from sqlbuild.cli.commands.models import LayerMoveSuggestion +from sqlbuild.compiler.refactoring.models import RefactorPlan +from tests.unit.src.sqlbuild.cli.commands._helpers.refactor._test_types import ( + LayerMoveSuggestionTestCase, +) +from tests.unit.src.sqlbuild.cli.commands._helpers.refactor.helpers import ( + build_rename_plan, + rule_diagnostics, +) + +_LAYER_CODES: tuple[str, str] = ("SQBRPROJECT102", "SQBRPROJECT105") + + +@pytest.mark.parametrize( + "test_case", + [ + LayerMoveSuggestionTestCase( + description="int_clean to int_enriched keeps domain and sub-folders", + old_path="models/orders/intermediate/clean/web/orders__int_clean__lines__web.sql", + new_name="orders__int_enriched__lines__web", + codes=_LAYER_CODES, + expected_command=( + "sqb mv orders__int_clean__lines__web " + "models/orders/intermediate/enriched/web/orders__int_enriched__lines__web.sql" + ), + expected_folder="models/orders/intermediate/enriched", + ), + LayerMoveSuggestionTestCase( + description="staging to a mart view replaces the one-segment layer folder", + old_path="models/customers/staging/customers__stg__accounts.sql", + new_name="customers__mart_v__accounts", + codes=("SQBRPROJECT105",), + expected_command=( + "sqb mv customers__stg__accounts models/customers/mart/customers__mart_v__accounts.sql" + ), + expected_folder="models/customers/mart", + ), + LayerMoveSuggestionTestCase( + description="a bare intermediate folder becomes the int_clean folder", + old_path="models/orders/intermediate/orders__int_v__totals.sql", + new_name="orders__int_clean__totals", + codes=("SQBRPROJECT102",), + expected_command=( + "sqb mv orders__int_v__totals " + "models/orders/intermediate/clean/orders__int_clean__totals.sql" + ), + expected_folder="models/orders/intermediate/clean", + ), + LayerMoveSuggestionTestCase( + description="no layer folder in the current path uses a placeholder", + old_path="models/orders/web/order_lines.sql", + new_name="orders__stg__order_lines", + codes=_LAYER_CODES, + expected_command="sqb mv order_lines '/orders__stg__order_lines.sql'", + expected_folder=None, + ), + LayerMoveSuggestionTestCase( + description="another rule failing as well gets no suggestion", + old_path="models/orders/intermediate/clean/orders__int_clean__totals.sql", + new_name="orders__int_enriched__totals", + codes=("SQBRPROJECT105", "SQBRMODEL103"), + expected_command=None, + expected_folder=None, + ), + ], + ids=lambda case: case.description, +) +def test_given_rename_compile_errors_when_suggesting_layer_move_then_one_step_mv_is_exact( + test_case: LayerMoveSuggestionTestCase, +) -> None: + plan: RefactorPlan = build_rename_plan(old_path=test_case.old_path, new_name=test_case.new_name) + + suggestion: LayerMoveSuggestion | None = layer_move_suggestion( + plan=plan, + diagnostics=rule_diagnostics(codes=test_case.codes, path=plan.changes[0].path), + project_dir=None, + ) + + assert ( + getattr(suggestion, "command", None), + getattr(suggestion, "folder", None), + ) == (test_case.expected_command, test_case.expected_folder) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"]) diff --git a/website/src/content/docs/docs/cli/rename.mdx b/website/src/content/docs/docs/cli/rename.mdx index f0c6b7dd6c..11f7708731 100644 --- a/website/src/content/docs/docs/cli/rename.mdx +++ b/website/src/content/docs/docs/cli/rename.mdx @@ -25,7 +25,20 @@ sqb rename stg_orders.amount revenue --cascade Targets are written the way [`sqb lineage`](/docs/cli/lineage) reads them: a bare name is a model, and `.` is one of its columns. The `model:` and `column:` prefixes are optional, so `sqb rename model:stg_orders stg_order_lines` and `sqb rename column:stg_orders.amount revenue` still work. A target that names a source, seed, or function is refused, because those are not renamed by these commands. -A model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` takes a new file path, which also sets the model name, or a folder ending in `/`, which keeps it. +## Rename or move + +`sqb rename` changes names and never moves files: a model rename keeps the file in its folder and renames it to `.sql`. `sqb mv` changes where a model lives and always takes a path: a new `.sql` file path, which also sets the model name, or a folder ending in `/`, which keeps it. Neither command picks a folder for you, because a folder can change the schema, materialization, tags, and declaration visibility the model gets from path defaults. + +```bash +# Name only: the file stays in models/orders/intermediate/clean/ +sqb rename orders__int_clean__order_lines orders__int_clean__order_items + +# Name and a specific folder, in one verified step +sqb mv orders__int_clean__order_lines models/orders/intermediate/enriched/orders__int_enriched__order_lines.sql + +# Folder only: the name stays +sqb mv orders__int_clean__order_lines models/orders/intermediate/clean/web/ +``` ## What changes @@ -75,6 +88,17 @@ The command refuses, and writes nothing, when: - a downstream model's `SELECT *` would silently rename its output, without `--cascade`; - Python code or Python node SQL strings name the model or column. +A rename whose new name belongs to another layer's folder, under the layer-folder rules `SQBRPROJECT102` and `SQBRPROJECT105`, fails verification because `sqb rename` keeps the file where it is. When those rules are the only failures, the output says where the model belongs and prints the one-step [`sqb mv`](#rename-or-move) command, with that layer's folder and the current sub-folders filled in: + +```text +orders__int_enriched__order_lines__web belongs under models/orders/intermediate/enriched/ + Rename and move in one step: + sqb mv orders__int_clean__order_lines__web models/orders/intermediate/enriched/web/orders__int_enriched__order_lines__web.sql + (change the folder if you want it somewhere else) +``` + +When the current path has no layer folder to mirror, the command uses a `/` placeholder for you to fill in. + Each location is listed with its file, line, and column, with the fix. Edit those locations, or pass the model or column to the macro as an argument, and run the command again. The command never applies part of a rename. References produced by a macro are only possible when a project turns off explicit references (`[references] enforce_explicit = false`); such projects get the refusal list for them. From 92d44ab248d29e8daac2e6679b438c7d19185f71 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 9 Oct 2026 01:24:32 +0100 Subject: [PATCH 2/2] fix(rename): mirror the current layer's folder and quote the mv hint per platform --- .../sqlbuild/references/docs/cli/rename.md | 2 +- .../commands/_helpers/refactor/layer_move.py | 75 ++++++++----- .../cli/commands/_helpers/refactor/output.py | 2 + src/sqlbuild/cli/commands/constants.py | 18 ++- src/sqlbuild/cli/commands/models.py | 2 +- .../refactor/test_rename_layer_refusals.py | 2 +- .../commands/_helpers/refactor/_test_types.py | 14 +++ .../_helpers/refactor/test_layer_move.py | 104 +++++++++++++++++- website/src/content/docs/docs/cli/rename.mdx | 2 +- 9 files changed, 185 insertions(+), 36 deletions(-) diff --git a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md index 7eff5fada5..85b3ab7a29 100644 --- a/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md +++ b/src/sqlbuild/.agents/skills/sqlbuild/references/docs/cli/rename.md @@ -112,7 +112,7 @@ orders__int_enriched__order_lines__web belongs under models/orders/intermediate/ (change the folder if you want it somewhere else) ``` -When the current path has no layer folder to mirror, the command uses a `/` placeholder for you to fill in. +When the current path has no folder for the model's current layer to swap, or the new layer has no canonical folder, the destination is `models//.sql`, with a `` placeholder for you to fill in. Each location is listed with its file, line, and column, with the fix. Edit those locations, or pass the model or column to the macro as an argument, and run the command again. The command never applies part of a rename. diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py b/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py index a5ca6a1ae2..6261679987 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/layer_move.py @@ -2,18 +2,22 @@ from __future__ import annotations +import os import shlex +import subprocess from pathlib import Path, PurePosixPath from sqlbuild.cli.commands.constants import ( - CURRENT_LAYER_FOLDERS, + CONTIGUOUS_LAYER_FOLDERS, FOLDER_PLACEHOLDER, LAYER_FOLDER_RULE_CODES, LAYER_FOLDERS, MODEL_NAME_LAYER_INDEX, MODEL_NAME_LAYER_SEPARATOR, + MODEL_NAME_PART_COUNTS, PROJECT_DIR_OPTION, SQL_FILE_SUFFIX, + WINDOWS_OS_NAME, ) from sqlbuild.cli.commands.models import LayerMoveSuggestion from sqlbuild.compiler.compile.models import CompilerDiagnostic @@ -26,6 +30,7 @@ def layer_move_suggestion( plan: RefactorPlan, diagnostics: tuple[CompilerDiagnostic, ...], project_dir: Path | None, + os_name: str = os.name, ) -> LayerMoveSuggestion | None: """Return the `sqb mv` that renames and moves in one step, or None for any other failure.""" @@ -43,19 +48,23 @@ def layer_move_suggestion( ): return None new_name: str = plan.request.new_name - name_parts: list[str] = new_name.split(MODEL_NAME_LAYER_SEPARATOR) + new_layer: str | None = _name_layer(new_name) layer_folder: tuple[str, ...] | None = ( - LAYER_FOLDERS.get(name_parts[MODEL_NAME_LAYER_INDEX]) - if len(name_parts) > MODEL_NAME_LAYER_INDEX - else None + LAYER_FOLDERS.get(new_layer) if new_layer is not None else None ) - if layer_folder is None: - return None - mirrored: tuple[tuple[str, ...], tuple[str, ...]] | None = _mirrored_folder( - original_path=renamed.original_path, layer_folder=layer_folder + mirrored: tuple[tuple[str, ...], tuple[str, ...]] | None = ( + _mirrored_folder( + original_path=renamed.original_path, + current_layer=_name_layer(plan.request.model_name), + layer_folder=layer_folder, + ) + if layer_folder is not None + else None ) folders: tuple[str, ...] = ( - (*mirrored[0], *mirrored[1]) if mirrored is not None else (FOLDER_PLACEHOLDER,) + (*mirrored[0], *mirrored[1]) + if mirrored is not None + else (PurePosixPath(renamed.original_path).parts[0], FOLDER_PLACEHOLDER) ) arguments: list[str] = [ "sqb", @@ -68,26 +77,42 @@ def layer_move_suggestion( return LayerMoveSuggestion( new_name=new_name, folder="/".join(mirrored[0]) if mirrored is not None else None, - layer_folder="/".join(layer_folder), - command=shlex.join(arguments), + layer_folder="/".join(layer_folder) if layer_folder is not None else None, + command=command_line(arguments=arguments, os_name=os_name), ) +def command_line(*, arguments: list[str], os_name: str) -> str: + """Quote a command for the shell of the platform it is printed on.""" + + if os_name == WINDOWS_OS_NAME: + return subprocess.list2cmdline(arguments) + return shlex.join(arguments) + + +def _name_layer(name: str) -> str | None: + """Return the layer segment of a name in the rule grammar, as the layer-folder rules read it.""" + + parts: list[str] = name.split(MODEL_NAME_LAYER_SEPARATOR) + return parts[MODEL_NAME_LAYER_INDEX] if len(parts) in MODEL_NAME_PART_COUNTS else None + + def _mirrored_folder( - *, original_path: str, layer_folder: tuple[str, ...] + *, original_path: str, current_layer: str | None, layer_folder: tuple[str, ...] ) -> tuple[tuple[str, ...], tuple[str, ...]] | None: - """Return the new layer folder in place of the current one, and the sub-path kept below it.""" + """Swap the current layer's own folder for the new one, keeping the folders around it.""" - parent: tuple[str, ...] = PurePosixPath(original_path).parent.parts - for start in range(1, len(parent)): - current: tuple[str, ...] | None = next( - ( - candidate - for candidate in CURRENT_LAYER_FOLDERS - if parent[start : start + len(candidate)] == candidate - ), - None, + candidates: tuple[tuple[str, ...], ...] = tuple( + folder + for folder in ( + LAYER_FOLDERS.get(current_layer or ""), + CONTIGUOUS_LAYER_FOLDERS.get(current_layer or ""), ) - if current is not None: - return (*parent[:start], *layer_folder), parent[start + len(current) :] + if folder is not None + ) + parent: tuple[str, ...] = PurePosixPath(original_path).parent.parts + for current in candidates: + for start in range(1, len(parent)): + if parent[start : start + len(current)] == current: + return (*parent[:start], *layer_folder), parent[start + len(current) :] return None diff --git a/src/sqlbuild/cli/commands/_helpers/refactor/output.py b/src/sqlbuild/cli/commands/_helpers/refactor/output.py index 9eaa243893..55d52bfb42 100644 --- a/src/sqlbuild/cli/commands/_helpers/refactor/output.py +++ b/src/sqlbuild/cli/commands/_helpers/refactor/output.py @@ -207,6 +207,8 @@ def _layer_move_lines(*, style: CliStyle, layer_move: LayerMoveSuggestion) -> li f"under {layer_move.folder}/" if layer_move.folder is not None else f"under a layer folder named {layer_move.layer_folder}/" + if layer_move.layer_folder is not None + else "under another layer's folder" ) note: str = ( "(change the folder if you want it somewhere else)" diff --git a/src/sqlbuild/cli/commands/constants.py b/src/sqlbuild/cli/commands/constants.py index c40b1c2339..83b25c8b9f 100644 --- a/src/sqlbuild/cli/commands/constants.py +++ b/src/sqlbuild/cli/commands/constants.py @@ -180,6 +180,7 @@ LAYER_FOLDER_RULE_CODES: frozenset[str] = frozenset({"SQBRPROJECT102", "SQBRPROJECT105"}) MODEL_NAME_LAYER_SEPARATOR: str = "__" MODEL_NAME_LAYER_INDEX: int = 1 +MODEL_NAME_PART_COUNTS: frozenset[int] = frozenset({3, 4}) LAYER_FOLDERS: MappingProxyType[str, tuple[str, ...]] = MappingProxyType( { "stg": ("staging",), @@ -191,12 +192,17 @@ "mart_v": ("mart",), } ) -CURRENT_LAYER_FOLDERS: tuple[tuple[str, ...], ...] = ( - ("intermediate", "clean"), - ("intermediate", "enriched"), - ("intermediate",), - ("staging",), - ("mart",), +CONTIGUOUS_LAYER_FOLDERS: MappingProxyType[str, tuple[str, ...]] = MappingProxyType( + { + "stg": ("staging",), + "stg_v": ("staging",), + "int_clean": ("intermediate", "clean"), + "int_enriched": ("intermediate", "enriched"), + "int_v": ("intermediate",), + "mart": ("mart",), + "mart_v": ("mart",), + } ) +WINDOWS_OS_NAME: str = "nt" FOLDER_PLACEHOLDER: str = "" PROJECT_DIR_OPTION: str = "--project-dir" diff --git a/src/sqlbuild/cli/commands/models.py b/src/sqlbuild/cli/commands/models.py index d96c4da5b9..da69ddaa54 100644 --- a/src/sqlbuild/cli/commands/models.py +++ b/src/sqlbuild/cli/commands/models.py @@ -1106,7 +1106,7 @@ class LayerMoveSuggestion: new_name: str folder: str | None - layer_folder: str + layer_folder: str | None command: str diff --git a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py index aa843fe0fd..97a117219c 100644 --- a/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py +++ b/tests/e2e/src/sqlbuild/cli/commands/main/refactor/test_rename_layer_refusals.py @@ -147,7 +147,7 @@ def test_given_rename_into_another_layer_when_refused_then_printed_mv_renames_an expected_hint=( f"{_ENRICHED_NAME} belongs under a layer folder named intermediate/enriched/", " Rename and move in one step:", - f" sqb mv order_lines_web '/{_ENRICHED_NAME}.sql' " + f" sqb mv order_lines_web 'models//{_ENRICHED_NAME}.sql' " "--project-dir {project_dir}", " (replace / with the folder the model should live in)", ), diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py index 8bb9cb6be1..8f9d5615da 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/_test_types.py @@ -44,3 +44,17 @@ class LayerMoveSuggestionTestCase: codes: tuple[str, ...] expected_command: str | None expected_folder: str | None + + +@dataclass(frozen=True) +class CommandLineQuotingTestCase: + description: str + os_name: str + project_dir: str + expected_command: str + + +@dataclass(frozen=True) +class LayerFolderMappingTestCase: + description: str + expected_layers: frozenset[str] diff --git a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py index 25fa7cfa67..be5696ecae 100644 --- a/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py +++ b/tests/unit/src/sqlbuild/cli/commands/_helpers/refactor/test_layer_move.py @@ -2,12 +2,17 @@ from __future__ import annotations +from pathlib import Path + import pytest from sqlbuild.cli.commands._helpers.refactor.layer_move import layer_move_suggestion +from sqlbuild.cli.commands.constants import CONTIGUOUS_LAYER_FOLDERS, LAYER_FOLDERS from sqlbuild.cli.commands.models import LayerMoveSuggestion from sqlbuild.compiler.refactoring.models import RefactorPlan from tests.unit.src.sqlbuild.cli.commands._helpers.refactor._test_types import ( + CommandLineQuotingTestCase, + LayerFolderMappingTestCase, LayerMoveSuggestionTestCase, ) from tests.unit.src.sqlbuild.cli.commands._helpers.refactor.helpers import ( @@ -58,7 +63,38 @@ old_path="models/orders/web/order_lines.sql", new_name="orders__stg__order_lines", codes=_LAYER_CODES, - expected_command="sqb mv order_lines '/orders__stg__order_lines.sql'", + expected_command="sqb mv order_lines 'models//orders__stg__order_lines.sql'", + expected_folder=None, + ), + LayerMoveSuggestionTestCase( + description="only the current layer's own folder is replaced", + old_path="models/mart/orders/staging/orders__stg__order_lines_x.sql", + new_name="orders__int_clean__order_lines_x", + codes=("SQBRPROJECT102",), + expected_command=( + "sqb mv orders__stg__order_lines_x " + "models/mart/orders/intermediate/clean/orders__int_clean__order_lines_x.sql" + ), + expected_folder="models/mart/orders/intermediate/clean", + ), + LayerMoveSuggestionTestCase( + description="a path without the current layer's folder uses a placeholder", + old_path="models/orders/mart/web/orders__stg__order_lines.sql", + new_name="orders__int_clean__order_lines", + codes=_LAYER_CODES, + expected_command=( + "sqb mv orders__stg__order_lines 'models//orders__int_clean__order_lines.sql'" + ), + expected_folder=None, + ), + LayerMoveSuggestionTestCase( + description="a layer without a folder mapping uses a placeholder", + old_path="models/orders/intermediate/clean/orders__int_clean__totals.sql", + new_name="orders__int_archive__totals", + codes=_LAYER_CODES, + expected_command=( + "sqb mv orders__int_clean__totals 'models//orders__int_archive__totals.sql'" + ), expected_folder=None, ), LayerMoveSuggestionTestCase( @@ -89,5 +125,71 @@ def test_given_rename_compile_errors_when_suggesting_layer_move_then_one_step_mv ) == (test_case.expected_command, test_case.expected_folder) +@pytest.mark.parametrize( + "test_case", + [ + CommandLineQuotingTestCase( + description="posix shells get POSIX quoting", + os_name="posix", + project_dir="/srv/order projects/orders", + expected_command=( + "sqb mv orders__int_clean__lines__web " + "models/orders/intermediate/enriched/web/orders__int_enriched__lines__web.sql " + "--project-dir '/srv/order projects/orders'" + ), + ), + CommandLineQuotingTestCase( + description="windows gets cmd.exe quoting", + os_name="nt", + project_dir="C:\\order projects\\orders", + expected_command=( + "sqb mv orders__int_clean__lines__web " + "models/orders/intermediate/enriched/web/orders__int_enriched__lines__web.sql " + '--project-dir "C:\\order projects\\orders"' + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_platform_when_suggesting_layer_move_then_command_is_quoted_for_its_shell( + test_case: CommandLineQuotingTestCase, +) -> None: + plan: RefactorPlan = build_rename_plan( + old_path="models/orders/intermediate/clean/web/orders__int_clean__lines__web.sql", + new_name="orders__int_enriched__lines__web", + ) + + suggestion: LayerMoveSuggestion | None = layer_move_suggestion( + plan=plan, + diagnostics=rule_diagnostics(codes=_LAYER_CODES, path=plan.changes[0].path), + project_dir=Path(test_case.project_dir), + os_name=test_case.os_name, + ) + + assert getattr(suggestion, "command", None) == test_case.expected_command + + +@pytest.mark.parametrize( + "test_case", + [ + LayerFolderMappingTestCase( + description="every parse_name layer, mirroring model_layers.rs and domain_layout.rs", + expected_layers=frozenset( + {"stg", "stg_v", "int_clean", "int_v", "int_enriched", "mart", "mart_v"} + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_rule_grammar_layers_when_reading_layer_folders_then_each_layer_has_both_folders( + test_case: LayerFolderMappingTestCase, +) -> None: + """LAYER_FOLDERS mirrors model_layers.rs (SQBRPROJECT105); the other, domain_layout.rs (102).""" + assert (frozenset(LAYER_FOLDERS), frozenset(CONTIGUOUS_LAYER_FOLDERS)) == ( + test_case.expected_layers, + test_case.expected_layers, + ) + + if __name__ == "__main__": pytest.main([__file__, "-vv"]) diff --git a/website/src/content/docs/docs/cli/rename.mdx b/website/src/content/docs/docs/cli/rename.mdx index 11f7708731..7431ffc6c2 100644 --- a/website/src/content/docs/docs/cli/rename.mdx +++ b/website/src/content/docs/docs/cli/rename.mdx @@ -97,7 +97,7 @@ orders__int_enriched__order_lines__web belongs under models/orders/intermediate/ (change the folder if you want it somewhere else) ``` -When the current path has no layer folder to mirror, the command uses a `/` placeholder for you to fill in. +When the current path has no folder for the model's current layer to swap, or the new layer has no canonical folder, the destination is `models//.sql`, with a `` placeholder for you to fill in. Each location is listed with its file, line, and column, with the fix. Edit those locations, or pass the model or column to the macro as an argument, and run the command again. The command never applies part of a rename.