Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Commit 89bafc3

Browse files
CTrandoCTrando
authored
Give Diagnostic messages on improper usage of Generic (#1248)
* Adding diagnostic message for improper use Generic. * Adding tests to make sure no diagnostics on valid uses of Generic
1 parent b917bda commit 89bafc3

File tree

5 files changed

+155
-23
lines changed

5 files changed

+155
-23
lines changed

‎src/Analysis/Ast/Impl/Analyzer/Evaluation/ExpressionEval.Generics.cs‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
using System;
1717
using System.Collections.Generic;
1818
using System.Linq;
19+
using Microsoft.Python.Analysis.Diagnostics;
1920
using Microsoft.Python.Analysis.Specializations.Typing;
2021
using Microsoft.Python.Analysis.Specializations.Typing.Types;
2122
using Microsoft.Python.Analysis.Types;
2223
using Microsoft.Python.Analysis.Values;
2324
using Microsoft.Python.Core;
25+
using Microsoft.Python.Parsing;
2426
using Microsoft.Python.Parsing.Ast;
27+
using ErrorCodes = Microsoft.Python.Analysis.Diagnostics.ErrorCodes;
2528

2629
namespace Microsoft.Python.Analysis.Analyzer.Evaluation {
2730
internal sealed partial class ExpressionEval {
@@ -59,40 +62,58 @@ private IMember GetValueFromGeneric(IMember target, Expression expr) {
5962
}
6063

6164
/// <summary>
62-
/// Given generic type and list of indices in the expression like
63-
/// Generic[T1, T2, ...] or List[str] creates generic class base
64-
/// (if the former) on specific type (if the latter).
65+
/// Returns whether the arguments to Generic are valid
6566
/// </summary>
66-
private IMember CreateSpecificTypeFromIndex(IGenericType gt, IReadOnlyList<IMember> indices, Expression expr) {
67-
// See which ones are generic parameters as defined by TypeVar()
68-
// and which are specific types. Normally there should not be a mix.
69-
var genericTypeArgs = indices.OfType<IGenericTypeDefinition>().ToArray();
70-
var specificTypes = indices.Where(i => !(i is IGenericTypeDefinition)).OfType<IPythonType>().ToArray();
71-
72-
if (genericTypeArgs.Length > 0 && genericTypeArgs.Length != indices.Count) {
73-
// TODO: report that some type arguments are not declared with TypeVar.
67+
private bool GenericClassParameterValid(IReadOnlyList<IGenericTypeDefinition> genericTypeArgs, IReadOnlyList<IMember> args, Expression expr) {
68+
// All arguments to Generic must be type parameters
69+
// e.g. Generic[T, str] throws a runtime error
70+
if (genericTypeArgs.Count != args.Count) {
71+
ReportDiagnostics(Module.Uri, new DiagnosticsEntry(
72+
Resources.GenericNotAllTypeParameters,
73+
GetLocation(expr).Span,
74+
ErrorCodes.TypingGenericArguments,
75+
Severity.Error,
76+
DiagnosticSource.Analysis));
77+
return false;
7478
}
75-
if (specificTypes.Length > 0 && specificTypes.Length != indices.Count) {
76-
// TODO: report that arguments are not specific types or are not declared.
79+
80+
// All arguments to Generic must be distinct
81+
if (genericTypeArgs.Distinct().Count() != genericTypeArgs.Count) {
82+
ReportDiagnostics(Module.Uri, new DiagnosticsEntry(
83+
Resources.GenericNotAllUnique,
84+
GetLocation(expr).Span,
85+
ErrorCodes.TypingGenericArguments,
86+
Severity.Error,
87+
DiagnosticSource.Analysis));
88+
return false;
7789
}
7890

91+
return true;
92+
}
93+
94+
/// <summary>
95+
/// Given generic type and list of arguments in the expression like
96+
/// Generic[T1, T2, ...] or List[str] creates generic class base
97+
/// (if the former) on specific type (if the latter).
98+
/// </summary>
99+
private IMember CreateSpecificTypeFromIndex(IGenericType gt, IReadOnlyList<IMember> args, Expression expr) {
100+
var genericTypeArgs = args.OfType<IGenericTypeDefinition>().ToArray();
101+
79102
if (gt.Name.EqualsOrdinal("Generic")) {
80-
// Generic[T1, T2, ...] expression. Create generic base for the class.
81-
if (genericTypeArgs.Length > 0) {
82-
return new GenericClassParameter(genericTypeArgs, Module);
83-
} else {
84-
// TODO: report too few type arguments for Generic[].
103+
if (!GenericClassParameterValid(genericTypeArgs, args, expr)) {
85104
return UnknownType;
86105
}
106+
107+
// Generic[T1, T2, ...] expression. Create generic base for the class.
108+
return new GenericClassParameter(genericTypeArgs, Module);
87109
}
88110

89111
// For other types just use supplied arguments
90-
if (indices.Count > 0) {
91-
return gt.CreateSpecificType(new ArgumentSet(indices, expr, this));
112+
if (args.Count > 0) {
113+
return gt.CreateSpecificType(new ArgumentSet(args, expr, this));
92114
}
93-
// TODO: report too few type arguments for the generic expression.
94-
return UnknownType;
95115

116+
return UnknownType;
96117
}
97118

98119
private IReadOnlyList<IMember> EvaluateIndex(IndexExpression expr) {
@@ -194,7 +215,7 @@ private static IReadOnlyList<IPythonType> GetSpecificTypeFromArgumentValue(objec
194215
var itemType = iter.GetIterator().Next.GetPythonType();
195216
if (!itemType.IsUnknown()) {
196217
specificTypes.Add(itemType);
197-
} else if(argumentValue is IPythonInstance inst) {
218+
} else if (argumentValue is IPythonInstance inst) {
198219
specificTypes.Add(inst.GetPythonType());
199220
}
200221
break;

‎src/Analysis/Ast/Impl/Diagnostics/ErrorCodes.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,6 @@ public static class ErrorCodes {
2626
public const string VariableNotDefinedGlobally= "variable-not-defined-globally";
2727
public const string VariableNotDefinedNonLocal = "variable-not-defined-nonlocal";
2828
public const string ReturnInInit = "return-in-init";
29+
public const string TypingGenericArguments = "typing-generic-arguments";
2930
}
3031
}

‎src/Analysis/Ast/Impl/Resources.Designer.cs‎

Lines changed: 18 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Analysis/Ast/Impl/Resources.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,10 @@
192192
<data name="ReturnInInit" xml:space="preserve">
193193
<value>Explicit return in __init__ </value>
194194
</data>
195+
<data name="GenericNotAllTypeParameters" xml:space="preserve">
196+
<value>Arguments to Generic must all be type parameters.</value>
197+
</data>
198+
<data name="GenericNotAllUnique" xml:space="preserve">
199+
<value>Arguments to Generic must all be unique.</value>
200+
</data>
195201
</root>

‎src/Analysis/Ast/Test/LintGenericTests.cs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
using System.Linq;
2+
using System.Threading.Tasks;
3+
using FluentAssertions;
4+
using Microsoft.Python.Analysis.Tests.FluentAssertions;
5+
using Microsoft.Python.Parsing.Tests;
6+
using Microsoft.VisualStudio.TestTools.UnitTesting;
7+
using TestUtilities;
8+
9+
10+
namespace Microsoft.Python.Analysis.Tests {
11+
12+
[TestClass]
13+
public class LintGenericTests : AnalysisTestBase {
14+
15+
public const string GenericSetup = @"
16+
from typing import Generic, TypeVar
17+
T = TypeVar('T', int, str)
18+
T1 = TypeVar('T1', int, str)
19+
20+
_X = TypeVar('_X', str, int)
21+
_T = _X
22+
";
23+
24+
public TestContext TestContext { get; set; }
25+
26+
[TestInitialize]
27+
public void TestInitialize()
28+
=> TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}");
29+
30+
[TestCleanup]
31+
public void Cleanup() => TestEnvironmentImpl.TestCleanup();
32+
33+
[DataRow("x = Generic[T, str]")]
34+
[DataRow("x = Generic[T, T1, int]")]
35+
[DataRow("x = Generic[T, str, int, T1]")]
36+
[DataRow("x = Generic[str, int]")]
37+
[DataRow("x = Generic[str]")]
38+
[DataTestMethod, Priority(0)]
39+
public async Task GenericNotAllTypeParameters(string decl) {
40+
string code = GenericSetup + decl;
41+
42+
var analysis = await GetAnalysisAsync(code);
43+
analysis.Diagnostics.Should().HaveCount(1);
44+
45+
var diagnostic = analysis.Diagnostics.ElementAt(0);
46+
var start = decl.IndexOf("Generic") + 1;
47+
// adding 1 because SourceSpan.End is exclusive and another 1 because SourceSpan is 1-indexed
48+
var end = decl.IndexOf("]", start) + 2;
49+
50+
diagnostic.SourceSpan.Should().Be(8, start, 8, end);
51+
diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingGenericArguments);
52+
diagnostic.Message.Should().Be(Resources.GenericNotAllTypeParameters);
53+
}
54+
55+
[DataRow("x = Generic[_T, _X]")]
56+
[DataRow("x = Generic[_T, T, T1, _X]")]
57+
[DataRow("x = Generic[_T,_T, T]")]
58+
[DataRow("x = Generic[T,T]")]
59+
[DataTestMethod, Priority(0)]
60+
public async Task GenericDuplicateArguments(string decl) {
61+
string code = GenericSetup + decl;
62+
var analysis = await GetAnalysisAsync(code);
63+
analysis.Diagnostics.Should().HaveCount(1);
64+
65+
var diagnostic = analysis.Diagnostics.ElementAt(0);
66+
var start = decl.IndexOf("Generic") + 1;
67+
// adding 1 because SourceSpan.End is exclusive and another 1 because SourceSpan is 1-indexed
68+
var end = decl.IndexOf("]", start) + 2;
69+
diagnostic.SourceSpan.Should().Be(8, start, 8, end);
70+
71+
diagnostic.ErrorCode.Should().Be(Diagnostics.ErrorCodes.TypingGenericArguments);
72+
diagnostic.Message.Should().Be(Resources.GenericNotAllUnique);
73+
}
74+
75+
[DataRow("x = Generic[_X, T]")]
76+
[DataRow("x = Generic[T1, T]")]
77+
[DataRow("x = Generic[T]")]
78+
[DataRow("x = Generic[T,T1, _X]")]
79+
[DataTestMethod, Priority(0)]
80+
public async Task GenericArgumentsNoDiagnosticOnValid(string decl) {
81+
string code = GenericSetup + decl;
82+
var analysis = await GetAnalysisAsync(code);
83+
analysis.Diagnostics.Should().HaveCount(0);
84+
}
85+
}
86+
}

0 commit comments

Comments
 (0)