Skip to content

Commit 45f7f38

Browse files
istatharistathar
committed
Better detection of invalid attribute assignments
1 parent d8ac45a commit 45f7f38

File tree

5 files changed

+102
-15
lines changed

5 files changed

+102
-15
lines changed

‎src/editor/server.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,9 @@ impl TechniqueLanguageServer {
487487
ParsingError::InvalidSubstep(_, _) => {
488488
("Invalid substep".to_string(), DiagnosticSeverity::ERROR)
489489
}
490+
ParsingError::InvalidAttribute(_, _) => {
491+
("Invalid attribute assignment".to_string(), DiagnosticSeverity::ERROR)
492+
}
490493
ParsingError::InvalidResponse(_, _) => {
491494
("Invalid response".to_string(), DiagnosticSeverity::ERROR)
492495
}

‎src/formatting/formatter.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,20 @@ pub fn render_procedure_declaration<'i>(procedure: &'i Procedure, renderer: &dyn
165165
render_fragments(&sub.fragments, renderer)
166166
}
167167

168+
pub fn render_scope<'i>(scope: &'i Scope, renderer: &dyn Render) -> String {
169+
let mut sub = Formatter::new(78);
170+
match scope {
171+
Scope::AttributeBlock { attributes, .. } => {
172+
// Render attributes without indentation for error messages
173+
sub.append_attributes(attributes);
174+
}
175+
_ => {
176+
panic!("Do not use for anything other than rendering Attribute line examples");
177+
}
178+
}
179+
render_fragments(&sub.fragments, renderer)
180+
}
181+
168182
/// Helper function to convert fragments to a styled string using a renderer
169183
fn render_fragments<'i>(fragments: &[(Syntax, Cow<'i, str>)], renderer: &dyn Render) -> String {
170184
let mut result = String::new();
@@ -806,6 +820,7 @@ impl<'i> Formatter<'i> {
806820
attributes,
807821
subscopes,
808822
} => {
823+
self.indent();
809824
self.append_attributes(attributes);
810825
self.add_fragment_reference(Syntax::Newline, "\n");
811826

@@ -906,7 +921,6 @@ impl<'i> Formatter<'i> {
906921
}
907922

908923
fn append_attributes(&mut self, attributes: &'i Vec<Attribute>) {
909-
self.indent();
910924
for (i, attribute) in attributes
911925
.iter()
912926
.enumerate()

‎src/parsing/parser.rs‎

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub enum ParsingError {
4848
InvalidCodeBlock(usize, usize),
4949
InvalidStep(usize, usize),
5050
InvalidSubstep(usize, usize),
51+
InvalidAttribute(usize, usize),
5152
InvalidResponse(usize, usize),
5253
InvalidMultiline(usize, usize),
5354
InvalidForeach(usize, usize),
@@ -87,6 +88,7 @@ impl ParsingError {
8788
ParsingError::InvalidMultiline(offset, _) => *offset,
8889
ParsingError::InvalidStep(offset, _) => *offset,
8990
ParsingError::InvalidSubstep(offset, _) => *offset,
91+
ParsingError::InvalidAttribute(offset, _) => *offset,
9092
ParsingError::InvalidForeach(offset, _) => *offset,
9193
ParsingError::InvalidResponse(offset, _) => *offset,
9294
ParsingError::InvalidIntegral(offset, _) => *offset,
@@ -123,6 +125,7 @@ impl ParsingError {
123125
ParsingError::InvalidMultiline(_, width) => *width,
124126
ParsingError::InvalidStep(_, width) => *width,
125127
ParsingError::InvalidSubstep(_, width) => *width,
128+
ParsingError::InvalidAttribute(_, width) => *width,
126129
ParsingError::InvalidForeach(_, width) => *width,
127130
ParsingError::InvalidResponse(_, width) => *width,
128131
ParsingError::InvalidIntegral(_, width) => *width,
@@ -1072,14 +1075,20 @@ impl<'i> Parser<'i> {
10721075
parser.skip_to_next_line();
10731076
}
10741077
}
1075-
} else if is_attribute_assignment(content) {
1076-
match parser.read_attribute_scope() {
1077-
Ok(attribute_block) => elements.push(Element::Steps(vec![attribute_block])),
1078-
Err(error) => {
1079-
self.problems
1080-
.push(error);
1081-
parser.skip_to_next_line();
1078+
} else if is_attribute_pattern(content) {
1079+
if is_attribute_assignment(content) {
1080+
match parser.read_attribute_scope() {
1081+
Ok(attribute_block) => elements.push(Element::Steps(vec![attribute_block])),
1082+
Err(error) => {
1083+
self.problems
1084+
.push(error);
1085+
parser.skip_to_next_line();
1086+
}
10821087
}
1088+
} else {
1089+
self.problems
1090+
.push(ParsingError::InvalidAttribute(parser.offset, content.len()));
1091+
parser.skip_to_next_line();
10831092
}
10841093
} else if is_step(content) {
10851094
let mut steps = vec![];
@@ -1124,14 +1133,14 @@ impl<'i> Parser<'i> {
11241133
&& !is_procedure_title(line)
11251134
&& !is_code_block(line)
11261135
&& !malformed_step_pattern(line)
1127-
&& !is_attribute_assignment(line)
1136+
&& !is_attribute_pattern(line)
11281137
},
11291138
|line| {
11301139
is_step(line)
11311140
|| is_procedure_title(line)
11321141
|| is_code_block(line)
11331142
|| malformed_step_pattern(line)
1134-
|| is_attribute_assignment(line)
1143+
|| is_attribute_pattern(line)
11351144
},
11361145
|inner| {
11371146
let content = inner.source;
@@ -2161,7 +2170,7 @@ impl<'i> Parser<'i> {
21612170
|| is_substep_dependent(line)
21622171
|| is_substep_parallel(line)
21632172
|| is_subsubstep_dependent(line)
2164-
|| is_attribute_assignment(line)
2173+
|| is_attribute_pattern(line)
21652174
|| is_enum_response(line)
21662175
|| malformed_step_pattern(line)
21672176
|| malformed_response_pattern(line)
@@ -2494,7 +2503,13 @@ impl<'i> Parser<'i> {
24942503
))?;
24952504
attributes.push(Attribute::Place(identifier));
24962505
} else {
2497-
return Err(ParsingError::InvalidStep(inner.offset, 0));
2506+
// Check if this looks like a malformed attribute (starts with @ or ^)
2507+
if is_attribute_pattern(trimmed) {
2508+
// This might be multiple attributes without proper + joiners
2509+
return Err(ParsingError::InvalidAttribute(inner.offset, line.len()));
2510+
} else {
2511+
return Err(ParsingError::InvalidStep(inner.offset, 0));
2512+
}
24982513
}
24992514
}
25002515

@@ -2515,9 +2530,13 @@ impl<'i> Parser<'i> {
25152530

25162531
let content = self.source;
25172532

2518-
if is_attribute_assignment(content) {
2519-
let block = self.read_attribute_scope()?;
2520-
scopes.push(block);
2533+
if is_attribute_pattern(content) {
2534+
if is_attribute_assignment(content) {
2535+
let block = self.read_attribute_scope()?;
2536+
scopes.push(block);
2537+
} else {
2538+
return Err(ParsingError::InvalidAttribute(self.offset, content.len()));
2539+
}
25212540
} else if is_substep_dependent(content) {
25222541
let block = self.read_substep_dependent()?;
25232542
scopes.push(block);
@@ -3037,6 +3056,12 @@ fn is_attribute_assignment(input: &str) -> bool {
30373056
re.is_match(input)
30383057
}
30393058

3059+
/// Detect the beginning of an attribute (role or place) assignment
3060+
fn is_attribute_pattern(input: &str) -> bool {
3061+
let trimmed = input.trim_ascii_start();
3062+
trimmed.starts_with('@') || trimmed.starts_with('^')
3063+
}
3064+
30403065
// This is a rather monsterous test battery, so we move it into a separate
30413066
// file. We use the path directive to avoid the need to put it into a
30423067
// subdirectory with the same name as this module.

‎src/problem/messages.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,45 @@ parallel steps, but again this is not compulsory.
630630
.trim_ascii()
631631
.to_string(),
632632
),
633+
ParsingError::InvalidAttribute(_, _) => {
634+
let examples = vec![
635+
Scope::AttributeBlock {
636+
attributes: vec![
637+
Attribute::Role(Identifier("president_of_the_galaxy")),
638+
Attribute::Role(Identifier("femme_fatale")),
639+
],
640+
subscopes: vec![],
641+
},
642+
Scope::AttributeBlock {
643+
attributes: vec![
644+
Attribute::Place(Identifier("milliways")),
645+
Attribute::Role(Identifier("waiter")),
646+
Attribute::Role(Identifier("dish_of_the_day")),
647+
],
648+
subscopes: vec![],
649+
},
650+
];
651+
652+
(
653+
"Invalid attribute assignment".to_string(),
654+
format!(
655+
r#"
656+
Multiple attributes (be they role or place assignments) must be joined using
657+
the '+' operator, for example:
658+
659+
{}
660+
{}
661+
662+
Note that an attribute creates a scope, so sub-steps and code blocks can be
663+
nested underneath a role or place assignment.
664+
"#,
665+
examples[0].present(renderer),
666+
examples[1].present(renderer)
667+
)
668+
.trim_ascii()
669+
.to_string(),
670+
)
671+
}
633672
ParsingError::InvalidForeach(_, _) => {
634673
let examples = vec![
635674
Expression::Foreach(

‎src/problem/present.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,9 @@ impl Present for Procedure<'_> {
9393
formatter::render_procedure_declaration(self, renderer)
9494
}
9595
}
96+
97+
impl Present for Scope<'_> {
98+
fn present(&self, renderer: &dyn Render) -> String {
99+
formatter::render_scope(self, renderer)
100+
}
101+
}

0 commit comments

Comments
 (0)