Skip to content

Commit 79775cb

Browse files
committed
Detect errors in function and invocation names
1 parent 6974fa2 commit 79775cb

File tree

3 files changed

+111
-7
lines changed

3 files changed

+111
-7
lines changed

src/parsing/checks/errors.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,32 @@ making_coffee :
239239
);
240240
}
241241

242+
#[test]
243+
fn invalid_function_with_space_in_name() {
244+
expect_error(
245+
r#"
246+
making_coffee :
247+
248+
1. Do something { re peat() }
249+
"#
250+
.trim_ascii(),
251+
ParsingError::InvalidFunction(38, 0),
252+
);
253+
}
254+
255+
#[test]
256+
fn invalid_function_with_space_and_invocation() {
257+
expect_error(
258+
r#"
259+
making_coffee :
260+
261+
1. Do something { re peat <thing>() }
262+
"#
263+
.trim_ascii(),
264+
ParsingError::InvalidFunction(38, 0),
265+
);
266+
}
267+
242268
#[test]
243269
fn invalid_invocation_in_repeat() {
244270
expect_error(
@@ -265,3 +291,16 @@ making_coffee :
265291
ParsingError::InvalidSubstep(37, 0),
266292
);
267293
}
294+
295+
#[test]
296+
fn invalid_code_block_with_leftover_content() {
297+
expect_error(
298+
r#"
299+
robot :
300+
301+
Your plastic pal who's fun to be with! { re peat <jingle> }
302+
"#
303+
.trim_ascii(),
304+
ParsingError::InvalidCodeBlock(43, 7),
305+
);
306+
}

src/parsing/checks/parser.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,8 +1587,8 @@ fn test_foreach_keyword_boundary() {
15871587
input.initialize("{ foreachitem in items }");
15881588

15891589
let result = input.read_code_block();
1590-
// Should parse as identifier, not foreach
1591-
assert_eq!(result, Ok(Expression::Variable(Identifier("foreachitem"))));
1590+
// Should fail because "foreachitem" is parsed but "in items" is leftover content
1591+
assert_eq!(result, Err(ParsingError::InvalidCodeBlock(2, 11)));
15921592
}
15931593

15941594
#[test]

src/parsing/parser.rs

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,8 +1353,56 @@ impl<'i> Parser<'i> {
13531353
}
13541354

13551355
fn read_code_block(&mut self) -> Result<Expression<'i>, ParsingError> {
1356-
self.take_block_chars("a code block", '{', '}', true, |outer| {
1357-
outer.read_expression()
1356+
self.take_block_chars("a code block", '{', '}', true, |inner| {
1357+
// Save the start position (accounting for leading whitespace that read_expression will trim)
1358+
inner.trim_whitespace();
1359+
let start = inner.offset;
1360+
1361+
let expression = inner.read_expression()?;
1362+
1363+
// Check if there's leftover content
1364+
let offset_before_trim = inner.offset;
1365+
inner.trim_whitespace();
1366+
if !inner
1367+
.source
1368+
.is_empty()
1369+
{
1370+
let mut width = offset_before_trim - start; // Width of what we parsed
1371+
1372+
// Check if leftover looks like continuation of identifier
1373+
let leftover = inner
1374+
.source
1375+
.chars()
1376+
.next()
1377+
.map(|ch| {
1378+
ch.is_ascii_lowercase()
1379+
&& !inner
1380+
.source
1381+
.starts_with("in ")
1382+
})
1383+
.unwrap_or(false);
1384+
1385+
if leftover {
1386+
// Include the space(s) between parts
1387+
width = inner.offset - start;
1388+
1389+
// Add identifier-like characters from leftover
1390+
for ch in inner
1391+
.source
1392+
.chars()
1393+
{
1394+
if ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_' {
1395+
width += ch.len_utf8();
1396+
} else {
1397+
break;
1398+
}
1399+
}
1400+
}
1401+
1402+
return Err(ParsingError::InvalidCodeBlock(start, width));
1403+
}
1404+
1405+
Ok(expression)
13581406
})
13591407
}
13601408

@@ -1396,7 +1444,19 @@ impl<'i> Parser<'i> {
13961444
let invocation = self.read_invocation()?;
13971445
Ok(Expression::Application(invocation))
13981446
} else if is_function(content) {
1399-
let target = self.read_identifier()?;
1447+
// Extract the entire text before the opening parenthesis
1448+
self.trim_whitespace();
1449+
let content = self.source;
1450+
let paren = content
1451+
.find('(')
1452+
.unwrap(); // is_function() already checked
1453+
let text = &content[0..paren];
1454+
1455+
// Validate that the entire text is a valid identifier
1456+
let target = validate_identifier(text)
1457+
.ok_or(ParsingError::InvalidFunction(self.offset, text.len()))?;
1458+
1459+
self.advance(text.len());
14001460
let parameters = self.read_parameters()?;
14011461

14021462
let function = Function { target, parameters };
@@ -1892,12 +1952,17 @@ impl<'i> Parser<'i> {
18921952

18931953
/// Parse a target like <procedure_name> or <https://example.com/proc>
18941954
fn read_target(&mut self) -> Result<Target<'i>, ParsingError> {
1955+
let start_offset = self.offset;
18951956
self.take_block_chars("an invocation", '<', '>', true, |inner| {
1896-
let content = inner.source;
1957+
let content = inner
1958+
.source
1959+
.trim();
18971960
if content.starts_with("https://") {
18981961
Ok(Target::Remote(External(content)))
18991962
} else {
1900-
let identifier = inner.read_identifier()?;
1963+
let identifier = validate_identifier(content).ok_or_else(|| {
1964+
ParsingError::InvalidInvocation(start_offset + 1, content.len())
1965+
})?;
19011966
Ok(Target::Local(identifier))
19021967
}
19031968
})

0 commit comments

Comments
 (0)