Skip to content
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

Implicit const parameters #10357

Merged
merged 3 commits into from
Aug 16, 2016
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
93 changes: 72 additions & 21 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ namespace ts {

function getNodeLinks(node: Node): NodeLinks {
const nodeId = getNodeId(node);
return nodeLinks[nodeId] || (nodeLinks[nodeId] = {});
return nodeLinks[nodeId] || (nodeLinks[nodeId] = { flags: 0 });
}

function isGlobalSourceFile(node: Node) {
Expand Down Expand Up @@ -8185,7 +8185,7 @@ namespace ts {
return incomplete ? { flags: 0, type } : type;
}

function getFlowTypeOfReference(reference: Node, declaredType: Type, assumeInitialized: boolean, includeOuterFunctions: boolean) {
function getFlowTypeOfReference(reference: Node, declaredType: Type, assumeInitialized: boolean, flowContainer: Node) {
let key: string;
if (!reference.flowNode || assumeInitialized && !(declaredType.flags & TypeFlags.Narrowable)) {
return declaredType;
Expand Down Expand Up @@ -8237,7 +8237,7 @@ namespace ts {
else if (flow.flags & FlowFlags.Start) {
// Check if we should continue with the control flow of the containing function.
const container = (<FlowStart>flow).container;
if (container && includeOuterFunctions) {
if (container && container !== flowContainer && reference.kind !== SyntaxKind.PropertyAccessExpression) {
flow = container.flowNode;
continue;
}
Expand Down Expand Up @@ -8708,21 +8708,52 @@ namespace ts {
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) {
if (isFunctionLike(node) && !getImmediatelyInvokedFunctionExpression(node) ||
node.kind === SyntaxKind.ModuleBlock ||
node.kind === SyntaxKind.SourceFile ||
node.kind === SyntaxKind.PropertyDeclaration) {
return node;
}
}
}

function isDeclarationIncludedInFlow(reference: Node, declaration: Declaration, includeOuterFunctions: boolean) {
const declarationContainer = getControlFlowContainer(declaration);
let container = getControlFlowContainer(reference);
while (container !== declarationContainer &&
(container.kind === SyntaxKind.FunctionExpression || container.kind === SyntaxKind.ArrowFunction) &&
(includeOuterFunctions || getImmediatelyInvokedFunctionExpression(<FunctionExpression>container))) {
container = getControlFlowContainer(container);
// Check if a parameter is assigned anywhere within its declaring function.
function isParameterAssigned(symbol: Symbol) {
const func = <FunctionLikeDeclaration>getRootDeclaration(symbol.valueDeclaration).parent;
const links = getNodeLinks(func);
if (!(links.flags & NodeCheckFlags.AssignmentsMarked)) {
links.flags |= NodeCheckFlags.AssignmentsMarked;
if (!hasParentWithAssignmentsMarked(func)) {
markParameterAssignments(func);
}
}
return symbol.isAssigned || false;
}

function hasParentWithAssignmentsMarked(node: Node) {
while (true) {
node = node.parent;
if (!node) {
return false;
}
if (isFunctionLike(node) && getNodeLinks(node).flags & NodeCheckFlags.AssignmentsMarked) {
return true;
}
}
}

function markParameterAssignments(node: Node) {
if (node.kind === SyntaxKind.Identifier) {
if (isAssignmentTarget(node)) {
const symbol = getResolvedSymbol(<Identifier>node);
if (symbol.valueDeclaration && getRootDeclaration(symbol.valueDeclaration).kind === SyntaxKind.Parameter) {
symbol.isAssigned = true;
}
}
}
else {
forEachChild(node, markParameterAssignments);
}
return container === declarationContainer;
}

function checkIdentifier(node: Identifier): Type {
Expand Down Expand Up @@ -8777,15 +8808,35 @@ namespace ts {
checkNestedBlockScopedBinding(node, symbol);

const type = getTypeOfSymbol(localOrExportSymbol);
if (!(localOrExportSymbol.flags & SymbolFlags.Variable) || isAssignmentTarget(node)) {
const declaration = localOrExportSymbol.valueDeclaration;
// We only narrow variables and parameters occurring in a non-assignment position. For all other
// entities we simply return the declared type.
if (!(localOrExportSymbol.flags & SymbolFlags.Variable) || isAssignmentTarget(node) || !declaration) {
return type;
}
const declaration = localOrExportSymbol.valueDeclaration;
const includeOuterFunctions = isReadonlySymbol(localOrExportSymbol);
const assumeInitialized = !strictNullChecks || (type.flags & TypeFlags.Any) !== 0 || !declaration ||
getRootDeclaration(declaration).kind === SyntaxKind.Parameter || isInAmbientContext(declaration) ||
!isDeclarationIncludedInFlow(node, declaration, includeOuterFunctions);
const flowType = getFlowTypeOfReference(node, type, assumeInitialized, includeOuterFunctions);
// The declaration container is the innermost function that encloses the declaration of the variable
// or parameter. The flow container is the innermost function starting with which we analyze the control
// flow graph to determine the control flow based type.
const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
const declarationContainer = getControlFlowContainer(declaration);
let flowContainer = getControlFlowContainer(node);
// When the control flow originates in a function expression or arrow function and we are referencing
// a const variable or parameter from an outer function, we extend the origin of the control flow
// analysis to include the immediately enclosing function.
while (flowContainer !== declarationContainer &&
(flowContainer.kind === SyntaxKind.FunctionExpression || flowContainer.kind === SyntaxKind.ArrowFunction) &&
(isReadonlySymbol(localOrExportSymbol) || isParameter && !isParameterAssigned(localOrExportSymbol))) {
flowContainer = getControlFlowContainer(flowContainer);
}
// We only look for uninitialized variables in strict null checking mode, and only when we can analyze
// the entire control flow graph from the variable's declaration (i.e. when the flow container and
// declaration container are the same).
const assumeInitialized = !strictNullChecks || (type.flags & TypeFlags.Any) !== 0 || isParameter ||
flowContainer !== declarationContainer || isInAmbientContext(declaration);
const flowType = getFlowTypeOfReference(node, type, assumeInitialized, flowContainer);
// A variable is considered uninitialized when it is possible to analyze the entire control flow graph
// from declaration to use, and when the variable's declared type doesn't include undefined but the
// control flow based type does include undefined.
if (!assumeInitialized && !(getFalsyFlags(type) & TypeFlags.Undefined) && getFalsyFlags(flowType) & TypeFlags.Undefined) {
error(node, Diagnostics.Variable_0_is_used_before_being_assigned, symbolToString(symbol));
// Return the declared type to reduce follow-on errors
Expand Down Expand Up @@ -9038,7 +9089,7 @@ namespace ts {
if (isClassLike(container.parent)) {
const symbol = getSymbolOfNode(container.parent);
const type = container.flags & NodeFlags.Static ? getTypeOfSymbol(symbol) : (<InterfaceType>getDeclaredTypeOfSymbol(symbol)).thisType;
return getFlowTypeOfReference(node, type, /*assumeInitialized*/ true, /*includeOuterFunctions*/ true);
return getFlowTypeOfReference(node, type, /*assumeInitialized*/ true, /*flowContainer*/ undefined);
}

if (isInJavaScriptFile(node)) {
Expand Down Expand Up @@ -10699,7 +10750,7 @@ namespace ts {
!(prop.flags & SymbolFlags.Method && propType.flags & TypeFlags.Union)) {
return propType;
}
return getFlowTypeOfReference(node, propType, /*assumeInitialized*/ true, /*includeOuterFunctions*/ false);
return getFlowTypeOfReference(node, propType, /*assumeInitialized*/ true, /*flowContainer*/ undefined);
}

function isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName, propertyName: string): boolean {
Expand Down
20 changes: 11 additions & 9 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,7 @@ namespace ts {
/* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol
/* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums
/* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere
/* @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments
}

/* @internal */
Expand Down Expand Up @@ -2209,23 +2210,24 @@ namespace ts {
AsyncMethodWithSuper = 0x00000800, // An async method that reads a value from a member of 'super'.
AsyncMethodWithSuperBinding = 0x00001000, // An async method that assigns a value to a member of 'super'.
CaptureArguments = 0x00002000, // Lexical 'arguments' used in body (for async functions)
EnumValuesComputed = 0x00004000, // Values for enum members have been computed, and any errors have been reported for them.
LexicalModuleMergesWithClass = 0x00008000, // Instantiated lexical module declaration is merged with a previous class declaration.
LoopWithCapturedBlockScopedBinding = 0x00010000, // Loop that contains block scoped variable captured in closure
CapturedBlockScopedBinding = 0x00020000, // Block-scoped binding that is captured in some function
BlockScopedBindingInLoop = 0x00040000, // Block-scoped binding with declaration nested inside iteration statement
ClassWithBodyScopedClassBinding = 0x00080000, // Decorated class that contains a binding to itself inside of the class body.
BodyScopedClassBinding = 0x00100000, // Binding to a decorated class inside of the class's body.
NeedsLoopOutParameter = 0x00200000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
EnumValuesComputed = 0x00004000, // Values for enum members have been computed, and any errors have been reported for them.
LexicalModuleMergesWithClass = 0x00008000, // Instantiated lexical module declaration is merged with a previous class declaration.
LoopWithCapturedBlockScopedBinding = 0x00010000, // Loop that contains block scoped variable captured in closure
CapturedBlockScopedBinding = 0x00020000, // Block-scoped binding that is captured in some function
BlockScopedBindingInLoop = 0x00040000, // Block-scoped binding with declaration nested inside iteration statement
ClassWithBodyScopedClassBinding = 0x00080000, // Decorated class that contains a binding to itself inside of the class body.
BodyScopedClassBinding = 0x00100000, // Binding to a decorated class inside of the class's body.
NeedsLoopOutParameter = 0x00200000, // Block scoped binding whose value should be explicitly copied outside of the converted loop
AssignmentsMarked = 0x00400000, // Parameter assignments have been marked
}

/* @internal */
export interface NodeLinks {
flags?: NodeCheckFlags; // Set of flags specific to Node
resolvedType?: Type; // Cached type of type node
resolvedSignature?: Signature; // Cached signature of signature node or call expression
resolvedSymbol?: Symbol; // Cached name resolution result
resolvedIndexInfo?: IndexInfo; // Cached indexing info resolution result
flags?: NodeCheckFlags; // Set of flags specific to Node
enumMemberValue?: number; // Constant value of enum member
isVisible?: boolean; // Is this node visible
hasReportedStatementInAmbientContext?: boolean; // Cache boolean if we report statements in ambient context
Expand Down
65 changes: 65 additions & 0 deletions tests/baselines/reference/implicitConstParameters.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
tests/cases/compiler/implicitConstParameters.ts(39,27): error TS2532: Object is possibly 'undefined'.
tests/cases/compiler/implicitConstParameters.ts(45,27): error TS2532: Object is possibly 'undefined'.


==== tests/cases/compiler/implicitConstParameters.ts (2 errors) ====

function doSomething(cb: () => void) {
cb();
}

function fn(x: number | string) {
if (typeof x === 'number') {
doSomething(() => x.toFixed());
}
}

function f1(x: string | undefined) {
if (!x) {
return;
}
doSomething(() => x.length);
}

function f2(x: string | undefined) {
if (x) {
doSomething(() => {
doSomething(() => x.length);
});
}
}

function f3(x: string | undefined) {
inner();
function inner() {
if (x) {
doSomething(() => x.length);
}
}
}

function f4(x: string | undefined) {
x = "abc"; // causes x to be considered non-const
if (x) {
doSomething(() => x.length);
~
!!! error TS2532: Object is possibly 'undefined'.
}
}

function f5(x: string | undefined) {
if (x) {
doSomething(() => x.length);
~
!!! error TS2532: Object is possibly 'undefined'.
}
x = "abc"; // causes x to be considered non-const
}


function f6(x: string | undefined) {
const y = x || "";
if (x) {
doSomething(() => y.length);
}
}
106 changes: 106 additions & 0 deletions tests/baselines/reference/implicitConstParameters.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
//// [implicitConstParameters.ts]

function doSomething(cb: () => void) {
cb();
}

function fn(x: number | string) {
if (typeof x === 'number') {
doSomething(() => x.toFixed());
}
}

function f1(x: string | undefined) {
if (!x) {
return;
}
doSomething(() => x.length);
}

function f2(x: string | undefined) {
if (x) {
doSomething(() => {
doSomething(() => x.length);
});
}
}

function f3(x: string | undefined) {
inner();
function inner() {
if (x) {
doSomething(() => x.length);
}
}
}

function f4(x: string | undefined) {
x = "abc"; // causes x to be considered non-const
if (x) {
doSomething(() => x.length);
}
}

function f5(x: string | undefined) {
if (x) {
doSomething(() => x.length);
}
x = "abc"; // causes x to be considered non-const
}


function f6(x: string | undefined) {
const y = x || "";
if (x) {
doSomething(() => y.length);
}
}

//// [implicitConstParameters.js]
function doSomething(cb) {
cb();
}
function fn(x) {
if (typeof x === 'number') {
doSomething(function () { return x.toFixed(); });
}
}
function f1(x) {
if (!x) {
return;
}
doSomething(function () { return x.length; });
}
function f2(x) {
if (x) {
doSomething(function () {
doSomething(function () { return x.length; });
});
}
}
function f3(x) {
inner();
function inner() {
if (x) {
doSomething(function () { return x.length; });
}
}
}
function f4(x) {
x = "abc"; // causes x to be considered non-const
if (x) {
doSomething(function () { return x.length; });
}
}
function f5(x) {
if (x) {
doSomething(function () { return x.length; });
}
x = "abc"; // causes x to be considered non-const
}
function f6(x) {
var y = x || "";
if (x) {
doSomething(function () { return y.length; });
}
}
Loading