New: Implement no-assign-exports rule (fixes #6)

README.md

@@ -52,6 +52,7 @@ Enable the rules that you would like to use, for example:
 * [no-amd-define](docs/rules/no-amd-define.md): Disallow AMD (dependency array) form of `define` (off by default)
 * [no-named-define](docs/rules/no-named-define.md): Disallow named module form of `define` (off by default)
 * [no-commonjs-wrapper](docs/rules/no-commonjs-wrapper.md): Disallow use of Simplified CommonJS Wrapper (off by default)
+* [no-assign-exports](docs/rules/no-assign-exports.md): Disallow assignment to `exports` when using Simplified CommonJS Wrapper
 
 ### Don't see the rule you're looking for?
 

docs/rules/no-assign-exports.md

@@ -0,0 +1,37 @@
+# Disallow assignment to `exports` when using Simplified CommonJS Wrapper (no-assign-exports)
+
+## Rule Details
+
+This rule aims to prevent assignment to the `exports` variable. This is generally an error, as assignment should be done to `module.exports` instead. This rule is only in effect when using the Simplified CommonJS Wrapper.
+
+The following patterns are considered warnings:
+
+```js
+define(function (require, exports) {
+    exports = function () {
+        /* ... */
+    };
+});
+
+define(function (require, exports) {
+    exports = {
+        doSomething: function () {
+            /* ... */
+        }
+    };
+});
+```
+
+The following patterns are not warnings:
+
+```js
+define(function (require, exports) {
+    exports.doSomething: function () {
+        /* ... */
+    };
+});
+```
+
+## When Not To Use It
+
+You should probably *not* disable this rule.

index.js

@@ -8,13 +8,15 @@ module.exports = {
         "no-function-define": require("./lib/rules/no-function-define"),
         "no-amd-define": require("./lib/rules/no-amd-define"),
         "no-named-define": require("./lib/rules/no-named-define"),
-        "no-commonjs-wrapper": require("./lib/rules/no-commonjs-wrapper")
+        "no-commonjs-wrapper": require("./lib/rules/no-commonjs-wrapper"),
+        "no-assign-exports": require("./lib/rules/no-assign-exports")
     },
     rulesConfig: {
 
         // Potential Errors
         "no-invalid-define": 2,
         "no-multiple-define": 2,
+        "no-assign-exports": 2,
 
         // Stylistic Choices
         "no-object-define": 0,

lib/rules/no-assign-exports.js

@@ -0,0 +1,49 @@
+/**
+ * @fileoverview Disallow assignment to `exports` when using Simplified CommonJS Wrapper
+ * @author Casey Visco
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var helpers = require("../helpers");
+
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function (context) {
+
+    var isCommonJS = false;
+
+    //------------------------------------------------------------------------------
+    // Public
+    //------------------------------------------------------------------------------
+
+    return {
+        "CallExpression": function (node) {
+            isCommonJS = isCommonJS || helpers.isCommonJsDef(node.arguments);
+        },
+
+        "CallExpression:exit": function (node) {
+            if (helpers.isCommonJsDef(node.arguments)) {
+                isCommonJS = false;
+            }
+        },
+
+        "AssignmentExpression": function (node) {
+            var isIdentifier = node.left.type === "Identifier",
+                isExports = node.left.name === "exports";
+
+            if (isCommonJS && isIdentifier && isExports) {
+                context.report(node, "Invalid assignment to `exports`.");
+            }
+        }
+    };
+
+};
+

tests/lib/fixtures.js

@@ -19,5 +19,16 @@ module.exports = {
         AMD_NAMED: "define('path/to/c', ['path/to/a', 'path/to/b'], function (a, b) { return true; });",
         AMD_NAMED_EMPTY: "define('path/to/c', [], function () { return true; });"
 
+    },
+
+    exports: {
+
+        // Always Invalid
+        ASSIGN_EXPORTS: "define(function (require, exports) { exports = { foo: 'bar' }; });",
+
+        // Valid unless disabled
+        MODIFY_EXPORTS: "define(function (require, exports) { exports.foo = 'bar'; });",
+        ASSIGN_MODULE_EXPORTS: "define(function (require, exports, module) { module.exports = { foo: 'bar' }; });",
+        NO_CJS_ASSIGN_EXPORTS: "exports = { foo: 'bar' };"
     }
 };

tests/lib/rules/no-assign-exports.js

@@ -0,0 +1,41 @@
+/**
+ * @fileoverview Tests for `no-assign-exports` rule
+ * @author Casey Visco <cvisco@gmail.com>
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var eslint = require("eslint"),
+    ESLintTester = require("eslint-tester"),
+    fixtures = require("../fixtures");
+
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+var eslintTester = new ESLintTester(eslint.linter);
+
+eslintTester.addRuleTest("lib/rules/no-assign-exports", {
+
+    valid: [
+        fixtures.exports.MODIFY_EXPORTS,
+        fixtures.exports.ASSIGN_MODULE_EXPORTS,
+        fixtures.exports.NO_CJS_ASSIGN_EXPORTS
+    ],
+
+    invalid: [
+        {
+            code: fixtures.exports.ASSIGN_EXPORTS,
+            errors: [{
+                message: "Invalid assignment to `exports`.",
+                type: "AssignmentExpression"
+            }]
+        }
+    ]
+
+});