Merge pull request microsoft#8014 from Microsoft/strictBlockScopeFunction

Make function block scoped in strict mode and report error in es5 for block scope level declaration of function

Author: sheetalkamat

src/compiler/binder.ts

@@ -104,6 +104,7 @@ namespace ts {
  function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
  let file: SourceFile;
  let options: CompilerOptions;
+ let languageVersion: ScriptTarget;
  let parent: Node;
  let container: Node;
  let blockScopeContainer: Node;
@@ -136,6 +137,7 @@ namespace ts {
  function bindSourceFile(f: SourceFile, opts: CompilerOptions) {
  file = f;
  options = opts;
+ languageVersion = getEmitScriptTarget(options);
  inStrictMode = !!file.externalModuleIndicator;
  classifiableNames = {};
 
@@ -149,6 +151,7 @@ namespace ts {
 
  file = undefined;
  options = undefined;
+ languageVersion = undefined;
  parent = undefined;
  container = undefined;
  blockScopeContainer = undefined;
@@ -1125,6 +1128,35 @@ namespace ts {
  }
  }
 
+ function getStrictModeBlockScopeFunctionDeclarationMessage(node: Node) {
+ // Provide specialized messages to help the user understand why we think they're in
+ // strict mode.
+ if (getContainingClass(node)) {
+ return Diagnostics.Function_declarations_are_not_allowed_inside_blocks_in_strict_mode_when_targeting_ES3_or_ES5_Class_definitions_are_automatically_in_strict_mode;
+ }
+
+ if (file.externalModuleIndicator) {
+ return Diagnostics.Function_declarations_are_not_allowed_inside_blocks_in_strict_mode_when_targeting_ES3_or_ES5_Modules_are_automatically_in_strict_mode;
+ }
+
+ return Diagnostics.Function_declarations_are_not_allowed_inside_blocks_in_strict_mode_when_targeting_ES3_or_ES5;
+ }
+
+ function checkStrictModeFunctionDeclaration(node: FunctionDeclaration) {
+ if (languageVersion < ScriptTarget.ES6) {
+ // Report error if function is not top level function declaration
+ if (blockScopeContainer.kind !== SyntaxKind.SourceFile &&
+ blockScopeContainer.kind !== SyntaxKind.ModuleDeclaration &&
+ !isFunctionLike(blockScopeContainer)) {
+ // We check first if the name is inside class declaration or class expression; if so give explicit message
+ // otherwise report generic error message.
+ const errorSpan = getErrorSpanForNode(file, node);
+ file.bindDiagnostics.push(createFileDiagnostic(file, errorSpan.start, errorSpan.length,
+ getStrictModeBlockScopeFunctionDeclarationMessage(node)));
+ }
+ }
+ }
+
  function checkStrictModeNumericLiteral(node: LiteralExpression) {
  if (inStrictMode && node.isOctalLiteral) {
  file.bindDiagnostics.push(createDiagnosticForNode(node, Diagnostics.Octal_literals_are_not_allowed_in_strict_mode));
@@ -1640,7 +1672,13 @@ namespace ts {
  }
 
  checkStrictModeFunctionName(<FunctionDeclaration>node);
- return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
+ if (inStrictMode) {
+ checkStrictModeFunctionDeclaration(node);
+ return bindBlockScopedDeclaration(node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
+ }
+ else {
+ return declareSymbolAndAddToSymbolTable(<Declaration>node, SymbolFlags.Function, SymbolFlags.FunctionExcludes);
+ }
  }
 
  function bindFunctionExpression(node: FunctionExpression) {

src/compiler/diagnosticMessages.json

@@ -811,6 +811,18 @@
  "category": "Error",
  "code": 1249
  },
+ "Function declarations are not allowed inside blocks in strict mode when targeting 'ES3' or 'ES5'.": {
+ "category": "Error",
+ "code": 1250
+ },
+ "Function declarations are not allowed inside blocks in strict mode when targeting 'ES3' or 'ES5'. Class definitions are automatically in strict mode.": {
+ "category": "Error",
+ "code": 1251
+ },
+ "Function declarations are not allowed inside blocks in strict mode when targeting 'ES3' or 'ES5'. Modules are automatically in strict mode.": {
+ "category": "Error",
+ "code": 1252
+ },
  "'with' statements are not allowed in an async function block.": {
  "category": "Error",
  "code": 1300

src/harness/compilerRunner.ts

@@ -256,125 +256,128 @@ class CompilerBaselineRunner extends RunnerBase {
  }
 
  // NEWTODO: Type baselines
- if (result.errors.length === 0) {
- // The full walker simulates the types that you would get from doing a full
- // compile. The pull walker simulates the types you get when you just do
- // a type query for a random node (like how the LS would do it). Most of the
- // time, these will be the same. However, occasionally, they can be different.
- // Specifically, when the compiler internally depends on symbol IDs to order
- // things, then we may see different results because symbols can be created in a
- // different order with 'pull' operations, and thus can produce slightly differing
- // output.
- //
- // For example, with a full type check, we may see a type displayed as: number | string
- // But with a pull type check, we may see it as: string | number
- //
- // These types are equivalent, but depend on what order the compiler observed
- // certain parts of the program.
-
- const program = result.program;
- const allFiles = toBeCompiled.concat(otherFiles).filter(file => !!program.getSourceFile(file.unitName));
-
- const fullWalker = new TypeWriterWalker(program, /*fullTypeCheck*/ true);
-
- const fullResults: ts.Map<TypeWriterResult[]> = {};
- const pullResults: ts.Map<TypeWriterResult[]> = {};
-
- for (const sourceFile of allFiles) {
- fullResults[sourceFile.unitName] = fullWalker.getTypeAndSymbols(sourceFile.unitName);
- pullResults[sourceFile.unitName] = fullWalker.getTypeAndSymbols(sourceFile.unitName);
- }
+ if (result.errors.length !== 0) {
+ return;
+ }
 
- // Produce baselines. The first gives the types for all expressions.
- // The second gives symbols for all identifiers.
- let e1: Error, e2: Error;
- try {
- checkBaseLines(/*isSymbolBaseLine*/ false);
- }
- catch (e) {
- e1 = e;
- }
+ // The full walker simulates the types that you would get from doing a full
+ // compile. The pull walker simulates the types you get when you just do
+ // a type query for a random node (like how the LS would do it). Most of the
+ // time, these will be the same. However, occasionally, they can be different.
+ // Specifically, when the compiler internally depends on symbol IDs to order
+ // things, then we may see different results because symbols can be created in a
+ // different order with 'pull' operations, and thus can produce slightly differing
+ // output.
+ //
+ // For example, with a full type check, we may see a type displayed as: number | string
+ // But with a pull type check, we may see it as: string | number
+ //
+ // These types are equivalent, but depend on what order the compiler observed
+ // certain parts of the program.
+
+ const program = result.program;
+ const allFiles = toBeCompiled.concat(otherFiles).filter(file => !!program.getSourceFile(file.unitName));
+
+ const fullWalker = new TypeWriterWalker(program, /*fullTypeCheck*/ true);
+
+ const fullResults: ts.Map<TypeWriterResult[]> = {};
+ const pullResults: ts.Map<TypeWriterResult[]> = {};
+
+ for (const sourceFile of allFiles) {
+ fullResults[sourceFile.unitName] = fullWalker.getTypeAndSymbols(sourceFile.unitName);
+ pullResults[sourceFile.unitName] = fullWalker.getTypeAndSymbols(sourceFile.unitName);
+ }
 
- try {
- checkBaseLines(/*isSymbolBaseLine*/ true);
- }
- catch (e) {
- e2 = e;
- }
+ // Produce baselines. The first gives the types for all expressions.
+ // The second gives symbols for all identifiers.
+ let e1: Error, e2: Error;
+ try {
+ checkBaseLines(/*isSymbolBaseLine*/ false);
+ }
+ catch (e) {
+ e1 = e;
+ }
 
- if (e1 || e2) {
- throw e1 || e2;
- }
+ try {
+ checkBaseLines(/*isSymbolBaseLine*/ true);
+ }
+ catch (e) {
+ e2 = e;
+ }
 
- return;
+ if (e1 || e2) {
+ throw e1 || e2;
+ }
 
- function checkBaseLines(isSymbolBaseLine: boolean) {
- const fullBaseLine = generateBaseLine(fullResults, isSymbolBaseLine);
- const pullBaseLine = generateBaseLine(pullResults, isSymbolBaseLine);
+ return;
 
- const fullExtension = isSymbolBaseLine ? ".symbols" : ".types";
- const pullExtension = isSymbolBaseLine ? ".symbols.pull" : ".types.pull";
+ function checkBaseLines(isSymbolBaseLine: boolean) {
+ const fullBaseLine = generateBaseLine(fullResults, isSymbolBaseLine);
+ const pullBaseLine = generateBaseLine(pullResults, isSymbolBaseLine);
 
- if (fullBaseLine !== pullBaseLine) {
- Harness.Baseline.runBaseline("Correct full information for " + fileName, justName.replace(/\.tsx?/, fullExtension), () => fullBaseLine);
- Harness.Baseline.runBaseline("Correct pull information for " + fileName, justName.replace(/\.tsx?/, pullExtension), () => pullBaseLine);
- }
- else {
- Harness.Baseline.runBaseline("Correct information for " + fileName, justName.replace(/\.tsx?/, fullExtension), () => fullBaseLine);
- }
+ const fullExtension = isSymbolBaseLine ? ".symbols" : ".types";
+ const pullExtension = isSymbolBaseLine ? ".symbols.pull" : ".types.pull";
+
+ if (fullBaseLine !== pullBaseLine) {
+ Harness.Baseline.runBaseline("Correct full information for " + fileName, justName.replace(/\.tsx?/, fullExtension), () => fullBaseLine);
+ Harness.Baseline.runBaseline("Correct pull information for " + fileName, justName.replace(/\.tsx?/, pullExtension), () => pullBaseLine);
  }
+ else {
+ Harness.Baseline.runBaseline("Correct information for " + fileName, justName.replace(/\.tsx?/, fullExtension), () => fullBaseLine);
+ }
+ }
 
-  function generateBaseLine(typeWriterResults: ts.Map<TypeWriterResult[]>, isSymbolBaseline: boolean): string {
-  const typeLines: string[] = [];
-  const typeMap: { [fileName: string]: { [lineNum: number]: string[]; } } = {};
+ function generateBaseLine(typeWriterResults: ts.Map<TypeWriterResult[]>, isSymbolBaseline: boolean): string {
+ const typeLines: string[] = [];
+ const typeMap: { [fileName: string]: { [lineNum: number]: string[]; } } = {};
 
-  allFiles.forEach(file => {
-  const codeLines = file.content.split("\n");
-  typeWriterResults[file.unitName].forEach(result => {
-  if (isSymbolBaseline && !result.symbol) {
-  return;
-  }
+ allFiles.forEach(file => {
+ const codeLines = file.content.split("\n");
+ typeWriterResults[file.unitName].forEach(result => {
+ if (isSymbolBaseline && !result.symbol) {
+ return;
+ }
 
-  const typeOrSymbolString = isSymbolBaseline ? result.symbol : result.type;
-  const formattedLine = result.sourceText.replace(/\r?\n/g, "") + " : " + typeOrSymbolString;
-  if (!typeMap[file.unitName]) {
-  typeMap[file.unitName] = {};
-  }
+ const typeOrSymbolString = isSymbolBaseline ? result.symbol : result.type;
+ const formattedLine = result.sourceText.replace(/\r?\n/g, "") + " : " + typeOrSymbolString;
+ if (!typeMap[file.unitName]) {
+ typeMap[file.unitName] = {};
+ }
 
- let typeInfo = [formattedLine];
- const existingTypeInfo = typeMap[file.unitName][result.line];
- if (existingTypeInfo) {
- typeInfo = existingTypeInfo.concat(typeInfo);
- }
- typeMap[file.unitName][result.line] = typeInfo;
- });
-
- typeLines.push("=== " + file.unitName + " ===\r\n");
- for (let i = 0; i < codeLines.length; i++) {
- const currentCodeLine = codeLines[i];
- typeLines.push(currentCodeLine + "\r\n");
- if (typeMap[file.unitName]) {
- const typeInfo = typeMap[file.unitName][i];
- if (typeInfo) {
- typeInfo.forEach(ty => {
- typeLines.push(">" + ty + "\r\n");
- });
- if (i + 1 < codeLines.length && (codeLines[i + 1].match(/^\s*[{|}]\s*$/) || codeLines[i + 1].trim() === "")) {
- }
- else {
- typeLines.push("\r\n");
- }
+ let typeInfo = [formattedLine];
+ const existingTypeInfo = typeMap[file.unitName][result.line];
+ if (existingTypeInfo) {
+ typeInfo = existingTypeInfo.concat(typeInfo);
+ }
+ typeMap[file.unitName][result.line] = typeInfo;
+ });
+
+ typeLines.push("=== " + file.unitName + " ===\r\n");
+ for (let i = 0; i < codeLines.length; i++) {
+ const currentCodeLine = codeLines[i];
+ typeLines.push(currentCodeLine + "\r\n");
+ if (typeMap[file.unitName]) {
+ const typeInfo = typeMap[file.unitName][i];
+ if (typeInfo) {
+ typeInfo.forEach(ty => {
+ typeLines.push(">" + ty + "\r\n");
+ });
+ if (i + 1 < codeLines.length && (codeLines[i + 1].match(/^\s*[{|}]\s*$/) || codeLines[i + 1].trim() === "")) {
+ }
+ else {
+ typeLines.push("\r\n");
  }
- }
- else {
- typeLines.push("No type information for this code.");
  }
  }
- });
+ else {
+ typeLines.push("No type information for this code.");
+ }
+ }
+ });
 
- return typeLines.join("");
- }
+ return typeLines.join("");
  }
+
  });
  });
  }

tests/baselines/reference/blockScopedFunctionDeclarationES5.js

@@ -0,0 +1,13 @@
+//// [blockScopedFunctionDeclarationES5.ts]
+if (true) {
+ function foo() { }
+ foo();
+}
+foo();
+
+//// [blockScopedFunctionDeclarationES5.js]
+if (true) {
+ function foo() { }
+ foo();
+}
+foo();

tests/baselines/reference/blockScopedFunctionDeclarationES5.symbols

@@ -0,0 +1,11 @@
+=== tests/cases/compiler/blockScopedFunctionDeclarationES5.ts ===
+if (true) {
+ function foo() { }
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES5.ts, 0, 11))
+
+ foo();
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES5.ts, 0, 11))
+}
+foo();
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES5.ts, 0, 11))
+

