Skip to content

Commit c269e7d

Browse files
committed
refactor: split validator into BasicHclValidator and FullHclValidator
Resolves: - solana-foundation#333 (comment) Make validation capabilities explicit at compile time by using separate types instead of constructor variants. This prevents confusion about what can be validated and follows Rust's principle of making invalid states unrepresentable.
1 parent ed8b294 commit c269e7d

2 files changed

Lines changed: 73 additions & 28 deletions

File tree

crates/txtx-core/src/validation/hcl_validator.rs

Lines changed: 62 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,56 @@ fn get_action_doc_link(namespace: &str, action: &str) -> Option<String> {
2828
}
2929
}
3030

31+
/// A basic HCL validator that performs structural validation without addon specifications.
32+
/// This validator can check HCL syntax, references between blocks, and structural correctness,
33+
/// but cannot validate action parameters since it lacks addon specifications.
34+
pub struct BasicHclValidator<'a> {
35+
inner: HclValidationVisitor<'a>,
36+
}
37+
38+
/// A full HCL validator with addon specifications for comprehensive validation.
39+
/// This validator can perform all validations including action parameter checking.
40+
pub struct FullHclValidator<'a> {
41+
inner: HclValidationVisitor<'a>,
42+
}
43+
44+
impl<'a> BasicHclValidator<'a> {
45+
/// Create a basic validator for structural validation only.
46+
/// This validator cannot check action parameters since it lacks addon specifications.
47+
pub fn new(result: &'a mut ValidationResult, file_path: &str, source: &'a str) -> Self {
48+
Self {
49+
inner: HclValidationVisitor::new_with_addons(result, file_path, source, HashMap::new()),
50+
}
51+
}
52+
53+
/// Run validation on the HCL body
54+
pub fn validate(&mut self, body: &Body) -> Vec<LocatedInputRef> {
55+
self.inner.validate(body);
56+
std::mem::take(&mut self.inner.input_refs)
57+
}
58+
}
59+
60+
impl<'a> FullHclValidator<'a> {
61+
/// Create a full validator with addon specifications for comprehensive validation.
62+
/// This validator can check all aspects including action parameters.
63+
pub fn new(
64+
result: &'a mut ValidationResult,
65+
file_path: &str,
66+
source: &'a str,
67+
addon_specs: HashMap<String, Vec<(String, CommandSpecification)>>,
68+
) -> Self {
69+
Self {
70+
inner: HclValidationVisitor::new_with_addons(result, file_path, source, addon_specs),
71+
}
72+
}
73+
74+
/// Run validation on the HCL body
75+
pub fn validate(&mut self, body: &Body) -> Vec<LocatedInputRef> {
76+
self.inner.validate(body);
77+
std::mem::take(&mut self.inner.input_refs)
78+
}
79+
}
80+
3181
/// A visitor that validates HCL runbooks
3282
pub struct HclValidationVisitor<'a> {
3383
/// Results collector
@@ -82,9 +132,8 @@ struct BlockContext {
82132
}
83133

84134
impl<'a> HclValidationVisitor<'a> {
85-
/// Create a validator with addon specifications for full validation
86-
/// This enables complete validation including action parameter checking
87-
pub fn new_with_addons(
135+
/// Internal constructor with addon specifications
136+
fn new_with_addons(
88137
result: &'a mut ValidationResult,
89138
file_path: &str,
90139
source: &'a str,
@@ -111,12 +160,6 @@ impl<'a> HclValidationVisitor<'a> {
111160
}
112161
}
113162

114-
/// Create a validator without addon specifications (limited validation)
115-
/// WARNING: This provides limited validation - cannot validate action parameters.
116-
/// Use only when addon specs are unavailable (e.g., in txtx-core without CLI context).
117-
pub fn new(result: &'a mut ValidationResult, file_path: &str, source: &'a str) -> Self {
118-
Self::new_with_addons(result, file_path, source, HashMap::new())
119-
}
120163

121164
/// Convert a span to line/column position
122165
fn span_to_position(&self, span: &std::ops::Range<usize>) -> (usize, usize) {
@@ -889,12 +932,11 @@ pub fn validate_with_hcl_and_addons(
889932
// Parse the content as HCL
890933
let body: Body = content.parse().map_err(|e| format!("Failed to parse runbook: {}", e))?;
891934

892-
// Create and run the validator with custom addon specs
893-
let mut visitor =
894-
HclValidationVisitor::new_with_addons(result, file_path, content, addon_specs);
895-
visitor.validate(&body);
935+
// Create and run the full validator with addon specs
936+
let mut validator = FullHclValidator::new(result, file_path, content, addon_specs);
937+
let input_refs = validator.validate(&body);
896938

897-
Ok(visitor.input_refs)
939+
Ok(input_refs)
898940
}
899941

900942
#[cfg(test)]
@@ -1179,7 +1221,7 @@ mod tests {
11791221
*/
11801222
}
11811223

1182-
/// Run HCL-based validation on a runbook (uses default addon specifications)
1224+
/// Run HCL-based validation on a runbook (limited validation without addon specs)
11831225
pub fn validate_with_hcl(
11841226
content: &str,
11851227
result: &mut ValidationResult,
@@ -1188,10 +1230,10 @@ pub fn validate_with_hcl(
11881230
// Parse the content as HCL
11891231
let body: Body = content.parse().map_err(|e| format!("Failed to parse runbook: {}", e))?;
11901232

1191-
// Create and run the validator without addon specs (limited validation)
1192-
// Note: This path cannot validate action parameters since addon specs are not available
1193-
let mut visitor = HclValidationVisitor::new(result, file_path, content);
1194-
visitor.validate(&body);
1233+
// Create and run the basic validator (structural validation only)
1234+
// Note: This validator cannot validate action parameters since addon specs are not available
1235+
let mut validator = BasicHclValidator::new(result, file_path, content);
1236+
let input_refs = validator.validate(&body);
11951237

1196-
Ok(visitor.input_refs)
1238+
Ok(input_refs)
11971239
}

crates/txtx-core/src/validation/validator.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! High-level validation API for runbook files
22
3-
use super::hcl_validator::HclValidationVisitor;
3+
use super::hcl_validator::{BasicHclValidator, FullHclValidator};
44
use super::types::ValidationResult;
55
use crate::kit::hcl::structure::Body;
66
use crate::kit::types::commands::{CommandSpecification, PreCommandSpecification};
@@ -35,16 +35,19 @@ pub fn validate_runbook(
3535
file_path: &str,
3636
source: &str,
3737
body: &Body,
38-
_config: ValidatorConfig,
38+
config: ValidatorConfig,
3939
) -> ValidationResult {
4040
let mut result = ValidationResult::new();
4141

42-
let mut visitor = HclValidationVisitor::new(&mut result, file_path, source);
43-
44-
// TODO: Need to update HclValidationVisitor to accept addon_specs as parameter
45-
// For now, it uses get_addon_specifications() internally
46-
47-
visitor.validate(body);
42+
if config.addon_specs.is_empty() {
43+
// Use basic validator when no addon specs are available
44+
let mut validator = BasicHclValidator::new(&mut result, file_path, source);
45+
validator.validate(body);
46+
} else {
47+
// Use full validator when addon specs are provided
48+
let mut validator = FullHclValidator::new(&mut result, file_path, source, config.addon_specs);
49+
validator.validate(body);
50+
}
4851

4952
result
5053
}

0 commit comments

Comments
 (0)