Skip to content
This repository was archived by the owner on Apr 4, 2025. It is now read-only.

Commit 0acada1

Browse files
committed
fix(modules): fix duplicate warnings on controller and modules
1 parent 5ef974e commit 0acada1

File tree

4 files changed

+60
-9
lines changed

4 files changed

+60
-9
lines changed

src/modules/controllers.js

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,14 @@ var nameToControllerMap = {};
1515
* log a message if the controller is instantiated on the window
1616
*/
1717
angular.module('ngHintControllers', []).
18-
config(['$provide', function ($provide) {
18+
config(['$provide', '$controllerProvider', function ($provide, $controllerProvider) {
1919
$provide.decorator('$controller', ['$delegate', controllerDecorator]);
20+
21+
var originalRegister = $controllerProvider.register;
22+
$controllerProvider.register = function(name, constructor) {
23+
stringOrObjectRegister(name);
24+
originalRegister.apply($controllerProvider, arguments);
25+
}
2026
}]);
2127

2228
function controllerDecorator($delegate) {
@@ -25,7 +31,10 @@ function controllerDecorator($delegate) {
2531
var match = ctrl.match(CNTRL_REG);
2632
var ctrlName = (match && match[1]) || ctrl;
2733

28-
sendMessageForControllerName(ctrlName);
34+
if (!nameToControllerMap[ctrlName]) {
35+
sendMessageForControllerName(ctrlName);
36+
}
37+
2938
if (!nameToControllerMap[ctrlName] && typeof window[ctrlName] === 'function') {
3039
sendMessageForGlobalController(ctrlName);
3140
}
@@ -41,6 +50,14 @@ function controllerDecorator($delegate) {
4150
*/
4251
var originalModule = angular.module;
4352

53+
function stringOrObjectRegister(controllerName) {
54+
if ((controllerName !== null) && (typeof controllerName === 'object')) {
55+
Object.keys(controllerName).forEach(processController);
56+
} else {
57+
processController(controllerName);
58+
}
59+
}
60+
4461
function processController(ctrlName) {
4562
nameToControllerMap[ctrlName] = true;
4663
sendMessageForControllerName(ctrlName);
@@ -98,11 +115,7 @@ angular.module = function() {
98115
originalController = module.controller;
99116

100117
module.controller = function(controllerName, controllerConstructor) {
101-
if ((controllerName !== null) && (typeof controllerName === 'object')) {
102-
Object.keys(controllerName).forEach(processController);
103-
} else {
104-
processController(controllerName);
105-
}
118+
stringOrObjectRegister(controllerName);
106119
return originalController.apply(this, arguments);
107120
};
108121

src/modules/modules.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ angular.module = function(name, requiresOriginal) {
1919

2020
module.requiresOriginal = requiresOriginal;
2121
modules[name] = module;
22-
hasNameSpace(name);
2322
var modToCheck = getModule(name, true);
2423
//check arguments to determine if called as setter or getter
2524
var modIsSetter = arguments.length > 1;
2625

26+
if (modIsSetter) {
27+
hasNameSpace(name);
28+
}
2729

2830
if(modToCheck && modToCheck.requiresOriginal !== module.requiresOriginal && modIsSetter) {
2931
if(!modData.createdMulti[name]) {

test/controllers.spec.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,20 @@ describe('controllerDecorator', function() {
184184
'Name controllers according to best practices');
185185
});
186186

187+
it('should only warn once about formatting of controller name', function() {
188+
$controllerProvider.register('sampleController', function() {});
189+
$controller('sampleController');
190+
$controller('sampleController');
191+
var formattingWarning = angular.hint.emit.calls.allArgs()
192+
.filter(function(warningArray) {
193+
return warningArray[0] === 'Controllers:rename' &&
194+
warningArray[1] === 'Consider renaming `sampleController`' +
195+
' to `SampleController`.' &&
196+
warningArray[2] === SEVERITY_WARNING;
197+
});
198+
expect(formattingWarning.length).toBe(1);
199+
});
200+
187201

188202
it('should not warn if a controller name begins with an uppercase letter', function(){
189203
$controllerProvider.register('SampleController', function() {});

test/hasNameSpace.spec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,32 @@
11
var hasNameSpace = require('../src/modules/angular-hint-modules/hasNameSpace');
22
var hintLog = angular.hint;
3+
var start = require('../src/modules/angular-hint-modules/start');
4+
5+
var SEVERITY_SUGGESTION = 3;
36

47
describe('hasNameSpace()', function() {
8+
beforeEach(function() {
9+
spyOn(hint, 'emit');
10+
});
11+
512
it('should check that a module has a correctly formatted name', function() {
613
expect(hasNameSpace('MyApp')).toBe(false);
714
expect(hasNameSpace('myapp')).toBe(false);
815
expect(hasNameSpace('myApp')).toBe(true);
916
});
10-
});
17+
18+
it('should only warn once about incorrect formatting for a module', function() {
19+
angular.module('MyApp', []);
20+
angular.module('MyApp');
21+
angular.module('MyApp');
22+
start();
23+
var formattingWarning = angular.hint.emit.calls.allArgs()
24+
.filter(function(warningArray) {
25+
return warningArray[0] === 'Modules' &&
26+
warningArray[1] === 'The best practice for module names is' +
27+
' to use lowerCamelCase. Check the name of "MyApp".' &&
28+
warningArray[2] === SEVERITY_SUGGESTION;
29+
});
30+
expect(formattingWarning.length).toBe(1);
31+
});
32+
});

0 commit comments

Comments
 (0)