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 2 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
80 changes: 59 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,22 @@ namespace ts {
checkNestedBlockScopedBinding(node, symbol);

const type = getTypeOfSymbol(localOrExportSymbol);
if (!(localOrExportSymbol.flags & SymbolFlags.Variable) || isAssignmentTarget(node)) {
const declaration = localOrExportSymbol.valueDeclaration;
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);

const isParameter = getRootDeclaration(declaration).kind === SyntaxKind.Parameter;
const declarationContainer = getControlFlowContainer(declaration);
let flowContainer = getControlFlowContainer(node);
while (flowContainer !== declarationContainer &&
(flowContainer.kind === SyntaxKind.FunctionExpression || flowContainer.kind === SyntaxKind.ArrowFunction) &&
(isReadonlySymbol(localOrExportSymbol) || isParameter && !isParameterAssigned(localOrExportSymbol))) {
flowContainer = getControlFlowContainer(flowContainer);
}
const assumeInitialized = !strictNullChecks || (type.flags & TypeFlags.Any) !== 0 || isParameter ||
flowContainer !== declarationContainer || isInAmbientContext(declaration);
const flowType = getFlowTypeOfReference(node, type, assumeInitialized, flowContainer);
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 +9076,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 +10737,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 has assignments
Copy link
Member

Choose a reason for hiding this comment

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

Clarify that this flag is only set for parameters

}

/* @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