From b25869acbd797e42a246b087456eb74af4876298 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Wed, 26 Jun 2019 16:37:48 -0700 Subject: [PATCH 1/4] Adding diagnostic error if the user calls NewType with the first arg not of string type --- .../Ast/Impl/Diagnostics/ErrorCodes.cs | 1 + src/Analysis/Ast/Impl/Resources.Designer.cs | 9 ++ src/Analysis/Ast/Impl/Resources.resx | 3 + .../Specializations/Typing/TypingModule.cs | 45 +++--- src/Analysis/Ast/Test/LintNewTypeTests.cs | 135 ++++++++++++++++++ 5 files changed, 177 insertions(+), 16 deletions(-) create mode 100644 src/Analysis/Ast/Test/LintNewTypeTests.cs diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs index 45c6a1407..d71308f85 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -25,5 +25,6 @@ public static class ErrorCodes { public const string UndefinedVariable = "undefined-variable"; public const string VariableNotDefinedGlobally= "variable-not-defined-globally"; public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal"; + public const string NewTypeArguments = "newtype-arguments"; } } diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs index 785cbf9a8..f20d0eee6 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -231,6 +231,15 @@ internal static string InterpreterNotFound { } } + /// + /// Looks up a localized string similar to The first argument to NewType must be a string but is of type '{0}'.. + /// + internal static string NewTypeFirstArgNotString { + get { + return ResourceManager.GetString("NewTypeFirstArgNotString", resourceCulture); + } + } + /// /// Looks up a localized string similar to property of type {0}. /// diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx index 95cd89002..1b2508c8d 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -189,4 +189,7 @@ Unable to determine analysis cache path. Exception: {0}. Using default '{1}'. + + The first argument to NewType must be a string but is of type '{0}'. + \ No newline at end of file diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs b/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs index 6fd4cb1e8..55a0ac535 100644 --- 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())); + 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 typeArgs) { return Interpreter.UnknownType; } - private IPythonType CreateTypeAlias(IReadOnlyList typeArgs) { + private IPythonType CreateTypeAlias(IArgumentSet args) { + var typeArgs = args.Values(); 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, + Diagnostics.ErrorCodes.NewTypeArguments, + Severity.Error, DiagnosticSource.Analysis) + ); } // TODO: report wrong number of arguments return Interpreter.UnknownType; @@ -331,7 +344,7 @@ private IPythonType CreateGenericClassParameter(IReadOnlyList 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 new file mode 100644 index 000000000..ac70404e1 --- /dev/null +++ b/src/Analysis/Ast/Test/LintNewTypeTests.cs @@ -0,0 +1,135 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Analyzer; +using Microsoft.Python.Analysis.Core.Interpreter; +using Microsoft.Python.Analysis.Diagnostics; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Core; +using Microsoft.Python.Parsing; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; +using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; + +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.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + 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.NewTypeArguments); + 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.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + 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.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + 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); + } + } +} From 2980368d3ca5d2bb66da45bab78023c6c1992d86 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Fri, 28 Jun 2019 15:13:06 -0700 Subject: [PATCH 2/4] Cleaning up imports --- src/Analysis/Ast/Test/LintNewTypeTests.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Analysis/Ast/Test/LintNewTypeTests.cs b/src/Analysis/Ast/Test/LintNewTypeTests.cs index ac70404e1..eb3ba2727 100644 --- a/src/Analysis/Ast/Test/LintNewTypeTests.cs +++ b/src/Analysis/Ast/Test/LintNewTypeTests.cs @@ -13,21 +13,14 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using FluentAssertions; -using Microsoft.Python.Analysis.Analyzer; -using Microsoft.Python.Analysis.Core.Interpreter; -using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Tests.FluentAssertions; -using Microsoft.Python.Analysis.Types; using Microsoft.Python.Core; -using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Tests; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; -using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; namespace Microsoft.Python.Analysis.Tests { [TestClass] From 1e6b669f6448a179028537a1ca0898acb2105875 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Tue, 2 Jul 2019 14:33:56 -0700 Subject: [PATCH 3/4] Editing error messages adding null checks --- src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs | 2 +- src/Analysis/Ast/Impl/Resources.Designer.cs | 2 +- src/Analysis/Ast/Impl/Resources.resx | 2 +- .../Impl/Specializations/Typing/TypingModule.cs | 4 ++-- src/Analysis/Ast/Test/LintNewTypeTests.cs | 14 +++++++------- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs index d71308f85..e89f60fce 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -25,6 +25,6 @@ public static class ErrorCodes { public const string UndefinedVariable = "undefined-variable"; public const string VariableNotDefinedGlobally= "variable-not-defined-globally"; public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal"; - public const string NewTypeArguments = "newtype-arguments"; + public const string TypingNewTypeArguments = "typing-newtype-arguments"; } } diff --git a/src/Analysis/Ast/Impl/Resources.Designer.cs b/src/Analysis/Ast/Impl/Resources.Designer.cs index f20d0eee6..2c01b6a38 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -232,7 +232,7 @@ internal static string InterpreterNotFound { } /// - /// Looks up a localized string similar to The first argument to NewType must be a string but is of type '{0}'.. + /// Looks up a localized string similar to The first argument to NewType must be a string, but it is of type '{0}'.. /// internal static string NewTypeFirstArgNotString { get { diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx index 1b2508c8d..3937360ce 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -190,6 +190,6 @@ Unable to determine analysis cache path. Exception: {0}. Using default '{1}'. - The first argument to NewType must be a string but is of type '{0}'. + The first argument to NewType must be a string, but it is of type '{0}'. \ No newline at end of file diff --git a/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs b/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs index 55a0ac535..960f507b2 100644 --- a/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs +++ b/src/Analysis/Ast/Impl/Specializations/Typing/TypingModule.cs @@ -233,8 +233,8 @@ private IPythonType CreateTypeAlias(IArgumentSet args) { eval.ReportDiagnostics( eval.Module?.Uri, new DiagnosticsEntry(Resources.NewTypeFirstArgNotString.FormatInvariant(firstArgType), - expression.GetLocation(eval.Module).Span, - Diagnostics.ErrorCodes.NewTypeArguments, + expression?.GetLocation(eval.Module)?.Span ?? default, + Diagnostics.ErrorCodes.TypingNewTypeArguments, Severity.Error, DiagnosticSource.Analysis) ); } diff --git a/src/Analysis/Ast/Test/LintNewTypeTests.cs b/src/Analysis/Ast/Test/LintNewTypeTests.cs index eb3ba2727..5060c7ad6 100644 --- a/src/Analysis/Ast/Test/LintNewTypeTests.cs +++ b/src/Analysis/Ast/Test/LintNewTypeTests.cs @@ -46,7 +46,7 @@ from typing import NewType analysis.Diagnostics.Should().HaveCount(1); var diagnostic = analysis.Diagnostics.ElementAt(0); - diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments); diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("int")); } @@ -65,7 +65,7 @@ from typing import NewType analysis.Diagnostics.Should().HaveCount(1); var diagnostic = analysis.Diagnostics.ElementAt(0); - diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments); diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant(nameType)); } @@ -86,7 +86,7 @@ def hello(): analysis.Diagnostics.Should().HaveCount(1); var diagnostic = analysis.Diagnostics.ElementAt(0); - diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments); diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("X")); } @@ -108,18 +108,18 @@ def __init__(self, p: T): analysis.Diagnostics.Should().HaveCount(1); var diagnostic = analysis.Diagnostics.ElementAt(0); - diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.NewTypeArguments); + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingNewTypeArguments); diagnostic.Message.Should().Be(Resources.NewTypeFirstArgNotString.FormatInvariant("X[int]")); } - [DataRow("'test'", "float")] - [DataRow("'testing'", "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}) +T = NewType('{name}', {type}) "; var analysis = await GetAnalysisAsync(code); analysis.Diagnostics.Should().HaveCount(0); From 3d8dda6ae2e16ea597fa5bc917c909e5053b17f5 Mon Sep 17 00:00:00 2001 From: Cameron Trando Date: Wed, 3 Jul 2019 10:06:08 -0700 Subject: [PATCH 4/4] Adding source span check to tests --- src/Analysis/Ast/Test/LintNewTypeTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Test/LintNewTypeTests.cs b/src/Analysis/Ast/Test/LintNewTypeTests.cs index 5060c7ad6..aacddbf87 100644 --- a/src/Analysis/Ast/Test/LintNewTypeTests.cs +++ b/src/Analysis/Ast/Test/LintNewTypeTests.cs @@ -40,12 +40,12 @@ public async Task NewTypeIntFirstArg() { 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")); } @@ -86,6 +86,7 @@ def hello(): 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")); } @@ -108,6 +109,7 @@ def __init__(self, p: T): 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]")); }