Skip to content
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

Reduce false positive reports of IsolatedDeclarations errors #93

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
15 changes: 11 additions & 4 deletions external-declarations/src/code-mod/code-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,19 @@ const canHaveExplicitTypeAnnotation = new Set<ts.SyntaxKind>([
ts.SyntaxKind.ArrayBindingPattern,
]);

// Currently, the diagnostics for the error is not given in the exact node of which that needs type annotation
function findNearestParentWithTypeAnnotation(node: ts.Node): ts.Node {
while (((ts.isObjectBindingPattern(node) || ts.isArrayBindingPattern(node)) && !ts.isVariableDeclaration(node.parent)) ||
!canHaveExplicitTypeAnnotation.has(node.kind)) {
// Currently, the diagnostics for the error is not given in the exact node of which that needs type annotation.
// If this is coming from an ill-formed AST with syntax errors, you cannot assume that it'll find a node
// to annotate types, this will return undefined - meaning that it couldn't find the node to annotate types.
function findNearestParentWithTypeAnnotation(node: ts.Node): ts.Node | undefined {
while (node &&
(((ts.isObjectBindingPattern(node) || ts.isArrayBindingPattern(node)) &&
!ts.isVariableDeclaration(node.parent)) ||
!canHaveExplicitTypeAnnotation.has(node.kind))) {
node = node.parent;
}
if (!node) {
return undefined;
}
if (ts.isObjectBindingPattern(node) || ts.isArrayBindingPattern(node)) {
// return VariableStatement
return node.parent.parent.parent;
Expand Down
21 changes: 17 additions & 4 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ import {
isInterfaceDeclaration,
isJsonSourceFile,
isLateVisibilityPaintedStatement,
isLiteralExpression,
isLiteralImportTypeNode,
isMappedTypeNode,
isMethodDeclaration,
Expand All @@ -151,6 +152,7 @@ import {
isTupleTypeNode,
isTypeAliasDeclaration,
isTypeElement,
isTypeLiteralNode,
isTypeNode,
isTypeParameterDeclaration,
isTypeQueryNode,
Expand Down Expand Up @@ -1147,7 +1149,10 @@ export function transformDeclarations(context: TransformationContext) {
if (isDeclaration(input)) {
if (isDeclarationAndNotVisible(input)) return;
if (hasDynamicName(input) && !resolver.isLateBound(getParseTreeNode(input) as Declaration)) {
if (isolatedDeclarations && hasIdentifierComputedName(input)) {
if (isolatedDeclarations && hasIdentifierComputedName(input) &&
// When --noImplicitAny is off, it's automatically 'any' type so we shouldn't complain.
// when it's on, it should be an error on the noImplicitAny side, so we also shouldn't complain.
!isInterfaceDeclaration(input.parent) && !isTypeLiteralNode(input.parent)) {
reportIsolatedDeclarationError(input);
}
else {
Expand Down Expand Up @@ -1784,7 +1789,13 @@ export function transformDeclarations(context: TransformationContext) {
// We must add a temporary declaration for the extends clause expression

// Isolated declarations does not allow inferred type in the extends clause
if(isolatedDeclarations) {
if (isolatedDeclarations &&
// Checking if it's a separate compiler error so we don't make it an isolatedDeclarations error.
// This is only an approximation as we need type information to figure out if something
// is a constructor type or not.
!isLiteralExpression(extendsClause.expression) &&
extendsClause.expression.kind !== SyntaxKind.FalseKeyword &&
extendsClause.expression.kind !== SyntaxKind.TrueKeyword) {
reportIsolatedDeclarationError(extendsClause);
return cleanup(factory.updateClassDeclaration(
input,
Expand Down Expand Up @@ -1850,12 +1861,14 @@ export function transformDeclarations(context: TransformationContext) {
case SyntaxKind.EnumDeclaration: {
return cleanup(factory.updateEnumDeclaration(
input,
factory.createNodeArray(ensureModifiers(input)),
factory.createNodeArray(ensureModifiers(input)),
input.name,
factory.createNodeArray(mapDefined(input.members, m => {
if (shouldStripInternal(m)) return;
if (isolatedDeclarations) {
if (m.initializer && !resolver.isLiteralConstDeclaration(m)) {
if (m.initializer && !resolver.isLiteralConstDeclaration(m) &&
// This will be its own compiler error instead, so don't report.
!isComputedPropertyName(m.name)) {
reportIsolatedDeclarationError(m);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Debug } from "../../debug";
import { Diagnostics } from "../../diagnosticInformationMap.generated";
import { isCallSignatureDeclaration, isComputedPropertyName, isConstructSignatureDeclaration, isExportAssignment, isGetAccessorDeclaration, isIdentifier, isLiteralTypeNode, isMethodDeclaration, isMethodSignature, isNoSubstitutionTemplateLiteral, isNumericLiteral, isOmittedExpression, isParameter, isPrivateIdentifier, isPropertyAccessExpression, isPropertyAssignment, isPropertyDeclaration, isPropertySignature, isSetAccessorDeclaration, isShorthandPropertyAssignment, isSpreadAssignment, isSpreadElement, isStringLiteral, isTypeParameterDeclaration, isTypeReferenceNode, isUnionTypeNode, isVariableDeclaration } from "../../factory/nodeTests";
import { isComputedPropertyName, isExportAssignment, isGetAccessorDeclaration, isIdentifier, isInterfaceDeclaration, isLiteralTypeNode, isMethodDeclaration, isNoSubstitutionTemplateLiteral, isNumericLiteral, isOmittedExpression, isParameter, isPrivateIdentifier, isPropertyAccessExpression, isPropertyAssignment, isPropertyDeclaration, isSetAccessorDeclaration, isShorthandPropertyAssignment, isSpreadAssignment, isSpreadElement, isStringLiteral, isTypeLiteralNode, isTypeParameterDeclaration, isTypeReferenceNode, isUnionTypeNode, isVariableDeclaration } from "../../factory/nodeTests";
import { setTextRange } from "../../factory/utilitiesPublic";
import { nullTransformationContext } from "../../transformer";
import { ArrayLiteralExpression, ArrowFunction, AsExpression, EntityNameOrEntityNameExpression, ExportAssignment, FunctionExpression, GetAccessorDeclaration, HasInferredType, Identifier, KeywordTypeSyntaxKind, LiteralExpression, MethodSignature, Modifier, Node, NodeArray, NodeFlags, ObjectLiteralElementLike, ObjectLiteralExpression, ParameterDeclaration, ParenthesizedExpression, PrefixUnaryExpression, PropertySignature, SetAccessorDeclaration, SourceFile, SyntaxKind, TransformationContext,TypeAssertion, TypeElement, TypeNode, Visitor, VisitResult } from "../../types";
Expand Down Expand Up @@ -254,7 +254,7 @@ export function createLocalInferenceResolver({

for (let propIndex = 0, length = objectLiteral.properties.length; propIndex < length; propIndex++) {
const prop = objectLiteral.properties[propIndex];

if (isShorthandPropertyAssignment(prop)) {
return invalid(prop);
}
Expand Down Expand Up @@ -543,10 +543,7 @@ export function createLocalInferenceResolver({
else if (isPropertyDeclaration(node) && node.initializer) {
localType = localInference(node.initializer);
}
else if(isMethodSignature(node)
|| isConstructSignatureDeclaration(node)
|| isCallSignatureDeclaration(node)
|| isPropertySignature(node)) {
else if(isInterfaceDeclaration(node.parent) || isTypeLiteralNode(node.parent)) {
return factory.createKeywordTypeNode(SyntaxKind.AnyKeyword);
}
else {
Expand Down
15 changes: 10 additions & 5 deletions src/services/codefixes/fixMissingTypeAnnotationOnExports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,18 @@ registerCodeFix({

function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, typeChecker: TypeChecker, nodeWithDiag: Node): void {
const nodeWithNoType = findNearestParentWithTypeAnnotation(nodeWithDiag);
fixupForIsolatedDeclarations(nodeWithNoType, nodeWithDiag, sourceFile, typeChecker, changes);
if (nodeWithNoType) {
fixupForIsolatedDeclarations(nodeWithNoType, nodeWithDiag, sourceFile, typeChecker, changes);
}
}

// Currently, the diagnostics for the error is not given in the exact node of which that needs type annotation
function findNearestParentWithTypeAnnotation(node: Node): Node {
while (((isObjectBindingPattern(node) || isArrayBindingPattern(node)) && !isVariableDeclaration(node.parent)) ||
!canHaveExplicitTypeAnnotation.has(node.kind)) {
// Currently, the diagnostics for the error is not given in the exact node of which that needs type annotation.
// If this is coming from an ill-formed AST with syntax errors, you cannot assume that it'll find a node
// to annotate types, this will return undefined - meaning that it couldn't find the node to annotate types.
function findNearestParentWithTypeAnnotation(node: Node): Node | undefined {
while (node &&
(((isObjectBindingPattern(node) || isArrayBindingPattern(node)) &&
!isVariableDeclaration(node.parent)) || !canHaveExplicitTypeAnnotation.has(node.kind))) {
node = node.parent;
}
return node;
Expand Down