Skip to content

Commit 98102ef

Browse files
committed
fix(native): show authored text unchanged in discovery failure messages
Discovery decoded raw-name markers across whole failure messages, so a NUL authored in a header key (for example a model's descri\0ption) was rewritten under every engine. Layout failures now show each listed path as it is displayed when the message is built, and the binding passes message text through untouched.
1 parent cdcd478 commit 98102ef

7 files changed

Lines changed: 178 additions & 15 deletions

File tree

‎crates/sqlbuild-discovery/src/declarations/_helpers/declaration_groups.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::constants::{
55
SCOPED_DECLARATION_DIRECTORIES,
66
};
77
use crate::declarations::_helpers::paths::parent;
8-
use crate::declarations::_helpers::scan::declaration_failure;
8+
use crate::declarations::_helpers::scan::{declaration_failure, shown, shown_list};
99
use crate::declarations::errors::ScanError;
1010
use crate::declarations::models::DeclarationGroup;
1111
use crate::models::StageFailure;
@@ -56,7 +56,7 @@ pub(crate) fn validate_declaration_groups(tree: &ProjectTree) -> Result<(), Scan
5656
"Grouped declaration root {}/ must be below a concrete owner directory; use the \
5757
project-wide macros/, enums/, constants/, audits/, schemas/, or hooks/ root \
5858
instead",
59-
group.directory
59+
shown(&group.directory)
6060
)));
6161
}
6262
let unsupported: Vec<String> = children(tree, &group.directory)?
@@ -67,8 +67,8 @@ pub(crate) fn validate_declaration_groups(tree: &ProjectTree) -> Result<(), Scan
6767
if !unsupported.is_empty() {
6868
return Err(declaration_failure(format!(
6969
"Declaration group {}/ contains unsupported entries: {}",
70-
group.directory,
71-
unsupported.join(", ")
70+
shown(&group.directory),
71+
shown_list(&unsupported)
7272
)));
7373
}
7474
}

‎crates/sqlbuild-discovery/src/declarations/_helpers/declaration_tree.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::constants::{
77
};
88
use crate::declarations::_helpers::declaration_groups::validate_declaration_groups;
99
use crate::declarations::_helpers::paths::{directory_facts, name, parent, stem};
10-
use crate::declarations::_helpers::scan::declaration_failure;
10+
use crate::declarations::_helpers::scan::{declaration_failure, shown};
1111
use crate::declarations::errors::ScanError;
1212
use crate::declarations::models::{DeclarationFileFact, DeclarationKind, ScopeKind};
1313
use crate::tree::main::directories::directories;
@@ -115,7 +115,8 @@ fn is_kind(kind: Option<DeclarationKind>, directory_name: &str) -> bool {
115115

116116
fn nested_root_failure(directory: &str) -> ScanError {
117117
declaration_failure(format!(
118-
"Declaration root {directory}/ is nested inside another declaration tree"
118+
"Declaration root {}/ is nested inside another declaration tree",
119+
shown(directory)
119120
))
120121
}
121122

‎crates/sqlbuild-discovery/src/declarations/_helpers/named_layout.rs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::constants::{
77
};
88
use crate::declarations::_helpers::declaration_groups::owned_groups;
99
use crate::declarations::_helpers::paths::{join, name};
10-
use crate::declarations::_helpers::scan::declaration_failure;
10+
use crate::declarations::_helpers::scan::{declaration_failure, shown, shown_list};
1111
use crate::declarations::errors::ScanError;
1212
use crate::declarations::models::DeclarationGroup;
1313
use crate::tree::main::children::children;
@@ -50,9 +50,10 @@ pub(crate) fn named_declaration_groups(
5050
.find(|directory| is_nested_role_directory(name(directory)))
5151
{
5252
return Err(declaration_failure(format!(
53-
"Declaration directory {nested}/ is not allowed inside the project-wide \
53+
"Declaration directory {}/ is not allowed inside the project-wide \
5454
{top_level}/ role; scoped declarations belong under \
55-
<folder>/{DECLARATION_GROUP_DIRECTORY}/ below a resource tree"
55+
<folder>/{DECLARATION_GROUP_DIRECTORY}/ below a resource tree",
56+
shown(&nested)
5657
)));
5758
}
5859
}
@@ -78,8 +79,9 @@ fn validate_group_roles(tree: &ProjectTree, group: &str) -> Result<(), ScanError
7879
let singular: String = join(&directory, SINGULAR_AUDIT_DIRECTORY);
7980
if role == LOCAL_AUDIT_ROLE_DIRECTORY && tree.exists(&singular) {
8081
return Err(declaration_failure(format!(
81-
"{singular}/ is invalid: singular audits are never used by name, so folder-only \
82-
visibility has no meaning; use {DECLARATION_GROUP_DIRECTORY}/audits/singular/"
82+
"{}/ is invalid: singular audits are never used by name, so folder-only \
83+
visibility has no meaning; use {DECLARATION_GROUP_DIRECTORY}/audits/singular/",
84+
shown(&singular)
8385
)));
8486
}
8587
let help_text: String = format!(
@@ -116,8 +118,8 @@ fn require_role_children(tree: &ProjectTree, role: &RoleChildren<'_>) -> Result<
116118
}
117119
Err(declaration_failure(format!(
118120
"Unsupported entries in {}/: {}; {}",
119-
role.directory,
120-
unsupported.join(", "),
121+
shown(role.directory),
122+
shown_list(&unsupported),
121123
role.help_text
122124
)))
123125
}

‎crates/sqlbuild-discovery/src/declarations/_helpers/scan.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,22 @@
22
33
use crate::declarations::errors::ScanError;
44
use crate::models::{DiscoveryFailure, FailureKind, StageFailure};
5+
use crate::tree::main::display_text::display_text;
6+
use std::borrow::Cow;
7+
8+
/// A listed relative path as a failure message shows it; message text is never decoded.
9+
pub(crate) fn shown(relative_path: &str) -> Cow<'_, str> {
10+
display_text(relative_path)
11+
}
12+
13+
/// The listed relative paths `paths`, shown and joined as Python lists them.
14+
pub(crate) fn shown_list(paths: &[String]) -> String {
15+
paths
16+
.iter()
17+
.map(|path| shown(path))
18+
.collect::<Vec<Cow<'_, str>>>()
19+
.join(", ")
20+
}
521

