Skip to content

Commit 43b355c

Browse files
rbucktonrbuckton
committed
Fix super method call chain, remove ? from method, improve vary-by
1 parent 165b4bc commit 43b355c

16 files changed

+335
-12
lines changed

‎src/compiler/binder.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3593,6 +3593,7 @@ namespace ts {
35933593
// overloads are TypeScript syntax.
35943594
if (node.decorators
35953595
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
3596+
|| node.questionToken
35963597
|| node.typeParameters
35973598
|| node.type
35983599
|| !node.body) {

‎src/compiler/transformers/esnext.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,16 @@ namespace ts {
6969

7070
let thisArg: Expression | undefined;
7171
if (captureThisArg) {
72-
// `a.b` -> { expression: `(_a = a).b`, thisArg: `_a` }
73-
thisArg = createTempVariable(hoistVariableDeclaration);
74-
expression = createParen(createAssignment(thisArg, expression));
72+
if (isSuperProperty(node)) {
73+
// `super.b` -> { expression: `a.b`, thisArg: `this` }
74+
thisArg = createThis();
75+
expression = expression;
76+
}
77+
else {
78+
// `a.b` -> { expression: `(_a = a).b`, thisArg: `_a` }
79+
thisArg = createTempVariable(hoistVariableDeclaration);
80+
expression = createParen(createAssignment(thisArg, expression));
81+
}
7582
}
7683

7784
expression = updatePropertyAccess(node, expression, visitNode(node.name, visitor, isIdentifier));
@@ -89,9 +96,16 @@ namespace ts {
8996

9097
let thisArg: Expression | undefined;
9198
if (captureThisArg) {
92-
// `a[b]` -> { expression: `(_a = a)[b]`, thisArg: `_a` }
93-
thisArg = createTempVariable(hoistVariableDeclaration);
94-
expression = createParen(createAssignment(thisArg, expression));
99+
if (isSuperProperty(node)) {
100+
// `super.b` -> { expression: `a.b`, thisArg: `this` }
101+
thisArg = createThis();
102+
expression = expression;
103+
}
104+
else {
105+
// `a[b]` -> { expression: `(_a = a)[b]`, thisArg: `_a` }
106+
thisArg = createTempVariable(hoistVariableDeclaration);
107+
expression = createParen(createAssignment(thisArg, expression));
108+
}
95109
}
96110

97111
expression = updateElementAccess(node, expression, visitNode(node.argumentExpression, visitor, isExpression));

‎src/harness/harness.ts

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,10 +1412,65 @@ namespace Harness {
14121412
[key: string]: string;
14131413
}
14141414

1415-
function splitVaryBySettingValue(text: string): string[] | undefined {
1415+
function splitVaryBySettingValue(text: string, varyBy: string): string[] | undefined {
14161416
if (!text) return undefined;
1417-
const entries = text.split(/,/).map(s => s.trim().toLowerCase()).filter(s => s.length > 0);
1418-
return entries && entries.length > 1 ? entries : undefined;
1417+
1418+
let star = false;
1419+
const includes: string[] = [];
1420+
const excludes: string[] = [];
1421+
for (let s of text.split(/,/g)) {
1422+
s = s.trim().toLowerCase();
1423+
if (s.length === 0) continue;
1424+
if (s === "*") {
1425+
star = true;
1426+
}
1427+
else if (ts.startsWith(s, "-") || ts.startsWith(s, "!")) {
1428+
excludes.push(s.slice(1));
1429+
}
1430+
else {
1431+
includes.push(s);
1432+
}
1433+
}
1434+
1435+
// do nothing if the setting has no variations
1436+
if (includes.length <= 1 && !star && excludes.length === 0) {
1437+
return undefined;
1438+
}
1439+
1440+
const variations: { key: string, value?: string | number }[] = [];
1441+
const values = getVaryByStarSettingValues(varyBy);
1442+
1443+
// add (and deduplicate) all included entries
1444+
for (const include of includes) {
1445+
const value = values?.get(include);
1446+
if (ts.findIndex(variations, v => v.key === include || value !== undefined && v.value === value) === -1) {
1447+
variations.push({ key: include, value });
1448+
}
1449+
}
1450+
1451+
if (star && values) {
1452+
// add all entries
1453+
for (const [key, value] of ts.arrayFrom(values.entries())) {
1454+
if (ts.findIndex(variations, v => v.key === key || v.value === value) === -1) {
1455+
variations.push({ key, value });
1456+
}
1457+
}
1458+
}
1459+
1460+
// remove all excluded entries
1461+
for (const exclude of excludes) {
1462+
const value = values?.get(exclude);
1463+
let index: number;
1464+
while ((index = ts.findIndex(variations, v => v.key === exclude || value !== undefined && v.value === value)) >= 0) {
1465+
ts.orderedRemoveItemAt(variations, index);
1466+
}
1467+
}
1468+
1469+
if (variations.length === 0) {
1470+
throw new Error(`Variations in test option '@${varyBy}' resulted in an empty set.`);
1471+
}
1472+
1473+
return ts.map(variations, v => v.key);
14191474
}
14201475

14211476
function computeFileBasedTestConfigurationVariations(configurations: FileBasedTestConfiguration[], variationState: FileBasedTestConfiguration, varyByEntries: [string, string[]][], offset: number) {
@@ -1433,17 +1488,37 @@ namespace Harness {
14331488
}
14341489
}
14351490

1491+
let booleanVaryByStarSettingValues: ts.Map<string | number> | undefined;
1492+
1493+
function getVaryByStarSettingValues(varyBy: string): ts.ReadonlyMap<string | number> | undefined {
1494+
const option = ts.forEach(ts.optionDeclarations, decl => ts.equateStringsCaseInsensitive(decl.name, varyBy) ? decl : undefined);
1495+
if (option) {
1496+
if (typeof option.type === "object") {
1497+
return option.type;
1498+
}
1499+
if (option.type === "boolean") {
1500+
return booleanVaryByStarSettingValues || (booleanVaryByStarSettingValues = ts.createMapFromTemplate({
1501+
true: 1,
1502+
false: 0
1503+
}));
1504+
}
1505+
}
1506+
}
1507+
14361508
/**
14371509
* Compute FileBasedTestConfiguration variations based on a supplied list of variable settings.
14381510
*/
1439-
export function getFileBasedTestConfigurations(settings: TestCaseParser.CompilerSettings, varyBy: string[]): FileBasedTestConfiguration[] | undefined {
1511+
export function getFileBasedTestConfigurations(settings: TestCaseParser.CompilerSettings, varyBy: readonly string[]): FileBasedTestConfiguration[] | undefined {
14401512
let varyByEntries: [string, string[]][] | undefined;
1513+
let variationCount = 1;
14411514
for (const varyByKey of varyBy) {
14421515
if (ts.hasProperty(settings, varyByKey)) {
14431516
// we only consider variations when there are 2 or more variable entries.
1444-
const entries = splitVaryBySettingValue(settings[varyByKey]);
1517+
const entries = splitVaryBySettingValue(settings[varyByKey], varyByKey);
14451518
if (entries) {
14461519
if (!varyByEntries) varyByEntries = [];
1520+
variationCount *= entries.length;
1521+
if (variationCount > 25) throw new Error(`Provided test options exceeded the maximum number of variations: ${varyBy.map(v => `'@${v}'`).join(", ")}`)
14471522
varyByEntries.push([varyByKey, entries]);
14481523
}
14491524
}

‎src/testRunner/compilerRunner.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,30 @@ class CompilerBaselineRunner extends RunnerBase {
112112
}
113113

114114
class CompilerTest {
115+
private static varyBy: readonly string[] = [
116+
"module",
117+
"target",
118+
"jsx",
119+
"removeComments",
120+
"importHelpers",
121+
"importHelpers",
122+
"downlevelIteration",
123+
"isolatedModules",
124+
"strict",
125+
"noImplicitAny",
126+
"strictNullChecks",
127+
"strictFunctionTypes",
128+
"strictBindCallApply",
129+
"strictPropertyInitialization",
130+
"noImplicitThis",
131+
"alwaysStrict",
132+
"allowSyntheticDefaultImports",
133+
"esModuleInterop",
134+
"emitDecoratorMetadata",
135+
"skipDefaultLibCheck",
136+
"preserveConstEnums",
137+
"skipLibCheck",
138+
];
115139
private fileName: string;
116140
private justName: string;
117141
private configuredName: string;
@@ -220,7 +244,7 @@ class CompilerTest {
220244
// also see `parseCompilerTestConfigurations` in tests/webTestServer.ts
221245
const content = Harness.IO.readFile(file)!;
222246
const settings = Harness.TestCaseParser.extractCompilerSettings(content);
223-
const configurations = Harness.getFileBasedTestConfigurations(settings, /*varyBy*/ ["module", "target"]);
247+
const configurations = Harness.getFileBasedTestConfigurations(settings, CompilerTest.varyBy);
224248
return { file, configurations, content };
225249
}
226250

‎tests/baselines/reference/callChainWithSuper(target=es2016).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=es2017).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=es2018).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=es2019).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=es2020).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=es5).js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
var __extends = (this && this.__extends) || (function () {
12+
var extendStatics = function (d, b) {
13+
extendStatics = Object.setPrototypeOf ||
14+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
15+
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
16+
return extendStatics(d, b);
17+
};
18+
return function (d, b) {
19+
extendStatics(d, b);
20+
function __() { this.constructor = d; }
21+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
22+
};
23+
})();
24+
// GH#34952
25+
var Base = /** @class */ (function () {
26+
function Base() {
27+
}
28+
Base.prototype.method = function () { };
29+
return Base;
30+
}());
31+
var Derived = /** @class */ (function (_super) {
32+
__extends(Derived, _super);
33+
function Derived() {
34+
return _super !== null && _super.apply(this, arguments) || this;
35+
}
36+
Derived.prototype.method1 = function () { var _a; return (_a = _super.prototype.method) === null || _a === void 0 ? void 0 : _a.call(this); };
37+
Derived.prototype.method2 = function () { var _a; return (_a = _super.prototype["method"]) === null || _a === void 0 ? void 0 : _a.call(this); };
38+
return Derived;
39+
}(Base));

‎tests/baselines/reference/callChainWithSuper(target=es6).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { var _a; return (_a = super.method) === null || _a === void 0 ? void 0 : _a.call(this); }
17+
method2() { var _a; return (_a = super["method"]) === null || _a === void 0 ? void 0 : _a.call(this); }
18+
}

‎tests/baselines/reference/callChainWithSuper(target=esnext).js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//// [callChainWithSuper.ts]
2+
// GH#34952
3+
class Base { method?() {} }
4+
class Derived extends Base {
5+
method1() { return super.method?.(); }
6+
method2() { return super["method"]?.(); }
7+
}
8+
9+
//// [callChainWithSuper.js]
10+
"use strict";
11+
// GH#34952
12+
class Base {
13+
method() { }
14+
}
15+
class Derived extends Base {
16+
method1() { return super.method?.(); }
17+
method2() { return super["method"]?.(); }
18+
}

‎tests/baselines/reference/optionalMethodDeclarations(target=es2016).js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//// [optionalMethodDeclarations.ts]
2+
// https://github.com/microsoft/TypeScript/issues/34952#issuecomment-552025027
3+
class C {
4+
// ? should be removed in emit
5+
method?() {}
6+
}
7+
8+
//// [optionalMethodDeclarations.js]
9+
// https://github.com/microsoft/TypeScript/issues/34952#issuecomment-552025027
10+
class C {
11+
// ? should be removed in emit
12+
method() { }
13+
}

‎tests/baselines/reference/optionalMethodDeclarations(target=esnext).js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//// [optionalMethodDeclarations.ts]
2+
// https://github.com/microsoft/TypeScript/issues/34952#issuecomment-552025027
3+
class C {
4+
// ? should be removed in emit
5+
method?() {}
6+
}
7+
8+
//// [optionalMethodDeclarations.js]
9+
// https://github.com/microsoft/TypeScript/issues/34952#issuecomment-552025027
10+
class C {
11+
// ? should be removed in emit
12+
method() { }
13+
}

0 commit comments

Comments
 (0)