Validate type literal parse (#2402)

Since we use generic expression parser for parsing type expression
within type literal node, we consider an expression like `Type(5+5)` to
be parsevalid.

This PR 
1. Adds a validator to allow only valid type expressions.
2. Simplifies `DTypeVisitor` to use `DefaultVisitor` 
3. overrides exception when visiting `Type(Type(..))`
4. Adds TexlPretty for `TypeLiteralNode` to enable ToString.

src/libraries/Microsoft.PowerFx.Core/Parser/ParseFormulasResult.cs

@@ -109,10 +109,13 @@ namespace Microsoft.PowerFx.Core.Parser
 
         internal TypeLiteralNode Type { get; }
 
-        public DefinedType(IdentToken ident, TypeLiteralNode type)
+        internal bool IsParseValid { get; }
+
+        public DefinedType(IdentToken ident, TypeLiteralNode type, bool isParseValid)
         {
             Ident = ident;
             Type = type;
+            IsParseValid = isParseValid;
         }
     }
 

src/libraries/Microsoft.PowerFx.Core/Parser/TexlParser.cs

@@ -317,7 +317,16 @@ namespace Microsoft.PowerFx.Core.Parser
                         var result = ParseExpr(Precedence.None);
                         if (result is TypeLiteralNode typeLiteralNode)
                         {
-                            definedTypes.Add(new DefinedType(thisIdentifier.As<IdentToken>(), typeLiteralNode));
+                            if (typeLiteralNode.IsValid(out var errors))
+                            {
+                                definedTypes.Add(new DefinedType(thisIdentifier.As<IdentToken>(), typeLiteralNode, true));
+                            }
+                            else
+                            {
+                                definedTypes.Add(new DefinedType(thisIdentifier.As<IdentToken>(), typeLiteralNode, false));
+                                CollectionUtils.Add(ref _errors, errors);
+                            }
+
                             continue;
                         }
 

src/libraries/Microsoft.PowerFx.Core/Public/DefinitionsCheckResult.cs