622
/// A `DeclarationParseError` with Python's message.
723
pub(crate) fn declaration_failure(message: String) -> ScanError {

‎crates/sqlbuild-python/src/bindings/_helpers/discovery/project_files.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ use sqlbuild_discovery::sql_tests::main::parse_sql_test_text::parse_sql_test_tex
2424
use sqlbuild_discovery::sql_tests::models::{
2525
DiscoveredScenarioFile, DiscoveredSqlTestFile, SqlTestBlock, SqlTestFileOptions,
2626
};
27-
use sqlbuild_discovery::tree::main::display_text::display_text;
2827
use sqlbuild_discovery::tree::main::listings::read_listings;
2928
use sqlbuild_discovery::tree::models::{ProjectTree, TreeEntry};
3029
use sqlbuild_discovery::yaml_files::main::load_yaml_files::load_yaml_files as load_yaml;
@@ -80,7 +79,7 @@ pub(crate) fn failure_object(py: Python<'_>, failure: DiscoveryFailure) -> PyRes
8079
vec![
8180
object(py, "error")?,
8281
object(py, failure.kind.as_str())?,
83-
object(py, display_text(&failure.message))?,
82+
object(py, failure.message)?,
8483
object(py, failure.help)?,
8584
],
8685
)

‎tests/integration/src/sqlbuild/compiler/discovery/_test_types.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,22 @@ class DeclarationReuseTestCase:
229229
edited_path: str
230230
edited_contents: bytes
231231
expected_reused_session: object = None
232+
233+
234+
@dataclass(frozen=True)
235+
class FailureTextTestCase:
236+
"""A project whose discovery failure message must keep its authored text unchanged."""
237+
238+
description: str
239+
files: tuple[tuple[str, bytes], ...]
240+
expected_error_type: str
241+
expected_message_suffix: str
242+
243+
244+
@dataclass(frozen=True)
245+
class TolerantFailureTextTestCase:
246+
"""A project whose broken declaration files tolerant discovery faults with authored text."""
247+
248+
description: str
249+
files: tuple[tuple[str, bytes], ...]
250+
expected_fault_keys: tuple[str, ...]
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
"""Failure messages show authored text as written and escape only raw path names."""
2+
3+
from __future__ import annotations
4+
5+
import os
6+
from pathlib import Path
7+
8+
import pytest
9+
10+
from sqlbuild.compiler.discovery.models import DiscoveryFileFault
11+
from tests.integration.src.sqlbuild.compiler.discovery._test_types import (
12+
FailureTextTestCase,
13+
TolerantFailureTextTestCase,
14+
)
15+
from tests.integration.src.sqlbuild.compiler.discovery.helpers import (
16+
stage_outcome,
17+
tolerant_declaration_outcome,
18+
write_project,
19+
)
20+
21+
_ENGINES: tuple[str, ...] = ("python", "native", "native-preview")
22+
_MODEL: tuple[str, bytes] = ("models/orders.sql", b"MODEL ();\nSELECT 1 AS order_id")
23+
_HOOK: tuple[str, bytes] = ("hooks/sql/refresh.sql", b"HOOK (descri\x00ption 'x');\nSELECT 1")
24+
_CONSTANT: tuple[str, bytes] = ("constants/limits.sql", b"CONSTANT (name cap, valu\x00e 1);")
25+
26+
27+
@pytest.mark.parametrize(
28+
"test_case",
29+
[
30+
FailureTextTestCase(
31+
description="a NUL in a model header key",
32+
files=(("models/orders.sql", b"MODEL (descri\x00ption 'x');\nSELECT 1"),),
33+
expected_error_type="ModelSqlParseError",
34+
expected_message_suffix="has unsupported keys: descri\x00ption",
35+
),
36+
FailureTextTestCase(
37+
description="a NUL in a SQL hook header key",
38+
files=(_MODEL, _HOOK),
39+
expected_error_type="SqlHookParseError",
40+
expected_message_suffix="has unsupported keys: descri\x00ption",
41+
),
42+
FailureTextTestCase(
43+
description="a NUL in a constant key",
44+
files=(_MODEL, _CONSTANT),
45+
expected_error_type="DeclarationParseError",
46+
expected_message_suffix="constant has unknown keys: valu\x00e",
47+
),
48+
FailureTextTestCase(
49+
description="a NUL in an audit header key",
50+
files=(
51+
_MODEL,
52+
("audits/generic/positive.sql", b"AUDIT (sever\x00ity warn);\nSELECT 1"),
53+
),
54+
expected_error_type="SqlAuditParseError",
55+
expected_message_suffix="has unsupported keys: sever\x00ity",
56+
),
57+
FailureTextTestCase(
58+
description="a NUL in a SQL function header key",
59+
files=(
60+
_MODEL,
61+
(
62+
"functions/sql/cents.sql",
63+
b'FUNCTION (arguments (), returns INTEGER, descri\x00ption "x");\nSELECT 1',
64+
),
65+
),
66+
expected_error_type="ModelSqlParseError",
67+
expected_message_suffix="has unsupported keys: descri\x00ption",
68+
),
69+
FailureTextTestCase(
70+
description="a raw entry name in a declaration group",
71+
files=(_MODEL, (os.fsdecode(b"models/orders/_sqlbuild/caf\xe9.txt"), b"x")),
72+
expected_error_type="DeclarationParseError",
73+
expected_message_suffix="unsupported entries: models/orders/_sqlbuild/caf\\xe9.txt",
74+
),
75+
],
76+
ids=lambda case: case.description,
77+
)
78+
def test_given_failing_project_when_discovering_then_every_engine_shows_authored_text(
79+
test_case: FailureTextTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
80+
) -> None:
81+
write_project(project_dir=tmp_path, files=test_case.files)
82+
83+
outcomes: list[tuple[object, ...]] = [
84+
stage_outcome(project_dir=tmp_path, engine=engine, monkeypatch=monkeypatch)
85+
for engine in _ENGINES
86+
]
87+
88+
assert {outcome[1:] for outcome in outcomes} == {outcomes[0][1:]}
89+
assert outcomes[0][1] == test_case.expected_error_type
90+
assert str(outcomes[0][2]).endswith(test_case.expected_message_suffix)
91+
92+
93+
@pytest.mark.parametrize(
94+
"test_case",
95+
[
96+
TolerantFailureTextTestCase(
97+
description="NUL header keys are faulted with their authored text",
98+
files=(_MODEL, _HOOK, _CONSTANT),
99+
expected_fault_keys=("descri\x00ption", "valu\x00e"),
100+
)
101+
],
102+
ids=lambda case: case.description,
103+
)
104+
def test_given_nul_header_keys_when_discovering_tolerantly_then_faults_match_python(
105+
test_case: TolerantFailureTextTestCase, tmp_path: Path, monkeypatch: pytest.MonkeyPatch
106+
) -> None:
107+
write_project(project_dir=tmp_path, files=test_case.files)
108+
python: tuple[object, tuple[DiscoveryFileFault, ...], tuple[DiscoveryFileFault, ...]] = (
109+
tolerant_declaration_outcome(project_dir=tmp_path, engine="python", monkeypatch=monkeypatch)
110+
)
111+
112+
native: tuple[object, tuple[DiscoveryFileFault, ...], tuple[DiscoveryFileFault, ...]] = (
113+
tolerant_declaration_outcome(
114+
project_dir=tmp_path, engine="native-preview", monkeypatch=monkeypatch
115+
)
116+
)
117+
118+
assert native == python
119+
assert (
120+
tuple(sorted(fault.message.rsplit("keys: ", 1)[1] for fault in (*native[1], *native[2])))
121+
== test_case.expected_fault_keys
122+
)
123+
124+
125+
if __name__ == "__main__":
126+
pytest.main([__file__, "-vv"])

0 commit comments

Comments
 (0)