no-constructor rule

README.md

@@ -10,4 +10,5 @@ eslint-plugin-backbone
 * [collection-model](docs/rules/collection-model.md)
 * [defaults-on-top](docs/rules/defaults-on-top.md)
 * [model-defaults](docs/rules/model-defaults.md)
+* [no-constructor](docs/rules/no-constructor.md)
 * [no-native-jquery](docs/rules/no-native-jquery.md)

backbone-helper.js

@@ -1,11 +1,40 @@
-exports.isBackboneModel = function(node) {
-	return node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "Model";
+function isBackboneModel(node) {
+	"use strict";
+	return node.type === "CallExpression" && node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "Model";
+}
+
+function isBackboneView(node) {
+	"use strict";
+	return node.type === "CallExpression" && node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "View";
+}
+
+function isBackboneCollection(node) {
+	"use strict";
+	return node.type === "CallExpression" && node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "Collection";
+}
+
+function isBackboneAny(node) {
+	"use strict";
+	return node.type === "CallExpression" && node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" &&
+		(node.callee.object.property.name === "Collection" || node.callee.object.property.name === "View" || node.callee.object.property.name === "Model");
+}
+
+exports.isBackboneAny = isBackboneAny;
+exports.isBackboneModel = isBackboneModel;
+exports.isBackboneView = isBackboneView;
+exports.isBackboneCollection = isBackboneCollection;
+
+exports.checkIfPropertyInBackbone = function(node) {
+	var parent = node.parent, grandparent = parent.parent, greatgrandparent = grandparent.parent;
+	return isBackboneAny(greatgrandparent);
 };
 
-exports.isBackboneView = function(node) {
-	return node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "View";
+exports.checkIfPropertyInBackboneModel = function(node) {
+	var parent = node.parent, grandparent = parent.parent, greatgrandparent = grandparent.parent;
+	return isBackboneModel(greatgrandparent);	
 };
 
-exports.isBackboneCollection = function(node) {
-	return node.callee.type === "MemberExpression" && node.callee.object.type === "MemberExpression" && node.callee.object.object.name === "Backbone" && node.callee.object.property.name === "Collection";
-};
+exports.checkIfPropertyInBackboneCollection = function(node) {
+	var parent = node.parent, grandparent = parent.parent, greatgrandparent = grandparent.parent;
+	return isBackboneCollection(greatgrandparent);
+};

docs/rules/no-constructor.md

@@ -0,0 +1,37 @@
+# Prevent overloading of constructor (no-constructor)
+
+Backbone Models, Views and Collections use `constructor` method to instantiate themselves. Overloading it might lead to unexpected results. When overloading `constructor`, you should always call base. In most cases it's easier to overload `initialize` method, which will be executed when Model, View or Collection is created.
+
+## Rule Details
+
+The following patterns are considered warnings:
+
+```js
+
+Backbone.Model.extend({
+	constructor: function() {
+		...
+	}
+});
+
+```
+
+The following patterns are not warnings:
+
+```js
+
+Backbone.Model.extend({
+	initialize: function() {
+		 ...
+	}
+});
+
+```
+
+## When Not To Use It
+
+If you are creating your own wrapper around Model, View or Collection, it's probably better to overload `constructor`. So this rule should be disabled for those files.
+
+## Further Reading
+
+[BackboneJS Documentation](http://backbonejs.org/#Model-constructor)

lib/rules/collection-model.js

@@ -36,7 +36,9 @@ module.exports = function(context) {
         },
         "Identifier": function(node) {
             if (backboneCollection && node.name === "model") {
-                foundModel = true;
+                if (helper.checkIfPropertyInBackboneCollection(node)) {
+                    foundModel = true;
+                }
             }
         }
     };

lib/rules/defaults-on-top.js

@@ -19,9 +19,8 @@ module.exports = function(context) {
     return {
         "Identifier": function(node) {
             if (node.name === "defaults") {
-                var ancestors = context.getAncestors(node);
-                var parent = ancestors.pop(), grandparent = ancestors.pop(), greatgrandparent = ancestors.pop();
-                if (helper.isBackboneModel(greatgrandparent) && grandparent.type === "ObjectExpression" &&  grandparent.properties[0] !== parent) {
+                var parent = node.parent, grandparent = parent.parent;
+                if (helper.checkIfPropertyInBackboneModel(node) && grandparent.type === "ObjectExpression" &&  grandparent.properties[0] !== parent) {
                     context.report(node, "defaults should be declared at the top of the model.");
                 }
             }

lib/rules/model-defaults.js

@@ -36,7 +36,9 @@ module.exports = function(context) {
         },
         "Identifier": function(node) {
             if (backboneModel && node.name === "defaults") {
-                foundDefaults = true;
+                if (helper.checkIfPropertyInBackboneModel(node)) {
+                    foundDefaults = true;
+                }
             }
         }
     };

lib/rules/no-constructor.js

@@ -0,0 +1,28 @@
+/**
+ * @fileoverview Prevent overloading of constructor
+ * @author Ilya Volodin
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+var helper = require("../../backbone-helper.js");
+
+module.exports = function(context) {
+
+    //--------------------------------------------------------------------------
+    // Public
+    //--------------------------------------------------------------------------
+
+    return {
+        "Identifier": function(node) {
+            if (node.name === "constructor") {
+                if (helper.checkIfPropertyInBackbone(node)) {
+                    context.report(node, "Overload initialize instead of constructor");
+                }
+            }
+        }
+    };
+};

tests/lib/rules/collection-model.js

@@ -38,6 +38,10 @@ eslintTester.addRuleTest("lib/rules/collection-model", {
         {
             code: "var a = Backbone.Collection.extend({}); var b = Backbone.Collection.extend();",
             errors: 2
-        }        
+        },
+        {
+            code: "Backbone.Collection.extend({ initialize: function() { var a = { model: {} }; } });",
+            errors: 1
+        }       
     ]
 });

tests/lib/rules/model-defaults.js

@@ -34,6 +34,10 @@ eslintTester.addRuleTest("lib/rules/model-defaults", {
         {
             code: "var a = Backbone.Model.extend({}); var b = Backbone.Model.extend({ defaults: {} });",
             errors: 1
+        },
+        {
+            code: "Backbone.Model.extend({ initialize: function() { var a = { defaults: {} }; } });",
+            errors: 1
         }
     ]
 });

tests/lib/rules/no-constructor.js

@@ -0,0 +1,42 @@
+/**
+ * @fileoverview Prevent overloading of constructor
+ * @author Ilya Volodin
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var eslint = require("eslint").linter,
+    ESLintTester = require("eslint-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+var eslintTester = new ESLintTester(eslint);
+eslintTester.addRuleTest("lib/rules/no-constructor", {
+
+    valid: [
+        "Backbone.Model.extend({ initialize: function() { } });",
+        "Backbone.View.extend({ initialize: function() { } });",
+        "Backbone.Collection.extend({ initialize: function() { } });",
+        "Backbone.Model.extend({ initialize: function() { var a = { constructor: function() { } }; } });"
+    ],
+
+    invalid: [
+        {
+            code: "Backbone.Model.extend({ constructor: function() {} });",
+            errors: 1
+        },
+        {
+            code: "Backbone.View.extend({ constructor: function() {} });",
+            errors: 1
+        },
+        {
+            code: "Backbone.Collection.extend({ constructor: function() {} });",
+            errors: 1
+        }
+    ]
+});

tests/lib/rules/no-native-jquery.js

@@ -23,7 +23,8 @@ eslintTester.addRuleTest("lib/rules/no-native-jquery", {
         "Backbone.View.extend({ render: function() { var a = this.$('.item').offset(); } });",
         "Backbone.View.extend({ test: function() { return $.isArray(a); } });",
         "Backbone.Model.extend({ initialize: function() { var a = $('.item').offset(); } });",
-        "var a = 6 * 7;"
+        "var a = 6 * 7;",
+        "var a = $('.item').offset();"
     ],
 
     invalid: [