Skip to content

Commit 4c43e09

Browse files
weswighamweswigham
authored
Add fallback error locations for nameless declarations, better class error locations (microsoft#42585)
1 parent aba932a commit 4c43e09

7 files changed

+360
-5
lines changed

‎src/compiler/transformers/declarations.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ namespace ts {
8080
reportNonlocalAugmentation
8181
};
8282
let errorNameNode: DeclarationName | undefined;
83+
let errorFallbackNode: Declaration | undefined;
8384

8485
let currentSourceFile: SourceFile;
8586
let refs: ESMap<NodeId, SourceFile>;
@@ -161,9 +162,9 @@ namespace ts {
161162
}
162163

163164
function reportPrivateInBaseOfClassExpression(propertyName: string) {
164-
if (errorNameNode) {
165+
if (errorNameNode || errorFallbackNode) {
165166
context.addDiagnostic(
166-
createDiagnosticForNode(errorNameNode, Diagnostics.Property_0_of_exported_class_expression_may_not_be_private_or_protected, propertyName));
167+
createDiagnosticForNode((errorNameNode || errorFallbackNode)!, Diagnostics.Property_0_of_exported_class_expression_may_not_be_private_or_protected, propertyName));
167168
}
168169
}
169170

@@ -199,8 +200,8 @@ namespace ts {
199200
}
200201

201202
function reportTruncationError() {
202-
if (errorNameNode) {
203-
context.addDiagnostic(createDiagnosticForNode(errorNameNode, Diagnostics.The_inferred_type_of_this_node_exceeds_the_maximum_length_the_compiler_will_serialize_An_explicit_type_annotation_is_needed));
203+
if (errorNameNode || errorFallbackNode) {
204+
context.addDiagnostic(createDiagnosticForNode((errorNameNode || errorFallbackNode)!, Diagnostics.The_inferred_type_of_this_node_exceeds_the_maximum_length_the_compiler_will_serialize_An_explicit_type_annotation_is_needed));
204205
}
205206
}
206207

@@ -1102,7 +1103,9 @@ namespace ts {
11021103
diagnosticMessage: Diagnostics.Default_export_of_the_module_has_or_is_using_private_name_0,
11031104
errorNode: input
11041105
});
1106+
errorFallbackNode = input;
11051107
const varDecl = factory.createVariableDeclaration(newId, /*exclamationToken*/ undefined, resolver.createTypeOfExpression(input.expression, input, declarationEmitNodeBuilderFlags, symbolTracker), /*initializer*/ undefined);
1108+
errorFallbackNode = undefined;
11061109
const statement = factory.createVariableStatement(needsDeclare ? [factory.createModifier(SyntaxKind.DeclareKeyword)] : [], factory.createVariableDeclarationList([varDecl], NodeFlags.Const));
11071110
return [statement, factory.updateExportAssignment(input, input.decorators, input.modifiers, newId)];
11081111
}
@@ -1326,6 +1329,8 @@ namespace ts {
13261329
}
13271330
}
13281331
case SyntaxKind.ClassDeclaration: {
1332+
errorNameNode = input.name;
1333+
errorFallbackNode = input;
13291334
const modifiers = factory.createNodeArray(ensureModifiers(input));
13301335
const typeParameters = ensureTypeParams(input, input.typeParameters);
13311336
const ctor = getFirstConstructorWithBody(input);
@@ -1462,6 +1467,8 @@ namespace ts {
14621467
if (node as Node === input) {
14631468
return node;
14641469
}
1470+
errorFallbackNode = undefined;
1471+
errorNameNode = undefined;
14651472
return node && setOriginalNode(preserveJsDoc(node, input), input);
14661473
}
14671474
}

