Skip to content

Commit 55f98a7

Browse files
author
mmews-n4
authored
GH-2576: TypeScript's meta type language has issues with circular type structures (#2578)
* add test * improve test * fix * improve test * use newer version of TypeScript * fix * improve test * fix * improve tests * fix * more fixes
1 parent 5c8b24e commit 55f98a7

File tree

4 files changed

+249
-31
lines changed

4 files changed

+249
-31
lines changed

‎plugins/org.eclipse.n4js.transpiler.dts/src/org/eclipse/n4js/transpiler/dts/transform/TypeReferenceTransformation.java

+115-29
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,13 @@
3131
import org.eclipse.emf.ecore.resource.Resource;
3232
import org.eclipse.n4js.AnnotationDefinition;
3333
import org.eclipse.n4js.N4JSGlobals;
34+
import org.eclipse.n4js.n4JS.FormalParameter;
3435
import org.eclipse.n4js.n4JS.FunctionDefinition;
3536
import org.eclipse.n4js.n4JS.ImportSpecifier;
37+
import org.eclipse.n4js.n4JS.N4FieldDeclaration;
3638
import org.eclipse.n4js.n4JS.N4JSPackage;
39+
import org.eclipse.n4js.n4JS.N4MethodDeclaration;
40+
import org.eclipse.n4js.n4JS.N4TypeDeclaration;
3741
import org.eclipse.n4js.n4JS.NamedImportSpecifier;
3842
import org.eclipse.n4js.n4JS.NamespaceImportSpecifier;
3943
import org.eclipse.n4js.n4JS.TypeReferenceNode;
@@ -66,12 +70,14 @@
6670
import org.eclipse.n4js.ts.types.TField;
6771
import org.eclipse.n4js.ts.types.TFormalParameter;
6872
import org.eclipse.n4js.ts.types.TGetter;
73+
import org.eclipse.n4js.ts.types.TInterface;
6974
import org.eclipse.n4js.ts.types.TMember;
7075
import org.eclipse.n4js.ts.types.TMethod;
7176
import org.eclipse.n4js.ts.types.TModule;
7277
import org.eclipse.n4js.ts.types.TSetter;
78+
import org.eclipse.n4js.ts.types.TStructField;
7379
import org.eclipse.n4js.ts.types.TStructMember;
74-
import org.eclipse.n4js.ts.types.TTypedElement;
80+
import org.eclipse.n4js.ts.types.TStructuralType;
7581
import org.eclipse.n4js.ts.types.Type;
7682
import org.eclipse.n4js.utils.N4JSLanguageUtils;
7783
import org.eclipse.n4js.utils.N4JSLanguageUtils.EnumKind;
@@ -171,10 +177,10 @@ private void convertTypeRefNode(TypeReferenceNode_IM<?> typeRefNode) {
171177
// special case: ArrayN in extends clause
172178
String referenceStr = getReferenceToType(declType, getState());
173179
write(referenceStr);
174-
convertTypeArguments((ParameterizedTypeRef) typeRef);
180+
convertTypeArguments(typeRefNode, (ParameterizedTypeRef) typeRef);
175181
} else {
176182
// standard case
177-
convertTypeRef(typeRef);
183+
convertTypeRef(typeRefNode, typeRef);
178184
}
179185

180186
if (isReturnType) {
@@ -188,15 +194,11 @@ private void convertTypeRefNode(TypeReferenceNode_IM<?> typeRefNode) {
188194
}
189195
}
190196

