Merge pull request #93 from h-joo/betterDiagnostics

Reduce false positive reports of IsolatedDeclarations errors

external-declarations/src/code-mod/code-transform.ts

@@ -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;

src/compiler/transformers/declarations.ts

@@ -128,6 +128,7 @@ import {
     isInterfaceDeclaration,
     isJsonSourceFile,
     isLateVisibilityPaintedStatement,
+    isLiteralExpression,
     isLiteralImportTypeNode,
     isMappedTypeNode,
     isMethodDeclaration,
@@ -151,6 +152,7 @@ import {
     isTupleTypeNode,
     isTypeAliasDeclaration,
     isTypeElement,
+    isTypeLiteralNode,
     isTypeNode,
     isTypeParameterDeclaration,
     isTypeQueryNode,
@@ -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 {
@@ -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,
@@ -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);
                             }
                         }

src/compiler/transformers/declarations/localInferenceResolver.ts

@@ -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";
@@ -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);
                     }
@@ -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 {

src/services/codefixes/fixMissingTypeAnnotationOnExports.ts

@@ -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;