When NewType is called and the first argument is not a string, make a diagnostic message (#1260)

CTrando · web-flow · commit 4ece7cb35361 · 2019-07-03T14:47:50.000-07:00
* Adding diagnostic error if the user calls NewType with the first arg not of string type
diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs
@@ -27,6 +27,7 @@ public static class ErrorCodes {
         public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal";
         public const string UnsupportedOperandType = "unsupported-operand-type";
         public const string ReturnInInit = "return-in-init";
+        public const string TypingNewTypeArguments = "typing-newtype-arguments";
         public const string TypingGenericArguments = "typing-generic-arguments";
     }
 }
diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs
diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx
@@ -189,6 +189,9 @@
   <data name="UnableToDetermineCachePathException" xml:space="preserve">
     <value>Unable to determine analysis cache path. Exception: {0}. Using default '{1}'.</value>
   </data>
+  <data name="NewTypeFirstArgNotString" xml:space="preserve">
+    <value>The first argument to NewType must be a string, but it is of type '{0}'.</value>
+  </data>
   <data name="UnsupporedOperandType" xml:space="preserve">
     <value>Unsupported operand types for '{0}': '{1}' and '{2}'</value>
   </data>
diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs b/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs
@@ -15,6 +15,7 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using Microsoft.Python.Analysis.Diagnostics;
 using Microsoft.Python.Analysis.Modules;
 using Microsoft.Python.Analysis.Specializations.Typing.Types;
 using Microsoft.Python.Analysis.Types;
