fix: Small code refactor for ECAL parser

lang/ecal/parser/helper.go

@@ -89,7 +89,7 @@ func (n *ASTNode) instance(p *parser, t *LexToken) *ASTNode {
 }
 
 /*
-Equal checks if this AST data equals another AST data. Returns also a message describing
+Equals checks if this AST data equals another AST data. Returns also a message describing
 what is the found difference.
 */
 func (n *ASTNode) Equals(other *ASTNode, ignoreTokenPosition bool) (bool, string) {
@@ -218,7 +218,7 @@ func (n *ASTNode) levelString(indent int, buf *bytes.Buffer, printChildren int)
 }
 
 /*
-ToJSON returns this ASTNode and all its children as a JSON object.
+ToJSONObject returns this ASTNode and all its children as a JSON object.
 */
 func (n *ASTNode) ToJSONObject() map[string]interface{} {
 	ret := make(map[string]interface{})
@@ -267,7 +267,7 @@ func (n *ASTNode) ToJSONObject() map[string]interface{} {
 }
 
 /*
-ASTFromPlain creates an AST from a JSON Object.
+ASTFromJSONObject creates an AST from a JSON Object.
 The following nested map structure is expected:
 
 	{
@@ -284,9 +284,10 @@ The following nested map structure is expected:
 func ASTFromJSONObject(jsonAST map[string]interface{}) (*ASTNode, error) {
 	var astMeta []MetaData
 	var astChildren []*ASTNode
-	var nodeID LexTokenID = TokenANY
 	var pos, line, linepos int
 
+	nodeID := TokenANY
+
 	name, ok := jsonAST["name"]
 	if !ok {
 		return nil, fmt.Errorf("Found json ast node without a name: %v", jsonAST)
@@ -393,7 +394,7 @@ func ASTFromJSONObject(jsonAST map[string]interface{}) (*ASTNode, error) {
 // =================
 
 /*
-ASTNode models a node in the AST
+LABuffer models a look-ahead buffer.
 */
 type LABuffer struct {
 	tokens chan LexToken
@@ -401,7 +402,7 @@ type LABuffer struct {
 }
 
 /*
-Create a new instance of this ASTNode which is connected to a concrete lexer token.
+NewLABuffer creates a new NewLABuffer instance.
 */
 func NewLABuffer(c chan LexToken, size int) *LABuffer {
 

lang/ecal/parser/helper_test.go

@@ -241,7 +241,7 @@ number: 1 # test
 	if _, err := ASTFromJSONObject(map[string]interface{}{
 		"name": "foo",
 		"children": []map[string]interface{}{
-			map[string]interface{}{
+			{
 				"value": "bar",
 			},
 		},
@@ -262,7 +262,7 @@ number: 1 # test
 	if ast, err := ASTFromJSONObject(map[string]interface{}{
 		"name": "foo",
 		"children": []map[string]interface{}{
-			map[string]interface{}{
+			{
 				"name": "bar",
 			},
 		},

lang/ecal/parser/lexer.go

@@ -50,46 +50,46 @@ func NewLexTokenInstance(t LexToken) *LexToken {
 }
 
 /*
-Equal checks if this LexToken equals another LexToken. Returns also a message describing
+Equals checks if this LexToken equals another LexToken. Returns also a message describing
 what is the found difference.
 */
-func (n LexToken) Equals(other LexToken, ignorePosition bool) (bool, string) {
+func (t LexToken) Equals(other LexToken, ignorePosition bool) (bool, string) {
 	var res = true
 	var msg = ""
 
-	if n.ID != other.ID {
+	if t.ID != other.ID {
 		res = false
-		msg += fmt.Sprintf("ID is different %v vs %v\n", n.ID, other.ID)
+		msg += fmt.Sprintf("ID is different %v vs %v\n", t.ID, other.ID)
 	}
 
-	if !ignorePosition && n.Pos != other.Pos {
+	if !ignorePosition && t.Pos != other.Pos {
 		res = false
-		msg += fmt.Sprintf("Pos is different %v vs %v\n", n.Pos, other.Pos)
+		msg += fmt.Sprintf("Pos is different %v vs %v\n", t.Pos, other.Pos)
 	}
 
-	if n.Val != other.Val {
+	if t.Val != other.Val {
 		res = false
-		msg += fmt.Sprintf("Val is different %v vs %v\n", n.Val, other.Val)
+		msg += fmt.Sprintf("Val is different %v vs %v\n", t.Val, other.Val)
 	}
 
-	if n.Identifier != other.Identifier {
+	if t.Identifier != other.Identifier {
 		res = false
-		msg += fmt.Sprintf("Identifier is different %v vs %v\n", n.Identifier, other.Identifier)
+		msg += fmt.Sprintf("Identifier is different %v vs %v\n", t.Identifier, other.Identifier)
 	}
 
-	if !ignorePosition && n.Lline != other.Lline {
+	if !ignorePosition && t.Lline != other.Lline {
 		res = false
-		msg += fmt.Sprintf("Lline is different %v vs %v\n", n.Lline, other.Lline)
+		msg += fmt.Sprintf("Lline is different %v vs %v\n", t.Lline, other.Lline)
 	}
 
-	if !ignorePosition && n.Lpos != other.Lpos {
+	if !ignorePosition && t.Lpos != other.Lpos {
 		res = false
-		msg += fmt.Sprintf("Lpos is different %v vs %v\n", n.Lpos, other.Lpos)
+		msg += fmt.Sprintf("Lpos is different %v vs %v\n", t.Lpos, other.Lpos)
 	}
 
 	if msg != "" {
 		var buf bytes.Buffer
-		out, _ := json.MarshalIndent(n, "", "  ")
+		out, _ := json.MarshalIndent(t, "", "  ")
 		buf.WriteString(string(out))
 		buf.WriteString("\nvs\n")
 		out, _ = json.MarshalIndent(other, "", "  ")

lang/ecal/parser/main_test.go

@@ -81,8 +81,8 @@ func UnitTestParseWithPPResult(name string, input string, expectedPPRes string)
 
 		if ok, msg := n.Equals(unmarshaledAST, false); !ok {
 			return nil, fmt.Errorf(
-				"Parsed AST is different from the unmarshaled AST.\n%v\n",
-				msg)
+				"Parsed AST is different from the unmarshaled AST."+
+					"\n%v\n", msg)
 		}
 	}
 

lang/ecal/parser/parser.go

@@ -400,7 +400,7 @@ func ndImport(p *parser, self *ASTNode) (*ASTNode, error) {
 ndSink is used to parse sinks.
 */
 func ndSkink(p *parser, self *ASTNode) (*ASTNode, error) {
-	var ret *ASTNode
+	var exp, ret *ASTNode
 
 	// Must specify a name
 
@@ -410,24 +410,24 @@ func ndSkink(p *parser, self *ASTNode) (*ASTNode, error) {
 
 		// Parse the rest of the parameters as children until we reach the body
 
-		for p.node.Token.ID != TokenEOF && p.node.Token.ID != TokenLBRACE {
-			exp, err := p.run(150)
-			if err != nil {
-				return nil, err
-			}
-
-			self.Children = append(self.Children, exp)
+		for err == nil && IsNotEndAndNotToken(p, TokenLBRACE) {
+			if exp, err = p.run(150); err == nil {
+				self.Children = append(self.Children, exp)
 
-			// Skip commas
+				// Skip commas
 
-			if p.node.Token.ID == TokenCOMMA {
-				skipToken(p, TokenCOMMA)
+				if p.node.Token.ID == TokenCOMMA {
+					err = skipToken(p, TokenCOMMA)
+				}
 			}
 		}
 
-		// Parse the body
+		if err == nil {
+
+			// Parse the body
 
-		ret, err = parseInnerStatements(p, self)
+			ret, err = parseInnerStatements(p, self)
+		}
 	}
 
 	return ret, err
@@ -437,6 +437,7 @@ func ndSkink(p *parser, self *ASTNode) (*ASTNode, error) {
 ndFunc is used to parse function definitions.
 */
 func ndFunc(p *parser, self *ASTNode) (*ASTNode, error) {
+	var exp *ASTNode
 
 	// Must specify a function name
 
@@ -450,12 +451,11 @@ func ndFunc(p *parser, self *ASTNode) (*ASTNode, error) {
 		params := astNodeMap[TokenPARAMS].instance(p, nil)
 		self.Children = append(self.Children, params)
 
-		for err == nil && p.node.Token.ID != TokenRPAREN {
+		for err == nil && IsNotEndAndNotToken(p, TokenRPAREN) {
 
 			// Parse all the expressions inside
 
-			exp, err := p.run(0)
-			if err == nil {
+			if exp, err = p.run(0); err == nil {
 				params.Children = append(params.Children, exp)
 
 				if p.node.Token.ID == TokenCOMMA {
@@ -535,6 +535,8 @@ func ndIdentifier(p *parser, self *ASTNode) (*ASTNode, error) {
 	}
 
 	parseFuncCall = func(current *ASTNode) error {
+		var exp *ASTNode
+
 		err := skipToken(p, TokenLPAREN)
 
 		fc := astNodeMap[TokenFUNCCALL].instance(p, nil)
@@ -542,12 +544,11 @@ func ndIdentifier(p *parser, self *ASTNode) (*ASTNode, error) {
 
 		// Read in parameters
 
-		for err == nil && p.node.Token.ID != TokenRPAREN {
+		for err == nil && IsNotEndAndNotToken(p, TokenRPAREN) {
 
 			// Parse all the expressions inside the directives
 
-			exp, err := p.run(0)
-			if err == nil {
+			if exp, err = p.run(0); err == nil {
 				fc.Children = append(fc.Children, exp)
 
 				if p.node.Token.ID == TokenCOMMA {
@@ -567,19 +568,23 @@ func ndIdentifier(p *parser, self *ASTNode) (*ASTNode, error) {
 	}
 
 	parseCompositionAccess = func(current *ASTNode) error {
+		var exp *ASTNode
+
 		err := skipToken(p, TokenLBRACK)
 
-		ca := astNodeMap[TokenCOMPACCESS].instance(p, nil)
-		current.Children = append(current.Children, ca)
+		if err == nil {
 
-		// Parse all the expressions inside the directives
+			ca := astNodeMap[TokenCOMPACCESS].instance(p, nil)
+			current.Children = append(current.Children, ca)
 
-		exp, err := p.run(0)
-		if err == nil {
-			ca.Children = append(ca.Children, exp)
+			// Parse all the expressions inside the directives
 
-			if err = skipToken(p, TokenRBRACK); err == nil {
-				err = parseMore(current)
+			if exp, err = p.run(0); err == nil {
+				ca.Children = append(ca.Children, exp)
+
+				if err = skipToken(p, TokenRBRACK); err == nil {
+					err = parseMore(current)
+				}
 			}
 		}
 
@@ -593,6 +598,8 @@ func ndIdentifier(p *parser, self *ASTNode) (*ASTNode, error) {
 ndList is used to collect elements of a list.
 */
 func ndList(p *parser, self *ASTNode) (*ASTNode, error) {
+	var err error
+	var exp *ASTNode
 
 	// Create a list token
 
@@ -600,31 +607,34 @@ func ndList(p *parser, self *ASTNode) (*ASTNode, error) {
 
 	// Get the inner expression
 
-	for p.node.Token.ID != TokenRBRACK {
+	for err == nil && IsNotEndAndNotToken(p, TokenRBRACK) {
 
 		// Parse all the expressions inside
 
-		exp, err := p.run(0)
-		if err != nil {
-			return nil, err
-		}
-
-		st.Children = append(st.Children, exp)
+		if exp, err = p.run(0); err == nil {
+			st.Children = append(st.Children, exp)
 
-		if p.node.Token.ID == TokenCOMMA {
-			skipToken(p, TokenCOMMA)
+			if p.node.Token.ID == TokenCOMMA {
+				err = skipToken(p, TokenCOMMA)
+			}
 		}
 	}
 
+	if err == nil {
+		err = skipToken(p, TokenRBRACK)
+	}
+
 	// Must have a closing bracket
 
-	return st, skipToken(p, TokenRBRACK)
+	return st, err
 }
 
 /*
 ndMap is used to collect elements of a map.
 */
 func ndMap(p *parser, self *ASTNode) (*ASTNode, error) {
+	var err error
+	var exp *ASTNode
 
 	// Create a map token
 
@@ -632,27 +642,26 @@ func ndMap(p *parser, self *ASTNode) (*ASTNode, error) {
 
 	// Get the inner expression
 
-	for p.node.Token.ID != TokenRBRACE {
+	for err == nil && IsNotEndAndNotToken(p, TokenRBRACE) {
 
 		// Parse all the expressions inside
 
-		exp, err := p.run(0)
-		if err != nil {
-			return nil, err
-		}
-
-		st.Children = append(st.Children, exp)
+		if exp, err = p.run(0); err == nil {
+			st.Children = append(st.Children, exp)
 
-		if p.node.Token.ID == TokenCOMMA {
-			if err := skipToken(p, TokenCOMMA); err != nil {
-				return nil, err
+			if p.node.Token.ID == TokenCOMMA {
+				err = skipToken(p, TokenCOMMA)
 			}
 		}
 	}
 
+	if err == nil {
+		err = skipToken(p, TokenRBRACE)
+	}
+
 	// Must have a closing brace
 
-	return st, skipToken(p, TokenRBRACE)
+	return st, err
 }
 
 /*
@@ -685,7 +694,7 @@ func ndGuard(p *parser, self *ASTNode) (*ASTNode, error) {
 
 	if err = parseGuardAndStatements(); err == nil {
 
-		for err == nil && p.node.Token.ID == TokenELIF {
+		for err == nil && IsNotEndAndToken(p, TokenELIF) {
 
 			// Parse an elif
 
@@ -763,6 +772,20 @@ func ldInfix(p *parser, self *ASTNode, left *ASTNode) (*ASTNode, error) {
 // Helper functions
 // ================
 
+/*
+IsNotEndAndToken checks if the next token is of a specific type or the end has been reached.
+*/
+func IsNotEndAndToken(p *parser, i LexTokenID) bool {
+	return p.node != nil && p.node.Name != NodeEOF && p.node.Token.ID == i
+}
+
+/*
+IsNotEndAndNotToken checks if the next token is not of a specific type or the end has been reached.
+*/
+func IsNotEndAndNotToken(p *parser, i LexTokenID) bool {
+	return p.node != nil && p.node.Name != NodeEOF && p.node.Token.ID != i
+}
+
 /*
 hasMoreStatements returns true if there are more statements to parse.
 */
@@ -816,17 +839,16 @@ func acceptChild(p *parser, self *ASTNode, id LexTokenID) error {
 
 	current := p.node
 
-	p.node, err = p.next()
-	if err != nil {
-		return err
-	}
+	if p.node, err = p.next(); err == nil {
 
-	if current.Token.ID == id {
-		self.Children = append(self.Children, current)
-		return nil
+		if current.Token.ID == id {
+			self.Children = append(self.Children, current)
+		} else {
+			err = p.newParserError(ErrUnexpectedToken, current.Token.Val, *current.Token)
+		}
 	}
 
-	return p.newParserError(ErrUnexpectedToken, current.Token.Val, *current.Token)
+	return err
 }
 
 /*

lang/ecal/parser/parser_main_test.go

@@ -125,3 +125,48 @@ number: 1 #  foo   foo bar
 		return
 	}
 }
+
+func TestErrorConditions(t *testing.T) {
+
+	input := ``
+	if ast, err := Parse("test", input); err == nil || err.Error() != "Parse error in test: Unexpected end" {
+		t.Errorf("Unexpected result: %v\nAST:\n%v", err, ast)
+		return
+	}
+
+	input = `a := 1 a`
+	if ast, err := Parse("test", input); err == nil || err.Error() != `Parse error in test: Unexpected end (extra token id:7 ("a")) (Line:1 Pos:8)` {
+		t.Errorf("Unexpected result: %v\nAST:\n%v", err, ast)
+		return
+	}
+
+	tokenStringEntry := astNodeMap[TokenSTRING]
+	delete(astNodeMap, TokenSTRING)
+	defer func() {
+		astNodeMap[TokenSTRING] = tokenStringEntry
+	}()
+
+	input = `"foo"`
+	if ast, err := Parse("test", input); err == nil || err.Error() != `Parse error in test: Unknown term (id:5 ("foo")) (Line:1 Pos:1)` {
+		t.Errorf("Unexpected result: %v\nAST:\n%v", err, ast)
+		return
+	}
+
+	// Test parser functions
+
+	input = `a := 1 + a`
+
+	p := &parser{"test", nil, NewLABuffer(Lex("test", input), 3), nil}
+	node, _ := p.next()
+	p.node = node
+
+	if err := skipToken(p, TokenAND); err == nil || err.Error() != "Parse error in test: Unexpected term (a) (Line:1 Pos:1)" {
+		t.Errorf("Unexpected result: %v", err)
+		return
+	}
+
+	if err := acceptChild(p, node, TokenAND); err == nil || err.Error() != "Parse error in test: Unexpected term (a) (Line:1 Pos:1)" {
+		t.Errorf("Unexpected result: %v", err)
+		return
+	}
+}

lang/ecal/parser/prettyprinter.go

@@ -161,7 +161,13 @@ func PrettyPrint(ast *ASTNode) (string, error) {
 
 	visit = func(ast *ASTNode, level int) (string, error) {
 		var buf bytes.Buffer
-		var numChildren = len(ast.Children)
+		var numChildren int
+
+		if ast == nil {
+			return "", fmt.Errorf("Nil pointer in AST at level: %v", level)
+		}
+
+		numChildren = len(ast.Children)
 
 		tempKey := ast.Name
 		tempParam := make(map[string]string)
@@ -332,10 +338,9 @@ func PrettyPrint(ast *ASTNode) (string, error) {
 		// Retrieve the template
 
 		temp, ok := prettyPrinterMap[tempKey]
-		if !ok {
-			return "", fmt.Errorf("Could not find template for %v (tempkey: %v)",
-				ast.Name, tempKey)
-		}
+		errorutil.AssertTrue(ok,
+			fmt.Sprintf("Could not find template for %v (tempkey: %v)",
+				ast.Name, tempKey))
 
 		// Use the children as parameters for template
 

lang/ecal/parser/prettyprinter_test.go

@@ -13,6 +13,27 @@ import (
 	"testing"
 )
 
+func TestErrorHandling(t *testing.T) {
+
+	input := "c:= a + b"
+
+	astres, err := ParseWithRuntime("mytest", input, &DummyRuntimeProvider{})
+	if err != nil {
+		t.Errorf("Unexpected parser output:\n%vError: %v", astres, err)
+		return
+	}
+
+	// Make ast invalid
+
+	astres.Children[1].Children[1] = nil
+
+	ppres, err := PrettyPrint(astres)
+	if err == nil || err.Error() != "Nil pointer in AST at level: 2" {
+		t.Errorf("Unexpected result: %v error: %v", ppres, err)
+		return
+	}
+}
+
 func TestArithmeticExpressionPrinting(t *testing.T) {
 
 	input := "a + b * 5 /2-1"