Skip to content

Commit

Permalink
Fix go/ts52upgrade JSC_MISPLACED_ANNOTATION issue
Browse files Browse the repository at this point in the history
For decorated classes, that reference the class name in a static block, TypeScript 5.2 changed the JS emit from

    /** @abstract */
    let Foo = class Foo {

to

    var Foo_1;
    /** @abstract */
    let Foo = Foo_1 = class Foo {

This caused JSC_MISPLACED_ANNOTATION errors for both `@abstract` and `@template`. The only fix needed is in the TypedScopeCreator.

Optimizations InlineAndCollapseProperties and RemoveUnusedCode already don't optimize the 5.2 emit. They work the same on 5.3 emit.

PiperOrigin-RevId: 561277685
  • Loading branch information
frigus02 authored and copybara-github committed Aug 30, 2023
1 parent 517545f commit 334e0b6
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/CheckJSDoc.java
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ private boolean isClassDecl(Node n) {
}

private boolean isNameInitializeWithClass(Node n) {
return n != null && n.isName() && n.hasChildren() && isClass(n.getFirstChild());
return n != null && n.isName() && n.hasChildren() && isClassDecl(n.getFirstChild());
}

private boolean isClass(Node n) {
Expand Down
14 changes: 8 additions & 6 deletions src/com/google/javascript/jscomp/TypedScopeCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -2397,6 +2397,14 @@ && shouldUseFunctionLiteralType(
}
}

if (rValue != null && rValue.isAssign()) {
// Handle nested assignments. For example, TypeScript generates code like this:
// var Foo_1;
// let Foo = Foo_1 = class Foo {}
// Foo = Foo_1 = tslib_1.decorate(..., Foo);
return getDeclaredType(info, lValue, rValue.getSecondChild(), null);
}

if (info != null && FunctionTypeBuilder.isFunctionTypeDeclaration(info)) {
String fnName = lValue.getQualifiedName();
return createFunctionTypeFromNodes(null, fnName, info, lValue);
Expand All @@ -2406,12 +2414,6 @@ && shouldUseFunctionLiteralType(
return getNativeType(JSTypeNative.NO_TYPE);
}

if (rValue != null && rValue.isAssign()) {
// Handle nested assignments. For example, tsickle generates code like this:
// let Foo = Foo_1 = tslib_1.decorate(...)
return getDeclaredType(info, lValue, rValue.getSecondChild(), null);
}

return null;
}

Expand Down
10 changes: 10 additions & 0 deletions test/com/google/javascript/jscomp/CheckJsDocTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1287,4 +1287,14 @@ public void testMangleClosureModuleExportsContentsTypes() {
"/** @type {!Array<module$exports$foo$bar>} */ let x;",
"/** @type {!Array<UnrecognizedType_module$exports$foo$bar>} */ let x;");
}

@Test
public void testNameDeclarationAndAssignForAbstractClass() {
testSame(lines("/** @abstract */", "let A = A_1 = class A {}"));
}

@Test
public void testNameDeclarationAndAssignForTemplatedClass() {
testSame(lines("/** @template T */", "let A = A_1 = class A {}"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2525,6 +2525,53 @@ public void testClassStaticInheritance_cantDetermineSuperclass() {
"use(C.foo);"));
}

@Test
public void testTypeScriptDecoratedClass() {
// TypeScript 5.1 emits this for decorated classes.
test(
lines(
"let A = class A { static getId() { return A.ID; } };", //
"A.ID = \"a\";",
"A = tslib.__decorate([], A);",
"if (false) {",
" /** @const {string} */ A.ID;",
"}",
"console.log(A.getId());"),
lines(
"let A = class A$jscomp$1 { static getId() { return A$jscomp$1.ID; } };",
"A.ID = \"a\";",
"A = tslib.__decorate([], A);",
"if (false) {",
" /** @const */ A.ID;",
"}",
"console.log(A.getId());"));
}

@Test
public void testTypeScriptDecoratedClass_withExtraVarAssignment() {
// TypeScript 5.2 emits this for decorated classes, which reference static properties on
// themselves.
test(
lines(
"var A_1;", //
"let A = A_1 = class A { static getId() { return A_1.ID; } };",
"A.ID = \"a\";",
"A = A_1 = tslib.__decorate([], A);",
"if (false) {",
" /** @const {string} */ A.ID;",
"}",
"console.log(A.getId());"),
lines(
"var A_1;", //
"let A = A_1 = class A$jscomp$1 { static getId() { return A_1.ID; } };",
"A.ID = \"a\";",
"A = A_1 = tslib.__decorate([], A);",
"if (false) {",
" /** @const */ A.ID;",
"}",
"console.log(A.getId());"));
}

@Test
public void testAliasForSuperclassNamespace() {
test(
Expand Down
3 changes: 3 additions & 0 deletions test/com/google/javascript/jscomp/NodeUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3589,6 +3589,9 @@ public void testGetBestJsDocInfoForClasses() {

classNode = parseFirst(CLASS, "/** @export */ var Foo = class Bar {}");
assertThat(NodeUtil.getBestJSDocInfo(classNode).isExport()).isTrue();

classNode = parseFirst(CLASS, "var Foo_1; /** @export */ let Foo = Foo_1 = class Foo {}");
assertThat(NodeUtil.getBestJSDocInfo(classNode).isExport()).isTrue();
}

@Test
Expand Down
24 changes: 24 additions & 0 deletions test/com/google/javascript/jscomp/TypedScopeCreatorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2736,6 +2736,30 @@ public void testClassDeclarationWithExtends_andIncompatibleJsdoc() {
assertThat(foo.isAmbiguousConstructor()).isTrue();
}

@Test
public void testAbstractClassDeclarationWithExtends_nameDeclAndAssign() {
testSame(
lines(
"var Bar_1;", //
"/** @abstract */",
"let Bar = Bar_1 = class Bar {}",
"class Foo extends Bar {}"));

FunctionType bar1 = (FunctionType) findNameType("Bar_1", globalScope);
FunctionType bar = (FunctionType) findNameType("Bar", globalScope);
FunctionType foo = (FunctionType) findNameType("Foo", globalScope);
assertThat(bar.isAbstract()).isTrue();
assertThat(bar).isEqualTo(bar1);
assertType(foo.getInstanceType()).isSubtypeOf(bar.getInstanceType());
assertType(foo.getImplicitPrototype()).isEqualTo(bar);
assertScope(globalScope).declares("Bar").withTypeThat().isEqualTo(bar);
assertScope(globalScope).declares("Foo").withTypeThat().isEqualTo(foo);

assertThat(foo.getInstanceType().loosenTypecheckingDueToForwardReferencedSupertype()).isFalse();
assertThat(foo.loosenTypecheckingDueToForwardReferencedSupertype()).isFalse();
assertThat(foo.isAmbiguousConstructor()).isFalse();
}

@Test
public void testClassDeclarationWithExtendsFromNamespace() {
testSame(
Expand Down

0 comments on commit 334e0b6

Please sign in to comment.