Skip to content

Commit 01fb511

Browse files
shengzhangscorehoe-jo
authored andcommitted
Add shared validation error formatting support
1 parent 90728a3 commit 01fb511

12 files changed

Lines changed: 207 additions & 42 deletions

File tree

‎tools/metamodel/common/source_location.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ impl SourceLocation {
3939
return normalize_source_path(relative.to_string_lossy().as_ref());
4040
}
4141
}
42+
43+
// TODO: Do not store that absolute sandbox path as the source
44+
// location. Source locations should be stable across actions, for
45+
// example repo-relative.
46+
if let Some(relative) = strip_bazel_execroot_prefix(self.file.as_ref()) {
47+
return relative;
48+
}
4249
}
4350

4451
normalize_source_path(self.file.as_ref())
@@ -53,6 +60,11 @@ fn normalize_source_path(path: &str) -> String {
5360
path.strip_prefix("./").unwrap_or(path).to_string()
5461
}
5562

63+
fn strip_bazel_execroot_prefix(path: &str) -> Option<String> {
64+
let (_, relative) = path.split_once("/execroot/_main/")?;
65+
Some(normalize_source_path(relative))
66+
}
67+
5668
impl fmt::Display for SourceLocation {
5769
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
5870
write!(f, "{}:{}", self.display_file(), self.line)

‎validation/core/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ rust_library(
5353
"src/readers/mod.rs",
5454
"src/readers/sequence_diagram_reader.rs",
5555
"src/results/diagnostics.rs",
56+
"src/results/error_models.rs",
5657
"src/results/mod.rs",
5758
"src/validators/bazel_component_validator.rs",
5859
"src/validators/class_design_implementation_validator.rs",
@@ -74,6 +75,7 @@ rust_library(
7475
visibility = ["//visibility:public"],
7576
deps = [
7677
"//tools/metamodel/class:class_diagram",
78+
"//tools/metamodel/common:source_location",
7779
"//tools/metamodel/component:component_diagram",
7880
"//tools/metamodel/sequence:sequence_diagram",
7981
"//tools/serialization/flatbuffers/class:class_fbs",

‎validation/core/integration_test/README.md‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ produced in layer 1. No copying or re-parsing occurs; the underlying Bazel
7272
action cache entry is reused.
7373

7474
A `filegroup` named `case_data` then bundles the `fbs` target together with the
75-
static fixture files (`architecture.json`, `expected.json` or `expected.yaml`),
75+
static fixture files (`architecture.json` and `expected.yaml`),
7676
making the whole case available as a single Bazel dependency.
7777

7878
For `ComponentInternalApi` and `SequenceInternalApi` suites, cases include
@@ -96,7 +96,7 @@ The shared `test_framework` library provides the following helpers:
9696
| Helper | Description |
9797
|--------|-------------|
9898
| `collect_case_fbs_files(suite, case, category)` | Returns sorted absolute paths to every `.fbs.bin` in a category subdirectory |
99-
| `load_expected_fixture(suite, case)` | Deserializes `expected.json` into `ExpectedFixture` |
99+
| `load_expected_fixture(suite, case)` | Deserializes `expected.yaml` into `ExpectedFixture` |
100100
| `load_expected_yaml_fixture(suite, case)` | Deserializes `expected.yaml` into `ExpectedFixture` |
101101
| `run_validation_profile(case_name, profile, input_bundle)` | Writes a profile-owned input bundle, spawns the CLI binary, and returns `CliRunResult` |
102102
| `assert_cli_result(case, expected, result)` | Asserts exit code and checks each string in `error_contains` against the log |
@@ -120,7 +120,7 @@ design/component_diagram.fbs.bin ← in ArchitecturalDesignInfo.static
120120
component/component_diagram.fbs.bin ← symlink → file above
121121
122122
│ filegroup case_data
123-
│ → bundles fbs + architecture.json + expected.json
123+
│ → bundles fbs + architecture.json + expected.yaml
124124
▼
125125
//.../bazel_component:bazel_component_test_data ← aggregated across all cases
126126
@@ -149,7 +149,7 @@ the validator under test.
149149
├── BUILD # architectural_design + provider_fbs_fixture_bundle + case_data
150150
├── component_diagram.puml
151151
├── architecture.json # Bazel build-graph snapshot (components/units)
152-
└── expected.json
152+
└── expected.yaml
153153
```
154154

155155
### ComponentClass cases
@@ -159,7 +159,7 @@ the validator under test.
159159
├── BUILD # architectural_design + unit_design + provider_fbs_fixture_bundle + case_data
160160
├── component_diagram.puml
161161
├── class_diagram.puml # one or more; multi-file: class_diagram_part1.puml, ...
162-
└── expected.json
162+
└── expected.yaml
163163
```
164164

165165
### ClassDesignImplementation cases
@@ -180,7 +180,7 @@ the validator under test.
180180
├── BUILD # architectural_design + provider_fbs_fixture_bundle + case_data
181181
├── component_diagram.puml
182182
├── sequence_diagram.puml
183-
└── expected.json
183+
└── expected.yaml
184184
```
185185

186186
### ComponentInternalApi cases
@@ -190,7 +190,7 @@ the validator under test.
190190
├── BUILD # architectural_design + provider_fbs_fixture_bundle + case_data
191191
├── component_diagram.puml
192192
├── internal_api_diagram.puml
193-
└── expected.json
193+
└── expected.yaml
194194
```
195195

196196
### SequenceInternalApi cases
@@ -201,16 +201,13 @@ the validator under test.
201201
├── component_diagram.puml
202202
├── sequence_diagram.puml
203203
├── internal_api_diagram.puml
204-
└── expected.json
204+
└── expected.yaml
205205
```
206206

207-
### `expected.json` / `expected.yaml` format
207+
### `expected.yaml` format
208208

209-
```json
210-
{
211-
"should_pass": true,
212-
"error_contains": []
213-
}
209+
```yaml
210+
should_pass: true
214211
```
215212
216213
| Field | Type | Description |
@@ -220,9 +217,7 @@ the validator under test.
220217

221218
The framework uses **substring matching** for `error_contains`, so entries do
222219
not need to reproduce exact formatting — just enough context to uniquely
223-
identify the error. Existing suites use `expected.json`; the
224-
`class_design_implementation` suite uses `expected.yaml` so larger multi-line
225-
error fragments stay readable.
220+
identify the error.
226221

227222
## Running the tests
228223

@@ -236,7 +231,6 @@ Run a single suite:
236231

237232
```bash
238233
bazel test //validation/core/integration_test/bazel_component:integration_test
239-
bazel test //validation/core/integration_test/component_class:integration_test
240234
bazel test //validation/core/integration_test/component_sequence:integration_test
241235
bazel test //validation/core/integration_test/component_internal_api:component_internal_api_integration_test
242236
bazel test //validation/core/integration_test/sequence_internal_api:sequence_internal_api_integration_test
@@ -253,16 +247,15 @@ bazel test //validation/core/integration_test/class_design_implementation:integr
253247
2. Add the `.puml` source file(s) and — for `bazel_component` — an
254248
`architecture.json`.
255249

256-
3. Write `expected.json` or `expected.yaml`. For negative cases add the error
250+
3. Write `expected.yaml`. For negative cases add the error
257251
substrings you expect to see in the CLI log.
258252

259253
4. Create a `BUILD` file following the pattern of an existing case in the same
260254
suite.
261255

262256
5. Add the new `case_data` target to the matching filegroup in
263257
[`BUILD`](BUILD) (`bazel_component_test_data`,
264-
`component_class_test_data`, `component_sequence_test_data`,
265-
`component_internal_api_test_data`, `sequence_internal_api_test_data`,
258+
`component_sequence_test_data`, `component_internal_api_test_data`, `sequence_internal_api_test_data`,
266259
or `class_design_implementation_test_data`).
267260

268261
6. Add a `#[test]` function in the matching suite file, such as

‎validation/core/integration_test/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@ mod test_framework;
1515

1616
pub use test_framework::{
1717
assert_cli_result, case_file_path, collect_case_fbs_files, load_expected_fixture,
18-
load_expected_yaml_fixture, normalize_yaml_result, run_validation_profile, CliRunResult,
19-
ExpectedFixture, ValidationIntegrationCase,
18+
normalize_yaml_result, run_validation_profile, CliRunResult, ExpectedFixture,
19+
ValidationIntegrationCase,
2020
};

‎validation/core/integration_test/src/test_framework.rs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,6 @@ pub fn collect_case_fbs_files(suite_dir: &str, case_dir: &str, category: &str) -
114114
matches
115115
}
116116

117-
pub fn load_expected_fixture(suite_dir: &str, case_dir: &str) -> ExpectedFixture {
118-
let expected_json = read_case_file(&format!(
119-
"validation/core/integration_test/{suite_dir}/{case_dir}/expected.json"
120-
));
121-
122-
serde_json::from_str(&expected_json).expect("failed to parse expected fixture")
123-
}
124-
125117
pub fn load_expected_yaml_fixture(suite_dir: &str, case_dir: &str) -> ExpectedFixture {
126118
let expected_yaml = read_case_file(&format!(
127119
"validation/core/integration_test/{suite_dir}/{case_dir}/expected.yaml"
@@ -169,6 +161,10 @@ fn is_source_file_line(line: &str) -> bool {
169161
line.starts_with(DESIGN_SOURCE_FILE_LABEL) || line.starts_with(IMPLEMENT_SOURCE_FILE_LABEL)
170162
}
171163

164+
pub fn load_expected_fixture(suite_dir: &str, case_dir: &str) -> ExpectedFixture {
165+
load_expected_yaml_fixture(suite_dir, case_dir)
166+
}
167+
172168
pub fn assert_cli_result(case_dir: &str, expected: &ExpectedFixture, result: &CliRunResult) {
173169
assert_eq!(
174170
result.success, expected.should_pass,

‎validation/core/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ mod results;
2323
mod validators;
2424

2525
pub use profiles::{read_profile_inputs, run_profile, Profile, ProfileInputs, ProfileRun};
26-
pub use results::{Diagnostics, ValidationResult};
26+
pub use results::{Diagnostics, ErrorBuilder, ErrorCategory, ValidationResult};

‎validation/core/src/models/class_diagram_models.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
1616
use std::collections::{BTreeMap, BTreeSet};
1717

18-
use class_diagram::{ClassDiagram as ClassDiagramInput, EntityType, SimpleEntity};
18+
use class_diagram::{ClassDiagram as ClassDiagramInput, EntityType, SimpleEntity, SourceLocation};
1919

2020
use crate::ValidationResult;
2121

@@ -67,6 +67,7 @@ impl ClassEntityIndex {
6767
pub struct InternalApiInterface {
6868
pub id: String,
6969
pub method_names: BTreeSet<String>,
70+
pub source_location: SourceLocation,
7071
}
7172

7273
/// Indexed internal-API data prepared for validators.
@@ -93,6 +94,7 @@ impl InternalApiIndex {
9394
.map(|method| method.name.clone())
9495
.filter(|name| !name.is_empty())
9596
.collect(),
97+
source_location: entity.source_location.clone(),
9698
};
9799

98100
interfaces.push(interface);

‎validation/core/src/readers/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
//! Input readers for Bazel JSON and PlantUML-derived FlatBuffer artifacts.
1515
16-
use component_diagram::SourceLocation;
16+
use source_location::SourceLocation;
1717

1818
mod bazel_reader;
1919
mod class_diagram_reader;
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
// *******************************************************************************
2+
// Copyright (c) 2026 Contributors to the Eclipse Foundation
3+
//
4+
// See the NOTICE file(s) distributed with this work for additional
5+
// information regarding copyright ownership.
6+
//
7+
// This program and the accompanying materials are made available under the
8+
// terms of the Apache License Version 2.0 which is available at
9+
// <https://www.apache.org/licenses/LICENSE-2.0>
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
// *******************************************************************************
13+
14+
//! Shared error message formatting helpers for validation results.
15+
16+
use std::fmt::Write;
17+
18+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
19+
pub enum ErrorCategory {
20+
Design,
21+
Class,
22+
Member,
23+
Implementation,
24+
Naming,
25+
Interface,
26+
Method,
27+
Coverage,
28+
}
29+
30+
impl ErrorCategory {
31+
fn as_tag(self) -> &'static str {
32+
match self {
33+
Self::Design => "Design",
34+
Self::Class => "Class",
35+
Self::Member => "Member",
36+
Self::Implementation => "Implementation",
37+
Self::Naming => "Naming",
38+
Self::Interface => "Interface",
39+
Self::Method => "Method",
40+
Self::Coverage => "Coverage",
41+
}
42+
}
43+
}
44+
45+
fn uppercase_first_char(value: impl Into<String>) -> String {
46+
let mut value = value.into();
47+
48+
if let Some((index, first_char)) = value.char_indices().next() {
49+
let upper = first_char.to_uppercase().to_string();
50+
value.replace_range(index..index + first_char.len_utf8(), &upper);
51+
}
52+
53+
value
54+
}
55+
56+
fn normalize_sentence_case(value: impl Into<String>) -> String {
57+
let mut value = uppercase_first_char(value);
58+
59+
if !value.ends_with('.') {
60+
value.push('.');
61+
}
62+
63+
value
64+
}
65+
66+
#[derive(Debug, Clone, PartialEq, Eq)]
67+
pub struct ErrorBuilder {
68+
category: ErrorCategory,
69+
title: Option<String>,
70+
fields: Vec<(String, String)>,
71+
fix: Option<String>,
72+
}
73+
74+
impl ErrorBuilder {
75+
pub fn new(category: ErrorCategory) -> Self {
76+
Self {
77+
category,
78+
title: None,
79+
fields: Vec::new(),
80+
fix: None,
81+
}
82+
}
83+
84+
pub fn title(mut self, title: impl Into<String>) -> Self {
85+
self.title = Some(normalize_sentence_case(title));
86+
self
87+
}
88+
89+
pub fn field(mut self, key: impl Into<String>, value: impl Into<String>) -> Self {
90+
self.fields.push((uppercase_first_char(key), value.into()));
91+
self
92+
}
93+
94+
pub fn fix(mut self, fix: impl Into<String>) -> Self {
95+
self.fix = Some(normalize_sentence_case(fix));
96+
self
97+
}
98+
99+
pub fn build(self) -> String {
100+
let label_width = self
101+
.fields
102+
.iter()
103+
.map(|(key, _)| key.len())
104+
.chain(self.fix.iter().map(|_| "Fix".len()))
105+
.max()
106+
.unwrap_or(0);
107+
108+
let title = self.title.unwrap_or_default();
109+
let mut message = String::new();
110+
write!(&mut message, "[{}] {title}", self.category.as_tag())
111+
.expect("writing to String cannot fail");
112+
113+
for (key, value) in self.fields {
114+
write!(&mut message, "\n {key:label_width$} : {value}")
115+
.expect("writing to String cannot fail");
116+
}
117+
118+
if let Some(fix) = self.fix {
119+
write!(&mut message, "\n {:label_width$} : {fix}", "Fix")
120+
.expect("writing to String cannot fail");
121+
}
122+
123+
message
124+
}
125+
}

‎validation/core/src/results/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@
1414
//! Validation execution results.
1515
1616
mod diagnostics;
17+
mod error_models;
1718

1819
pub use diagnostics::Diagnostics;
20+
pub use error_models::{ErrorBuilder, ErrorCategory};
1921

2022
#[derive(Debug, Default)]
2123
pub struct ValidationResult {

0 commit comments

Comments
 (0)