diff --git a/src/sqlquality/cli.py b/src/sqlquality/cli.py index 8f5cf8c..c5dd76b 100644 --- a/src/sqlquality/cli.py +++ b/src/sqlquality/cli.py @@ -110,6 +110,27 @@ def read_sql_file(path: Path) -> str: raise typer.Exit(code=2) +def _write_report_or_exit(path: Path, text: str, flag: str) -> None: + """Write a rendered report, or exit 2 naming the flag that pointed at the bad path. + + Two failures are folded together here because both used to escape as exit **1**, and + exit 1 is how `check` and `lint` report real findings — so an unwritable path was + indistinguishable from a failed gate, and CI would block on a healthy run. + + * ``encoding="utf-8"`` because the default is platform-dependent and these reports + carry non-ASCII (the gate verdict is an emoji), so an ASCII locale made every + ``--markdown`` write a guaranteed crash. + * ``UnicodeError`` alongside ``OSError`` because ``UnicodeEncodeError`` is a + ``ValueError``: pinning the encoding makes it unlikely, not unreachable, since a lone + surrogate in an identifier is unencodable in any codec. + """ + try: + path.write_text(text, encoding="utf-8") + except (OSError, UnicodeError) as exc: + typer.echo(f"Could not write {flag} to {path}: {exc}", err=True) + raise typer.Exit(code=2) + + def _validate_dialect_or_exit(name: str) -> str: """Normalize a dialect name or print the friendly error and exit 2.""" try: @@ -300,11 +321,12 @@ def check( deltas, skipped = compute_deltas(baseline, candidate, changeset.changed, resolved_dialect) report = evaluate_gate(deltas, cfg) + # Rendered first, then written, so a renderer bug cannot be reported as a write failure. if html is not None: - Path(html).write_text(render_html(report, skipped)) + _write_report_or_exit(Path(html), render_html(report, skipped), "--html") if markdown is not None: - Path(markdown).write_text(render_markdown(report, skipped)) + _write_report_or_exit(Path(markdown), render_markdown(report, skipped), "--markdown") if json_out: typer.echo( @@ -377,7 +399,11 @@ def lint( if fix: fixed_sql = fix_sql(sql, dialect, excl, config_path) if fixed_sql != sql: - path.write_text(fixed_sql) + # `--fix` rewrites the user's own source. read_sql_file already reads as + # UTF-8, so writing without an encoding could round-trip a non-ASCII + # comment into mojibake, or raise and exit 1 — which `lint` also uses for + # findings. + _write_report_or_exit(path, fixed_sql, "--fix") changed = True # INFO (unresolved-Jinja) findings are advisory and never gate the commit. gating = gating or any(f.severity in (Severity.WARNING, Severity.ERROR) for f in findings) diff --git a/tests/test_check_gate.py b/tests/test_check_gate.py index c479d61..fe95a42 100644 --- a/tests/test_check_gate.py +++ b/tests/test_check_gate.py @@ -229,3 +229,87 @@ def test_check_old_schema_warns(tmp_path): # Warning only: no behavior change, still exits normally. assert result.exit_code == 0 assert "v12" in result.stderr and "v9" in result.stderr + + +def test_check_html_write_failure_exits_2_not_1(tmp_path): + """A bad --html path must not be mistaken for a failed gate. + + `check` uses exit 1 legitimately, for a regression over the threshold, so an unwritable + output path escaping as exit 1 is indistinguishable from a real finding — CI would + report a healthy run as a blocked one. + """ + proj, state = _project_with_baseline(tmp_path) + with _mock_changed(): + result = runner.invoke( + app, + [ + "check", + "--project-dir", + str(proj), + "--state", + str(state), + "--html", + str(tmp_path / "nope" / "missing.html"), + ], + ) + assert result.exit_code == 2, result.output + assert "--html" in result.output + + +def test_check_markdown_write_failure_exits_2_not_1(tmp_path): + proj, state = _project_with_baseline(tmp_path) + a_directory = tmp_path / "adir" + a_directory.mkdir() + with _mock_changed(): + result = runner.invoke( + app, + [ + "check", + "--project-dir", + str(proj), + "--state", + str(state), + "--markdown", + str(a_directory), + ], + ) + assert result.exit_code == 2, result.output + assert "--markdown" in result.output + + +def test_check_writes_reports_as_utf8(tmp_path, monkeypatch): + """Both report writes must name their encoding. + + The verdict line carries an emoji, so a platform-encoded write is a guaranteed crash on + an ASCII locale — and UnicodeEncodeError is a ValueError, so it escapes the OSError + handler and exits 1, which `check` also uses for a failed gate. Asserting the argument + rather than emulating a locale, because this Python resolves the default encoding + somewhere the locale module cannot be patched from. + """ + seen: list[str | None] = [] + real = Path.write_text + + def spy(self, data, encoding=None, *args, **kwargs): + seen.append(encoding) + return real(self, data, encoding=encoding or "utf-8", *args, **kwargs) + + proj, state = _project_with_baseline(tmp_path) + monkeypatch.setattr(Path, "write_text", spy) + seen.clear() # the fixture writes manifests of its own; only the reports matter here + with _mock_changed(): + result = runner.invoke( + app, + [ + "check", + "--project-dir", + str(proj), + "--state", + str(state), + "--markdown", + str(tmp_path / "r.md"), + "--html", + str(tmp_path / "r.html"), + ], + ) + assert result.exit_code in (0, 1), result.output + assert seen == ["utf-8", "utf-8"], f"reports written without an explicit encoding: {seen}" diff --git a/tests/test_lint_cli.py b/tests/test_lint_cli.py index 85557b4..81ab23a 100644 --- a/tests/test_lint_cli.py +++ b/tests/test_lint_cli.py @@ -220,3 +220,29 @@ def test_lint_fix_bad_second_path_leaves_first_unmodified(tmp_path): result = runner.invoke(app, ["lint", "--fix", str(good), str(missing)]) assert result.exit_code == 2 assert good.read_text() == before # untouched + + +def test_lint_fix_writes_utf8(tmp_path, monkeypatch): + """--fix rewrites the user's own source file. + + read_sql_file already reads as UTF-8; writing without an encoding meant a non-ASCII + comment could come back as mojibake, or raise UnicodeEncodeError — which escapes as + exit 1, the code `lint` also uses for findings. + """ + from pathlib import Path as _P + + seen: list[str | None] = [] + real = _P.write_text + + def spy(self, data, encoding=None, *args, **kwargs): + seen.append(encoding) + return real(self, data, encoding=encoding or "utf-8", *args, **kwargs) + + monkeypatch.setattr(_P, "write_text", spy) + f = tmp_path / "m.sql" + f.write_text("-- \u2705 v\u00e9rifi\u00e9\nselect a from t as t\n", encoding="utf-8") + seen.clear() + result = runner.invoke(app, ["lint", str(f), "--fix"]) + assert result.exit_code in (0, 1), result.output + assert seen == ["utf-8"], f"--fix wrote the source file without an encoding: {seen}" + assert "v\u00e9rifi\u00e9" in f.read_text(encoding="utf-8")