Update: Module overrides all 'strict' rule options (fixes #4936)

docs/rules/README.md

@@ -105,9 +105,9 @@ These are rules designed to prevent you from making mistakes. They either prescr
 
 ## Strict Mode
 
-These rules relate to using strict mode.
+These rules relate to using strict mode and strict mode directives.
 
-* [strict](strict.md) - controls location of Use Strict Directives
+* [strict](strict.md) - require effective use of strict mode directives
 
 ## Variables
 

docs/rules/strict.md

@@ -1,6 +1,6 @@
-# Strict Mode (strict)
+# Strict Mode Directives (strict)
 
-A Use Strict Directive at the beginning of a script or function body enables strict mode semantics:
+A strict mode directive at the beginning of a script or function body enables strict mode semantics:
 
 ```js
 "use strict";
@@ -20,9 +20,11 @@ var bar = function() {
 };
 ```
 
+Unlike scripts, ECMAScript modules are always in strict mode. Strict mode directives in ECMAScript modules have no effect.
+
 ## Rule Details
 
-This rule is aimed at using strict directives effectively, and as such, will flag any unexpected uses or omissions of strict directives.
+This rule is aimed at using strict mode directives effectively, and as such, will flag any unexpected uses or omissions of strict mode directives.
 
 ### Options
 
@@ -33,9 +35,11 @@ There are four options for this rule:
 1. `"function"` - require `"use strict"` in function scopes only
 1. `"safe"` - require `"use strict"` globally when inside a module wrapper and in function scopes everywhere else.
 
+All strict mode directives are flagged as unnecessary if ECMAScript modules are enabled (see [Configuring ESLint](configuring)). This behaviour does not depend on the rule options, but can be silenced by disabling this rule.
+
 ### "never"
 
-This mode forbids any occurrence of a Use Strict Directive.
+This mode forbids any occurrence of a strict mode directive.
 
 The following patterns are considered problems:
 
@@ -77,7 +81,7 @@ bar();
 
 ### "global"
 
-This mode ensures that all code is in strict mode and that there are no extraneous Use Strict Directives at the top level or in nested functions, which are themselves already strict by virtue of being contained in strict global code. It requires that global code contains exactly one Use Strict Directive. Use Strict Directives inside functions are considered unnecessary. Multiple Use Strict Directives at any level also trigger warnings.
+This mode ensures that all code is in strict mode and that there are no extraneous strict mode directives at the top level or in nested functions, which are themselves already strict by virtue of being contained in strict global code. It requires that global code contains exactly one strict mode directive. Strict mode directives inside functions are considered unnecessary. Multiple strict mode directives at any level also trigger warnings.
 
 The following patterns are considered problems:
 
@@ -119,7 +123,7 @@ foo();
 
 ### "function"
 
-This mode ensures that all function bodies are strict mode code, while global code is not. Particularly if a build step concatenates multiple scripts, a Use Strict Directive in global code of one script could unintentionally enable strict mode in another script that was not intended to be strict code. It forbids any occurrence of a Use Strict Directive in global code. It requires exactly one Use Strict Directive in each function declaration or expression whose parent is global code. Use Strict Directives inside nested functions are considered unnecessary. Multiple Use Strict Directives at any level also trigger warnings.
+This mode ensures that all function bodies are strict mode code, while global code is not. Particularly if a build step concatenates multiple scripts, a strict mode directive in global code of one script could unintentionally enable strict mode in another script that was not intended to be strict code. It forbids any occurrence of a strict mode directive in global code. It requires exactly one strict mode directive in each function declaration or expression whose parent is global code. Strict mode directives inside nested functions are considered unnecessary. Multiple strict mode directives at any level also trigger warnings.
 
 The following patterns are considered problems:
 
@@ -129,10 +133,10 @@ The following patterns are considered problems:
 "use strict";           /*error Use the function form of 'use strict'.*/
 
 function foo() {        /*error Use the function form of 'use strict'.*/
-    // Missing Use Strict Directive
+    // Missing strict mode directive
 
     return function() {
-        "use strict";   // Unnecessary; parent should contain a Strict Mode Directive
+        "use strict";   // Unnecessary; parent should contain a strict mode directive
         "use strict";   /*error Multiple 'use strict' directives.*/
 
         return;
@@ -166,13 +170,13 @@ foo();
 
 ### "safe"  (default)
 
-Node.js and the CommonJS module system wrap modules inside a hidden function wrapper that defines each module's scope. The wrapper makes it safe to concatenate strict-mode modules while maintaining their original `"use strict"` directives. When the `node` or `commonjs` environments are enabled or `globalReturn` is enabled in `ecmaFeatures`, ESLint considers code to be inside the module wrapper, and `"safe"` mode corresponds to `"global"` mode and enforces global `"use strict"` directives. Everywhere else, `"safe"` mode corresponds to `"function"` mode and enforces `"use strict"` directives inside top-level functions.
+Node.js and the CommonJS module system wrap modules inside a hidden function wrapper that defines each module's scope. The wrapper makes it safe to concatenate strict mode modules while maintaining their original strict mode directives. When the `node` or `commonjs` environments are enabled or `globalReturn` is enabled in `ecmaFeatures`, ESLint considers code to be inside the module wrapper, and `"safe"` mode corresponds to `"global"` mode and enforces global strict mode directives. Everywhere else, `"safe"` mode corresponds to `"function"` mode and enforces strict mode directives inside top-level functions.
 
 ### "deprecated" (Removed)
 
 **Replacement notice**: This mode, previously enabled by turning on the rule without specifying a mode, has been removed in ESLint v1.0. `"function"` mode is most similar to the deprecated behavior.
 
-This mode ensures that all functions are executed in strict mode. A Use Strict Directive must be present in global code or in every top-level function declaration or expression. It does not concern itself with unnecessary Use Strict Directives in nested functions that are already strict, nor with multiple Use Strict Directives at the same level.
+This mode ensures that all functions are executed in strict mode. A strict mode directive must be present in global code or in every top-level function declaration or expression. It does not concern itself with unnecessary strict mode directives in nested functions that are already strict, nor with multiple strict mode directives at the same level.
 
 The following patterns are considered problems:
 

lib/rules/strict.js

@@ -8,6 +8,12 @@
 
 "use strict";
 
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var assign = require("object-assign");
+
 //------------------------------------------------------------------------------
 // Helpers
 //------------------------------------------------------------------------------
@@ -18,7 +24,7 @@ var messages = {
     multiple: "Multiple 'use strict' directives.",
     never: "Strict mode is not permitted.",
     unnecessary: "Unnecessary 'use strict' directive.",
-    unnecessaryInModules: "'use strict' is unnecessary inside of modules.",
+    module: "'use strict' is unnecessary inside of modules.",
     unnecessaryInClasses: "'use strict' is unnecessary inside of classes."
 };
 
@@ -55,172 +61,147 @@ function getUseStrictDirectives(statements) {
 
 module.exports = function(context) {
 
-    var mode = context.options[0];
+    var mode = context.options[0] || "safe",
+        scopes = [],
+        classScopes = [],
+        rule;
 
-    if (["never", "global", "function"].indexOf(mode) < 0) {
+    if (mode === "safe") {
         mode = context.parserOptions.ecmaFeatures &&
             context.parserOptions.ecmaFeatures.globalReturn ?
             "global" : "function";
     }
 
     /**
-     * Report a node or array of nodes with a given message.
-     * @param {(ASTNode|ASTNode[])} nodes Node or nodes to report.
+     * Report a slice of an array of nodes with a given message.
+     * @param {ASTNode[]} nodes Nodes.
+     * @param {string} start Index to start from.
+     * @param {string} end Index to end before.
      * @param {string} message Message to display.
      * @returns {void}
      */
-    function report(nodes, message) {
+    function reportSlice(nodes, start, end, message) {
         var i;
 
-        if (Array.isArray(nodes)) {
-            for (i = 0; i < nodes.length; i++) {
-                context.report(nodes[i], message);
-            }
-        } else {
-            context.report(nodes, message);
+        for (i = start; i < end; i++) {
+            context.report(nodes[i], message);
         }
     }
 
-    //--------------------------------------------------------------------------
-    // "never" mode
-    //--------------------------------------------------------------------------
-
-    if (mode === "never") {
-        return {
-            "Program": function(node) {
-                report(getUseStrictDirectives(node.body), messages.never);
-            },
-            "FunctionDeclaration": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.never);
-            },
-            "FunctionExpression": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.never);
-            },
-            "ArrowFunctionExpression": function(node) {
-                if (node.body.type === "BlockStatement") {
-                    report(getUseStrictDirectives(node.body.body), messages.never);
-                }
-            }
-        };
-    }
-
-    //--------------------------------------------------------------------------
-    // If this is modules, all "use strict" directives are unnecessary.
-    //--------------------------------------------------------------------------
-
-    if (context.parserOptions.sourceType === "module") {
-        return {
-            "Program": function(node) {
-                report(getUseStrictDirectives(node.body), messages.unnecessaryInModules);
-            },
-            "FunctionDeclaration": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.unnecessaryInModules);
-            },
-            "FunctionExpression": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.unnecessaryInModules);
-            },
-            "ArrowFunctionExpression": function(node) {
-                if (node.body.type === "BlockStatement") {
-                    report(getUseStrictDirectives(node.body.body), messages.unnecessaryInModules);
-                }
-            }
-        };
+    /**
+     * Report all nodes in an array with a given message.
+     * @param {ASTNode[]} nodes Nodes.
+     * @param {string} message Message to display.
+     * @returns {void}
+     */
+    function reportAll(nodes, message) {
+        reportSlice(nodes, 0, nodes.length, message);
     }
 
-    //--------------------------------------------------------------------------
-    // "global" mode
-    //--------------------------------------------------------------------------
-
-    if (mode === "global") {
-        return {
-            "Program": function(node) {
-                var useStrictDirectives = getUseStrictDirectives(node.body);
-
-                if (node.body.length > 0 && useStrictDirectives.length === 0) {
-                    report(node, messages.global);
-                } else {
-                    report(useStrictDirectives.slice(1), messages.multiple);
-                }
-            },
-            "FunctionDeclaration": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.global);
-            },
-            "FunctionExpression": function(node) {
-                report(getUseStrictDirectives(node.body.body), messages.global);
-            },
-            "ArrowFunctionExpression": function(node) {
-                if (node.body.type === "BlockStatement") {
-                    report(getUseStrictDirectives(node.body.body), messages.global);
-                }
-            }
-        };
+    /**
+     * Report all nodes in an array, except the first, with a given message.
+     * @param {ASTNode[]} nodes Nodes.
+     * @param {string} message Message to display.
+     * @returns {void}
+     */
+    function reportAllExceptFirst(nodes, message) {
+        reportSlice(nodes, 1, nodes.length, message);
     }
 
