Skip to content

Make it a noImplicitAny error to fail to provide type arguments to a superclass via @augments #18778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
7 commits merged into from
Oct 20, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6352,11 +6352,11 @@ namespace ts {
* @param typeParameters The requested type parameters.
* @param minTypeArgumentCount The minimum number of required type arguments.
*/
function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScript: boolean) {
function fillMissingTypeArguments(typeArguments: Type[] | undefined, typeParameters: TypeParameter[] | undefined, minTypeArgumentCount: number, isJavaScriptImplicitAny: boolean) {
const numTypeParameters = length(typeParameters);
if (numTypeParameters) {
const numTypeArguments = length(typeArguments);
if ((isJavaScript || numTypeArguments >= minTypeArgumentCount) && numTypeArguments <= numTypeParameters) {
if ((isJavaScriptImplicitAny || numTypeArguments >= minTypeArgumentCount) && numTypeArguments <= numTypeParameters) {
if (!typeArguments) {
typeArguments = [];
}
Expand All @@ -6365,12 +6365,12 @@ namespace ts {
// If a type parameter does not have a default type, or if the default type
// is a forward reference, the empty object type is used.
for (let i = numTypeArguments; i < numTypeParameters; i++) {
typeArguments[i] = getDefaultTypeArgumentType(isJavaScript);
typeArguments[i] = getDefaultTypeArgumentType(isJavaScriptImplicitAny);
}
for (let i = numTypeArguments; i < numTypeParameters; i++) {
const mapper = createTypeMapper(typeParameters, typeArguments);
const defaultType = getDefaultFromTypeParameter(typeParameters[i]);
typeArguments[i] = defaultType ? instantiateType(defaultType, mapper) : getDefaultTypeArgumentType(isJavaScript);
typeArguments[i] = defaultType ? instantiateType(defaultType, mapper) : getDefaultTypeArgumentType(isJavaScriptImplicitAny);
}
}
}
Expand Down Expand Up @@ -6806,21 +6806,24 @@ namespace ts {
if (typeParameters) {
const numTypeArguments = length(node.typeArguments);
const minTypeArgumentCount = getMinTypeArgumentCount(typeParameters);
const isJavascript = isInJavaScriptFile(node);
if (!isJavascript && (numTypeArguments < minTypeArgumentCount || numTypeArguments > typeParameters.length)) {
error(node,
minTypeArgumentCount === typeParameters.length
? Diagnostics.Generic_type_0_requires_1_type_argument_s
: Diagnostics.Generic_type_0_requires_between_1_and_2_type_arguments,
typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType),
minTypeArgumentCount,
typeParameters.length);
const isJs = isInJavaScriptFile(node);
const isJsImplicitAny = !compilerOptions.noImplicitAny && isJs;
if (!isJsImplicitAny && (numTypeArguments < minTypeArgumentCount || numTypeArguments > typeParameters.length)) {
const diag = minTypeArgumentCount === typeParameters.length
? isJs
? Diagnostics.Generic_type_0_requires_1_type_arguments_provide_these_with_an_augments_or_extends_tag
: Diagnostics.Generic_type_0_requires_1_type_argument_s
: isJs
? Diagnostics.Generic_type_0_requires_between_1_and_2_type_arguments_provide_these_with_an_augments_or_extends_tag
: Diagnostics.Generic_type_0_requires_between_1_and_2_type_arguments;
const typeStr = typeToString(type, /*enclosingDeclaration*/ undefined, TypeFormatFlags.WriteArrayAsGenericType);
error(node, diag, typeStr, minTypeArgumentCount, typeParameters.length);
return unknownType;
}
// In a type reference, the outer type parameters of the referenced class or interface are automatically
// supplied as type arguments and the type reference only specifies arguments for the local type parameters
// of the class or interface.
const typeArguments = concatenate(type.outerTypeParameters, fillMissingTypeArguments(typeArgs, typeParameters, minTypeArgumentCount, isJavascript));
const typeArguments = concatenate(type.outerTypeParameters, fillMissingTypeArguments(typeArgs, typeParameters, minTypeArgumentCount, isJsImplicitAny));
return createTypeReference(<GenericType>type, typeArguments);
}
if (node.typeArguments) {
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3511,6 +3511,14 @@
"category": "Error",
"code": 8021
},
"Generic type '{0}' requires '{1}' type arguments; provide these with an '@augments' or '@extends' tag.": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd choose one of @augments or @extends for the error, preferably the one that is in the source if it is already there.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There probably won't be one already there; we might just recommend @extends since that's the more intuitive name.

"category": "Error",
"code": 8022
},
"Generic type '{0}' requires between '{1}' and '{2}' type arguments; provide these with an '@augments' or '@extends' tag.": {
"category": "Error",
"code": 8023
},
"Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clause.": {
"category": "Error",
"code": 9002
Expand Down
11 changes: 11 additions & 0 deletions tests/baselines/reference/jsExtendsImplicitAny.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/b.js(1,17): error TS8022: Generic type 'A<T>' requires '1' type arguments; provide these with an '@augments' or '@extends' tag.


==== /a.d.ts (0 errors) ====
declare class A<T> { x: T; }

==== /b.js (1 errors) ====
class B extends A {}
~
!!! error TS8022: Generic type 'A<T>' requires '1' type arguments; provide these with an '@augments' or '@extends' tag.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for

/** @augments A */
class B { }

There should be an error on A on the first line, I think.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires getting #18775 in first.


12 changes: 12 additions & 0 deletions tests/baselines/reference/jsExtendsImplicitAny.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /a.d.ts ===
declare class A<T> { x: T; }
>A : Symbol(A, Decl(a.d.ts, 0, 0))
>T : Symbol(T, Decl(a.d.ts, 0, 16))
>x : Symbol(A.x, Decl(a.d.ts, 0, 20))
>T : Symbol(T, Decl(a.d.ts, 0, 16))

=== /b.js ===
class B extends A {}
>B : Symbol(B, Decl(b.js, 0, 0))
>A : Symbol(A, Decl(a.d.ts, 0, 0))

12 changes: 12 additions & 0 deletions tests/baselines/reference/jsExtendsImplicitAny.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== /a.d.ts ===
declare class A<T> { x: T; }
>A : A<T>
>T : T
>x : T
>T : T

=== /b.js ===
class B extends A {}
>B : B
>A : typeof A

10 changes: 10 additions & 0 deletions tests/cases/compiler/jsExtendsImplicitAny.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @noImplicitAny: true

// @Filename: /a.d.ts
declare class A<T> { x: T; }

// @Filename: /b.js
class B extends A {}