Skip to content

Fix control flow analysis for property initializers #8970

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.

Sign up for GitHub

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
ahejlsberg merged 2 commits into from
Jun 7, 2016
Merged

Fix control flow analysis for property initializers #8970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb7818b
Consider property declarations to be control flow containers
ahejlsberg Jun 4, 2016
3b1effb
Adding regression test
ahejlsberg Jun 4, 2016
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
15 changes: 12 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8091,13 +8091,22 @@ namespace ts {
return expression;
}

function getControlFlowContainer(node: Node): Node {
while (true) {
node = node.parent;
if (isFunctionLike(node) || node.kind === SyntaxKind.ModuleBlock || node.kind === SyntaxKind.SourceFile || node.kind === SyntaxKind.PropertyDeclaration) {

@DanielRosenwasser DanielRosenwasser Jun 5, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this cover computed properties?

const LANG = "German";
class WurstLanguage {
    [LANG]() {
    }
}

Maybe the class itself should be a control flow container.

@DanielRosenwasser DanielRosenwasser Jun 5, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, it should work since the parent will be a method declaration with a computed property name. It still might be worth adding as a test.

return node;
}
}
}

function isDeclarationIncludedInFlow(reference: Node, declaration: Declaration, includeOuterFunctions: boolean) {
const declarationContainer = getContainingFunctionOrModule(declaration);
let container = getContainingFunctionOrModule(reference);
const declarationContainer = getControlFlowContainer(declaration);
let container = getControlFlowContainer(reference);
while (container !== declarationContainer &&
(container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.ArrowFunction) &&
(includeOuterFunctions || getImmediatelyInvokedFunctionExpression(<FunctionExpression>container))) {
container = getContainingFunctionOrModule(container);
container = getControlFlowContainer(container);
}
return container === declarationContainer;
}
Expand Down
9 changes: 0 additions & 9 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,15 +880,6 @@ namespace ts {
}
}

export function getContainingFunctionOrModule(node: Node): Node {
while (true) {
node = node.parent;
if (isFunctionLike(node) || node.kind === SyntaxKind.ModuleDeclaration || node.kind === SyntaxKind.SourceFile) {
return node;
}
}
}

export function getContainingClass(node: Node): ClassLikeDeclaration {
while (true) {
node = node.parent;
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/controlFlowPropertyInitializer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [controlFlowPropertyInitializer.ts]

// Repro from #8967

const LANG = "Turbo Pascal"

class BestLanguage {
name = LANG;
}

//// [controlFlowPropertyInitializer.js]
// Repro from #8967
var LANG = "Turbo Pascal";
var BestLanguage = (function () {
function BestLanguage() {
this.name = LANG;
}
return BestLanguage;
}());
14 changes: 14 additions & 0 deletions tests/baselines/reference/controlFlowPropertyInitializer.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
=== tests/cases/compiler/controlFlowPropertyInitializer.ts ===

// Repro from #8967

const LANG = "Turbo Pascal"
>LANG : Symbol(LANG, Decl(controlFlowPropertyInitializer.ts, 3, 5))

class BestLanguage {
>BestLanguage : Symbol(BestLanguage, Decl(controlFlowPropertyInitializer.ts, 3, 27))

name = LANG;
>name : Symbol(BestLanguage.name, Decl(controlFlowPropertyInitializer.ts, 5, 20))
>LANG : Symbol(LANG, Decl(controlFlowPropertyInitializer.ts, 3, 5))
}
15 changes: 15 additions & 0 deletions tests/baselines/reference/controlFlowPropertyInitializer.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
=== tests/cases/compiler/controlFlowPropertyInitializer.ts ===

// Repro from #8967

const LANG = "Turbo Pascal"
>LANG : string
>"Turbo Pascal" : string

class BestLanguage {
>BestLanguage : BestLanguage

name = LANG;
>name : string
>LANG : string
}
9 changes: 9 additions & 0 deletions tests/cases/compiler/controlFlowPropertyInitializer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// @strictNullChecks: true

// Repro from #8967

const LANG = "Turbo Pascal"

class BestLanguage {
name = LANG;
}