Skip to content

Commit f6bd78b

Browse files
authored
Improve test coverage (#78)
The _parser.rs_ module was becoming enormous. Move the test code that was in a module in that file to a new location _checks/_ but jump through some minor hoops so that name didn't need to match the parent. Also adds a fix to resolve mishandling of blank lines, and adds a new error case if parenthesis are omitted when binding to more than one variable. Handle the `$*` special case that resets the attribute assignment. Prevent the formatter from wrapping code inlines and within string interpolations. This is corner case territory, but it looks rubbish to have a single '}' sitting by itself on a line just because it got wrapped. So, when formatting, we instead keep the binding and surrounding code inline together as an atomic unit.
2 parents e88e78f + ec9b3a3 commit f6bd78b

File tree

20 files changed

+4225
-4112
lines changed

20 files changed

+4225
-4112
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "technique"
3-
version = "0.4.1"
3+
version = "0.4.2"
44
edition = "2021"
55
description = "A domain specific language for procedures."
66
authors = [ "Andrew Cowie" ]

src/formatting/formatter.rs

Lines changed: 71 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ pub fn render_invocation<'i>(invocation: &'i Invocation, renderer: &dyn Render)
127127
render_fragments(&sub.fragments, renderer)
128128
}
129129

130+
pub fn render_descriptive<'i>(descriptive: &'i Descriptive, renderer: &dyn Render) -> String {
131+
let mut sub = Formatter::new(78);
132+
sub.append_descriptive(descriptive);
133+
render_fragments(&sub.fragments, renderer)
134+
}
135+
130136
pub fn render_function<'i>(function: &'i Function, renderer: &dyn Render) -> String {
131137
let mut sub = Formatter::new(78);
132138
sub.append_function(function);
@@ -322,16 +328,41 @@ impl<'i> Formatter<'i> {
322328
sub.add_fragment_reference(Syntax::Neutral, " ");
323329
sub.add_fragment_reference(Syntax::Structure, "}");
324330
sub.flush_current();
325-
sub.fragments
331+
332+
let mut combined = String::new();
333+
for (_syntax, content) in &sub.fragments {
334+
combined.push_str(&content);
335+
}
336+
337+
vec![(Syntax::Structure, Cow::Owned(combined))]
326338
}
327339
}
328340
}
329341

342+
fn render_string_interpolation(&self, expr: &'i Expression) -> Vec<(Syntax, Cow<'i, str>)> {
343+
let mut sub = self.subformatter();
344+
sub.add_fragment_reference(Syntax::Structure, "{");
345+
sub.add_fragment_reference(Syntax::Neutral, " ");
346+
sub.append_expression(expr);
347+
sub.add_fragment_reference(Syntax::Neutral, " ");
348+
sub.add_fragment_reference(Syntax::Structure, "}");
349+
sub.flush_current();
350+
sub.fragments
351+
}
352+
330353
fn render_application(&self, invocation: &'i Invocation) -> Vec<(Syntax, Cow<'i, str>)> {
331354
let mut sub = self.subformatter();
332355
sub.append_application(invocation);
333356
sub.flush_current();
334-
sub.fragments
357+
358+
// Combine all fragments into a single atomic fragment to prevent wrapping
359+
let mut combined = String::new();
360+
for (_syntax, content) in &sub.fragments {
361+
combined.push_str(&content);
362+
}
363+
364+
// Return as a single fragment
365+
vec![(Syntax::Invocation, Cow::Owned(combined))]
335366
}
336367