@@ -61,7 +62,7 @@ private void SpecializeMembers() {
             o = new PythonFunctionOverload(fn.Name, location);
             // When called, create generic parameter type. For documentation
             // use original TypeVar declaration so it appear as a tooltip.
-            o.SetReturnValueProvider((interpreter, overload, args) => CreateTypeAlias(args.Values<IMember>()));
+            o.SetReturnValueProvider((interpreter, overload, args) => CreateTypeAlias(args));
             fn.AddOverload(o);
             _members["NewType"] = fn;
 
@@ -81,41 +82,41 @@ private void SpecializeMembers() {
 
             _members["Iterable"] = new GenericType("Iterable", typeArgs => CreateListType("Iterable", BuiltinTypeId.List, typeArgs, false), this);
             _members["Sequence"] = new GenericType("Sequence", typeArgs => CreateListType("Sequence", BuiltinTypeId.List, typeArgs, false), this);
-            _members["MutableSequence"] = new GenericType("MutableSequence", 
+            _members["MutableSequence"] = new GenericType("MutableSequence",
                 typeArgs => CreateListType("MutableSequence", BuiltinTypeId.List, typeArgs, true), this);
-            _members["List"] = new GenericType("List", 
+            _members["List"] = new GenericType("List",
                 typeArgs => CreateListType("List", BuiltinTypeId.List, typeArgs, true), this);
 
-            _members["MappingView"] = new GenericType("MappingView", 
+            _members["MappingView"] = new GenericType("MappingView",
                 typeArgs => CreateDictionary("MappingView", typeArgs, false), this);
 
             _members["KeysView"] = new GenericType("KeysView", CreateKeysViewType, this);
             _members["ValuesView"] = new GenericType("ValuesView", CreateValuesViewType, this);
             _members["ItemsView"] = new GenericType("ItemsView", CreateItemsViewType, this);
 
-            _members["Set"] = new GenericType("Set", 
+            _members["Set"] = new GenericType("Set",
                 typeArgs => CreateListType("Set", BuiltinTypeId.Set, typeArgs, true), this);
-            _members["MutableSet"] = new GenericType("MutableSet", 
+            _members["MutableSet"] = new GenericType("MutableSet",
                 typeArgs => CreateListType("MutableSet", BuiltinTypeId.Set, typeArgs, true), this);
-            _members["FrozenSet"] = new GenericType("FrozenSet", 
+            _members["FrozenSet"] = new GenericType("FrozenSet",
                 typeArgs => CreateListType("FrozenSet", BuiltinTypeId.Set, typeArgs, false), this);
 
             _members["Tuple"] = new GenericType("Tuple", CreateTupleType, this);
 
-            _members["Mapping"] = new GenericType("Mapping", 
+            _members["Mapping"] = new GenericType("Mapping",
                 typeArgs => CreateDictionary("Mapping", typeArgs, false), this);
-            _members["MutableMapping"] = new GenericType("MutableMapping", 
+            _members["MutableMapping"] = new GenericType("MutableMapping",
                 typeArgs => CreateDictionary("MutableMapping", typeArgs, true), this);
-            _members["Dict"] = new GenericType("Dict", 
+            _members["Dict"] = new GenericType("Dict",
                 typeArgs => CreateDictionary("Dict", typeArgs, true), this);
-            _members["OrderedDict"] = new GenericType("OrderedDict", 
+            _members["OrderedDict"] = new GenericType("OrderedDict",
                 typeArgs => CreateDictionary("OrderedDict", typeArgs, true), this);
-            _members["DefaultDict"] = new GenericType("DefaultDict", 
+            _members["DefaultDict"] = new GenericType("DefaultDict",
                 typeArgs => CreateDictionary("DefaultDict", typeArgs, true), this);
 
             _members["Union"] = new GenericType("Union", CreateUnion, this);
 
-            _members["Counter"] = Specialized.Function("Counter", this, GetMemberDocumentation("Counter"), 
+            _members["Counter"] = Specialized.Function("Counter", this, GetMemberDocumentation("Counter"),
                 new PythonInstance(Interpreter.GetBuiltinType(BuiltinTypeId.Int)));
 
             _members["SupportsInt"] = Interpreter.GetBuiltinType(BuiltinTypeId.Int);
@@ -217,13 +218,25 @@ private IPythonType CreateItemsViewType(IReadOnlyList<IPythonType> typeArgs) {
             return Interpreter.UnknownType;
         }
 
-        private IPythonType CreateTypeAlias(IReadOnlyList<IMember> typeArgs) {
+        private IPythonType CreateTypeAlias(IArgumentSet args) {
+            var typeArgs = args.Values<IMember>();
             if (typeArgs.Count == 2) {
                 var typeName = (typeArgs[0] as IPythonConstant)?.Value as string;
                 if (!string.IsNullOrEmpty(typeName)) {
                     return new TypeAlias(typeName, typeArgs[1].GetPythonType() ?? Interpreter.UnknownType);
                 }
-                // TODO: report incorrect first argument to NewVar
+
+                var firstArgType = (typeArgs[0] as PythonInstance)?.Type.Name;
+                var eval = args.Eval;
+                var expression = args.Expression;
+
+                eval.ReportDiagnostics(
+                    eval.Module?.Uri,
+                    new DiagnosticsEntry(Resources.NewTypeFirstArgNotString.FormatInvariant(firstArgType), 
+                        expression?.GetLocation(eval.Module)?.Span ?? default, 
+                        Diagnostics.ErrorCodes.TypingNewTypeArguments,
+                        Severity.Error, DiagnosticSource.Analysis)
+                );
             }
             // TODO: report wrong number of arguments
             return Interpreter.UnknownType;
@@ -331,7 +344,7 @@ private IPythonType CreateGenericClassParameter(IReadOnlyList<IPythonType> typeA
             return Interpreter.UnknownType;
         }
 
-        private IPythonType ToGenericTemplate(string typeName, IGenericTypeDefinition[] typeArgs, BuiltinTypeId typeId) 
+        private IPythonType ToGenericTemplate(string typeName, IGenericTypeDefinition[] typeArgs, BuiltinTypeId typeId)
             => _members[typeName] is GenericType gt
                 ? new GenericType(CodeFormatter.FormatSequence(typeName, '[', typeArgs), gt.SpecificTypeConstructor, this, typeId, typeArgs)
                 : Interpreter.UnknownType;
diff --git a/src/Analysis/Ast/Test/LintNewTypeTests.cs b/src/Analysis/Ast/Test/LintNewTypeTests.cs
@@ -0,0 +1,130 @@
+﻿// Copyright(c) Microsoft Corporation
+// All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the License); you may not use
+// this file except in compliance with the License. You may obtain a copy of the
+// License at http://www.apache.org/licenses/LICENSE-2.0
+//
+// THIS CODE IS PROVIDED ON AN  *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
+// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
+// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
+// MERCHANTABILITY OR NON-INFRINGEMENT.
+//
+// See the Apache Version 2.0 License for specific language governing
+// permissions and limitations under the License.
+
+using System.Linq;
+using System.Threading.Tasks;
+using FluentAssertions;
+using Microsoft.Python.Analysis.Tests.FluentAssertions;
+using Microsoft.Python.Core;
+using Microsoft.Python.Parsing.Tests;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using TestUtilities;
+
+namespace Microsoft.Python.Analysis.Tests {
+    [TestClass]
+    public class LintNewTypeTests : AnalysisTestBase {
+        public TestContext TestContext { get; set; }
+
+        [TestInitialize]
+        public void TestInitialize()
+            => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}");
+
+        [TestCleanup]
+        public void Cleanup() => TestEnvironmentImpl.TestCleanup();
+
+        [TestMethod, Priority(0)]
+        public async Task NewTypeIntFirstArg() {
+            const string code = @"
+from typing import NewType
+
+T = NewType(5, int)
+";
+            var analysis = await GetAnalysisAsync(code);
+            analysis.Diagnostics.Should().HaveCount(1);
+
+            var diagnostic = analysis.Diagnostics.ElementAt(0);
+            diagnostic.SourceSpan.Should().Be(4, 5, 4, 20);
+            diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments);
+            diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("int"));
+        }
+
+        [DataRow("float", "float")]
+        [DataRow("int", "int")]
+        [DataRow("complex", "str")]
+        [DataTestMethod, Priority(0)]
+        public async Task DifferentTypesFirstArg(string nameType, string type) {
+            string code = $@"
+from typing import NewType
+
+T = NewType({nameType}(10), {type})
+
+";
+            var analysis = await GetAnalysisAsync(code);
+            analysis.Diagnostics.Should().HaveCount(1);
+
+            var diagnostic = analysis.Diagnostics.ElementAt(0);
+            diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments);
+            diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant(nameType));
+        }
+
+        [TestMethod, Priority(0)]
+        public async Task ObjectFirstArg() {
+            string code = $@"
+from typing import NewType
+
+class X:
+    def hello():
+        pass
+
+h = X()
+
+T = NewType(h, int)
+";
+            var analysis = await GetAnalysisAsync(code);
+            analysis.Diagnostics.Should().HaveCount(1);
+
+            var diagnostic = analysis.Diagnostics.ElementAt(0);
+            diagnostic.SourceSpan.Should().Be(10, 5, 10, 20);
+            diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments);
+            diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("X"));
+        }
+
+        [TestMethod, Priority(0)]
+        public async Task GenericFirstArg() {
+            string code = $@"
+from typing import NewType, Generic, TypeVar
+
+T = TypeVar('T', str, int)
+
+class X(Generic[T]):
+    def __init__(self, p: T):
+        self.x = p
+
+h = X(5)
+T = NewType(h, int)
+";
+            var analysis = await GetAnalysisAsync(code);
+            analysis.Diagnostics.Should().HaveCount(1);
+
+            var diagnostic = analysis.Diagnostics.ElementAt(0);
+            diagnostic.SourceSpan.Should().Be(11, 5, 11, 20);
+            diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments);
+            diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("X[int]"));
+        }
+
+        [DataRow("test", "float")]
+        [DataRow("testing", "int")]
+        [DataTestMethod, Priority(0)]
+        public async Task NoDiagnosticOnStringFirstArg(string name, string type) {
+            string code = $@"
+from typing import NewType
+
+T = NewType('{name}', {type})
+";
+            var analysis = await GetAnalysisAsync(code);
+            analysis.Diagnostics.Should().HaveCount(0);
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,7 @@ public static class ErrorCodes {`
`27`	`27`	`public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal";`
`28`	`28`	`public const string UnsupportedOperandType = "unsupported-operand-type";`
`29`	`29`	`public const string ReturnInInit = "return-in-init";`
	`30`	`+ public const string TypingNewTypeArguments = "typing-newtype-arguments";`
`30`	`31`	`public const string TypingGenericArguments = "typing-generic-arguments";`
`31`	`32`	`}`
`32`	`33`	`}`