@@ -113,7 +113,7 @@ namespace Microsoft.PowerFx
             {
                 if (_parse.DefinedTypes.Any())
                 {
-                    this._resolvedTypes = DefinedTypeResolver.ResolveTypes(_parse.DefinedTypes, _symbols, out var errors);
+                    this._resolvedTypes = DefinedTypeResolver.ResolveTypes(_parse.DefinedTypes.Where(dt => dt.IsParseValid), _symbols, out var errors);
                     _errors.AddRange(ExpressionError.New(errors, _defaultErrorCulture));
                 }
                 else

src/libraries/Microsoft.PowerFx.Core/Syntax/Nodes/TypeLiteralNode.cs

@@ -2,6 +2,10 @@
 // Licensed under the MIT license.
 
 using System.Collections.Generic;
+using System.Linq;
+using Microsoft.PowerFx.Core.Errors;
+using Microsoft.PowerFx.Core.Localization;
+using Microsoft.PowerFx.Core.Parser;
 using Microsoft.PowerFx.Core.Types;
 using Microsoft.PowerFx.Core.Utils;
 using Microsoft.PowerFx.Syntax;
@@ -19,6 +23,8 @@ namespace Microsoft.PowerFx.Syntax
 
     public sealed class TypeLiteralNode : TexlNode
     {
+        private IEnumerable<TexlError> _errors;
+
         internal TexlNode TypeRoot { get; }
 
         internal TypeLiteralNode(ref int idNext, Token firstToken, TexlNode type, SourceList sources)
@@ -47,5 +53,186 @@ namespace Microsoft.PowerFx.Syntax
 
         /// <inheritdoc />
         public override NodeKind Kind => NodeKind.TypeLiteral;
+
+        internal bool IsValid(out IEnumerable<TexlError> errors)
+        {
+            if (_errors == null)
+            {
+                var validator = new Validator();
+                this.TypeRoot.Accept(validator);
+                this._errors = validator.Errors;
+            }
+
+            errors = _errors;
+            return !_errors.Any();
+        }
+
+        private class Validator : TexlVisitor
+        {
+            private readonly List<TexlError> _errors;
+
+            internal IEnumerable<TexlError> Errors => _errors;
+
+            public Validator()
+            {
+                _errors = new List<TexlError>();
+            }
+
+            // Valid Nodes
+            public override void Visit(FirstNameNode node)
+            {
+            }
+
+            public override bool PreVisit(RecordNode node)
+            {
+                return true;
+            }
+
+            public override void PostVisit(RecordNode node)
+            {
+            }
+
+            public override bool PreVisit(TableNode node)
+            {
+                if (node.ChildNodes.Count > 1)
+                {
+                    _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                    return false;
+                }
+
+                return true;
+            }
+
+            public override void PostVisit(TableNode node)
+            {
+            }
+
+            // Invalid nodes
+            public override void Visit(ErrorNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(BlankNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(BoolLitNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(StrLitNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(NumLitNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(DecLitNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(ParentNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(SelfNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override void Visit(TypeLiteralNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+            }
+
+            public override bool PreVisit(StrInterpNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(DottedNameNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(UnaryOpNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(BinaryOpNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(VariadicOpNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(CallNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(ListNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            public override bool PreVisit(AsNode node)
+            {
+                _errors.Add(new TexlError(node, DocumentErrorSeverity.Severe, TexlStrings.ErrTypeLiteral_InvalidTypeDefinition, node.ToString()));
+                return false;
+            }
+
+            // Do nothing in PostVisit for these nodes as we fail and add errors in PreVisit for these nodes
+            public override void PostVisit(StrInterpNode node)
+            {
+            }
+
+            public override void PostVisit(DottedNameNode node)
+            {
+            }
+
+            public override void PostVisit(UnaryOpNode node)
+            {
+            }
+
+            public override void PostVisit(BinaryOpNode node)
+            {
+            }
+
+            public override void PostVisit(VariadicOpNode node)
+            {
+            }
+
+            public override void PostVisit(CallNode node)
+            {
+            }
+
+            public override void PostVisit(ListNode node)
+            {
+            }
+
+            public override void PostVisit(AsNode node)
+            {
+            }
+        }
     }
 }

src/libraries/Microsoft.PowerFx.Core/Syntax/TexlPretty.cs

@@ -403,6 +403,23 @@ namespace Microsoft.PowerFx.Syntax
             return result;
         }
 
+        public override LazyList<string> Visit(TypeLiteralNode node, Precedence parentPrecedence)
+        {
+            Contracts.AssertValue(node);
+
+            var result = LazyList<string>.Empty;
+            var sb = new StringBuilder();
+
+            result = result
+                .With(
+                    "Type",
+                    TexlLexer.PunctuatorParenOpen)
+                .With(node.TypeRoot.Accept(this, Precedence.Atomic))
+                .With(TexlLexer.PunctuatorParenClose);
+
+            return result;
+        }
+
         public virtual string GetRightToken(TexlNode leftNode, Identifier right)
         {
             return right.Token.ToString();

src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DTypeVisitor.cs

@@ -15,9 +15,10 @@ using Microsoft.PowerFx.Types;
 namespace Microsoft.PowerFx.Core.Syntax.Visitors
 {
     // Visitor to resolve TypeLiteralNode.TypeRoot into DType.
-    internal class DTypeVisitor : TexlFunctionalVisitor<DType, INameResolver>
+    internal class DTypeVisitor : DefaultVisitor<DType, INameResolver>
     {
         private DTypeVisitor() 
+            : base(DType.Invalid)
         {
         }
 
@@ -106,85 +107,5 @@ namespace Microsoft.PowerFx.Core.Syntax.Visitors
 
             return rowType.ToTable();
         }
-
-        public override DType Visit(ErrorNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(BlankNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(BoolLitNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(StrLitNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(NumLitNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(DecLitNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(ParentNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(SelfNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(StrInterpNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(DottedNameNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(UnaryOpNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(BinaryOpNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(VariadicOpNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(CallNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(ListNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
-
-        public override DType Visit(AsNode node, INameResolver context)
-        {
-            return DType.Invalid;
-        }
     }
 }

src/libraries/Microsoft.PowerFx.Core/Syntax/Visitors/DefaultVisitor.cs

@@ -104,5 +104,15 @@ namespace Microsoft.PowerFx.Syntax
         {
             return Default;
         }
+
+        public override TResult Visit(StrInterpNode node, TContext context)
+        {
+            return Default;
+        }
+
+        public override TResult Visit(TypeLiteralNode node, TContext context)
+        {
+            return Default;
+        }
     }
 }

src/strings/PowerFxResources.en-US.resx

@@ -4410,8 +4410,8 @@
     <comment>Warning for makers that a certain capability should be avoided.</comment>
   </data>
   <data name="ErrTypeLiteral_InvalidTypeDefinition" xml:space="preserve">
-    <value>Type literal declaration was invalid.</value>
-    <comment>Error message when type binding is invalid.</comment>
+    <value>Type literal declaration is invalid. The expression '{0}' cannot be used in a type definition.</value>
+    <comment>Error message when validation of type literal fails.</comment>
   </data>
   <data name="ErrNamedType_InvalidTypeDefinition" xml:space="preserve">
     <value>Definition of type {0} is invalid.</value>

src/tests/Microsoft.PowerFx.Core.Tests/UserDefinedTypeTests.cs

@@ -138,5 +138,38 @@ namespace Microsoft.PowerFx.Core.Tests
             Assert.Equal(expectedErrorCount, errors.Count());
             Assert.All(errors, e => Assert.Contains(expectedMessageKey, e.MessageKey));
         }
+
+        [Theory]
+        [InlineData("T = Type({ x: 5+5, y: -5 });", 2)]
+        [InlineData("T = Type(Type(Number));", 1)]
+        [InlineData("T = Type({+});", 1)]
+        [InlineData("T = Type({);", 1)]
+        [InlineData("T = Type({x: true, y: \"Number\"});", 2)]
+        [InlineData("T1 = Type({A: Number}); T2 = Type(T1.A);", 1)]
+        [InlineData("T = Type((1, 2));", 1)]
+        [InlineData("T1 = Type(UniChar(955)); T2 = Type([Table(Number)])", 2)]
+        [InlineData("T = Type((1; 2));", 1)]
+        [InlineData("T = Type(Self.T);", 1)]
+        [InlineData("T = Type(Parent.T);", 1)]
+        [InlineData("T = Type(Number As T1);", 1)]
+        [InlineData("T = Type(Text); T1 = Type(Not T);", 1)]
+        [InlineData("T1 = Type({V: Number}); T2 = Type(T1[@V]);", 1)]
+        [InlineData("T = Type([{a: {b: {c: [{d: 10e+4}]}}}]);", 1)]
+        public void TestUDTParseErrors(string typeDefinition, int expectedErrorCount)
+        {
+            var parseOptions = new ParserOptions
+            {
+                AllowParseAsTypeLiteral = true
+            };
+
+            var checkResult = new DefinitionsCheckResult()
+                                            .SetText(typeDefinition, parseOptions);
+
+            var parseResult = checkResult.ApplyParse();
+            Assert.True(parseResult.HasErrors);
+
+            var validatorErrors = parseResult.Errors.Where(e => e.MessageKey.Contains("ErrTypeLiteral_InvalidTypeDefinition"));
+            Assert.Equal(expectedErrorCount, validatorErrors.Count());
+        }
     }
 }