diff --git a/esphome/__main__.py b/esphome/__main__.py index c1451c5fafd..a0ee5a359f4 100644 --- a/esphome/__main__.py +++ b/esphome/__main__.py @@ -1813,11 +1813,32 @@ def command_rename(args: ArgsProtocol, config: ConfigType) -> int | None: old_name = yaml[CONF_ESPHOME][CONF_NAME] match = re.match(r"^\$\{?([a-zA-Z0-9_]+)\}?$", old_name) if match is None: - new_raw = re.sub( - rf"name:\s+[\"']?{old_name}[\"']?", - f'name: "{new_name}"', - raw_contents, + # Only swap the ``name:`` line that sits directly under the + # top-level ``esphome:`` block. A naked ``re.sub`` would + # also clobber any other ``name:`` line whose value happens + # to match (e.g. a sensor / output / wifi entry sharing the + # device's hostname), silently rewriting unrelated user + # configuration. The pattern anchors: + # - at the start of the line so ``friendly_name:``, + # ``device_name:`` etc. don't match the trailing ``name:`` + # substring; and + # - at the end of the value (lookahead for whitespace + + # comment + EOL) so ``old_name`` doesn't match as a + # prefix of a longer value (``kitchen`` vs ``kitchen2``). + name_pattern = re.compile( + rf"^(\s*)name:\s+[\"']?{re.escape(old_name)}[\"']?(?=\s*(?:#|$))" ) + out_lines: list[str] = [] + in_esphome_block = False + for line in raw_contents.splitlines(keepends=True): + if line and not line[0].isspace() and line.strip(): + in_esphome_block = line.lstrip().startswith("esphome:") + out_lines.append(line) + continue + if in_esphome_block: + line = name_pattern.sub(rf'\1name: "{new_name}"', line, count=1) + out_lines.append(line) + new_raw = "".join(out_lines) else: old_name = yaml[CONF_SUBSTITUTIONS][match.group(1)] if ( @@ -1840,7 +1861,40 @@ def command_rename(args: ArgsProtocol, config: ConfigType) -> int | None: flags=re.MULTILINE, ) + # ``new_name == old_name`` (after substitution resolution) is + # a no-op rewrite that would still queue a pointless re-flash. + # Catch it before the path-equality check below — covers the + # case where the config filename doesn't match the device name + # (e.g. ``weird-file.yaml`` whose ``esphome.name`` is + # ``kitchen``; running ``esphome rename weird-file.yaml kitchen`` + # would otherwise just re-flash the same hostname). + if new_name == old_name: + print( + color( + AnsiFore.BOLD_RED, + f"'{new_name}' is already the device's name.", + ) + ) + return 1 + new_path: Path = CORE.config_dir / (new_name + ".yaml") + if new_path.resolve() == CORE.config_path.resolve(): + print( + color( + AnsiFore.BOLD_RED, + f"'{new_name}' is already the device's name.", + ) + ) + return 1 + if new_path.exists(): + print( + color( + AnsiFore.BOLD_RED, + f"Cannot rename: {new_path} already exists. " + "Refusing to overwrite an existing configuration.", + ) + ) + return 1 print( f"Updating {color(AnsiFore.CYAN, str(CORE.config_path))} to {color(AnsiFore.CYAN, str(new_path))}" ) diff --git a/tests/unit_tests/test_main.py b/tests/unit_tests/test_main.py index 4b0590cf76d..2823310f0e4 100644 --- a/tests/unit_tests/test_main.py +++ b/tests/unit_tests/test_main.py @@ -3595,6 +3595,467 @@ esp32: assert "Rename failed" in captured.out +def test_command_rename_install_failure_reverts( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename when the install (esphome run) step fails.""" + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + + # First call (config validation) succeeds; second (esphome run) fails. + mock_run_external_process.side_effect = [0, 1] + + result = command_rename(args, {}) + + assert result == 1 + + # New file was unlinked when install failed. + new_file = tmp_path / "newname.yaml" + assert not new_file.exists() + + # Old file is preserved so the device stays reachable under the + # original hostname. + assert config_file.exists() + + +def test_command_rename_target_exists_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the target filename already exists. + + Without this guard, the rename would overwrite the unrelated + device's YAML and OTA-install our firmware to the wrong device. + """ + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + +esp32: + board: nodemcu-32s +""") + target_file = tmp_path / "newname.yaml" + target_file.write_text(""" +esphome: + name: someoneelse + +esp32: + board: nodemcu-32s +""") + target_original = target_file.read_text() + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + # No subprocess work happened — refusal is up-front. + mock_run_external_process.assert_not_called() + # Target file untouched: same content, still on disk. + assert target_file.exists() + assert target_file.read_text() == target_original + # Source file untouched. + assert config_file.exists() + + captured = capfd.readouterr() + assert "already exists" in captured.out + + +def test_command_rename_same_name_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the new name matches the current name. + + A same-name rename would otherwise re-write the YAML and queue + a redundant compile + install — wasted work the user almost + certainly didn't intend. + """ + config_file = tmp_path / "samename.yaml" + config_file.write_text(""" +esphome: + name: samename + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "samename"}} + + args = MockArgs(name="samename", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # File preserved verbatim — no rewrite happened. + assert config_file.exists() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_does_not_touch_friendly_name_substring( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + r"""Test rename does not match the ``name:`` substring of ``friendly_name:``. + + Without anchoring the regex at line start, the pattern + ``\s*name:\s+`` could match the trailing ``name:`` + substring inside ``friendly_name: ``. The rewrite would + flip both lines to the new name, leaving the user with a + silently corrupted ``friendly_name``. + """ + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +esphome: + name: oldname + friendly_name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "oldname"}} + + args = MockArgs(name="newname", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "newname.yaml" + content = new_file.read_text() + # esphome.name swapped. + assert 'name: "newname"' in content + # friendly_name kept verbatim. + assert "friendly_name: oldname" in content + + +def test_command_rename_does_not_match_old_name_as_value_prefix( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + r"""Test rename does not match ``old_name`` as a prefix of a longer value. + + With ``old_name = kitchen`` the value ``kitchen2`` (a sensor + or wifi entry) would otherwise match the unanchored + ``["']?kitchen["']?`` pattern at the prefix and get + rewritten to the new name. The end-of-value lookahead keeps + the match restricted to whole tokens. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s + +wifi: + ap: + ssid: kitchen2 +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + assert 'name: "garage"' in content + # The wifi ssid value is unrelated and stays intact. + assert "ssid: kitchen2" in content + + +def test_command_rename_same_resolved_name_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when ``new_name`` matches the resolved device name. + + The path-equality check only catches the case where the + config filename matches the device name. For a config whose + filename and ``esphome.name`` differ (here ``weird-file.yaml`` + holds ``esphome.name: kitchen``), running + ``esphome rename weird-file.yaml kitchen`` would otherwise + fall through to the rewrite + install: the YAML's name stays + ``kitchen``, the file is renamed to ``kitchen.yaml``, and the + device gets a redundant flash. Refuse up-front so the + "already the device's name" message matches reality. + """ + config_file = tmp_path / "weird-file.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="kitchen", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # Source file untouched, no derived target written. + assert config_file.exists() + assert not (tmp_path / "kitchen.yaml").exists() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_target_path_equals_source_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when the new path resolves to the source file. + + Reachable only when the YAML's filename and ``esphome.name`` + disagree — here ``kitchen.yaml`` holds ``esphome.name: garage`` + and the user runs ``esphome rename kitchen.yaml kitchen``. The + name-equality check above passes (``garage != kitchen``), but + ``/kitchen.yaml`` resolves to the source file + itself, so the rewrite would clobber the source mid-rename. + Refuse rather than silently overwriting. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: garage + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "garage"}} + + args = MockArgs(name="kitchen", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # Source file still present and unmodified. + assert config_file.exists() + assert "name: garage" in config_file.read_text() + + captured = capfd.readouterr() + assert "already" in captured.out.lower() + + +def test_command_rename_does_not_touch_lookalike_name_in_other_blocks( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename only swaps the esphome.name line. + + A device whose name happens to match a sensor's / output's + ``name:`` value must not have those other names rewritten — + they're independent. Without an anchor for the esphome block + a naive regex would clobber every line whose value matches. + """ + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen + +esp32: + board: nodemcu-32s + +sensor: + - platform: template + name: kitchen + lambda: 'return 0;' +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + # esphome.name renamed. + assert 'name: "garage"' in content + # Sensor's name is the user's entity name — must not be touched. + assert " name: kitchen\n" in content + + +def test_command_rename_preserves_trailing_comment( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename preserves a trailing ``# comment`` on the name line.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: kitchen # primary device + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + + new_file = tmp_path / "garage.yaml" + content = new_file.read_text() + assert "# primary device" in content + + +def test_command_rename_handles_double_quoted_value( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename matches when the existing value is double-quoted.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: "kitchen" + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + assert 'name: "garage"' in new_file.read_text() + + +def test_command_rename_handles_single_quoted_value( + tmp_path: Path, + mock_run_external_process: Mock, +) -> None: + """Test rename matches when the existing value is single-quoted.""" + config_file = tmp_path / "kitchen.yaml" + config_file.write_text(""" +esphome: + name: 'kitchen' + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = {CONF_ESPHOME: {CONF_NAME: "kitchen"}} + + args = MockArgs(name="garage", dashboard=False) + mock_run_external_process.return_value = 0 + + result = command_rename(args, {}) + + assert result == 0 + new_file = tmp_path / "garage.yaml" + assert 'name: "garage"' in new_file.read_text() + + +def test_command_rename_too_many_substitution_matches_refuses( + tmp_path: Path, + capfd: CaptureFixture[str], + mock_run_external_process: Mock, +) -> None: + """Test rename refuses when ``${var}`` resolves to multiple matches. + + When ``esphome.name: ${device_name}`` and the substitution + definition ``device_name: foo`` appears more than once in the + YAML (e.g. inside multiple included blocks), the regex rewrite + can't tell which one to flip. Rather than silently picking one + or rewriting both, the command refuses. + """ + config_file = tmp_path / "oldname.yaml" + config_file.write_text(""" +substitutions: + device_name: oldname + +esphome: + name: ${device_name} + +# A copy-pasted block that re-declares the substitution at the +# same indent level - happens when users splice in a packaged +# fragment without renaming the variable. +example: + device_name: oldname + +esp32: + board: nodemcu-32s +""") + setup_core(tmp_path=tmp_path) + CORE.config_path = config_file + CORE.config = { + CONF_ESPHOME: {CONF_NAME: "oldname"}, + CONF_SUBSTITUTIONS: {"device_name": "oldname"}, + } + + args = MockArgs(name="newname", dashboard=False) + + result = command_rename(args, {}) + + assert result == 1 + mock_run_external_process.assert_not_called() + # File untouched. + assert config_file.exists() + assert "device_name: oldname" in config_file.read_text() + + captured = capfd.readouterr() + assert "Too many matches" in captured.out + + def test_command_update_all_path_string_conversion( tmp_path: Path, mock_run_external_process: Mock,