‎tests/baselines/reference/declarationEmitMixinPrivateProtected.errors.txt

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
tests/cases/compiler/another.ts(11,1): error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
2+
tests/cases/compiler/another.ts(11,1): error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
3+
tests/cases/compiler/first.ts(12,1): error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
4+
tests/cases/compiler/first.ts(12,1): error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
5+
tests/cases/compiler/first.ts(13,14): error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
6+
tests/cases/compiler/first.ts(13,14): error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
7+
8+
9+
==== tests/cases/compiler/first.ts (4 errors) ====
10+
declare function mix<TMix>(mixin: TMix): TMix;
11+
12+
const DisposableMixin = class {
13+
protected _onDispose() {
14+
this._assertIsStripped()
15+
}
16+
private _assertIsStripped() {
17+
}
18+
};
19+
20+
// No error, but definition is wrong.
21+
export default mix(DisposableMixin);
22+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
23+
!!! error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
24+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
25+
!!! error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
26+
export class Monitor extends mix(DisposableMixin) {
27+
~~~~~~~
28+
!!! error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
29+
~~~~~~~
30+
!!! error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
31+
protected _onDispose() {
32+
}
33+
}
34+
35+
==== tests/cases/compiler/another.ts (2 errors) ====
36+
declare function mix<TMix>(mixin: TMix): TMix;
37+
38+
const DisposableMixin = class {
39+
protected _onDispose() {
40+
this._assertIsStripped()
41+
}
42+
private _assertIsStripped() {
43+
}
44+
};
45+
46+
export default class extends mix(DisposableMixin) {
47+
~~~~~~
48+
!!! error TS4094: Property '_assertIsStripped' of exported class expression may not be private or protected.
49+
~~~~~~
50+
!!! error TS4094: Property '_onDispose' of exported class expression may not be private or protected.
51+
protected _onDispose() {
52+
}
53+
}

‎tests/baselines/reference/declarationEmitMixinPrivateProtected.js

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
//// [tests/cases/compiler/declarationEmitMixinPrivateProtected.ts] ////
2+
3+
//// [first.ts]
4+
declare function mix<TMix>(mixin: TMix): TMix;
5+
6+
const DisposableMixin = class {
7+
protected _onDispose() {
8+
this._assertIsStripped()
9+
}
10+
private _assertIsStripped() {
11+
}
12+
};
13+
14+
// No error, but definition is wrong.
15+
export default mix(DisposableMixin);
16+
export class Monitor extends mix(DisposableMixin) {
17+
protected _onDispose() {
18+
}
19+
}
20+
21+
//// [another.ts]
22+
declare function mix<TMix>(mixin: TMix): TMix;
23+
24+
const DisposableMixin = class {
25+
protected _onDispose() {
26+
this._assertIsStripped()
27+
}
28+
private _assertIsStripped() {
29+
}
30+
};
31+
32+
export default class extends mix(DisposableMixin) {
33+
protected _onDispose() {
34+
}
35+
}
36+
37+
//// [first.js]
38+
"use strict";
39+
var __extends = (this && this.__extends) || (function () {
40+
var extendStatics = function (d, b) {
41+
extendStatics = Object.setPrototypeOf ||
42+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
43+
function (d, b) { for (var p in b) if (Object.prototype.hasOwnProperty.call(b, p)) d[p] = b[p]; };
44+
return extendStatics(d, b);
45+
};
46+
return function (d, b) {
47+
if (typeof b !== "function" && b !== null)
48+
throw new TypeError("Class extends value " + String(b) + " is not a constructor or null");
49+
extendStatics(d, b);
50+
function __() { this.constructor = d; }
51+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
52+
};
53+
})();
54+
exports.__esModule = true;
55+
exports.Monitor = void 0;
56+
var DisposableMixin = /** @class */ (function () {
57+
function class_1() {
58+
}
59+
class_1.prototype._onDispose = function () {
60+
this._assertIsStripped();
61+
};
62+
class_1.prototype._assertIsStripped = function () {
63+
};
64+
return class_1;
65+
}());
66+
// No error, but definition is wrong.
67+
exports["default"] = mix(DisposableMixin);
68+
var Monitor = /** @class */ (function (_super) {
69+
__extends(Monitor, _super);
70+
function Monitor() {
71+
return _super !== null && _super.apply(this, arguments) || this;
72+
}
73+
Monitor.prototype._onDispose = function () {
74+
};
75+
return Monitor;
76+
}(mix(DisposableMixin)));
77+
exports.Monitor = Monitor;
78+
//// [another.js]
79+
"use strict";
80+
var __extends = (this && this.__extends) || (function () {
81+
var extendStatics = function (d, b) {
82+
extendStatics = Object.setPrototypeOf ||
83+
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
84+
function (d, b) { for (var p in b) if (Object.prototype.hasOwnProperty.call(b, p)) d[p] = b[p]; };
85+
return extendStatics(d, b);
86+
};
87+
return function (d, b) {
88+
if (typeof b !== "function" && b !== null)
89+
throw new TypeError("Class extends value " + String(b) + " is not a constructor or null");
90+
extendStatics(d, b);
91+
function __() { this.constructor = d; }
92+
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
93+
};
94+
})();
95+
exports.__esModule = true;
96+
var DisposableMixin = /** @class */ (function () {
97+
function class_1() {
98+
}
99+
class_1.prototype._onDispose = function () {
100+
this._assertIsStripped();
101+
};
102+
class_1.prototype._assertIsStripped = function () {
103+
};
104+
return class_1;
105+
}());
106+
var default_1 = /** @class */ (function (_super) {
107+
__extends(default_1, _super);
108+
function default_1() {
109+
return _super !== null && _super.apply(this, arguments) || this;
110+
}
111+
default_1.prototype._onDispose = function () {
112+
};
113+
return default_1;
114+
}(mix(DisposableMixin)));
115+
exports["default"] = default_1;

