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

initial revision of reachability checks #4788

Merged
merged 25 commits into from
Nov 2, 2015
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b62ef0d
initial revision of reachability checks in binder
vladima Sep 3, 2015
beb1aa3
addressed PR feedback
vladima Sep 15, 2015
9aeb0f8
merge with master
vladima Sep 15, 2015
682c14c
addressed PR feedback
vladima Sep 16, 2015
69321a0
merge with master
vladima Sep 16, 2015
4711f0e
merge with master
vladima Sep 22, 2015
98ac805
Merge branch 'master' into reachabilityChecks
vladima Sep 29, 2015
ae175d0
merge with master
vladima Oct 1, 2015
9d24e0f
Merge branch 'master' into reachabilityChecks
vladima Oct 2, 2015
ca0d580
merge with master, fix linter issues
vladima Oct 2, 2015
f0f5a0d
updated command line options, accepted baselines
vladima Oct 3, 2015
eb04f32
Merge branch 'master' into reachabilityChecks
vladima Oct 3, 2015
ebfcd25
merge with master
vladima Oct 11, 2015
938dd74
Merge branch 'master' into reachabilityChecks
vladima Oct 13, 2015
17716fb
accepted baselines
vladima Oct 13, 2015
bc02341
addressed PR feedback, updated tests to suppress reachability errors …
vladima Oct 13, 2015
238e1c6
partially suppress reachability errors in tests
vladima Oct 13, 2015
5532778
suppress reachability errors in remaining tests
vladima Oct 13, 2015
7b12617
Merge branch 'master' into reachabilityChecks
vladima Oct 16, 2015
f9eaed7
Merge branch 'master' into reachabilityChecks
vladima Oct 19, 2015
f96980d
merge with master
vladima Oct 22, 2015
2779352
make binder singleton, inline bindWithReachabilityChecks
vladima Oct 22, 2015
7d09f26
defer allocation of error message text in binder
vladima Oct 22, 2015
d2a11b5
merge with master
vladima Oct 27, 2015
3f11c0b
merge with master
vladima Oct 29, 2015
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
448 changes: 430 additions & 18 deletions src/compiler/binder.ts

Large diffs are not rendered by default.

