Merge pull request #166 from asger-semmle/documentable-self-assign

Approved by esben-semmle, xiemaisi

javascript/ql/src/Expressions/Clones.qll

@@ -152,7 +152,14 @@ class SelfAssignment extends StructurallyCompared {
   }
 
   override Expr candidate() {
-    result = getParent().(AssignExpr).getRhs()
+    result = getAssignment().getRhs()
+  }
+
+  /**
+   * Gets the enclosing assignment.
+   */
+  AssignExpr getAssignment() {
+    result.getLhs() = this
   }
 }
 

javascript/ql/src/Expressions/SelfAssignment.qhelp

@@ -15,6 +15,11 @@ Assigning a variable to itself typically indicates a mistake such as a missing
 Carefully inspect the assignment to check for misspellings or missing qualifiers.
 </p>
 
+<p>
+If the self-assignment is intentional and is needed for documentation or optimization purposes,
+add a JSDoc comment with a <tt>@type</tt> tag. This will indicate the self-assignment is intentional.
+</p>
+
 </recommendation>
 <example>
 

javascript/ql/src/Expressions/SelfAssignment.ql

@@ -43,5 +43,7 @@ where e.same(_) and
         propName = any(AccessorMethodDeclaration amd).getName()
       ) and
       // exclude DOM properties
-      not isDOMProperty(e.(PropAccess).getPropertyName())
-select e.getParent(), "This expression assigns " + dsc + " to itself."
+      not isDOMProperty(e.(PropAccess).getPropertyName()) and
+      // exclude self-assignments that have been inserted to satisfy the TypeScript JS-checker
+      not e.getAssignment().getParent().(ExprStmt).getDocumentation().getATag().getTitle() = "type"
+select e.getParent(), "This expression assigns " + dsc + " to itself."

javascript/ql/test/query-tests/Expressions/SelfAssignment/SelfAssignment.expected

@@ -1,3 +1,5 @@
+| jsdoc.js:9:5:9:19 | this.y = this.y | This expression assigns property y to itself. |
+| jsdoc.js:11:5:11:23 | this.arg = this.arg | This expression assigns property arg to itself. |
 | tst.js:5:2:5:14 | width = width | This expression assigns variable width to itself. |
 | tst.js:24:1:24:19 | array[1] = array[1] | This expression assigns element 1 to itself. |
 | tst.js:27:1:27:9 | o.x = o.x | This expression assigns property x to itself. |

javascript/ql/test/query-tests/Expressions/SelfAssignment/jsdoc.js

@@ -0,0 +1,15 @@
+class C extends Q {
+  constructor(arg) {
+    /**
+     * Something.
+     * @type {string | undefined}
+     */
+    this.x = this.x; // OK - documentation
+
+    this.y = this.y; // NOT OK
+
+    this.arg = this.arg; // NOT OK
+  }
+}
+
+// semmle-extractor-options: --experimental