Skip to content

Add diagnostic context for expando property declarations #29905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/compiler/transformers/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,9 @@ namespace ts {
if (!isPropertyAccessExpression(p.valueDeclaration)) {
return undefined;
}
getSymbolAccessibilityDiagnostic = createGetSymbolAccessibilityDiagnosticForNode(p.valueDeclaration);
const type = resolver.createTypeOfDeclaration(p.valueDeclaration, enclosingDeclaration, declarationEmitNodeBuilderFlags, symbolTracker);
getSymbolAccessibilityDiagnostic = oldDiag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know this part of the codebase, but I don't get why you're not stashing into oldDiag first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we already stashed it for the context we made for the associated function declaration ❤️

const varDecl = createVariableDeclaration(unescapeLeadingUnderscores(p.escapedName), type, /*initializer*/ undefined);
return createVariableStatement(/*modifiers*/ undefined, createVariableDeclarationList([varDecl]));
});
Expand Down
10 changes: 6 additions & 4 deletions src/compiler/transformers/declarations/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ namespace ts {
| ImportEqualsDeclaration
| TypeAliasDeclaration
| ConstructorDeclaration
| IndexSignatureDeclaration;
| IndexSignatureDeclaration
| PropertyAccessExpression;

export function canProduceDiagnostics(node: Node): node is DeclarationDiagnosticProducing {
return isVariableDeclaration(node) ||
Expand All @@ -46,7 +47,8 @@ namespace ts {
isImportEqualsDeclaration(node) ||
isTypeAliasDeclaration(node) ||
isConstructorDeclaration(node) ||
isIndexSignatureDeclaration(node);
isIndexSignatureDeclaration(node) ||
isPropertyAccessExpression(node);
}

export function createGetSymbolAccessibilityDiagnosticForNodeName(node: DeclarationDiagnosticProducing) {
Expand Down Expand Up @@ -123,7 +125,7 @@ namespace ts {
}

export function createGetSymbolAccessibilityDiagnosticForNode(node: DeclarationDiagnosticProducing): (symbolAccessibilityResult: SymbolAccessibilityResult) => SymbolAccessibilityDiagnostic | undefined {
if (isVariableDeclaration(node) || isPropertyDeclaration(node) || isPropertySignature(node) || isBindingElement(node) || isConstructorDeclaration(node)) {
if (isVariableDeclaration(node) || isPropertyDeclaration(node) || isPropertySignature(node) || isPropertyAccessExpression(node) || isBindingElement(node) || isConstructorDeclaration(node)) {
return getVariableDeclarationTypeVisibilityError;
}
else if (isSetAccessor(node) || isGetAccessor(node)) {
Expand Down Expand Up @@ -164,7 +166,7 @@ namespace ts {
}
// This check is to ensure we don't report error on constructor parameter property as that error would be reported during parameter emit
// The only exception here is if the constructor was marked as private. we are not emitting the constructor parameters at all.
else if (node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertySignature ||
else if (node.kind === SyntaxKind.PropertyDeclaration || node.kind === SyntaxKind.PropertyAccessExpression || node.kind === SyntaxKind.PropertySignature ||
(node.kind === SyntaxKind.Parameter && hasModifier(node.parent, ModifierFlags.Private))) {
// TODO(jfreeman): Deal with computed properties in error reporting.
if (hasModifier(node, ModifierFlags.Static)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/compiler/b.ts(4,1): error TS4032: Property 'val' of exported interface has or is using name 'I' from private module '"tests/cases/compiler/a"'.


==== tests/cases/compiler/a.ts (0 errors) ====
interface I {}
export function f(): I { return null as I; }
==== tests/cases/compiler/b.ts (1 errors) ====
import {f} from "./a";

export function q() {}
q.val = f();
~~~~~
!!! error TS4032: Property 'val' of exported interface has or is using name 'I' from private module '"tests/cases/compiler/a"'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/declarationEmitExpandoPropertyPrivateName.ts] ////

//// [a.ts]
interface I {}
export function f(): I { return null as I; }
//// [b.ts]
import {f} from "./a";

export function q() {}
q.val = f();


//// [a.js]
"use strict";
exports.__esModule = true;
function f() { return null; }
exports.f = f;
//// [b.js]
"use strict";
exports.__esModule = true;
var a_1 = require("./a");
function q() { }
exports.q = q;
q.val = a_1.f();


//// [a.d.ts]
interface I {
}
export declare function f(): I;
export {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
=== tests/cases/compiler/a.ts ===
interface I {}
>I : Symbol(I, Decl(a.ts, 0, 0))

export function f(): I { return null as I; }
>f : Symbol(f, Decl(a.ts, 0, 14))
>I : Symbol(I, Decl(a.ts, 0, 0))
>I : Symbol(I, Decl(a.ts, 0, 0))

=== tests/cases/compiler/b.ts ===
import {f} from "./a";
>f : Symbol(f, Decl(b.ts, 0, 8))

export function q() {}
>q : Symbol(q, Decl(b.ts, 0, 22), Decl(b.ts, 2, 22))

q.val = f();
>q.val : Symbol(q.val, Decl(b.ts, 2, 22))
>q : Symbol(q, Decl(b.ts, 0, 22), Decl(b.ts, 2, 22))
>val : Symbol(q.val, Decl(b.ts, 2, 22))
>f : Symbol(f, Decl(b.ts, 0, 8))

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
=== tests/cases/compiler/a.ts ===
interface I {}
export function f(): I { return null as I; }
>f : () => I
>null as I : I
>null : null

=== tests/cases/compiler/b.ts ===
import {f} from "./a";
>f : () => I

export function q() {}
>q : typeof q

q.val = f();
>q.val = f() : I
>q.val : I
>q : typeof q
>val : I
>f() : I
>f : () => I

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @declaration: true
// @filename: a.ts
interface I {}
export function f(): I { return null as I; }
// @filename: b.ts
import {f} from "./a";

export function q() {}
q.val = f();
Submodule TypeScript-Node-Starter updated 49 files
+2 −11 .env.example
+1 −7 .gitignore
+0 −1 .vscode/extensions.json
+15 −3 .vscode/launch.json
+2 −8 .vscode/settings.json
+75 −190 README.md
+5 −0 copyStaticAssets.js
+0 −5 copyStaticAssets.ts
+0 −18 jest.config.js
+1,346 −1,722 package-lock.json
+59 −51 package.json
+19 −22 src/app.ts
+5 −5 src/config/passport.ts
+3 −3 src/controllers/api.ts
+1 −1 src/controllers/contact.ts
+6 −6 src/controllers/user.ts
+6 −9 src/models/User.ts
+1 −1 src/public/css/lib/bootstrap/_button-groups.scss
+1 −1 src/public/css/lib/bootstrap/_forms.scss
+1 −1 src/public/css/lib/bootstrap/_input-groups.scss
+1 −1 src/public/css/lib/bootstrap/_panels.scss
+1 −1 src/public/css/lib/bootstrap/_scaffolding.scss
+2 −2 src/public/css/lib/bootstrap/_theme.scss
+1 −1 src/public/css/lib/bootstrap/_variables.scss
+2 −2 src/public/css/lib/bootstrap/bootstrap.scss
+3 −3 src/public/css/lib/bootstrap/mixins/_tab-focus.scss
+ src/public/fonts/glyphicons-halflings-regular.eot
+0 −288 src/public/fonts/glyphicons-halflings-regular.svg
+ src/public/fonts/glyphicons-halflings-regular.ttf
+ src/public/fonts/glyphicons-halflings-regular.woff
+ src/public/fonts/glyphicons-halflings-regular.woff2
+4 −4 src/public/js/lib/jquery-3.1.1.min.js
+4 −8 src/server.ts
+2 −0 src/types/lusca.d.ts
+48 −0 src/types/passport-local.d.ts
+0 −17 src/util/logger.ts
+0 −26 src/util/secrets.ts
+2 −2 test/api.test.ts
+2 −2 test/app.test.ts
+2 −20 test/contact.test.ts
+2 −2 test/home.test.ts
+2 −20 test/user.test.ts
+0 −1 tsconfig.json
+1 −1 views/account/profile.pug
+1 −1 views/api/facebook.pug
+1 −1 views/api/index.pug
+9 −8 views/layout.pug
+3 −2 views/partials/footer.pug
+2 −2 views/partials/header.pug
2 changes: 1 addition & 1 deletion tests/cases/user/prettier/prettier
Submodule prettier updated 1906 files
2 changes: 1 addition & 1 deletion tests/cases/user/webpack/webpack
Submodule webpack updated 1279 files