Skip to content

Commit

Permalink
More strict function parsing
Browse files Browse the repository at this point in the history 
Currently original KSC can parse more wider range of call expressions then it can handle,
actually it can handle only `[expr.]$name($args)` construction, but it can parse, for example:
- ()(args)
- [](args)
- 42(args)
- ""(args)
- true(args)

This is fixed in the original compiler in the kaitai-io/kaitai_struct_compiler#227
but it still not merged
  • Loading branch information
@Mingun
Mingun committed Aug 9, 2024
1 parent 3b25682 commit 6f2cec0
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 12 deletions.
7 changes: 5 additions & 2 deletions src/model/expressions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,12 @@ pub enum OwningNode {
bit: bool,
},

/// Calling function or method: `${expr}(${args})`.
/// Calling function or method: `[${callee}.]${method}(${args})`.
Call {
/// Expression which is called
callee: Box<OwningNode>,
/// Name of method to call
method: FieldName,
/// Arguments of method call
args: Vec<OwningNode>,
},
Expand Down Expand Up @@ -146,8 +148,9 @@ impl OwningNode {

Node::SizeOf { type_, bit } => SizeOf { type_: type_.into(), bit },

Node::Call { callee, args } => Call {
Node::Call { callee, method, args } => Call {
callee: Box::new(Self::validate(*callee)?),
method: FieldName::valid(method),
args: Self::validate_all(args)?,
},
Node::Cast { expr, to_type } => Cast {
Expand Down
175 changes: 165 additions & 10 deletions src/parser/expressions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ pub enum Node<'input> {
bit: bool,
},

/// Calling function or method: `${expr}(${args})`.
/// Calling function or method: `[${callee}.]${method}(${args})`.
Call {
/// Expression which is called
callee: Box<Node<'input>>,
/// Name of method to call
method: &'input str,
/// Arguments of method call
args: Vec<Node<'input>>,
},
Expand Down Expand Up @@ -392,7 +394,7 @@ pub struct ProcessAlgo<'input> {
/// then converted to `Node` with `make_chain` call.
enum Postfix<'input> {
/// Calling function or method with specified arguments.
Args(Vec<Node<'input>>),
Call(&'input str, Vec<Node<'input>>),
/// Conversion to specified type.
CastTo(TypeRef<'input>),
/// Index expression
Expand Down Expand Up @@ -440,10 +442,10 @@ fn to_escaped(ch: &str) -> char {
fn make_chain<'i>(mut expr: Node<'i>, tail: Vec<Postfix<'i>>) -> Node<'i> {
for p in tail {
expr = match p {
Postfix::Args(args) => Node::Call { callee: Box::new(expr), args },
Postfix::CastTo(to_type) => Node::Cast { expr: Box::new(expr), to_type },
Postfix::Index(index) => Node::Index { expr: Box::new(expr), index: Box::new(index) },
Postfix::Field(attr) => Node::Access { expr: Box::new(expr), attr },
Postfix::Call(method, args) => Node::Call { callee: Box::new(expr), method, args },
Postfix::CastTo(to_type) => Node::Cast { expr: Box::new(expr), to_type },
Postfix::Index(index) => Node::Index { expr: Box::new(expr), index: Box::new(index) },
Postfix::Field(attr) => Node::Access { expr: Box::new(expr), attr },
}
}
expr
Expand Down Expand Up @@ -654,10 +656,10 @@ peg::parser! {
/ "_" { Node::SpecialName(SpecialName::Value) }
;
rule postfix() -> Postfix<'input>
= "(" _ a:args() _ ")" { Postfix::Args(a) }// call
/ "[" _ e:expr() _ "]" { Postfix::Index(e) }// indexing
/ "." _ "as" _ "<" _ t:type_ref() _ ">" { Postfix::CastTo(t) }// type cast
/ "." _ n:name() { Postfix::Field(n) }// attribute access
= "[" _ e:expr() _ "]" { Postfix::Index(e) }// indexing
/ "." _ "as" _ "<" _ t:type_ref() _ ">" { Postfix::CastTo(t) }// type cast
/ "." _ n:name() _ "(" _ a:args() _ ")" { Postfix::Call(n, a) }// call
/ "." _ n:name() { Postfix::Field(n) }// attribute access
;

/// List of names, delimited by `::`, with an optional leading `::`.
Expand Down Expand Up @@ -1587,6 +1589,159 @@ mod parse {
}
}

mod call {
// Colorful diffs in assertions - resolve ambiguous
use pretty_assertions::assert_eq;
use super::*;

#[test]
fn without_params() {
assert_eq!(parse_single("123.bar()"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![],
}));
assert_eq!(parse_single("123. bar()"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![],
}));
assert_eq!(parse_single("123.\nbar()"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![],
}));
assert_eq!(parse_single("foo.bar()"), Ok(Call {
callee: Box::new(Attr("foo")),
method: "bar",
args: vec![],
}));
}

#[test]
fn with_params() {
assert_eq!(parse_single("123.bar(42)"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![Int(42.into())],
}));
assert_eq!(parse_single("123. bar(42)"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![Int(42.into())],
}));
assert_eq!(parse_single("123.\nbar(42)"), Ok(Call {
callee: Box::new(Int(123.into())),
method: "bar",
args: vec![Int(42.into())],
}));
assert_eq!(parse_single("foo.bar(42)"), Ok(Call {
callee: Box::new(Attr("foo")),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_strings() {
assert_eq!(parse_single(r#""foo".bar(42)"#), Ok(Call {
callee: Box::new(Str("foo".into())),
method: "bar",
args: vec![Int(42.into())],
}));
assert_eq!(parse_single(r#"'foo'.bar(42)"#), Ok(Call {
callee: Box::new(Str("foo".into())),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_booleans() {
assert_eq!(parse_single("true.bar(42)"), Ok(Call {
callee: Box::new(Bool(true)),
method: "bar",
args: vec![Int(42.into())],
}));
assert_eq!(parse_single("false.bar(42)"), Ok(Call {
callee: Box::new(Bool(false)),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_integer() {
assert_eq!(parse_single("42.bar(42)"), Ok(Call {
callee: Box::new(Int(42.into())),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_float() {
assert_eq!(parse_single("42.0.bar(42)"), Ok(Call {
callee: Box::new(Float(42.into())),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_array() {
assert_eq!(parse_single("[].bar(42)"), Ok(Call {
callee: Box::new(List(vec![])),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_slice() {
assert_eq!(parse_single("foo[1].bar(42)"), Ok(Call {
callee: Box::new(Index {
expr: Box::new(Attr("foo")),
index: Box::new(Int(1.into())),
}),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_group() {
assert_eq!(parse_single("(42).bar(42)"), Ok(Call {
callee: Box::new(Int(42.into())),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn on_expression() {
assert_eq!(parse_single("(1+2).bar(42)"), Ok(Call {
callee: Box::new(Binary {
op: Add,
left: Box::new(Int(1.into())),
right: Box::new(Int(2.into())),
}),
method: "bar",
args: vec![Int(42.into())],
}));
}

#[test]
fn should_fail() {
assert!(parse_single("''(42)").is_err());
assert!(parse_single("()(42)").is_err());
assert!(parse_single("[](42)").is_err());
assert!(parse_single("12(42)").is_err());
assert!(parse_single("12.3(42)").is_err());
assert!(parse_single(r#"""(42)"#).is_err());
}
}

/// Tests for parsing of attributes' type reference definitions
mod type_ref {
use super::*;
Expand Down

0 comments on commit 6f2cec0

Please sign in to comment.