tests/baselines/reference/blockScopedFunctionDeclarationES5.types

@@ -0,0 +1,15 @@
+=== tests/cases/compiler/blockScopedFunctionDeclarationES5.ts ===
+if (true) {
+>true : boolean
+
+ function foo() { }
+>foo : () => void
+
+ foo();
+>foo() : void
+>foo : () => void
+}
+foo();
+>foo() : void
+>foo : () => void
+

tests/baselines/reference/blockScopedFunctionDeclarationES6.js

@@ -0,0 +1,13 @@
+//// [blockScopedFunctionDeclarationES6.ts]
+if (true) {
+ function foo() { }
+ foo();
+}
+foo();
+
+//// [blockScopedFunctionDeclarationES6.js]
+if (true) {
+ function foo() { }
+ foo();
+}
+foo();

tests/baselines/reference/blockScopedFunctionDeclarationES6.symbols

@@ -0,0 +1,11 @@
+=== tests/cases/compiler/blockScopedFunctionDeclarationES6.ts ===
+if (true) {
+ function foo() { }
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES6.ts, 0, 11))
+
+ foo();
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES6.ts, 0, 11))
+}
+foo();
+>foo : Symbol(foo, Decl(blockScopedFunctionDeclarationES6.ts, 0, 11))
+