-    //--------------------------------------------------------------------------
-    // "function" mode
-    //--------------------------------------------------------------------------
-
-    var scopes = [],
-        classScopes = [];
-
     /**
-     * Entering a function pushes a new nested scope onto the stack. The new
-     * scope is true if the nested function is strict mode code.
+     * Entering a function in 'function' mode pushes a new nested scope onto the
+     * stack. The new scope is true if the nested function is strict mode code.
      * @param {ASTNode} node The function declaration or expression.
+     * @param {ASTNode[]} useStrictDirectives The Use Strict Directives of the node.
      * @returns {void}
      */
-    function enterFunction(node) {
+    function enterFunctionInFunctionMode(node, useStrictDirectives) {
         var isInClass = classScopes.length > 0,
             isParentGlobal = scopes.length === 0 && classScopes.length === 0,
             isParentStrict = scopes.length > 0 && scopes[scopes.length - 1],
-            isNotBlock = node.body.type !== "BlockStatement",
-            useStrictDirectives = isNotBlock ? [] : getUseStrictDirectives(node.body.body),
             isStrict = useStrictDirectives.length > 0;
 
         if (isStrict) {
             if (isParentStrict) {
-                report(useStrictDirectives[0], messages.unnecessary);
+                context.report(useStrictDirectives[0], messages.unnecessary);
             } else if (isInClass) {
-                report(useStrictDirectives[0], messages.unnecessaryInClasses);
+                context.report(useStrictDirectives[0], messages.unnecessaryInClasses);
             }
 
-            report(useStrictDirectives.slice(1), messages.multiple);
+            reportAllExceptFirst(useStrictDirectives, messages.multiple);
         } else if (isParentGlobal) {
-            report(node, messages.function);
+            context.report(node, messages.function);
         }
 
         scopes.push(isParentStrict || isStrict);
     }
 
     /**
-     * Exiting a function pops its scope off the stack.
+     * Exiting a function in 'function' mode pops its scope off the stack.
      * @returns {void}
      */
-    function exitFunction() {
+    function exitFunctionInFunctionMode() {
         scopes.pop();
     }
 
-    return {
+    /**
+     * Enter a function and either:
+     * - Push a new nested scope onto the stack (in 'function' mode).
+     * - Report all the Use Strict Directives (in the other modes).
+     * @param {ASTNode} node The function declaration or expression.
+     * @returns {void}
+     */
+    function enterFunction(node) {
+        var isBlock = node.body.type === "BlockStatement",
+            useStrictDirectives = isBlock ?
+                getUseStrictDirectives(node.body.body) : [];
+
+        if (mode === "function") {
+            enterFunctionInFunctionMode(node, useStrictDirectives);
+        } else {
+            reportAll(useStrictDirectives, messages[mode]);
+        }
+    }
+
+    rule = {
         "Program": function(node) {
-            report(getUseStrictDirectives(node.body), messages.function);
-        },
+            var useStrictDirectives = getUseStrictDirectives(node.body);
 
-        // Inside of class bodies are always strict mode.
-        "ClassBody": function() {
-            classScopes.push(true);
-        },
-        "ClassBody:exit": function() {
-            classScopes.pop();
-        },
+            if (node.sourceType === "module") {
+                mode = "module";
+            }
 
+            if (mode === "global") {
+                if (node.body.length > 0 && useStrictDirectives.length === 0) {
+                    context.report(node, messages.global);
+                }
+                reportAllExceptFirst(useStrictDirectives, messages.multiple);
+            } else {
+                reportAll(useStrictDirectives, messages[mode]);
+            }
+        },
         "FunctionDeclaration": enterFunction,
         "FunctionExpression": enterFunction,
-        "ArrowFunctionExpression": enterFunction,
-
-        "FunctionDeclaration:exit": exitFunction,
-        "FunctionExpression:exit": exitFunction,
-        "ArrowFunctionExpression:exit": exitFunction
+        "ArrowFunctionExpression": enterFunction
     };
+
+    if (mode === "function") {
+        assign(rule, {
+            // Inside of class bodies are always strict mode.
+            "ClassBody": function() {
+                classScopes.push(true);
+            },
+            "ClassBody:exit": function() {
+                classScopes.pop();
+            },
+
+            "FunctionDeclaration:exit": exitFunctionInFunctionMode,
+            "FunctionExpression:exit": exitFunctionInFunctionMode,
+            "ArrowFunctionExpression:exit": exitFunctionInFunctionMode
+        });
+    }
+
+    return rule;
 };
 
 module.exports.schema = [

tests/lib/rules/strict.js

@@ -109,6 +109,13 @@ ruleTester.run("strict", rule, {
                 { message: "Strict mode is not permitted.", type: "ExpressionStatement" },
                 { message: "Strict mode is not permitted.", type: "ExpressionStatement" }
             ]
+        }, {
+            code: "\"use strict\"; foo();",
+            options: ["never"],
+            parserOptions: { sourceType: "module" },
+            errors: [
+                { message: "'use strict' is unnecessary inside of modules.", type: "ExpressionStatement" }
+            ]
         },
 
         // "global" mode