Merge pull request #36 from maelmahdy/master

Fix NullReferenceException when NodeCreator and DefaultNodeCreator are both null

Irony/Ast/AstBuilder.cs

@@ -18,81 +18,95 @@
 using Irony.Parsing;
 using System.Collections.Concurrent;
 
-namespace Irony.Ast {
+namespace Irony.Ast
+{
 
-  public class AstBuilder {
-    public AstContext Context; 
+  public class AstBuilder
+  {
+    public AstContext Context;
 
-    public AstBuilder(AstContext context) {
-      Context = context; 
+    public AstBuilder(AstContext context)
+    {
+      Context = context;
     }
 
-    public virtual void BuildAst(ParseTree parseTree) {
+    public virtual void BuildAst(ParseTree parseTree)
+    {
       if (parseTree.Root == null)
-        return; 
+        return;
       Context.Messages = parseTree.ParserMessages;
       if (!Context.Language.AstDataVerified)
         VerifyLanguageData();
       if (Context.Language.ErrorLevel == GrammarErrorLevel.Error)
-        return; 
+        return;
       BuildAst(parseTree.Root);
     }
 
-    public virtual void VerifyLanguageData() {
-      var gd = Context.Language.GrammarData; 
+    public virtual void VerifyLanguageData()
+    {
+      var gd = Context.Language.GrammarData;
       //Collect all terminals and non-terminals
-      var terms = new BnfTermSet(); 
+      var terms = new BnfTermSet();
       //SL does not understand co/contravariance, so doing merge one-by-one
       foreach (var t in gd.Terminals) terms.Add(t);
       foreach (var t in gd.NonTerminals) terms.Add(t);
       var missingList = new BnfTermList();
-      foreach (var term in terms) {
+      foreach (var term in terms)
+      {
         var terminal = term as Terminal;
         if (terminal != null && terminal.Category != TokenCategory.Content) continue; //only content terminals
         if (term.Flags.IsSet(TermFlags.NoAstNode)) continue;
-        var config = term.AstConfig; 
-        if (config.NodeCreator != null || config.DefaultNodeCreator != null) continue; 
+        var config = term.AstConfig;
+        if (config.NodeCreator != null || config.DefaultNodeCreator != null) continue;
         //We must check NodeType
         if (config.NodeType == null)
           config.NodeType = GetDefaultNodeType(term);
         if (config.NodeType == null)
           missingList.Add(term);
-        else 
-          config.DefaultNodeCreator = CompileDefaultNodeCreator(config.NodeType);   
+        else
+          config.DefaultNodeCreator = CompileDefaultNodeCreator(config.NodeType);
       }
       if (missingList.Count > 0)
         // AST node type is not specified for term {0}. Either assign Term.AstConfig.NodeType, or specify default type(s) in AstBuilder.  
-        Context.AddMessage(ErrorLevel.Error, SourceLocation.Empty, Resources.ErrNodeTypeNotSetOn, string.Join(", " , missingList));
-      Context.Language.AstDataVerified = true; 
+        Context.AddMessage(ErrorLevel.Error, SourceLocation.Empty, Resources.ErrNodeTypeNotSetOn, string.Join(", ", missingList));
+      Context.Language.AstDataVerified = true;
     }
 
-    protected virtual Type GetDefaultNodeType(BnfTerm term) {
+    protected virtual Type GetDefaultNodeType(BnfTerm term)
+    {
       if (term is NumberLiteral || term is StringLiteral)
         return Context.DefaultLiteralNodeType;
       else if (term is IdentifierTerminal)
         return Context.DefaultIdentifierNodeType;
       else
-        return Context.DefaultNodeType; 
+        return Context.DefaultNodeType;
     }
 
-    public virtual void BuildAst(ParseTreeNode parseNode) {
+    public virtual void BuildAst(ParseTreeNode parseNode)
+    {
       var term = parseNode.Term;
-      if (term.Flags.IsSet(TermFlags.NoAstNode) || parseNode.AstNode != null) return; 
+      if (term.Flags.IsSet(TermFlags.NoAstNode) || parseNode.AstNode != null) return;
       //children first
       var processChildren = !parseNode.Term.Flags.IsSet(TermFlags.AstDelayChildren) && parseNode.ChildNodes.Count > 0;
-      if (processChildren) {
+      if (processChildren)
+      {
         var mappedChildNodes = parseNode.GetMappedChildNodes();
         for (int i = 0; i < mappedChildNodes.Count; i++)
           BuildAst(mappedChildNodes[i]);
       }
       //create the node
       //We know that either NodeCreator or DefaultNodeCreator is set; VerifyAstData create the DefaultNodeCreator
       var config = term.AstConfig;
-      if (config.NodeCreator != null) {
+      if (config.NodeCreator == null && config.DefaultNodeCreator == null) return;
+
+      if (config.NodeCreator != null)
+      {
         config.NodeCreator(Context, parseNode);
         // We assume that Node creator method creates node and initializes it, so parser does not need to call 
         // IAstNodeInit.Init() method on node object. But we do call AstNodeCreated custom event on term.
-      } else {
+      }
+      else
+      {
         //Invoke the default creator compiled when we verified the data
         parseNode.AstNode = config.DefaultNodeCreator();
         //Initialize node
@@ -104,22 +118,22 @@ public virtual void BuildAst(ParseTreeNode parseNode) {
       term.OnAstNodeCreated(parseNode);
     }//method
 
-    private Func<object> CompileDefaultNodeCreator(Type nodeType) {
-      if(_cachedNodeCreators.TryGetValue(nodeType, out var creator))
+    private Func<object> CompileDefaultNodeCreator(Type nodeType)
+    {
+      if (_cachedNodeCreators.TryGetValue(nodeType, out var creator))
         return creator;
       var constr = nodeType.GetConstructor(Type.EmptyTypes);
-      if (constr == null) {
+      if (constr == null)
+      {
         this.Context.AddMessage(ErrorLevel.Error, SourceLocation.Empty, "AST node type {0} does not have default constructor.", nodeType);
-        return null; 
+        return null;
       }
       creator = () => constr.Invoke(null);
-      _cachedNodeCreators[nodeType] = creator; 
-      return creator; 
+      _cachedNodeCreators[nodeType] = creator;
+      return creator;
     }
 
     static ConcurrentDictionary<Type, Func<object>> _cachedNodeCreators = new ConcurrentDictionary<Type, Func<object>>();
 
   }//class
-
-
 }