diff --git a/.travis.yml b/.travis.yml index 71812dc1..d807dd0f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,5 +2,5 @@ language: node_js node_js: - "5.0" - "4.2" - - "0.10" + - "0.12" after_script: "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js" diff --git a/README.md b/README.md index 0405727a..b7716a44 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,7 @@ Angular often provide multi ways to to something. These rules help you to define * [di](docs/di.md) - require a consistent DI syntax * [di-order](docs/di-order.md) - require DI parameters to be sorted alphabetically + * [dumb-inject](docs/dumb-inject.md) - unittest `inject` functions should only consist of assignments from injected values to describe block variables * [function-type](docs/function-type.md) - require and specify a consistent function style for components ('named' or 'anonymous') ([y024](https://github.com/johnpapa/angular-styleguide#style-y024)) * [module-dependency-order](docs/module-dependency-order.md) - require a consistent order of module dependencies * [no-service-method](docs/no-service-method.md) - use `factory()` instead of `service()` ([y040](https://github.com/johnpapa/angular-styleguide#style-y040)) diff --git a/docs/dumb-inject.md b/docs/dumb-inject.md new file mode 100644 index 00000000..e4d50319 --- /dev/null +++ b/docs/dumb-inject.md @@ -0,0 +1,64 @@ + + +# dumb-inject - unittest `inject` functions should only consist of assignments from injected values to describe block variables + +`inject` functions in unittests should only contain a sorted mapping of injected values to values in the `describe` block with matching names. +This way the dependency injection setup is separated from the other setup logic, improving readability of the test. + +## Examples + +The following patterns are considered problems; + + /*eslint angular/dumb-inject: 2*/ + + // invalid + describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + + $httpBackend.whenGET('/data').respond([]); + })); + }); // error: inject functions may only consist of assignments in the form myService = _myService_ + + // invalid + describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $rootScope = _$rootScope_; + $httpBackend = _$httpBackend_; + })); + }); // error: '$httpBackend' must be sorted before '$rootScope' + +The following patterns are **not** considered problems; + + /*eslint angular/dumb-inject: 2*/ + + // valid + describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + })); + + beforeEach(function() { + $httpBackend.whenGET('/data').respond([]); + }); + }); + +## Version + +This rule was introduced in eslint-plugin-angular 0.15.0 + +## Links + +* [Rule source](../rules/dumb-inject.js) +* [Example source](../examples/dumb-inject.js) diff --git a/examples/dumb-inject.js b/examples/dumb-inject.js new file mode 100644 index 00000000..4e5152ec --- /dev/null +++ b/examples/dumb-inject.js @@ -0,0 +1,38 @@ +// example - valid: true +describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + })); + + beforeEach(function() { + $httpBackend.whenGET('/data').respond([]); + }); +}); + +// example - valid: false, errorMessage: "inject functions may only consist of assignments in the form myService = _myService_" +describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $httpBackend = _$httpBackend_; + $rootScope = _$rootScope_; + + $httpBackend.whenGET('/data').respond([]); + })); +}); + +// example - valid: false, errorMessage: "'$httpBackend' must be sorted before '$rootScope'" +describe(function() { + var $httpBackend; + var $rootScope; + + beforeEach(inject(function(_$httpBackend_, _$rootScope_) { + $rootScope = _$rootScope_; + $httpBackend = _$httpBackend_; + })); +}); diff --git a/index.js b/index.js index dcfd851f..0d029540 100644 --- a/index.js +++ b/index.js @@ -16,6 +16,7 @@ rulesConfiguration.addRule('di-unused', 0); rulesConfiguration.addRule('directive-name', 0); rulesConfiguration.addRule('directive-restrict', 0); rulesConfiguration.addRule('document-service', 2); +rulesConfiguration.addRule('dumb-inject', 0); rulesConfiguration.addRule('empty-controller', 0); rulesConfiguration.addRule('file-name', 0); rulesConfiguration.addRule('filter-name', 0); diff --git a/rules/dumb-inject.js b/rules/dumb-inject.js new file mode 100644 index 00000000..91f647ff --- /dev/null +++ b/rules/dumb-inject.js @@ -0,0 +1,71 @@ +/** + * unittest `inject` functions should only consist of assignments from injected values to describe block variables + * + * `inject` functions in unittests should only contain a sorted mapping of injected values to values in the `describe` block with matching names. + * This way the dependency injection setup is separated from the other setup logic, improving readability of the test. + * + * @version 0.15.0 + * @category conventions + */ +'use strict'; + +var angularRule = require('./utils/angular-rule'); + + +module.exports = angularRule(function(context) { + function report(node, name) { + context.report(node, 'inject functions may only consist of assignments in the form {{name}} = _{{name}}_', { + name: name || 'myService' + }); + } + + return { + 'angular:inject': function(callExpression, fn) { + if (!fn) { + return; + } + var valid = []; + // Report bad statement types + fn.body.body.forEach(function(statement) { + if (statement.type !== 'ExpressionStatement') { + return report(statement); + } + if (statement.expression.type !== 'AssignmentExpression') { + return report(statement); + } + if (statement.expression.right.type !== 'Identifier') { + return report(statement); + } + // From this point there is more context on what to report. + var name = statement.expression.right.name.replace(/^_(.+)_$/, '$1'); + if (statement.expression.left.type !== 'Identifier') { + return report(statement, name); + } + if (statement.expression.right.name !== '_' + name + '_') { + return report(statement, name); + } + if (statement.expression.left.name !== name) { + return report(statement, name); + } + // Register valid statements for sort order validation + valid.push(statement); + }); + // Validate the sorting order + var lastValid; + valid.forEach(function(statement) { + if (!lastValid) { + lastValid = statement.expression.left.name; + return; + } + if (statement.expression.left.name.localeCompare(lastValid) !== -1) { + lastValid = statement.expression.left.name; + return; + } + context.report(statement, "'{{current}}' must be sorted before '{{previous}}'", { + current: statement.expression.left.name, + previous: lastValid + }); + }); + } + }; +}); diff --git a/test/dumb-inject.js b/test/dumb-inject.js new file mode 100644 index 00000000..488a8556 --- /dev/null +++ b/test/dumb-inject.js @@ -0,0 +1,61 @@ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +var rule = require('../rules/dumb-inject'); +var RuleTester = require('eslint').RuleTester; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +var eslintTester = new RuleTester(); +eslintTester.run('log', rule, { + valid: [ + // Don't crash if no function is passed. + 'inject()', + // Some valid examples + 'inject(function() {$httpBackend = _$httpBackend_})', + 'inject(function() {$controller = _$controller_; $rootScope = _$rootScope_})', + 'inject(function() {$httpBackend = _$httpBackend_; myService = _myService_})' + ], + invalid: [ + // Not an expression statement + { + code: 'inject(function() {function foo() {}})', + errors: [{message: 'inject functions may only consist of assignments in the form myService = _myService_'}] + }, + // Not an assignment expression + { + code: 'inject(function() {$httpBackend.whenGET()})', + errors: [{message: 'inject functions may only consist of assignments in the form myService = _myService_'}] + }, + // Right is not a simple identifier + { + code: 'inject(function() {navigator = $window.navigator})', + errors: [{message: 'inject functions may only consist of assignments in the form myService = _myService_'}] + }, + // Left is not a simple identifier + { + code: 'inject(function() {foo.bar = _bar_})', + errors: [{message: 'inject functions may only consist of assignments in the form bar = _bar_'}] + }, + // Right is not wrapped using underscores + { + code: 'inject(function() {bar = bar})', + errors: [{message: 'inject functions may only consist of assignments in the form bar = _bar_'}] + }, + // Left does not match right + { + code: 'inject(function() {foo = _bar_})', + errors: [{message: 'inject functions may only consist of assignments in the form bar = _bar_'}] + }, + // Bad sorting of statements + { + code: 'inject(function() {foo = _foo_; bar = _bar_})', + errors: [{message: "'bar' must be sorted before 'foo'"}] + } + ] +});