Skip to content

getEffectiveTypeParameterDeclarations: Always return a defined result #24251

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.

Sign up for GitHub

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
1 commit merged into from
May 18, 2018
Merged

getEffectiveTypeParameterDeclarations: Always return a defined result #24251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 16 additions & 19 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5165,7 +5165,7 @@ namespace ts {
// Appends the type parameters given by a list of declarations to a set of type parameters and returns the resulting set.
// The function allocates a new array if the input type parameter set is undefined, but otherwise it modifies the set
// in-place and returns the same array.
function appendTypeParameters(typeParameters: TypeParameter[], declarations: ReadonlyArray<TypeParameterDeclaration>): TypeParameter[] {
function appendTypeParameters(typeParameters: TypeParameter[] | undefined, declarations: ReadonlyArray<TypeParameterDeclaration>): TypeParameter[] {
for (const declaration of declarations) {
typeParameters = appendIfUnique(typeParameters, getDeclaredTypeOfTypeParameter(getSymbolOfNode(declaration)));
}
Expand Down Expand Up @@ -5206,7 +5206,7 @@ namespace ts {
else if (node.kind === SyntaxKind.ConditionalType) {
return concatenate(outerTypeParameters, getInferTypeParameters(<ConditionalTypeNode>node));
}
const outerAndOwnTypeParameters = appendTypeParameters(outerTypeParameters, getEffectiveTypeParameterDeclarations(<DeclarationWithTypeParameters>node) || emptyArray);
const outerAndOwnTypeParameters = appendTypeParameters(outerTypeParameters, getEffectiveTypeParameterDeclarations(<DeclarationWithTypeParameters>node));
const thisType = includeThisTypes &&
(node.kind === SyntaxKind.ClassDeclaration || node.kind === SyntaxKind.ClassExpression || node.kind === SyntaxKind.InterfaceDeclaration) &&
getDeclaredTypeOfClassOrInterface(getSymbolOfNode(node)).thisType;
Expand All @@ -5224,17 +5224,14 @@ namespace ts {
// The local type parameters are the combined set of type parameters from all declarations of the class,
// interface, or type alias.
function getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): TypeParameter[] {
let result: TypeParameter[];
let result: TypeParameter[] | undefined;
for (const node of symbol.declarations) {
if (node.kind === SyntaxKind.InterfaceDeclaration ||
node.kind === SyntaxKind.ClassDeclaration ||
node.kind === SyntaxKind.ClassExpression ||
isTypeAlias(node)) {
const declaration = <InterfaceDeclaration | TypeAliasDeclaration | JSDocTypedefTag | JSDocCallbackTag>node;
const typeParameters = getEffectiveTypeParameterDeclarations(declaration);
if (typeParameters) {
result = appendTypeParameters(result, typeParameters);
}
result = appendTypeParameters(result, getEffectiveTypeParameterDeclarations(declaration));
}
}
return result;
Expand Down Expand Up @@ -5744,7 +5741,7 @@ namespace ts {
const typeParameters = getEffectiveTypeParameterDeclarations(node);
return (node.kind === SyntaxKind.Constructor || (returnType && isThislessType(returnType))) &&
node.parameters.every(isThislessVariableLikeDeclaration) &&
(!typeParameters || typeParameters.every(isThislessTypeParameter));
typeParameters.every(isThislessTypeParameter);
}

/**
Expand Down Expand Up @@ -7071,11 +7068,11 @@ namespace ts {

// Return list of type parameters with duplicates removed (duplicate identifier errors are generated in the actual
// type checking functions).
function getTypeParametersFromDeclaration(declaration: DeclarationWithTypeParameters): TypeParameter[] {
let result: TypeParameter[];
forEach(getEffectiveTypeParameterDeclarations(declaration), node => {
function getTypeParametersFromDeclaration(declaration: DeclarationWithTypeParameters): TypeParameter[] | undefined {
let result: TypeParameter[] | undefined;
for (const node of getEffectiveTypeParameterDeclarations(declaration)) {
result = appendIfUnique(result, getDeclaredTypeOfTypeParameter(node.symbol));
});
}
return result;
}

Expand Down Expand Up @@ -22679,7 +22676,7 @@ namespace ts {
// Only report errors on the last declaration for the type parameter container;
// this ensures that all uses have been accounted for.
const typeParameters = getEffectiveTypeParameterDeclarations(node);
if (!(node.flags & NodeFlags.Ambient) && typeParameters && last(getSymbolOfNode(node)!.declarations) === node) {
if (!(node.flags & NodeFlags.Ambient) && last(getSymbolOfNode(node)!.declarations) === node) {
for (const typeParameter of typeParameters) {
if (!(getMergedSymbol(typeParameter.symbol).isReferenced & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {
addDiagnostic(UnusedKind.Parameter, createDiagnosticForNode(typeParameter.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(typeParameter.symbol)));
Expand Down Expand Up @@ -24047,7 +24044,7 @@ namespace ts {
/**
* Check each type parameter and check that type parameters have no duplicate type parameter declarations
*/
function checkTypeParameters(typeParameterDeclarations: ReadonlyArray<TypeParameterDeclaration>) {
function checkTypeParameters(typeParameterDeclarations: ReadonlyArray<TypeParameterDeclaration> | undefined) {
if (typeParameterDeclarations) {
let seenDefault = false;
for (let i = 0; i < typeParameterDeclarations.length; i++) {
Expand Down Expand Up @@ -24103,7 +24100,7 @@ namespace ts {
for (const declaration of declarations) {
// If this declaration has too few or too many type parameters, we report an error
const sourceParameters = getEffectiveTypeParameterDeclarations(declaration);
const numTypeParameters = length(sourceParameters);
const numTypeParameters = sourceParameters.length;
if (numTypeParameters < minTypeArgumentCount || numTypeParameters > maxTypeArgumentCount) {
return false;
}
Expand Down Expand Up @@ -27371,7 +27368,7 @@ namespace ts {
}
}

function checkGrammarTypeParameterList(typeParameters: NodeArray<TypeParameterDeclaration>, file: SourceFile): boolean {
function checkGrammarTypeParameterList(typeParameters: NodeArray<TypeParameterDeclaration> | undefined, file: SourceFile): boolean {
if (typeParameters && typeParameters.length === 0) {
const start = typeParameters.pos - "<".length;
const end = skipTrivia(file.text, typeParameters.end) + ">".length;
Expand Down Expand Up @@ -27427,7 +27424,7 @@ namespace ts {

function checkGrammarClassLikeDeclaration(node: ClassLikeDeclaration): boolean {
const file = getSourceFileOfNode(node);
return checkGrammarClassDeclarationHeritageClauses(node) || checkGrammarTypeParameterList(getEffectiveTypeParameterDeclarations(node), file);
return checkGrammarClassDeclarationHeritageClauses(node) || checkGrammarTypeParameterList(node.typeParameters, file);
}

function checkGrammarArrowFunction(node: Node, file: SourceFile): boolean {
Expand Down Expand Up @@ -28199,8 +28196,8 @@ namespace ts {

function checkGrammarConstructorTypeParameters(node: ConstructorDeclaration) {
const typeParameters = getEffectiveTypeParameterDeclarations(node);
if (typeParameters) {
const { pos, end } = isNodeArray(typeParameters) ? typeParameters : first(typeParameters);
if (isNodeArray(typeParameters)) {
Copy link
Member

Choose a reason for hiding this comment

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

Based on my reading of the new code, isNodeArray is only false when typeParameters is empty. So how did first(typeParameters) ever work? Was it just dead code?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, dead code. Maybe we should be linting that we never call isFoo(x) if x is already Foo, or is a type not assignable to Foo. (E.g. we shouldn't be able to call isIfStatement on an Expression.)

const { pos, end } = typeParameters;
return grammarErrorAtPos(node, pos, end - pos, Diagnostics.Type_parameters_cannot_appear_on_a_constructor_declaration);
}
}
Expand Down
17 changes: 7 additions & 10 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3101,9 +3101,9 @@ namespace ts {
* Gets the effective type parameters. If the node was parsed in a
* JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
*/
export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters) {
export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration> {
if (isJSDocSignature(node)) {
return undefined;
return emptyArray;
}
if (isJSDocTypeAlias(node)) {
Debug.assert(node.parent.kind === SyntaxKind.JSDocComment);
Expand All @@ -3114,17 +3114,14 @@ namespace ts {
templateTagNodes.hasTrailingComma = false;
return templateTagNodes;
}
return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : undefined);
return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : emptyArray);
}

export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters) {
export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters): ReadonlyArray<TypeParameterDeclaration> {
const tags = filter(getJSDocTags(node), isJSDocTemplateTag);
for (const tag of tags) {
if (!(tag.parent.kind === SyntaxKind.JSDocComment && find(tag.parent.tags, isJSDocTypeAlias))) {
// template tags are only available when a typedef isn't already using them
return tag.typeParameters;
}
}
// template tags are only available when a typedef isn't already using them
const tag = find(tags, tag => !(tag.parent.kind === SyntaxKind.JSDocComment && find(tag.parent.tags, isJSDocTypeAlias)));
return (tag && tag.typeParameters) || emptyArray;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/services/codefixes/annotateWithTypeFromJSDoc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ namespace ts.codefix {
if (isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)))) {
if (!decl.typeParameters) {
const typeParameters = getJSDocTypeParameterDeclarations(decl);
if (typeParameters) changes.insertTypeParameters(sourceFile, decl, typeParameters);
if (typeParameters.length) changes.insertTypeParameters(sourceFile, decl, typeParameters);
}
const needParens = isArrowFunction(decl) && !findChildOfKind(decl, SyntaxKind.OpenParenToken, sourceFile);
if (needParens) changes.insertNodeBefore(sourceFile, first(decl.parameters), createToken(SyntaxKind.OpenParenToken));
Expand Down
5 changes: 3 additions & 2 deletions src/services/codefixes/fixUnusedIdentifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,9 @@ namespace ts.codefix {
case SyntaxKind.TypeParameter:
const typeParameters = getEffectiveTypeParameterDeclarations(<DeclarationWithTypeParameters>parent.parent);
if (typeParameters.length === 1) {
const previousToken = getTokenAtPosition(sourceFile, typeParameters.pos - 1, /*includeJsDocComment*/ false);
const nextToken = getTokenAtPosition(sourceFile, typeParameters.end, /*includeJsDocComment*/ false);
const { pos, end } = cast(typeParameters, isNodeArray);
const previousToken = getTokenAtPosition(sourceFile, pos - 1, /*includeJsDocComment*/ false);
const nextToken = getTokenAtPosition(sourceFile, end, /*includeJsDocComment*/ false);
Debug.assert(previousToken.kind === SyntaxKind.LessThanToken);
Debug.assert(nextToken.kind === SyntaxKind.GreaterThanToken);

Expand Down
16 changes: 2 additions & 14 deletions src/services/refactors/extractSymbol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,7 @@ namespace ts.refactor.extractSymbol {
}

// Note that we add the current node's type parameters *after* updating the corresponding scope.
if (isDeclarationWithTypeParameters(curr) && getEffectiveTypeParameterDeclarations(curr)) {
if (isDeclarationWithTypeParameters(curr)) {
for (const typeParameterDecl of getEffectiveTypeParameterDeclarations(curr)) {
const typeParameter = checker.getTypeAtLocation(typeParameterDecl) as TypeParameter;
if (allTypeParameterUsages.has(typeParameter.id.toString())) {
Expand Down Expand Up @@ -1534,20 +1534,8 @@ namespace ts.refactor.extractSymbol {

return { target, usagesPerScope, functionErrorsPerScope, constantErrorsPerScope, exposedVariableDeclarations };

function hasTypeParameters(node: Node) {
return isDeclarationWithTypeParameters(node) &&
getEffectiveTypeParameterDeclarations(node) &&
getEffectiveTypeParameterDeclarations(node).length > 0;
}

function isInGenericContext(node: Node) {
for (; node; node = node.parent) {
if (hasTypeParameters(node)) {
return true;
}
}

return false;
return !!findAncestor(node, n => isDeclarationWithTypeParameters(n) && getEffectiveTypeParameterDeclarations(n).length !== 0);
}

function recordTypeParameterUsages(type: Type) {
Expand Down