191-
private void convertDeclaredTypeRef(TTypedElement elem) {
192-
TypeRef declaredTypeRef = elem.getTypeRef();
193-
if (declaredTypeRef != null) {
194-
write(": ");
195-
convertTypeRef(declaredTypeRef);
196-
}
197+
private void convertTypeRef(TypeRef typeRefRaw) {
198+
convertTypeRef(null, typeRefRaw);
197199
}
198200

199-
private void convertTypeRef(TypeRef typeRefRaw) {
201+
private void convertTypeRef(TypeReferenceNode_IM<?> typeRefNode, TypeRef typeRefRaw) {
200202
if (typeRefRaw == null) {
201203
return;
202204
}
@@ -217,7 +219,7 @@ private void convertTypeRef(TypeRef typeRefRaw) {
217219
} else if (typeRef instanceof FunctionTypeExprOrRef) {
218220
convertFunctionTypeExprOrRef((FunctionTypeExprOrRef) typeRef);
219221
} else if (typeRef instanceof ParameterizedTypeRef) {
220-
convertParameterizedTypeRef((ParameterizedTypeRef) typeRef);
222+
convertParameterizedTypeRef(typeRefNode, (ParameterizedTypeRef) typeRef);
221223
} else if (typeRef instanceof ThisTypeRef) {
222224
convertThisTypeRef((ThisTypeRef) typeRef);
223225
} else if (typeRef instanceof ExistentialTypeRef) {
@@ -278,15 +280,15 @@ private void convertFunctionTypeExprOrRef(FunctionTypeExprOrRef typeRef) {
278280
}
279281
}
280282

281-
private void convertParameterizedTypeRef(ParameterizedTypeRef typeRef) {
283+
private void convertParameterizedTypeRef(TypeReferenceNode_IM<?> typeRefNode, ParameterizedTypeRef typeRef) {
282284
if (typeRef instanceof FunctionTypeRef) {
283285
convertFunctionTypeExprOrRef((FunctionTypeRef) typeRef);
284286
return;
285287
}
286288
Type declType = typeRef.getDeclaredType();
287289

288290
if (isArrayN(getState().G, declType)) {
289-
convertTypeArguments(typeRef, "[", "]");
291+
convertTypeArguments(typeRefNode, typeRef, "[", "]");
290292
return;
291293
}
292294

@@ -302,11 +304,11 @@ private void convertParameterizedTypeRef(ParameterizedTypeRef typeRef) {
302304
if (showDeclaredType) {
303305
String referenceStr = (declType == null) ? null : getReferenceToType(declType, getState());
304306
if (referenceStr != null) {
305-
String prependType = getStructuralTypeReplacements(typeRef);
307+
String prependType = getStructuralTypeReplacements(typeRefNode, typeRef);
306308
write(prependType);
307309

308310
write(referenceStr);
309-
convertTypeArguments(typeRef);
311+
convertTypeArguments(typeRefNode, typeRef);
310312

311313
write(Strings.isNullOrEmpty(prependType) ? "" : ">");
312314

@@ -321,7 +323,7 @@ private void convertParameterizedTypeRef(ParameterizedTypeRef typeRef) {
321323
write(" & ");
322324
}
323325
write("{");
324-
write(members, m -> convertTMember(m), "; "); // ',' would also be allowed as separator
326+
write(members, m -> convertTMember(typeRefNode, m), "; "); // ',' would also be allowed as separator
325327
write("}");
326328
}
327329

@@ -331,7 +333,7 @@ private void convertParameterizedTypeRef(ParameterizedTypeRef typeRef) {
331333
}
332334

333335
private void convertThisTypeRef(ThisTypeRef typeRef) {
334-
String prependType = getStructuralTypeReplacements(typeRef);
336+
String prependType = getStructuralTypeReplacements(null, typeRef);
335337
write(prependType);
336338

337339
write("this");
@@ -361,7 +363,19 @@ private void convertLiteralTypeRef(LiteralTypeRef typeRef) {
361363
}
362364
}
363365

364-
private String getStructuralTypeReplacements(TypeRef typeRef) {
366+
private String getStructuralTypeReplacements(TypeReferenceNode_IM<?> typeRefNode, TypeRef typeRef) {
367+
if (isCaseOfTypeScriptCircularityIssue(typeRefNode, typeRef)) {
368+
switch (typeRef.getTypingStrategy()) {
369+
case STRUCTURAL_READ_ONLY_FIELDS:
370+
case STRUCTURAL_FIELD_INITIALIZER:
371+
return "Readonly<";
372+
case STRUCTURAL_FIELDS:
373+
case STRUCTURAL_WRITE_ONLY_FIELDS:
374+
default:
375+
return "";
376+
}
377+
}
378+
365379
switch (typeRef.getTypingStrategy()) {
366380
case STRUCTURAL_FIELDS:
367381
return "StructuralFields<";
@@ -376,11 +390,79 @@ private String getStructuralTypeReplacements(TypeRef typeRef) {
376390
}
377391
}
378392

379-
private void convertTypeArguments(ParameterizedTypeRef typeRef) {
380-
convertTypeArguments(typeRef, "<", ">");
393+
/**
394+
* Mitigates TypeScript issue occurring on circular type references. Details see here:
395+
* ReadonlyStructuralTypes_TS_circularity.n4js.xt
396+
*/
397+
private boolean isCaseOfTypeScriptCircularityIssue(TypeReferenceNode_IM<?> typeRefNode, TypeRef typeRef) {
398+
if (typeRef instanceof ParameterizedTypeRefStructural
399+
&& typeRef.eContainer() instanceof ParameterizedTypeRefStructural) {
400+
401+
EObject trTmp = typeRef;
402+
while (trTmp instanceof TypeRef) {
403+
trTmp = trTmp.eContainer();
404+
}
405+
if (trTmp instanceof TStructField) {
406+
// case for fields type aliases
407+
ParameterizedTypeRefStructural propTypeRef = (ParameterizedTypeRefStructural) typeRef.eContainer();
408+
TStructuralType propType = propTypeRef.getStructuralType();
409+
410+
if (propType == trTmp.eContainer()) {
411+
return true;
412+
}
413+
}
414+
if (trTmp instanceof TFormalParameter && trTmp.eContainer() != null) {
415+
// case for formal parameters in type aliases
416+
ParameterizedTypeRefStructural paramTypeRef = (ParameterizedTypeRefStructural) typeRef.eContainer();
417+
TStructuralType paramType = paramTypeRef.getStructuralType();
418+
419+
if (paramType == trTmp.eContainer().eContainer()) {
420+
return true;
421+
}
422+
}
423+
}
424+
425+
if (typeRefNode != null
426+
&& typeRefNode.eContainer() instanceof N4FieldDeclaration
427+
&& typeRefNode.eContainer().eContainer() instanceof N4TypeDeclaration) {
428+
429+
// case for interfaces and classes
430+
Type referencedType = typeRef.getDeclaredType();
431+
N4TypeDeclaration containingTDecl = (N4TypeDeclaration) typeRefNode.eContainer().eContainer();
432+
Type containingType = getState().info.getOriginalDefinedType(containingTDecl);
433+
434+
if (referencedType == containingType) {
435+
return true;
436+
}
437+
}
438+
439+
if (typeRefNode != null
440+
&& typeRefNode.eContainer() instanceof FormalParameter
441+
&& typeRefNode.eContainer().eContainer() instanceof N4MethodDeclaration
442+
&& typeRefNode.eContainer().eContainer().eContainer() instanceof N4TypeDeclaration) {
443+
444+
// case for formal parameters
445+
Type referencedType = typeRef.getDeclaredType();
446+
N4MethodDeclaration method = (N4MethodDeclaration) typeRefNode.eContainer().eContainer();
447+
N4TypeDeclaration containingTDecl = (N4TypeDeclaration) typeRefNode.eContainer().eContainer().eContainer();
448+
Type containingType = getState().info.getOriginalDefinedType(containingTDecl);
449+
450+
if (referencedType == containingType && !method.isConstructor()) {
451+
if (containingType instanceof TInterface) {
452+
return !method.isConstructSignature();
453+
}
454+
return !method.isConstructor();
455+
}
456+
}
457+
return false;
458+
}
459+
460+
private void convertTypeArguments(TypeReferenceNode_IM<?> typeRefNode, ParameterizedTypeRef typeRef) {
461+
convertTypeArguments(typeRefNode, typeRef, "<", ">");
381462
}
382463

383-
private void convertTypeArguments(ParameterizedTypeRef typeRef, String prefix, String suffix) {
464+
private void convertTypeArguments(TypeReferenceNode_IM<?> typeRefNode, ParameterizedTypeRef typeRef, String prefix,
465+
String suffix) {
384466
List<TypeArgument> typeArgs = typeRef.getDeclaredTypeArgs();
385467
if (typeArgs.isEmpty()) {
386468
return;
@@ -394,15 +476,15 @@ private void convertTypeArguments(ParameterizedTypeRef typeRef, String prefix, S
394476
}
395477

396478
write(prefix);
397-
write(typeArgs, ta -> convertTypeArgument(ta), ", ");
479+
write(typeArgs, ta -> convertTypeArgument(typeRefNode, ta), ", ");
398480
write(suffix);
399481
}
400482

401-
private void convertTypeArgument(TypeArgument typeArg) {
483+
private void convertTypeArgument(TypeReferenceNode_IM<?> typeRefNode, TypeArgument typeArg) {
402484
if (typeArg instanceof Wildcard) {
403485
TypeRef upperBound = ((Wildcard) typeArg).getDeclaredOrImplicitUpperBound();
404486
if (upperBound != null) {
405-
convertTypeArgument(upperBound);
487+
convertTypeArgument(typeRefNode, upperBound);
406488
} else {
407489
write("any"); // TypeScript does not support lower bounds
408490
}
@@ -421,7 +503,7 @@ private void convertTypeArgument(TypeArgument typeArg) {
421503
// return;
422504
}
423505

424-
convertTypeRef(typeRef);
506+
convertTypeRef(typeRefNode, typeRef);
425507
}
426508

427509
private void convertTFormalParameters(Iterable<? extends TFormalParameter> fpars) {
@@ -453,9 +535,9 @@ private void convertTFormalParameter(TFormalParameter fpar) {
453535
}
454536
}
455537

456-
private void convertTMember(TMember member) {
538+
private void convertTMember(TypeReferenceNode_IM<?> typeRefNode, TMember member) {
457539
if (member instanceof TField) {
458-
convertTMember((TField) member);
540+
convertTMember(typeRefNode, (TField) member);
459541
} else if (member instanceof TGetter) {
460542
convertTMember((TGetter) member);
461543
} else if (member instanceof TSetter) {
@@ -468,12 +550,16 @@ private void convertTMember(TMember member) {
468550
}
469551
}
470552

471-
private void convertTMember(TField field) {
553+
private void convertTMember(TypeReferenceNode_IM<?> typeRefNode, TField field) {
472554
writeQuotedIfNonIdentifier(field.getName());
473555
if (field.isOptional()) {
474556
write("?");
475557
}
476-
convertDeclaredTypeRef(field);
558+
TypeRef declaredTypeRef = field.getTypeRef();
559+
if (declaredTypeRef != null) {
560+
write(": ");
561+
convertTypeRef(typeRefNode, declaredTypeRef);
562+
}
477563
}
478564

479565
private void convertTMember(TGetter getter) {

‎testhelpers/org.eclipse.n4js.ide.tests.helper/src/org/eclipse/n4js/ide/tests/helper/server/xt/XtIdeTest.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
*/
8282
public class XtIdeTest extends AbstractIdeTest {
8383

84+
static final String TSC_VERSION = "5.3.2";
8485
static final File TSC_PROVIDER = new File("test-tscProvider");
8586

8687
static final File TSC2 = new File(TSC_PROVIDER, N4JSGlobals.NODE_MODULES + "/.bin/tsc");
@@ -802,8 +803,9 @@ private void ensureTSC(CliTools cliTools) {
802803
}
803804
TSC_PROVIDER.mkdirs();
804805

805-
// npm install --prefix . [email protected]
806-
ProcessResult result = cliTools.npmRun(TSC_PROVIDER.toPath(), "install", "--prefix", ".", "[email protected]");
806+
// npm install --prefix . typescript@<TSC_VERSION>
807+
ProcessResult result = cliTools.npmRun(TSC_PROVIDER.toPath(), "install", "--prefix", ".",
808+
"typescript@" + TSC_VERSION);
807809

808810
assertEquals(0, result.getExitCode());
809811
}

‎tests/org.eclipse.n4js.spec.tests/xt-tests/dts-export/ReadonlyStructuralTypes_TS_circularity.n4js.xt

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
* Copyright (c) 2021 NumberFour AG.
3+
* All rights reserved. This program and the accompanying materials
4+
* are made available under the terms of the Eclipse Public License v1.0
5+
* which accompanies this distribution, and is available at
6+
* http://www.eclipse.org/legal/epl-v10.html
7+
*
8+
* Contributors:
9+
* NumberFour AG - Initial API and implementation
10+
*/
11+
12+
/* XPECT_SETUP org.eclipse.n4js.spec.tests.SpecXtTest
13+
GENERATE_DTS
14+
END_SETUP
15+
*/
16+
17+
18+
19+
/*
20+
* Background is GH-2576:
21+
* TypeScript cannot handle recursive types when used with type meta language (a.k.a. utitlity types).
22+
* We use the utility types (e.g. StructuralReadOnly<T>) to mimic N4JS structural type strategies (e.g. ~r~) when generating d.ts files.
23+
* Those utility types we use are defined in n4jsglobals.d.ts.
24+
* They rely on the TypeScript meta language feature: [K in keyof T]: T[K] extends Function ? K : never.
25+
* However, this feature is not compatible with recursive types, i.e. types that have members of the same type.
26+
* As a fallback in those situations, we use the utility type Readonly<T> instead to avoid errors by the TypeScript compiler.
27+
*/
28+
29+
export public interface CircularProblem1 {
30+
field_f: ~~CircularProblem1;
31+
field_r: ~r~CircularProblem1;
32+
field_w: ~w~CircularProblem1;
33+
array_r1: ~r~CircularProblem1[];
34+
array_r2: Array<~r~CircularProblem1>;
35+
field_i: ~Object with {prop: ~i~CircularProblem1};
36+
method(arg: ~r~CircularProblem1): ~r~CircularProblem1; // method returns are fine
37+
}
38+
39+
export public class CircularProblem2 {
40+
constructor(spec: ~i~CircularProblem2) {}
41+
field_f: ~~CircularProblem2;
42+
field_r: ~r~CircularProblem2;
43+
field_w: ~w~CircularProblem2;
44+
array_r1: ~r~CircularProblem2[];
45+
array_r2: Array<~r~CircularProblem2>;
46+
field_i: ~Object with {prop: ~i~CircularProblem2};
47+
method(arg: ~r~CircularProblem2): ~r~CircularProblem2 { return null; } // method returns are fine
48+
}
49+
50+
export public type CircularProblem3 = ~Object with {
51+
field_f: ~~CircularProblem3;
52+
field_r: ~r~CircularProblem3;
53+
field_w: ~w~CircularProblem3;
54+
array_r1: ~r~CircularProblem3[];
55+
array_r2: Array<~r~CircularProblem3>;
56+
field_i: ~Object with {prop: ~i~CircularProblem3};
57+
method(arg: ~r~CircularProblem3): ~r~CircularProblem3; // method returns are fine
58+
}
59+
60+
61+
/* XPECT generated_dts ---
62+
export interface CircularProblem1 {
63+
field_f: CircularProblem1;
64+
field_r: Readonly<CircularProblem1>;
65+
field_w: CircularProblem1;
66+
array_r1: Array<Readonly<CircularProblem1>>;
67+
array_r2: Array<Readonly<CircularProblem1>>;
68+
field_i: {prop: Readonly<CircularProblem1>};
69+
method(arg: Readonly<CircularProblem1>): StructuralReadOnly<CircularProblem1>;
70+
}
71+
export class CircularProblem2 {
72+
constructor(spec: StructuralInititializers<CircularProblem2>);
73+
field_f: CircularProblem2;
74+
field_r: Readonly<CircularProblem2>;
75+
field_w: CircularProblem2;
76+
array_r1: Array<Readonly<CircularProblem2>>;
77+
array_r2: Array<Readonly<CircularProblem2>>;
78+
field_i: {prop: Readonly<CircularProblem2>};
79+
method(arg: Readonly<CircularProblem2>): StructuralReadOnly<CircularProblem2>;
80+
}
81+
export type CircularProblem3 = {field_f: CircularProblem3; field_r: Readonly<CircularProblem3>; field_w: CircularProblem3; array_r1: Array<Readonly<CircularProblem3>>; array_r2: Array<Readonly<CircularProblem3>>; field_i: {prop: StructuralInititializers<CircularProblem3>}; method(arg: Readonly<CircularProblem3>): StructuralReadOnly<CircularProblem3>};
82+
--- */

0 commit comments

Comments
 (0)