diff --git a/README.md b/README.md index f3a83460..63e8f5e0 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ We provide also three samples : | 'ng_definedundefined': 2 | You should use the angular.isUndefined or angular.isDefined methods instead of using the keyword undefined. We also check the use of !angular.isUndefined and !angular.isDefined (should prefer the reverse function)| | 'ng_di': [2, 'function'] | All your DI should use the same syntax : the Array or function syntaxes ("ng_di": [2, "function or array"])| | 'ng_di_order': 0 | Injected dependencies should be sorted alphabetically. | +| 'ng_di_unused': 0 | Unused dependencies should not be injected. | | 'ng_directive_name': 0 | All your directives should have a name starting with the parameter you can define in your config object. The second parameter can be a Regexp wrapped in quotes. You can not prefix your directives by "ng" (reserved keyword for AngularJS directives) ("ng_directive_name": [2, "ng"]) [Y073](https://github.com/johnpapa/angular-styleguide#style-y073), [Y126](https://github.com/johnpapa/angular-styleguide#style-y126) | | 'ng_document_service': 2 | Instead of the default document object, you should prefer the AngularJS wrapper service $document. [Y180](https://github.com/johnpapa/angular-styleguide#style-y180) | | 'ng_empty_controller': 0 | If you have one empty controller, maybe you have linked it in your Router configuration or in one of your views. You can remove this declaration because this controller is useless | @@ -135,7 +136,7 @@ Here are the things you should do before sending a Pull Request with a new Rule * Update the main index.js file, in order to add the new rule in the 'rules' property, and set the default configuration in the rulesConfig property * Update the "Rules" part of the README.md file with a small description of the rule and its default configuration. -We can use a property, defined in the ESLint configuration file, in order to know which version is used : Angular 1 or Angular 2. based on this property, you can create rules for each version. +We can use a property, defined in the ESLint configuration file, in order to know which version is used : Angular 1 or Angular 2. based on this property, you can create rules for each version. ```yaml plugins: @@ -153,7 +154,7 @@ settings: angular: 2 ``` -And in your rule, you can access to this property thanks to the `context` object : +And in your rule, you can access to this property thanks to the `context` object : ```javascript //If Angular 2 is used, we disabled the rule diff --git a/index.js b/index.js index e51ccc16..7c635773 100644 --- a/index.js +++ b/index.js @@ -12,6 +12,7 @@ 'ng_definedundefined': require('./rules/ng_definedundefined'), 'ng_di': require('./rules/ng_di'), 'ng_di_order': require('./rules/ng_di_order'), + 'ng_di_unused': require('./rules/ng_di_unused'), 'ng_directive_name': require('./rules/ng_directive_name'), 'ng_document_service': require('./rules/ng_document_service'), 'ng_empty_controller': require('./rules/ng_empty_controller'), @@ -54,6 +55,7 @@ 'ng_definedundefined': 2, 'ng_di': [2, 'function'], 'ng_di_order': 0, + 'ng_di_unused': 0, 'ng_directive_name': 0, 'ng_document_service': 2, 'ng_empty_controller': 0, diff --git a/rules/ng_di_order.js b/rules/ng_di_order.js index 009edaee..7f55012b 100644 --- a/rules/ng_di_order.js +++ b/rules/ng_di_order.js @@ -12,15 +12,19 @@ module.exports = function(context) { 'provider', 'service' ]; + var setupCalls = [ + 'config', + 'run' + ]; - function checkOrder(node, fn) { + function checkOrder(fn) { var args = fn.params.map(function(arg) { return arg.name; }); var sortedArgs = args.slice().sort(); sortedArgs.some(function(value, index) { if(args.indexOf(value) !== index) { - context.report(node, 'Injected values should be sorted alphabetically'); + context.report(fn, 'Injected values should be sorted alphabetically'); return true; } }); @@ -31,18 +35,22 @@ module.exports = function(context) { 'AssignmentExpression': function(node) { // The $get function of a provider. if(node.left.type === 'MemberExpression' && node.left.property.name === '$get') { - return checkOrder(node, node.right); + return checkOrder(node.right); } }, 'CallExpression': function(node) { // An Angular component definition. - if(utils.isAngularComponent(node) && node.callee.type === 'MemberExpression' && angularNamedObjectList.indexOf(node.callee.property.name) >= 0){ - return checkOrder(node, node.arguments[1]); + if(utils.isAngularComponent(node) && node.callee.type === 'MemberExpression' && node.arguments[1].type === 'FunctionExpression' && angularNamedObjectList.indexOf(node.callee.property.name) >= 0){ + return checkOrder(node.arguments[1]); + } + // Config and run functions. + if(node.callee.type === 'MemberExpression' && node.arguments.length > 0 && setupCalls.indexOf(node.callee.property.name) !== -1 && node.arguments[0].type === 'FunctionExpression') { + return checkOrder(node.arguments[0]); } // Injected values in unittests. if(node.callee.type === 'Identifier' && node.callee.name === 'inject') { - return checkOrder(node, node.arguments[0]); + return checkOrder(node.arguments[0]); } } }; diff --git a/rules/ng_di_unused.js b/rules/ng_di_unused.js new file mode 100644 index 00000000..d46612e2 --- /dev/null +++ b/rules/ng_di_unused.js @@ -0,0 +1,93 @@ +module.exports = function(context) { + + 'use strict'; + + var utils = require('./utils/utils'); + + var angularNamedObjectList = [ + 'controller', + 'directive', + 'factory', + 'filter', + 'provider', + 'service' + ]; + var setupCalls = [ + 'config', + 'run' + ]; + + var injectFunctions = []; + + // Keeps track of visited scopes in the collectAngularScopes function to prevent infinite recursion on circular references. + var visitedScopes = []; + + // This collects the variable scopes for the injectible functions which have been collected. + function collectAngularScopes(scope) { + if(visitedScopes.indexOf(scope) === -1) { + visitedScopes.push(scope); + injectFunctions.forEach(function(value) { + if(scope.block === value.node) { + value.scope = scope; + } + }); + scope.childScopes.forEach(function(child) { + collectAngularScopes(child); + }); + } + } + + return { + + 'AssignmentExpression': function(node) { + // Colllect the $get function of a providers. + if(node.left.type === 'MemberExpression' && node.left.property.name === '$get') { + injectFunctions.push({ + node: node.right + }); + } + }, + + 'CallExpression': function(node) { + // An Angular component definition. + if(utils.isAngularComponent(node) && node.callee.type === 'MemberExpression' && node.arguments[1].type === 'FunctionExpression' && angularNamedObjectList.indexOf(node.callee.property.name) >= 0){ + return injectFunctions.push({ + node: node.arguments[1] + }); + } + // Config and run functions. + if(node.callee.type === 'MemberExpression' && node.arguments.length > 0 && setupCalls.indexOf(node.callee.property.name) !== -1 && node.arguments[0].type === 'FunctionExpression') { + return injectFunctions.push({ + node: node.arguments[0] + }); + } + // Injected values in unittests. + if(node.callee.type === 'Identifier' && node.callee.name === 'inject') { + return injectFunctions.push({ + node: node.arguments[0] + }); + } + }, + + // Actually find and report unused injected variables. + 'Program:exit': function(node) { + var globalScope = context.getScope(); + collectAngularScopes(globalScope); + injectFunctions.forEach(function(value) { + if(value.scope) { + value.scope.variables.forEach(function(variable) { + if(variable.name === 'arguments') { + return; + } + if(value.node.params.indexOf(variable.identifiers[0]) === -1) { + return; + } + if(variable.references.length === 0) { + context.report(value.node, 'Unused injected value {{name}}', variable); + } + }); + } + }) + } + }; +}; diff --git a/test/ng_di_unused.js b/test/ng_di_unused.js new file mode 100644 index 00000000..313ed77d --- /dev/null +++ b/test/ng_di_unused.js @@ -0,0 +1,70 @@ +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require('../rules/ng_di_unused'), + RuleTester = require("eslint").RuleTester; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var eslintTester = new RuleTester(); +eslintTester.run('ng_di_unused', rule, { + valid: [ + 'app.controller("", function($q){return $q;});', + 'app.directive("", function($q){return $q;});', + 'app.factory("", function($q){return $q;});', + 'app.factory("", function($q){return function(){return $q;};});', + 'app.factory("", function(){var myVar;});', + 'app.filter("", function($q){return $q;});', + 'app.provider("", function($httpProvider){return $httpProvider;});', + 'app.service("", function($q){return $q;});', + 'app.config(function($httpProvider) {$httpProvider.defaults.headers.post.answer="42"})', + 'app.run(function($q) {$q()})', + 'inject(function($q){_$q_ = $q;});', + 'this.$get = function($q){return $q;};', + ], + invalid: [{ + code: 'app.controller("", function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'app.directive("", function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'app.factory("", function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'app.factory("", function($http, $q){});', + errors: [ + {message: 'Unused injected value $http'}, + {message: 'Unused injected value $q'} + ] + }, { + code: 'app.factory("", function($http, $q){return $q.resolve()});', + errors: [ + {message: 'Unused injected value $http'} + ] + }, { + code: 'app.filter("", function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'app.provider("", function($httpProvider){});', + errors: [{message: 'Unused injected value $httpProvider'}] + }, { + code: 'app.service("", function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'app.config(function($httpProvider) {})', + errors: [{message: 'Unused injected value $httpProvider'}] + }, { + code: 'app.run(function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'inject(function($q){});', + errors: [{message: 'Unused injected value $q'}] + }, { + code: 'this.$get = function($q){};', + errors: [{message: 'Unused injected value $q'}] + }] +});