‎tests/baselines/reference/declarationEmitMixinPrivateProtected.symbols

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
=== tests/cases/compiler/first.ts ===
2+
declare function mix<TMix>(mixin: TMix): TMix;
3+
>mix : Symbol(mix, Decl(first.ts, 0, 0))
4+
>TMix : Symbol(TMix, Decl(first.ts, 0, 21))
5+
>mixin : Symbol(mixin, Decl(first.ts, 0, 27))
6+
>TMix : Symbol(TMix, Decl(first.ts, 0, 21))
7+
>TMix : Symbol(TMix, Decl(first.ts, 0, 21))
8+
9+
const DisposableMixin = class {
10+
>DisposableMixin : Symbol(DisposableMixin, Decl(first.ts, 2, 5))
11+
12+
protected _onDispose() {
13+
>_onDispose : Symbol(DisposableMixin._onDispose, Decl(first.ts, 2, 31))
14+
15+
this._assertIsStripped()
16+
>this._assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(first.ts, 5, 5))
17+
>this : Symbol(DisposableMixin, Decl(first.ts, 2, 23))
18+
>_assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(first.ts, 5, 5))
19+
}
20+
private _assertIsStripped() {
21+
>_assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(first.ts, 5, 5))
22+
}
23+
};
24+
25+
// No error, but definition is wrong.
26+
export default mix(DisposableMixin);
27+
>mix : Symbol(mix, Decl(first.ts, 0, 0))
28+
>DisposableMixin : Symbol(DisposableMixin, Decl(first.ts, 2, 5))
29+
30+
export class Monitor extends mix(DisposableMixin) {
31+
>Monitor : Symbol(Monitor, Decl(first.ts, 11, 36))
32+
>mix : Symbol(mix, Decl(first.ts, 0, 0))
33+
>DisposableMixin : Symbol(DisposableMixin, Decl(first.ts, 2, 5))
34+
35+
protected _onDispose() {
36+
>_onDispose : Symbol(Monitor._onDispose, Decl(first.ts, 12, 51))
37+
}
38+
}
39+
40+
=== tests/cases/compiler/another.ts ===
41+
declare function mix<TMix>(mixin: TMix): TMix;
42+
>mix : Symbol(mix, Decl(another.ts, 0, 0))
43+
>TMix : Symbol(TMix, Decl(another.ts, 0, 21))
44+
>mixin : Symbol(mixin, Decl(another.ts, 0, 27))
45+
>TMix : Symbol(TMix, Decl(another.ts, 0, 21))
46+
>TMix : Symbol(TMix, Decl(another.ts, 0, 21))
47+
48+
const DisposableMixin = class {
49+
>DisposableMixin : Symbol(DisposableMixin, Decl(another.ts, 2, 5))
50+
51+
protected _onDispose() {
52+
>_onDispose : Symbol(DisposableMixin._onDispose, Decl(another.ts, 2, 31))
53+
54+
this._assertIsStripped()
55+
>this._assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(another.ts, 5, 5))
56+
>this : Symbol(DisposableMixin, Decl(another.ts, 2, 23))
57+
>_assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(another.ts, 5, 5))
58+
}
59+
private _assertIsStripped() {
60+
>_assertIsStripped : Symbol(DisposableMixin._assertIsStripped, Decl(another.ts, 5, 5))
61+
}
62+
};
63+
64+
export default class extends mix(DisposableMixin) {
65+
>mix : Symbol(mix, Decl(another.ts, 0, 0))
66+
>DisposableMixin : Symbol(DisposableMixin, Decl(another.ts, 2, 5))
67+
68+
protected _onDispose() {
69+
>_onDispose : Symbol(default._onDispose, Decl(another.ts, 10, 51))
70+
}
71+
}