58 changes: 25 additions & 33 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ namespace ts {
let getGlobalPromiseConstructorLikeType: () => ObjectType;
let getGlobalThenableType: () => ObjectType;

let jsxElementClassType: Type;

let tupleTypes: Map<TupleType> = {};
let unionTypes: Map<UnionType> = {};
let intersectionTypes: Map<IntersectionType> = {};
Expand Down Expand Up @@ -7632,7 +7634,6 @@ namespace ts {
return prop || unknownSymbol;
}

let jsxElementClassType: Type = undefined;
function getJsxGlobalElementClassType(): Type {
if (!jsxElementClassType) {
jsxElementClassType = getExportedTypeFromNamespace(JsxNames.JSX, JsxNames.ElementClass);
Expand Down Expand Up @@ -9349,21 +9350,11 @@ namespace ts {
return aggregatedTypes;
}

function bodyContainsAReturnStatement(funcBody: Block) {
return forEachReturnStatement(funcBody, returnStatement => {
return true;
});
}

function bodyContainsSingleThrowStatement(body: Block) {
return (body.statements.length === 1) && (body.statements[0].kind === SyntaxKind.ThrowStatement);
}

// TypeScript Specification 1.0 (6.3) - July 2014
// An explicitly typed function whose return type isn't the Void or the Any type
// must have at least one return statement somewhere in its body.
// An exception to this rule is if the function implementation consists of a single 'throw' statement.
function checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(func: FunctionLikeDeclaration, returnType: Type): void {
function checkAllCodePathsInNonVoidFunctionReturnOrThrow(func: FunctionLikeDeclaration, returnType: Type): void {
if (!produceDiagnostics) {
return;
}
Expand All @@ -9374,26 +9365,20 @@ namespace ts {
}

// If all we have is a function signature, or an arrow function with an expression body, then there is nothing to check.
if (nodeIsMissing(func.body) || func.body.kind !== SyntaxKind.Block) {
// also if HasImplicitReturnValue flags is not set this means that all codepaths in function body end with return of throw
if (nodeIsMissing(func.body) || func.body.kind !== SyntaxKind.Block || !(func.flags & NodeFlags.HasImplicitReturn)) {
return;
}

let bodyBlock = <Block>func.body;

// Ensure the body has at least one return expression.
if (bodyContainsAReturnStatement(bodyBlock)) {
return;

if (func.flags & NodeFlags.HasExplicitReturn) {
if (compilerOptions.noImplicitReturns) {
error(func.type, Diagnostics.Not_all_code_paths_return_a_value);
}
}

// If there are no return expressions, then we need to check if
// the function body consists solely of a throw statement;
// this is to make an exception for unimplemented functions.
if (bodyContainsSingleThrowStatement(bodyBlock)) {
return;
else {
// This function does not conform to the specification.
error(func.type, Diagnostics.A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value);
}

// This function does not conform to the specification.
error(func.type, Diagnostics.A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value_or_consist_of_a_single_throw_statement);
}

function checkFunctionExpressionOrObjectLiteralMethod(node: FunctionExpression | MethodDeclaration, contextualMapper?: TypeMapper): Type {
Expand Down Expand Up @@ -9473,7 +9458,7 @@ namespace ts {
}

if (returnType && !node.asteriskToken) {
checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, isAsync ? promisedType : returnType);
checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, isAsync ? promisedType : returnType);
}

if (node.body) {
Expand Down Expand Up @@ -10651,8 +10636,15 @@ namespace ts {
checkGrammarFunctionLikeDeclaration(node) || checkGrammarAccessor(node) || checkGrammarComputedPropertyName(node.name);

if (node.kind === SyntaxKind.GetAccessor) {
if (!isInAmbientContext(node) && nodeIsPresent(node.body) && !(bodyContainsAReturnStatement(<Block>node.body) || bodyContainsSingleThrowStatement(<Block>node.body))) {
error(node.name, Diagnostics.A_get_accessor_must_return_a_value_or_consist_of_a_single_throw_statement);
if (!isInAmbientContext(node) && nodeIsPresent(node.body) && (node.flags & NodeFlags.HasImplicitReturn)) {
if (node.flags & NodeFlags.HasExplicitReturn) {
if (compilerOptions.noImplicitReturns) {
error(node.name, Diagnostics.Not_all_code_paths_return_a_value);
}
}
else {
error(node.name, Diagnostics.A_get_accessor_must_return_a_value);
}
}
}

Expand Down Expand Up @@ -11577,7 +11569,7 @@ namespace ts {
promisedType = checkAsyncFunctionReturnType(node);
}

checkIfNonVoidFunctionHasReturnExpressionsOrSingleThrowStatment(node, isAsync ? promisedType : returnType);
checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, isAsync ? promisedType : returnType);
}

if (produceDiagnostics && !node.type) {
Expand Down Expand Up @@ -14609,7 +14601,7 @@ namespace ts {
function initializeTypeChecker() {
// Bind all source files and propagate errors
forEach(host.getSourceFiles(), file => {
bindSourceFile(file);
bindSourceFile(file, compilerOptions);
});

// Initialize global symbol table
Expand Down
20 changes: 20 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,26 @@ namespace ts {
},
description: Diagnostics.Specifies_module_resolution_strategy_Colon_node_Node_js_or_classic_TypeScript_pre_1_6,
error: Diagnostics.Argument_for_moduleResolution_option_must_be_node_or_classic,
},
{
name: "noUnusedLabels",
type: "boolean",
description: Diagnostics.Report_error_on_unused_labels
Copy link
Contributor

Choose a reason for hiding this comment

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

nit : Report_errors

},
{
name: "noImplicitReturns",
type: "boolean",
description: Diagnostics.Report_error_when_not_all_code_paths_in_function_return_a_value
Copy link
Contributor

Choose a reason for hiding this comment

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

nit : Report_errors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't have a consistent style here. My understanding of phrasing is: 'use this option to do...' but not 'this option does...' so I'd prefer to keep existing version. Probably @JsonFreeman has different opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yuit's comment was about singular vs plural, rather than the mood, if I understood correctly. The mood inconsistency is a different issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JsonFreeman @vladima yep, it is more about the consistency on plural or singular on the word "error". I will be ok either one but I think it is nice to keep them consistent

},
{
name: "noFallthroughCasesInSwitch",
type: "boolean",
description: Diagnostics.Report_errors_for_fallthrough_cases_in_switch_statement
},
{
name: "noUnreachableCode",
type: "boolean",
description: Diagnostics.Report_errors_on_unreachable_code
}
];

Expand Down
12 changes: 10 additions & 2 deletions src/compiler/diagnosticInformationMap.generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ namespace ts {
Neither_type_0_nor_type_1_is_assignable_to_the_other: { code: 2352, category: DiagnosticCategory.Error, key: "Neither type '{0}' nor type '{1}' is assignable to the other." },
Object_literal_may_only_specify_known_properties_and_0_does_not_exist_in_type_1: { code: 2353, category: DiagnosticCategory.Error, key: "Object literal may only specify known properties, and '{0}' does not exist in type '{1}'." },
No_best_common_type_exists_among_return_expressions: { code: 2354, category: DiagnosticCategory.Error, key: "No best common type exists among return expressions." },
A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value_or_consist_of_a_single_throw_statement: { code: 2355, category: DiagnosticCategory.Error, key: "A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement." },
A_function_whose_declared_type_is_neither_void_nor_any_must_return_a_value: { code: 2355, category: DiagnosticCategory.Error, key: "A function whose declared type is neither 'void' nor 'any' must return a value." },
An_arithmetic_operand_must_be_of_type_any_number_or_an_enum_type: { code: 2356, category: DiagnosticCategory.Error, key: "An arithmetic operand must be of type 'any', 'number' or an enum type." },
The_operand_of_an_increment_or_decrement_operator_must_be_a_variable_property_or_indexer: { code: 2357, category: DiagnosticCategory.Error, key: "The operand of an increment or decrement operator must be a variable, property or indexer." },
The_left_hand_side_of_an_instanceof_expression_must_be_of_type_any_an_object_type_or_a_type_parameter: { code: 2358, category: DiagnosticCategory.Error, key: "The left-hand side of an 'instanceof' expression must be of type 'any', an object type or a type parameter." },
Expand All @@ -277,7 +277,7 @@ namespace ts {
Duplicate_number_index_signature: { code: 2375, category: DiagnosticCategory.Error, key: "Duplicate number index signature." },
A_super_call_must_be_the_first_statement_in_the_constructor_when_a_class_contains_initialized_properties_or_has_parameter_properties: { code: 2376, category: DiagnosticCategory.Error, key: "A 'super' call must be the first statement in the constructor when a class contains initialized properties or has parameter properties." },
Constructors_for_derived_classes_must_contain_a_super_call: { code: 2377, category: DiagnosticCategory.Error, key: "Constructors for derived classes must contain a 'super' call." },
A_get_accessor_must_return_a_value_or_consist_of_a_single_throw_statement: { code: 2378, category: DiagnosticCategory.Error, key: "A 'get' accessor must return a value or consist of a single 'throw' statement." },
A_get_accessor_must_return_a_value: { code: 2378, category: DiagnosticCategory.Error, key: "A 'get' accessor must return a value." },
Getter_and_setter_accessors_do_not_agree_in_visibility: { code: 2379, category: DiagnosticCategory.Error, key: "Getter and setter accessors do not agree in visibility." },
get_and_set_accessor_must_have_the_same_type: { code: 2380, category: DiagnosticCategory.Error, key: "'get' and 'set' accessor must have the same type." },
A_signature_with_an_implementation_cannot_use_a_string_literal_type: { code: 2381, category: DiagnosticCategory.Error, key: "A signature with an implementation cannot use a string literal type." },
Expand Down Expand Up @@ -576,6 +576,10 @@ namespace ts {
Initializes_a_TypeScript_project_and_creates_a_tsconfig_json_file: { code: 6070, category: DiagnosticCategory.Message, key: "Initializes a TypeScript project and creates a tsconfig.json file." },
Successfully_created_a_tsconfig_json_file: { code: 6071, category: DiagnosticCategory.Message, key: "Successfully created a tsconfig.json file." },
Suppress_excess_property_checks_for_object_literals: { code: 6072, category: DiagnosticCategory.Message, key: "Suppress excess property checks for object literals." },
Report_error_on_unused_labels: { code: 6073, category: DiagnosticCategory.Message, key: "Report error on unused labels." },
Report_error_when_not_all_code_paths_in_function_return_a_value: { code: 6074, category: DiagnosticCategory.Message, key: "Report error when not all code paths in function return a value." },
Report_errors_for_fallthrough_cases_in_switch_statement: { code: 6075, category: DiagnosticCategory.Message, key: "Report errors for fallthrough cases in switch statement." },
Report_errors_on_unreachable_code: { code: 6076, category: DiagnosticCategory.Message, key: "Report errors on unreachable code." },
Variable_0_implicitly_has_an_1_type: { code: 7005, category: DiagnosticCategory.Error, key: "Variable '{0}' implicitly has an '{1}' type." },
Parameter_0_implicitly_has_an_1_type: { code: 7006, category: DiagnosticCategory.Error, key: "Parameter '{0}' implicitly has an '{1}' type." },
Member_0_implicitly_has_an_1_type: { code: 7008, category: DiagnosticCategory.Error, key: "Member '{0}' implicitly has an '{1}' type." },
Expand All @@ -593,6 +597,10 @@ namespace ts {
Function_implicitly_has_return_type_any_because_it_does_not_have_a_return_type_annotation_and_is_referenced_directly_or_indirectly_in_one_of_its_return_expressions: { code: 7024, category: DiagnosticCategory.Error, key: "Function implicitly has return type 'any' because it does not have a return type annotation and is referenced directly or indirectly in one of its return expressions." },
Generator_implicitly_has_type_0_because_it_does_not_yield_any_values_Consider_supplying_a_return_type: { code: 7025, category: DiagnosticCategory.Error, key: "Generator implicitly has type '{0}' because it does not yield any values. Consider supplying a return type." },
JSX_element_implicitly_has_type_any_because_no_interface_JSX_0_exists: { code: 7026, category: DiagnosticCategory.Error, key: "JSX element implicitly has type 'any' because no interface 'JSX.{0}' exists" },
Unreachable_code_detected: { code: 7027, category: DiagnosticCategory.Error, key: "Unreachable code detected." },
Unused_label: { code: 7028, category: DiagnosticCategory.Error, key: "Unused label." },
Fallthrough_case_in_switch: { code: 7029, category: DiagnosticCategory.Error, key: "Fallthrough case in switch." },
Not_all_code_paths_return_a_value: { code: 7030, category: DiagnosticCategory.Error, key: "Not all code paths return a value." },
You_cannot_rename_this_element: { code: 8000, category: DiagnosticCategory.Error, key: "You cannot rename this element." },
You_cannot_rename_elements_that_are_defined_in_the_standard_TypeScript_library: { code: 8001, category: DiagnosticCategory.Error, key: "You cannot rename elements that are defined in the standard TypeScript library." },
import_can_only_be_used_in_a_ts_file: { code: 8002, category: DiagnosticCategory.Error, key: "'import ... =' can only be used in a .ts file." },
Expand Down
39 changes: 34 additions & 5 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@
"category": "Error",
"code": 2354
},
"A function whose declared type is neither 'void' nor 'any' must return a value or consist of a single 'throw' statement.": {
"A function whose declared type is neither 'void' nor 'any' must return a value.": {
"category": "Error",
"code": 2355
},
Expand Down Expand Up @@ -1097,7 +1097,7 @@
"category": "Error",
"code": 2377
},
"A 'get' accessor must return a value or consist of a single 'throw' statement.": {
"A 'get' accessor must return a value.": {
"category": "Error",
"code": 2378
},
Expand Down Expand Up @@ -2295,7 +2295,22 @@
"category": "Message",
"code": 6072
},

"Report error on unused labels.": {
"category": "Message",
"code": 6073
},
"Report error when not all code paths in function return a value.": {
"category": "Message",
"code": 6074
},
"Report errors for fallthrough cases in switch statement.": {
"category": "Message",
"code": 6075
},
"Report errors on unreachable code.": {
"category": "Message",
"code": 6076
},
"Variable '{0}' implicitly has an '{1}' type.": {
"category": "Error",
"code": 7005
Expand Down Expand Up @@ -2364,8 +2379,22 @@
"category": "Error",
"code": 7026
},


"Unreachable code detected.": {
"category": "Error",
"code": 7027
},
"Unused label.": {
"category": "Error",
"code": 7028
},
"Fallthrough case in switch.": {
"category": "Error",
"code": 7029
},
"Not all code paths return a value.": {
"category": "Error",
"code": 7030
},
"You cannot rename this element.": {
"category": "Error",
"code": 8000
Expand Down
8 changes: 7 additions & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,8 @@ namespace ts {
OctalLiteral = 0x00010000, // Octal numeric literal
Namespace = 0x00020000, // Namespace declaration
ExportContext = 0x00040000, // Export context (initialized by binding)
HasImplicitReturn = 0x00080000, // If function implicitly returns on one of codepaths (initialized by binding)
HasExplicitReturn = 0x00100000, // If function has explicit reachable return on one of codepaths (initialized by binding)

Modifier = Export | Ambient | Public | Private | Protected | Static | Abstract | Default | Async,
AccessibilityModifier = Public | Private | Protected,
Expand Down Expand Up @@ -2061,7 +2063,11 @@ namespace ts {
experimentalDecorators?: boolean;
experimentalAsyncFunctions?: boolean;
emitDecoratorMetadata?: boolean;
moduleResolution?: ModuleResolutionKind
moduleResolution?: ModuleResolutionKind,
noUnusedLabels?: boolean,
noImplicitReturns?: boolean,
noFallthroughCasesInSwitch?: boolean,
noUnreachableCode?: boolean,
/* @internal */ stripInternal?: boolean;

// Skip checking lib.d.ts to help speed up tests.
Expand Down
5 changes: 1 addition & 4 deletions src/services/breakpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ namespace ts.BreakpointResolver {
// fall through.

case SyntaxKind.CatchClause:
return spanInNode(lastOrUndefined((<Block>node.parent).statements));;
return spanInNode(lastOrUndefined((<Block>node.parent).statements));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hah! Was this reported as unreachable? Awesome.


case SyntaxKind.CaseBlock:
// breakpoint in last statement of the last clause
Expand Down Expand Up @@ -493,9 +493,6 @@ namespace ts.BreakpointResolver {
default:
return spanInNode(node.parent);
}

// Default to parent node
return spanInNode(node.parent);
}

function spanInColonToken(node: Node): TextSpan {
Expand Down
1 change: 0 additions & 1 deletion src/services/formatting/smartIndenter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ namespace ts.formatting {
return node;
}
}
return node;
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/services/navigateTo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ namespace ts.NavigateTo {
let patternMatcher = createPatternMatcher(searchValue);
let rawItems: RawNavigateToItem[] = [];

// This means "compare in a case insensitive manner."
let baseSensitivity: Intl.CollatorOptions = { sensitivity: "base" };

// Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[]
forEach(program.getSourceFiles(), sourceFile => {
cancellationToken.throwIfCancellationRequested();
Expand Down Expand Up @@ -162,8 +165,6 @@ namespace ts.NavigateTo {
return bestMatchKind;
}

// This means "compare in a case insensitive manner."
let baseSensitivity: Intl.CollatorOptions = { sensitivity: "base" };
function compareNavigateToItems(i1: RawNavigateToItem, i2: RawNavigateToItem) {
// TODO(cyrusn): get the gamut of comparisons that VS already uses here.
// Right now we just sort by kind first, and then by name of the item.
Expand Down
Loading