diff --git a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Generics.cs b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Generics.cs index f815271f7..b36242ba2 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Generics.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Generics.cs @@ -16,12 +16,15 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.Python.Analysis.Diagnostics; using Microsoft.Python.Analysis.Specializations.Typing; using Microsoft.Python.Analysis.Specializations.Typing.Types; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; +using Microsoft.Python.Parsing; using Microsoft.Python.Parsing.Ast; +using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes; namespace Microsoft.Python.Analysis.Analyzer.Evaluation { internal sealed partial class ExpressionEval { @@ -59,40 +62,58 @@ private IMember GetValueFromGeneric(IMember target, Expression expr) { } /// - /// Given generic type and list of indices in the expression like - /// Generic[T1, T2, ...] or List[str] creates generic class base - /// (if the former) on specific type (if the latter). + /// Returns whether the arguments to Generic are valid /// - private IMember CreateSpecificTypeFromIndex(IGenericType gt, IReadOnlyList indices, Expression expr) { - // See which ones are generic parameters as defined by TypeVar() - // and which are specific types. Normally there should not be a mix. - var genericTypeArgs = indices.OfType().ToArray(); - var specificTypes = indices.Where(i => !(i is IGenericTypeDefinition)).OfType().ToArray(); - - if (genericTypeArgs.Length > 0 && genericTypeArgs.Length != indices.Count) { - // TODO: report that some type arguments are not declared with TypeVar. + private bool GenericClassParameterValid(IReadOnlyList genericTypeArgs, IReadOnlyList args, Expression expr) { + // All arguments to Generic must be type parameters + // e.g. Generic[T, str] throws a runtime error + if (genericTypeArgs.Count != args.Count) { + ReportDiagnostics(Module.Uri, new DiagnosticsEntry( + Resources.GenericNotAllTypeParameters, + GetLocation(expr).Span, + ErrorCodes.TypingGenericArguments, + Severity.Error, + DiagnosticSource.Analysis)); + return false; } - if (specificTypes.Length > 0 && specificTypes.Length != indices.Count) { - // TODO: report that arguments are not specific types or are not declared. + + // All arguments to Generic must be distinct + if (genericTypeArgs.Distinct().Count() != genericTypeArgs.Count) { + ReportDiagnostics(Module.Uri, new DiagnosticsEntry( + Resources.GenericNotAllUnique, + GetLocation(expr).Span, + ErrorCodes.TypingGenericArguments, + Severity.Error, + DiagnosticSource.Analysis)); + return false; } + return true; + } + + /// + /// Given generic type and list of arguments in the expression like + /// Generic[T1, T2, ...] or List[str] creates generic class base + /// (if the former) on specific type (if the latter). + /// + private IMember CreateSpecificTypeFromIndex(IGenericType gt, IReadOnlyList args, Expression expr) { + var genericTypeArgs = args.OfType().ToArray(); + if (gt.Name.EqualsOrdinal("Generic")) { - // Generic[T1, T2, ...] expression. Create generic base for the class. - if (genericTypeArgs.Length > 0) { - return new GenericClassParameter(genericTypeArgs, Module); - } else { - // TODO: report too few type arguments for Generic[]. + if (!GenericClassParameterValid(genericTypeArgs, args, expr)) { return UnknownType; } + + // Generic[T1, T2, ...] expression. Create generic base for the class. + return new GenericClassParameter(genericTypeArgs, Module); } // For other types just use supplied arguments - if (indices.Count > 0) { - return gt.CreateSpecificType(new ArgumentSet(indices, expr, this)); + if (args.Count > 0) { + return gt.CreateSpecificType(new ArgumentSet(args, expr, this)); } - // TODO: report too few type arguments for the generic expression. - return UnknownType; + return UnknownType; } private IReadOnlyList EvaluateIndex(IndexExpression expr) { @@ -194,7 +215,7 @@ private static IReadOnlyList GetSpecificTypeFromArgumentValue(objec var itemType = iter.GetIterator().Next.GetPythonType(); if (!itemType.IsUnknown()) { specificTypes.Add(itemType); - } else if(argumentValue is IPythonInstance inst) { + } else if (argumentValue is IPythonInstance inst) { specificTypes.Add(inst.GetPythonType()); } break; diff --git a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs index fdf6f03f8..fd6d5f0c9 100644 --- a/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs +++ b/src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs @@ -26,5 +26,6 @@ public static class ErrorCodes { public const string VariableNotDefinedGlobally= "variable-not-defined-globally"; public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal"; public const string ReturnInInit = "return-in-init"; + 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 index 365bba623..df406b0eb 100644 --- a/src/Analysis/Ast/Impl/Resources.Designer.cs +++ b/src/Analysis/Ast/Impl/Resources.Designer.cs @@ -222,6 +222,24 @@ internal static string ExceptionGettingSearchPaths { } } + /// + /// Looks up a localized string similar to Arguments to Generic must all be type parameters.. + /// + internal static string GenericNotAllTypeParameters { + get { + return ResourceManager.GetString("GenericNotAllTypeParameters", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Arguments to Generic must all be unique.. + /// + internal static string GenericNotAllUnique { + get { + return ResourceManager.GetString("GenericNotAllUnique", resourceCulture); + } + } + /// /// Looks up a localized string similar to Interpreter does not exist; analysis will not be available.. /// diff --git a/src/Analysis/Ast/Impl/Resources.resx b/src/Analysis/Ast/Impl/Resources.resx index b882f93f7..e9a661561 100644 --- a/src/Analysis/Ast/Impl/Resources.resx +++ b/src/Analysis/Ast/Impl/Resources.resx @@ -192,4 +192,10 @@ Explicit return in __init__ + + Arguments to Generic must all be type parameters. + + + Arguments to Generic must all be unique. + \ No newline at end of file diff --git a/src/Analysis/Ast/Test/LintGenericTests.cs b/src/Analysis/Ast/Test/LintGenericTests.cs new file mode 100644 index 000000000..86f09faba --- /dev/null +++ b/src/Analysis/Ast/Test/LintGenericTests.cs @@ -0,0 +1,86 @@ +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Analysis.Tests.FluentAssertions; +using Microsoft.Python.Parsing.Tests; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + + +namespace Microsoft.Python.Analysis.Tests { + + [TestClass] + public class LintGenericTests : AnalysisTestBase { + + public const string GenericSetup = @" +from typing import Generic, TypeVar +T = TypeVar('T', int, str) +T1 = TypeVar('T1', int, str) + +_X = TypeVar('_X', str, int) +_T = _X +"; + + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [DataRow("x = Generic[T, str]")] + [DataRow("x = Generic[T, T1, int]")] + [DataRow("x = Generic[T, str, int, T1]")] + [DataRow("x = Generic[str, int]")] + [DataRow("x = Generic[str]")] + [DataTestMethod, Priority(0)] + public async Task GenericNotAllTypeParameters(string decl) { + string code = GenericSetup + decl; + + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + var start = decl.IndexOf("Generic") + 1; + // adding 1 because SourceSpan.End is exclusive and another 1 because SourceSpan is 1-indexed + var end = decl.IndexOf("]", start) + 2; + + diagnostic.SourceSpan.Should().Be(8, start, 8, end); + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingGenericArguments); + diagnostic.Message.Should().Be(Resources.GenericNotAllTypeParameters); + } + + [DataRow("x = Generic[_T, _X]")] + [DataRow("x = Generic[_T, T, T1, _X]")] + [DataRow("x = Generic[_T,_T, T]")] + [DataRow("x = Generic[T,T]")] + [DataTestMethod, Priority(0)] + public async Task GenericDuplicateArguments(string decl) { + string code = GenericSetup + decl; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(1); + + var diagnostic = analysis.Diagnostics.ElementAt(0); + var start = decl.IndexOf("Generic") + 1; + // adding 1 because SourceSpan.End is exclusive and another 1 because SourceSpan is 1-indexed + var end = decl.IndexOf("]", start) + 2; + diagnostic.SourceSpan.Should().Be(8, start, 8, end); + + diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingGenericArguments); + diagnostic.Message.Should().Be(Resources.GenericNotAllUnique); + } + + [DataRow("x = Generic[_X, T]")] + [DataRow("x = Generic[T1, T]")] + [DataRow("x = Generic[T]")] + [DataRow("x = Generic[T,T1, _X]")] + [DataTestMethod, Priority(0)] + public async Task GenericArgumentsNoDiagnosticOnValid(string decl) { + string code = GenericSetup + decl; + var analysis = await GetAnalysisAsync(code); + analysis.Diagnostics.Should().HaveCount(0); + } + } +}