diff --git a/src/com/google/javascript/jscomp/CheckJSDoc.java b/src/com/google/javascript/jscomp/CheckJSDoc.java index 5b773eebf7a..9a8f3c5c4f8 100644 --- a/src/com/google/javascript/jscomp/CheckJSDoc.java +++ b/src/com/google/javascript/jscomp/CheckJSDoc.java @@ -169,6 +169,10 @@ private void validateAbstractJsDoc(Node n, JSDocInfo info) { if (info == null || !info.isAbstract()) { return; } + if (n.isClass()) { + return; + } + Node functionNode = getFunctionDecl(n); if (functionNode == null) { @@ -190,7 +194,9 @@ private void validateAbstractJsDoc(Node n, JSDocInfo info) { return; } - if (!n.isMemberFunctionDef() && !NodeUtil.isPrototypeMethod(functionNode)) { + if (!info.isConstructor() + && !n.isMemberFunctionDef() + && !NodeUtil.isPrototypeMethod(functionNode)) { // @abstract annotation on a non-method (or static method) in ES5 report(n, MISPLACED_ANNOTATION, "@abstract", "only functions or methods can be abstract"); return; diff --git a/src/com/google/javascript/jscomp/TypeCheck.java b/src/com/google/javascript/jscomp/TypeCheck.java index 68d652a3ee4..ccf90dc873b 100644 --- a/src/com/google/javascript/jscomp/TypeCheck.java +++ b/src/com/google/javascript/jscomp/TypeCheck.java @@ -116,6 +116,11 @@ public final class TypeCheck implements NodeTraversal.Callback, CompilerPass { "JSC_NOT_A_CONSTRUCTOR", "cannot instantiate non-constructor"); + static final DiagnosticType INSTANTIATE_ABSTRACT_CLASS = + DiagnosticType.warning( + "JSC_INSTANTIATE_ABSTRACT_CLASS", + "cannot instantiate abstract class"); + static final DiagnosticType BIT_OPERATION = DiagnosticType.warning( "JSC_BAD_TYPE_FOR_BIT_OPERATION", @@ -265,6 +270,7 @@ public final class TypeCheck implements NodeTraversal.Callback, CompilerPass { POSSIBLE_INEXISTENT_PROPERTY, INEXISTENT_PROPERTY_WITH_SUGGESTION, NOT_A_CONSTRUCTOR, + INSTANTIATE_ABSTRACT_CLASS, BIT_OPERATION, NOT_CALLABLE, CONSTRUCTOR_NOT_CALLABLE, @@ -1562,18 +1568,28 @@ private void visitVar(NodeTraversal t, Node n) { private void visitNew(NodeTraversal t, Node n) { Node constructor = n.getFirstChild(); JSType type = getJSType(constructor).restrictByNotNullOrUndefined(); - if (type.isConstructor() || type.isEmptyType() || type.isUnknownType()) { - FunctionType fnType = type.toMaybeFunctionType(); - if (fnType != null && fnType.hasInstanceType()) { - visitParameterList(t, n, fnType); - ensureTyped(t, n, fnType.getInstanceType()); - } else { - ensureTyped(t, n); - } - } else { + if (!couldBeAConstructor(type)) { report(t, n, NOT_A_CONSTRUCTOR); ensureTyped(t, n); + return; } + + Var var = t.getScope().getVar(constructor.getQualifiedName()); + if (var != null && var.getJSDocInfo() != null && var.getJSDocInfo().isAbstract()) { + report(t, n, INSTANTIATE_ABSTRACT_CLASS); + } + + FunctionType fnType = type.toMaybeFunctionType(); + if (fnType != null && fnType.hasInstanceType()) { + visitParameterList(t, n, fnType); + ensureTyped(t, n, fnType.getInstanceType()); + } else { + ensureTyped(t, n); + } + } + + private boolean couldBeAConstructor(JSType type) { + return type.isConstructor() || type.isEmptyType() || type.isUnknownType(); } /** diff --git a/src/com/google/javascript/rhino/JSDocInfoBuilder.java b/src/com/google/javascript/rhino/JSDocInfoBuilder.java index 80b8cfcd468..98b7695661f 100644 --- a/src/com/google/javascript/rhino/JSDocInfoBuilder.java +++ b/src/com/google/javascript/rhino/JSDocInfoBuilder.java @@ -866,8 +866,7 @@ public boolean recordNoCollapse() { */ public boolean recordConstructor() { if (!hasAnySingletonTypeTags() - && !currentInfo.isConstructorOrInterface() - && !currentInfo.isAbstract()) { + && !currentInfo.isConstructorOrInterface()) { currentInfo.setConstructor(true); populated = true; return true; @@ -939,7 +938,6 @@ public boolean isUnrestrictedRecorded() { public boolean recordAbstract() { if (!hasAnySingletonTypeTags() && !currentInfo.isInterface() - && !currentInfo.isConstructor() && !currentInfo.isAbstract()) { currentInfo.setAbstract(); populated = true; diff --git a/test/com/google/javascript/jscomp/CheckJsDocTest.java b/test/com/google/javascript/jscomp/CheckJsDocTest.java index 422aa8dd0e1..0392f5a6b32 100644 --- a/test/com/google/javascript/jscomp/CheckJsDocTest.java +++ b/test/com/google/javascript/jscomp/CheckJsDocTest.java @@ -117,17 +117,15 @@ public void testAbstract_staticMethod() { } public void testAbstract_class() { - testWarningEs6( - "/** @abstract */class Foo { constructor() {}}", - MISPLACED_ANNOTATION); - // Adding @abstract to an ES5 class/constructor is a parse error. + testSameEs6("/** @abstract */class Foo { constructor() {}}"); + testSame("/** @abstract @constructor */ var Foo = function() {};"); } public void testAbstract_constructor() { testWarningEs6( "class Foo { /** @abstract */ constructor() {}}", MISPLACED_ANNOTATION); - // Adding @abstract to an ES5 class/constructor is a parse error. + // ES5 constructors are treated as class definitions and tested above. } public void testAbstract_field() { diff --git a/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java b/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java index 681b28a4014..b10281070a5 100644 --- a/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java +++ b/test/com/google/javascript/jscomp/Es6ToEs3ConverterTest.java @@ -19,6 +19,7 @@ import static com.google.javascript.jscomp.Es6ToEs3Converter.CANNOT_CONVERT; import static com.google.javascript.jscomp.Es6ToEs3Converter.CANNOT_CONVERT_YET; import static com.google.javascript.jscomp.Es6ToEs3Converter.CONFLICTING_GETTER_SETTER_TYPE; +import static com.google.javascript.jscomp.TypeCheck.INSTANTIATE_ABSTRACT_CLASS; import com.google.javascript.jscomp.CompilerOptions.LanguageMode; @@ -502,6 +503,15 @@ public void testClassExpression() { "(condition ? obj1 : obj2).prop = testcode$classdecl$var0;")); } + public void testAbstractClass() { + enableTypeCheck(); + test( + "/** @abstract */ class Foo {} var x = new Foo();", + "/** @abstract @constructor @struct */ var Foo = function() {}; var x = new Foo();", + null, + INSTANTIATE_ABSTRACT_CLASS); + } + /** * We don't bother transpiling this case because the transpiled code will be very difficult to * typecheck. diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index 66e71a99f7a..40f037de286 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -17,6 +17,7 @@ package com.google.javascript.jscomp; import static com.google.common.truth.Truth.assertThat; +import static com.google.javascript.jscomp.TypeCheck.INSTANTIATE_ABSTRACT_CLASS; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; @@ -7884,6 +7885,12 @@ public void testNew18() { "/** @constructor */ goog.G = goog.F;"); } + public void testNew19() { + testTypes( + "/** @constructor @abstract */ var Foo = function() {}; var foo = new Foo();", + INSTANTIATE_ABSTRACT_CLASS); + } + public void testName1() { assertTypeEquals(VOID_TYPE, testNameNode("undefined")); } diff --git a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java index e85afe11d65..442a7a30de9 100644 --- a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java @@ -1349,12 +1349,6 @@ public void testParseAbstract_typeBeforeAbstract() throws Exception { "Bad type annotation. type annotation incompatible with other annotations"); } - public void testParseAbstract_constructorBeforeAbstract() throws Exception { - // TODO(michaelthomas): Remove when @abstract is supported on constructors. - parse("@constructor \n * @abstract */", - "Bad type annotation. type annotation incompatible with other annotations"); - } - public void testParseAbstract_interfaceBeforeAbstract() throws Exception { parse("* @interface \n * @abstract */", "Bad type annotation. type annotation incompatible with other annotations"); @@ -1375,12 +1369,6 @@ public void testParseAbstract_abstractBeforeInterface() throws Exception { "Bad type annotation. type annotation incompatible with other annotations"); } - public void testParseAbstract_abstractBeforeConstructor() throws Exception { - // TODO(michaelthomas): Remove when @abstract is supported on constructors. - parse("* @abstract \n * @constructor */", - "Bad type annotation. type annotation incompatible with other annotations"); - } - public void testStackedAnnotation() throws Exception { JSDocInfo info = parse("@const @type {string}*/"); assertThat(info.isConstant()).isTrue();