‎tests/baselines/reference/declarationEmitMixinPrivateProtected.types

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
=== tests/cases/compiler/first.ts ===
2+
declare function mix<TMix>(mixin: TMix): TMix;
3+
>mix : <TMix>(mixin: TMix) => TMix
4+
>mixin : TMix
5+
6+
const DisposableMixin = class {
7+
>DisposableMixin : typeof DisposableMixin
8+
>class { protected _onDispose() { this._assertIsStripped() } private _assertIsStripped() { }} : typeof DisposableMixin
9+
10+
protected _onDispose() {
11+
>_onDispose : () => void
12+
13+
this._assertIsStripped()
14+
>this._assertIsStripped() : void
15+
>this._assertIsStripped : () => void
16+
>this : this
17+
>_assertIsStripped : () => void
18+
}
19+
private _assertIsStripped() {
20+
>_assertIsStripped : () => void
21+
}
22+
};
23+
24+
// No error, but definition is wrong.
25+
export default mix(DisposableMixin);
26+
>mix(DisposableMixin) : typeof DisposableMixin
27+
>mix : <TMix>(mixin: TMix) => TMix
28+
>DisposableMixin : typeof DisposableMixin
29+
30+
export class Monitor extends mix(DisposableMixin) {
31+
>Monitor : Monitor
32+
>mix(DisposableMixin) : DisposableMixin
33+
>mix : <TMix>(mixin: TMix) => TMix
34+
>DisposableMixin : typeof DisposableMixin
35+
36+
protected _onDispose() {
37+
>_onDispose : () => void
38+
}
39+
}
40+
41+
=== tests/cases/compiler/another.ts ===
42+
declare function mix<TMix>(mixin: TMix): TMix;
43+
>mix : <TMix>(mixin: TMix) => TMix
44+
>mixin : TMix
45+
46+
const DisposableMixin = class {
47+
>DisposableMixin : typeof DisposableMixin
48+
>class { protected _onDispose() { this._assertIsStripped() } private _assertIsStripped() { }} : typeof DisposableMixin
49+
50+
protected _onDispose() {
51+
>_onDispose : () => void
52+
53+
this._assertIsStripped()
54+
>this._assertIsStripped() : void
55+
>this._assertIsStripped : () => void
56+
>this : this
57+
>_assertIsStripped : () => void
58+
}
59+
private _assertIsStripped() {
60+
>_assertIsStripped : () => void
61+
}
62+
};
63+
64+
export default class extends mix(DisposableMixin) {
65+
>mix(DisposableMixin) : DisposableMixin
66+
>mix : <TMix>(mixin: TMix) => TMix
67+
>DisposableMixin : typeof DisposableMixin
68+
69+
protected _onDispose() {
70+
>_onDispose : () => void
71+
}
72+
}

‎tests/baselines/reference/emitClassExpressionInDeclarationFile2.errors.txt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts(1,12): error TS4094: Property 'p' of exported class expression may not be private or protected.
22
tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts(1,12): error TS4094: Property 'ps' of exported class expression may not be private or protected.
33
tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts(16,17): error TS4094: Property 'property' of exported class expression may not be private or protected.
4+
tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts(23,14): error TS4094: Property 'property' of exported class expression may not be private or protected.
45

56

6-
==== tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts (3 errors) ====
7+
==== tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts (4 errors) ====
78
export var noPrivates = class {
89
~~~~~~~~~~
910
!!! error TS4094: Property 'p' of exported class expression may not be private or protected.
@@ -33,6 +34,8 @@ tests/cases/compiler/emitClassExpressionInDeclarationFile2.ts(16,17): error TS40
3334
}
3435

3536
export class Test extends WithTags(FooItem) {}
37+
~~~~
38+
!!! error TS4094: Property 'property' of exported class expression may not be private or protected.
3639

3740
const test = new Test();
3841

0 commit comments

Comments
 (0)