From 2b8dda14e34342552bb12ef1144c0b4cda5fe1e9 Mon Sep 17 00:00:00 2001 From: Jeff Date: Tue, 13 Aug 2024 13:12:13 -0400 Subject: [PATCH] Add anaylsis to check for valid fields and indexes --- dust-lang/src/abstract_tree.rs | 3 +- dust-lang/src/analyzer.rs | 96 +++++++++++++++++++++++++++--- dust-lang/src/built_in_function.rs | 2 +- dust-lang/src/type.rs | 66 +++++++++++++++++++- dust-lang/src/value.rs | 1 + 5 files changed, 157 insertions(+), 11 deletions(-) diff --git a/dust-lang/src/abstract_tree.rs b/dust-lang/src/abstract_tree.rs index 5748362..c7f212b 100644 --- a/dust-lang/src/abstract_tree.rs +++ b/dust-lang/src/abstract_tree.rs @@ -147,7 +147,7 @@ impl Statement { BinaryOperator::ListIndex => { let left_type = left.inner.expected_type(context)?; - if let Type::List { item_type } = left_type { + if let Type::List { item_type, .. } = left_type { Some(*item_type) } else { None @@ -167,6 +167,7 @@ impl Statement { Some(Type::List { item_type: Box::new(item_type), + length: nodes.len(), }) } Statement::Map(nodes) => { diff --git a/dust-lang/src/analyzer.rs b/dust-lang/src/analyzer.rs index 1b1b607..0d79a58 100644 --- a/dust-lang/src/analyzer.rs +++ b/dust-lang/src/analyzer.rs @@ -11,7 +11,7 @@ use std::{ use crate::{ abstract_tree::{BinaryOperator, UnaryOperator}, - parse, AbstractSyntaxTree, Context, DustError, Node, Span, Statement, Type, + parse, AbstractSyntaxTree, Context, DustError, Identifier, Node, Span, Statement, Type, }; /// Analyzes the abstract syntax tree for errors. @@ -105,8 +105,11 @@ impl<'a> Analyzer<'a> { self.analyze_statement(right)?; } - if let Some(Type::Map { .. }) = left.inner.expected_type(self.context) { - if let Some(Type::String) = right.inner.expected_type(self.context) { + let left_type = left.inner.expected_type(self.context); + let right_type = right.inner.expected_type(self.context); + + if let Some(Type::Map { .. }) = left_type { + if let Some(Type::String) = right_type { // Allow indexing maps with strings } else if let Statement::Identifier(_) = right.inner { // Allow indexing maps with identifiers @@ -121,6 +124,31 @@ impl<'a> Analyzer<'a> { }); } + // If the accessor is an identifier, check if it is a valid field + if let Statement::Identifier(identifier) = &right.inner { + if let Some(Type::Map(fields)) = &left_type { + if !fields.contains_key(identifier) { + return Err(AnalyzerError::UndefinedField { + identifier: right.as_ref().clone(), + map: left.as_ref().clone(), + }); + } + } + } + // If the accessor is a constant, check if it is a valid field + if let Statement::Constant(value) = &right.inner { + if let Some(field_name) = value.as_string() { + if let Some(Type::Map(fields)) = left_type { + if !fields.contains_key(&Identifier::new(field_name)) { + return Err(AnalyzerError::UndefinedField { + identifier: right.as_ref().clone(), + map: left.as_ref().clone(), + }); + } + } + } + } + return Ok(()); } @@ -128,7 +156,8 @@ impl<'a> Analyzer<'a> { self.analyze_statement(left)?; self.analyze_statement(right)?; - if let Some(Type::List { .. }) = left.inner.expected_type(self.context) { + if let Some(Type::List { length, .. }) = left.inner.expected_type(self.context) + { let index_type = right.inner.expected_type(self.context); if let Some(Type::Integer | Type::Range) = index_type { @@ -138,6 +167,22 @@ impl<'a> Analyzer<'a> { actual: right.as_ref().clone(), }); } + + // If the index is a constant, check if it is out of bounds + if let Statement::Constant(value) = &right.inner { + if let Some(index_value) = value.as_integer() { + let index_value = index_value as usize; + + if index_value >= length { + return Err(AnalyzerError::IndexOutOfBounds { + list: left.as_ref().clone(), + index: right.as_ref().clone(), + index_value, + length, + }); + } + } + } } else { return Err(AnalyzerError::ExpectedList { actual: left.as_ref().clone(), @@ -459,6 +504,10 @@ pub enum AnalyzerError { UndefinedVariable { identifier: Node, }, + UndefinedField { + identifier: Node, + map: Node, + }, UnexpectedIdentifier { identifier: Node, }, @@ -484,6 +533,7 @@ impl AnalyzerError { AnalyzerError::TypeConflict { actual_statement, .. } => actual_statement.position, + AnalyzerError::UndefinedField { identifier, .. } => identifier.position, AnalyzerError::UndefinedVariable { identifier } => identifier.position, AnalyzerError::UnexpectedIdentifier { identifier } => identifier.position, AnalyzerError::UnexectedString { actual } => actual.position, @@ -518,9 +568,9 @@ impl Display for AnalyzerError { } => write!(f, "Expected {} value arguments, found {}", expected, actual), AnalyzerError::IndexOutOfBounds { list, - index, index_value, length, + .. } => write!( f, "Index {} out of bounds for list {} with length {}", @@ -537,6 +587,9 @@ impl Display for AnalyzerError { expected, actual_statement, actual_type ) } + AnalyzerError::UndefinedField { identifier, map } => { + write!(f, "Undefined field {} in map {}", identifier, map) + } AnalyzerError::UndefinedVariable { identifier } => { write!(f, "Undefined variable {}", identifier) } @@ -582,14 +635,43 @@ mod tests { } #[test] - fn nonexistant_field() { + fn nonexistant_field_identifier() { let source = "{ x = 1 }.y"; assert_eq!( analyze(source), Err(DustError::AnalyzerError { - analyzer_error: AnalyzerError::UndefinedVariable { + analyzer_error: AnalyzerError::UndefinedField { identifier: Node::new(Statement::Identifier(Identifier::new("y")), (10, 11)), + map: Node::new( + Statement::Map(vec![( + Node::new(Statement::Identifier(Identifier::new("x")), (2, 3)), + Node::new(Statement::Constant(Value::integer(1)), (6, 7)) + )]), + (0, 9) + ) + }, + source + }) + ); + } + + #[test] + fn nonexistant_field_string() { + let source = "{ x = 1 }.'y'"; + + assert_eq!( + analyze(source), + Err(DustError::AnalyzerError { + analyzer_error: AnalyzerError::UndefinedField { + identifier: Node::new(Statement::Constant(Value::string("y")), (10, 13)), + map: Node::new( + Statement::Map(vec![( + Node::new(Statement::Identifier(Identifier::new("x")), (2, 3)), + Node::new(Statement::Constant(Value::integer(1)), (6, 7)) + )]), + (0, 9) + ) }, source }) diff --git a/dust-lang/src/built_in_function.rs b/dust-lang/src/built_in_function.rs index 92e4210..718ba33 100644 --- a/dust-lang/src/built_in_function.rs +++ b/dust-lang/src/built_in_function.rs @@ -47,7 +47,7 @@ impl BuiltInFunction { BuiltInFunction::Length => { vec![( "value".into(), - Type::List { + Type::ListOf { item_type: Box::new(Type::Any), }, )] diff --git a/dust-lang/src/type.rs b/dust-lang/src/type.rs index 278cc97..e222377 100644 --- a/dust-lang/src/type.rs +++ b/dust-lang/src/type.rs @@ -50,6 +50,10 @@ pub enum Type { Integer, List { item_type: Box, + length: usize, + }, + ListOf { + item_type: Box, }, Map(BTreeMap), Number, @@ -139,9 +143,35 @@ impl Type { ( Type::List { item_type: left_type, + length: left_length, }, Type::List { item_type: right_type, + length: right_length, + }, + ) => { + if left_length != right_length { + return Err(TypeConflict { + actual: other.clone(), + expected: self.clone(), + }); + } + + if left_type.check(right_type).is_err() { + return Err(TypeConflict { + actual: other.clone(), + expected: self.clone(), + }); + } + + return Ok(()); + } + ( + Type::ListOf { + item_type: left_type, + }, + Type::ListOf { + item_type: right_type, }, ) => { if left_type.check(right_type).is_err() { @@ -150,8 +180,36 @@ impl Type { expected: self.clone(), }); } + } + ( + Type::List { + item_type: list_item_type, + .. + }, + Type::ListOf { + item_type: list_of_item_type, + }, + ) + | ( + Type::ListOf { + item_type: list_of_item_type, + }, + Type::List { + item_type: list_item_type, + .. + }, + ) => { + // TODO: This is a hack, remove it. + if let Type::Any = **list_of_item_type { + return Ok(()); + } - return Ok(()); + if list_item_type.check(list_of_item_type).is_err() { + return Err(TypeConflict { + actual: other.clone(), + expected: self.clone(), + }); + } } ( Type::Function { @@ -248,7 +306,8 @@ impl Display for Type { } } Type::Integer => write!(f, "int"), - Type::List { item_type } => write!(f, "[{item_type}]"), + Type::List { item_type, length } => write!(f, "[{item_type}; {length}]"), + Type::ListOf { item_type } => write!(f, "list_of({item_type})"), Type::Map(map) => { write!(f, "{{ ")?; @@ -316,9 +375,11 @@ mod tests { assert_eq!( Type::List { item_type: Box::new(Type::Boolean), + length: 42 } .check(&Type::List { item_type: Box::new(Type::Boolean), + length: 42 }), Ok(()) ); @@ -360,6 +421,7 @@ mod tests { Type::Integer, Type::List { item_type: Box::new(Type::Integer), + length: 42, }, Type::Map(BTreeMap::new()), Type::Range, diff --git a/dust-lang/src/value.rs b/dust-lang/src/value.rs index 585a1ba..a6d37f8 100644 --- a/dust-lang/src/value.rs +++ b/dust-lang/src/value.rs @@ -679,6 +679,7 @@ impl ValueInner { Type::List { item_type: Box::new(item_type), + length: values.len(), } } ValueInner::Map(value_map) => {