337368
fn render_binding(
@@ -363,7 +394,14 @@ impl<'i> Formatter<'i> {
363394
sub.add_fragment_reference(Syntax::Neutral, " ");
364395
sub.append_variables(variables);
365396
sub.flush_current();
366-
sub.fragments
397+
398+
// Combine all fragments into a single atomic fragment to prevent wrapping
399+
let mut combined = String::new();
400+
for (_syntax, content) in &sub.fragments {
401+
combined.push_str(&content);
402+
}
403+
404+
vec![(Syntax::Structure, Cow::Owned(combined))]
367405
}
368406

369407
pub fn append_char(&mut self, c: char) {
@@ -584,7 +622,16 @@ impl<'i> Formatter<'i> {
584622
}
585623
}
586624

587-
fn append_descriptives(&mut self, descriptives: &'i Vec<Descriptive>) {
625+
// This is a helper for rendering a single descriptives in error messages.
626+
// The real method is append_decriptives() below; this method simply
627+
// creates a single element slice that can be passed to it.
628+
fn append_descriptive(&mut self, descriptive: &'i Descriptive) {
629+
use std::slice;
630+
let slice = slice::from_ref(descriptive);
631+
self.append_descriptives(slice);
632+
}
633+
634+
fn append_descriptives(&mut self, descriptives: &'i [Descriptive<'i>]) {
588635
let syntax = self.current;
589636
let mut line = self.builder();
590637

@@ -894,7 +941,7 @@ impl<'i> Formatter<'i> {
894941
self.add_fragment_reference(Syntax::String, text);
895942
}
896943
Piece::Interpolation(expr) => {
897-
let fragments = self.render_inline_code(expr);
944+
let fragments = self.render_string_interpolation(expr);
898945
for (syntax, content) in fragments {
899946
self.add_fragment(syntax, content);
900947
}
@@ -1223,30 +1270,41 @@ impl<'a, 'i> Line<'a, 'i> {
12231270
let fragments = self
12241271
.output
12251272
.render_inline_code(expr);
1226-
self.add_fragments(fragments);
1273+
for (syntax, content) in fragments {
1274+
self.add_no_wrap(syntax, content);
1275+
}
12271276
}
12281277

12291278
fn add_application(&mut self, invocation: &'i Invocation) {
12301279
let fragments = self
12311280
.output
12321281
.render_application(invocation);
1233-
self.add_fragments(fragments);
1282+
for (syntax, content) in fragments {
1283+
self.add_no_wrap(syntax, content);
1284+
}
12341285
}
12351286

12361287
fn add_binding(&mut self, inner_descriptive: &'i Descriptive, variables: &'i Vec<Identifier>) {
12371288
let fragments = self
12381289
.output
12391290
.render_binding(inner_descriptive, variables);
1240-
self.add_fragments(fragments);
1241-
}
1242-
1243-
fn add_fragments(&mut self, fragments: Vec<(Syntax, Cow<'i, str>)>) {
1244-
// All fragments should be atomic - the formatter is responsible for breaking up content
1291+
// Bindings should not wrap - add as a single non-wrapping unit
12451292
for (syntax, content) in fragments {
1246-
self.add_atomic_cow(syntax, content);
1293+
self.add_no_wrap(syntax, content);
12471294
}
12481295
}
12491296

1297+
fn add_no_wrap(&mut self, syntax: Syntax, content: Cow<'i, str>) {
1298+
// Add content that must never wrap mid-construct (inline code,
1299+
// applications, bindings) Unlike add_atomic_cow(), this bypasses
1300+
// width checking entirely to preserve the integrity of these language
1301+
// constructs on single lines
1302+
let len = content.len() as u8;
1303+
self.current
1304+
.push((syntax, content));
1305+
self.position += len;
1306+
}
1307+
12501308
fn wrap_line(&mut self) {
12511309
// Emit all current fragments to the output
12521310
for (syntax, content) in self

src/language/types.rs

Lines changed: 7 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Types representing an Abstract Syntax Tree for the Technique language
22
3-
use crate::{language::quantity::parse_quantity, regex::*};
3+
use crate::regex::*;
44

55
#[derive(Eq, Debug, PartialEq)]
66
pub struct Document<'i> {
@@ -214,7 +214,7 @@ pub use crate::language::quantity::Quantity;
214214
// the validate functions all need to have start and end anchors, which seems
215215
// like it should be abstracted away.
216216

217-
pub fn validate_license(input: &str) -> Option<&str> {
217+
pub(crate) fn validate_license(input: &str) -> Option<&str> {
218218
let re = regex!(r"^[A-Za-z0-9.,\-_ \(\)\[\]]+$");
219219

220220
if re.is_match(input) {
@@ -224,7 +224,7 @@ pub fn validate_license(input: &str) -> Option<&str> {
224224
}
225225
}
226226

227-
pub fn validate_copyright(input: &str) -> Option<&str> {
227+
pub(crate) fn validate_copyright(input: &str) -> Option<&str> {
228228
let re = regex!(r"^[A-Za-z0-9.,\-_ \(\)\[\]]+$");
229229

230230
if re.is_match(input) {
@@ -234,7 +234,7 @@ pub fn validate_copyright(input: &str) -> Option<&str> {
234234
}
235235
}
236236

237-
pub fn validate_template(input: &str) -> Option<&str> {
237+
pub(crate) fn validate_template(input: &str) -> Option<&str> {
238238
let re = regex!(r"^[A-Za-z0-9.,\-]+$");
239239

240240
if re.is_match(input) {
@@ -244,7 +244,7 @@ pub fn validate_template(input: &str) -> Option<&str> {
244244
}
245245
}
246246

247-
pub fn validate_identifier(input: &str) -> Option<Identifier<'_>> {
247+
pub(crate) fn validate_identifier(input: &str) -> Option<Identifier<'_>> {
248248
if input.len() == 0 {
249249
return None;
250250
}
@@ -257,7 +257,7 @@ pub fn validate_identifier(input: &str) -> Option<Identifier<'_>> {
257257
}
258258
}
259259

260-
pub fn validate_forma(input: &str) -> Option<Forma<'_>> {
260+
pub(crate) fn validate_forma(input: &str) -> Option<Forma<'_>> {
261261
if input.len() == 0 {
262262
return None;
263263
}
@@ -294,7 +294,7 @@ fn parse_tuple(input: &str) -> Option<Vec<Forma<'_>>> {
294294
}
295295

296296
/// This one copes with (and discards) any internal whitespace encountered.
297-
pub fn validate_genus(input: &str) -> Option<Genus<'_>> {
297+
pub(crate) fn validate_genus(input: &str) -> Option<Genus<'_>> {
298298
let first = input
299299
.chars()
300300
.next()
@@ -371,33 +371,6 @@ pub fn validate_response(input: &str) -> Option<Response<'_>> {
371371
Some(Response { value, condition })
372372
}
373373

374-
fn _validate_decimal(_input: &str) -> Option<Numeric<'_>> {
375-
// Test the regex macro availability within types.rs
376-
let _decimal_regex = regex!(r"^\s*-?[0-9]+\.[0-9]+\s*$");
377-
// For now, just return None since we removed Decimal variant
378-
None
379-
}
380-
381-
pub fn validate_numeric(input: &str) -> Option<Numeric<'_>> {
382-
if input.is_empty() {
383-
return None;
384-
}
385-
386-
let input = input.trim_ascii();
387-
388-
// Try to parse as a simple Integral first
389-
if let Ok(amount) = input.parse::<i64>() {
390-
return Some(Numeric::Integral(amount));
391-
}
392-
393-
// Try to parse as a Quantity (scientific notation with units)
394-
if let Some(quantity) = parse_quantity(input) {
395-
return Some(Numeric::Scientific(quantity));
396-
}
397-
398-
None
399-
}
400-
401374
#[cfg(test)]
402375
mod check {
403376
use super::*;
@@ -582,18 +555,6 @@ mod check {
582555
t1
583556
}
584557

585-
#[test]
586-
fn numeric_rules() {
587-
// Test simple integers
588-
assert_eq!(validate_numeric("42"), Some(Numeric::Integral(42)));
589-
assert_eq!(validate_numeric("0"), Some(Numeric::Integral(0)));
590-
assert_eq!(validate_numeric("-123"), Some(Numeric::Integral(-123)));
591-
assert_eq!(
592-
validate_numeric("9223372036854775807"),
593-
Some(Numeric::Integral(9223372036854775807))
594-
);
595-
}
596-
597558
#[test]
598559
fn ast_construction() {
599560
let t1 = Metadata {

0 commit comments

Comments
 (0)