Merge pull request #660 from esben-semmle/js/angularjs-alert-locations

Approved by xiemaisi

Author: semmle-qlci

javascript/ql/src/AngularJS/DuplicateDependency.ql

@@ -10,6 +10,7 @@
  */
 
 import javascript
+import semmle.javascript.RestrictedLocations
 
 predicate isRepeatedDependency(AngularJS::InjectableFunction f, string name, ASTNode location) {
   exists(int i, int j | i < j and
@@ -20,4 +21,4 @@ predicate isRepeatedDependency(AngularJS::InjectableFunction f, string name, AST
 from AngularJS::InjectableFunction f, ASTNode node, string name
 where isRepeatedDependency(f, name, node) and
       not count(f.asFunction().getParameterByName(name)) > 1 // avoid duplicating reports from js/duplicate-parameter-name
-select f, "This function has a duplicate dependency '$@'.", node, name
+select (FirstLineOf)f.asFunction(), "This function has a duplicate dependency '$@'.", node, name

javascript/ql/src/AngularJS/RepeatedInjection.ql

@@ -10,8 +10,9 @@
  */
 
 import javascript
+import semmle.javascript.RestrictedLocations
 
 from AngularJS::InjectableFunction f, ASTNode explicitInjection
 where count(f.getAnExplicitDependencyInjection()) > 1 and
       explicitInjection = f.getAnExplicitDependencyInjection()
-select f.asFunction(), "This function has $@ defined in multiple places.", explicitInjection, "dependency injections"
+select (FirstLineOf)f.asFunction(), "This function has $@ defined in multiple places.", explicitInjection, "dependency injections"

javascript/ql/src/AngularJS/UnusedAngularDependency.ql

@@ -11,6 +11,7 @@
 
 import javascript
 import Declarations.UnusedParameter
+import semmle.javascript.RestrictedLocations
 
 predicate isUnusedParameter(Function f, string msg, Parameter parameter) {
   exists(Variable pv |
@@ -36,4 +37,4 @@ predicate isMissingParameter(AngularJS::InjectableFunction f, string msg, ASTNod
 
 from AngularJS::InjectableFunction f, string message, ASTNode location
 where isUnusedParameter(f.asFunction(), message, location) or isMissingParameter(f, message, location)
-select location, message
+select (FirstLineOf)location, message

javascript/ql/test/query-tests/AngularJS/DuplicateDependency/DuplicateDependency.expected

@@ -1,4 +1,5 @@
 | duplicates.js:2:5:2:18 | function f(){} | This function has a duplicate dependency '$@'. | duplicates.js:3:26:3:31 | 'dup5' | dup5 |
-| duplicates.js:6:14:6:57 | ['dup2a ... p2b){}] | This function has a duplicate dependency '$@'. | duplicates.js:6:24:6:30 | 'dup2a' | dup2a |
-| duplicates.js:7:14:7:57 | ['dup3b ... p3b){}] | This function has a duplicate dependency '$@'. | duplicates.js:7:24:7:30 | 'dup3b' | dup3b |
-| duplicates.js:8:14:8:79 | ['dup4' ... p4C){}] | This function has a duplicate dependency '$@'. | duplicates.js:8:35:8:40 | 'dup4' | dup4 |
+| duplicates.js:6:33:6:56 | functio ... up2b){} | This function has a duplicate dependency '$@'. | duplicates.js:6:24:6:30 | 'dup2a' | dup2a |
+| duplicates.js:7:33:7:56 | functio ... up3b){} | This function has a duplicate dependency '$@'. | duplicates.js:7:24:7:30 | 'dup3b' | dup3b |
+| duplicates.js:8:43:8:78 | functio ... up4C){} | This function has a duplicate dependency '$@'. | duplicates.js:8:35:8:40 | 'dup4' | dup4 |
+| duplicates.js:15:35:15:112 | functio ...       } | This function has a duplicate dependency '$@'. | duplicates.js:15:25:15:32 | 'dup11a' | dup11a |

javascript/ql/test/query-tests/AngularJS/DuplicateDependency/duplicates.js

@@ -12,5 +12,7 @@
         .run(['notDup8a', 'notDup8b', function(notDup8a, notDup8b){}]) // OK
         .run(['notDup9a', 'notDup9b', function(notDup9c, notDup9d){}]) // OK
         .run(['dup10a', 'dup10a', 'dup10a', function(dup10a, dup10a, dup10a){}]) // OK (flagged by js/duplicate-parameter-name)
+        .run(['dup11a', 'dup11a', function(dup11a, dup11b){ // NOT OK (alert formatting for multi-line function)
+        }])
     ;
 })();

javascript/ql/test/query-tests/AngularJS/RepeatedInjection/RepeatedInjection.expected

@@ -2,3 +2,5 @@
 | repeated-injection.js:6:5:6:31 | functio ... name){} | This function has $@ defined in multiple places. | repeated-injection.js:8:54:8:73 | ['name', $Injected2] | dependency injections |
 | repeated-injection.js:10:5:10:31 | functio ... name){} | This function has $@ defined in multiple places. | repeated-injection.js:11:5:11:22 | $Injected3.$inject | dependency injections |
 | repeated-injection.js:10:5:10:31 | functio ... name){} | This function has $@ defined in multiple places. | repeated-injection.js:12:5:12:22 | $Injected3.$inject | dependency injections |
+| repeated-injection.js:33:5:33:84 | functio ... )\\n    } | This function has $@ defined in multiple places. | repeated-injection.js:35:5:35:23 | $Injected10.$inject | dependency injections |
+| repeated-injection.js:33:5:33:84 | functio ... )\\n    } | This function has $@ defined in multiple places. | repeated-injection.js:36:56:36:76 | ['name' ... cted10] | dependency injections |

javascript/ql/test/query-tests/AngularJS/RepeatedInjection/repeated-injection.js

@@ -30,4 +30,9 @@
 
     angular.module('app9').controller('controller9', ['name', function inline9(name){}]); // OK
 
+    function $Injected10(name){ // NOT OK (alert formatting for multi-line function)
+    }
+    $Injected10.$inject = ['name'];
+    angular.module('app10').controller('controller10', ['name', $Injected10]);
+
 })();

javascript/ql/test/query-tests/AngularJS/UnusedAngularDependency/UnusedAngularDependency.expected

@@ -2,3 +2,4 @@
 | unused-angular-dependency.js:14:14:14:39 | ["unuse ... n() {}] | This function has 0 parameters, but 1 dependency is injected into it. |
 | unused-angular-dependency.js:16:14:16:53 | ["used2 ... d2) {}] | This function has 1 parameter, but 2 dependencies are injected into it. |
 | unused-angular-dependency.js:17:14:17:52 | ["unuse ... n() {}] | This function has 0 parameters, but 2 dependencies are injected into it. |
+| unused-angular-dependency.js:18:14:18:105 | ["used2 ...      }] | This function has 1 parameter, but 2 dependencies are injected into it. |

javascript/ql/test/query-tests/AngularJS/UnusedAngularDependency/unused-angular-dependency.js

@@ -15,6 +15,8 @@
         .run(f2)
         .run(["used2", "unused9", function(used2) {}]) // NOT OK
         .run(["unused10", "unused11", function() {}]) // NOT OK
+        .run(["used2", "unused12", function(used2) { // NOT OK (alert formatting for multi-line function)
+        }])
     ;
 })();
 angular.module('app2')