remove false positive in missingSpaceInAppend by requring the presence of a word-like fragment

Author: erik-krogh

change-notes/1.22/analysis-javascript.md

@@ -36,6 +36,7 @@
 | Shift out of range (`js/shift-out-of-range`| Fewer false positive results | This rule now correctly handles BigInt shift operands. |
 | Superfluous trailing arguments (`js/superfluous-trailing-arguments`) | Fewer false-positive results. | This rule no longer flags calls to placeholder functions that trivially throw an exception. |
 | Undocumented parameter (`js/jsdoc/missing-parameter`) | No changes to results | This rule is now run on LGTM, although its results are still not shown by default. |
+| Missing space in string concatenation (`js/missing-space-in-concatenation`) | Fewer false positive results | The rule now requires a word-like part exists in the string concatenation. | 
 
 ## Changes to QL libraries
 

javascript/ql/src/Expressions/MissingSpaceInAppend.ql

@@ -22,14 +22,51 @@ Expr leftChild(Expr e) {
   result = e.(AddExpr).getLeftOperand()
 }
 
-class LiteralOrTemplate extends Expr {
-  LiteralOrTemplate() {
-    this instanceof TemplateLiteral or
-    this instanceof Literal
+predicate isInConcat(Expr e) {
+    exists(ParExpr par | par.getExpression() = e)
+    or 
+    exists(AddExpr a | a.getAnOperand() = e)
+}
+
+class ConcatenationLiteral extends Expr {
+  ConcatenationLiteral() {
+    (
+      this instanceof TemplateLiteral 
+      or
+      this instanceof Literal
+    )
+    and isInConcat(this)
   }
 }
 
-from AddExpr e, LiteralOrTemplate l, LiteralOrTemplate r, string word
+Expr getConcatChild(Expr e) {
+    result = rightChild(e) or 
+    result = leftChild(e)
+}
+
+Expr getConcatParent(Expr e) {
+    e = getConcatChild(result)
+}
+
+predicate isWordLike(ConcatenationLiteral lit) {
+    lit.getStringValue().regexpMatch("(?i).*[a-z]{3,}.*")
+}
+
+class ConcatRoot extends AddExpr {
+    ConcatRoot() {
+        not isInConcat(this)
+    }
+}
+
+ConcatRoot getAddRoot(AddExpr e) {
+    result = getConcatParent*(e)
+}
+
+predicate hasWordLikeFragment(AddExpr e) {
+    isWordLike(getConcatChild*(getAddRoot(e)))
+}
+
+from AddExpr e, ConcatenationLiteral l, ConcatenationLiteral r, string word
 where
   // l and r are appended together
   l = rightChild*(e.getLeftOperand()) and
@@ -41,5 +78,8 @@ where
   // needed, and intra-identifier punctuation in, for example, a qualified name.
   word = l.getStringValue().regexpCapture(".* (([-A-Za-z/'\\.:,]*[a-zA-Z]|[0-9]+)[\\.:,!?']*)", 1) and
   r.getStringValue().regexpMatch("[a-zA-Z].*") and
-  not word.regexpMatch(".*[,\\.:].*[a-zA-Z].*[^a-zA-Z]")
+  not word.regexpMatch(".*[,\\.:].*[a-zA-Z].*[^a-zA-Z]") and
+  
+  // There must be a constant-string in the concatenation that looks like a word.
+  hasWordLikeFragment(e)
 select l, "This string appears to be missing a space after '" + word + "'."

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

@@ -11,3 +11,4 @@
 | missing.js:24:5:24:21 | `missing a space` | This string appears to be missing a space after 'space'. |
 | missing.js:26:5:26:21 | "missing a space" | This string appears to be missing a space after 'space'. |
 | missing.js:28:5:28:21 | `missing a space` | This string appears to be missing a space after 'space'. |
+| missing.js:31:7:31:12 | "h. 0" | This string appears to be missing a space after '0'. |

javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/missing.js

@@ -27,3 +27,5 @@ s = "missing a space" +
     `here`;
 s = `missing a space` +
     `here`;
+
+s = (("h. 0" + "h")) + "word"

javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/ok.js

@@ -8,4 +8,7 @@ s = "the class java.util." +
 s = "some data: a,b,c," +
   "d,e,f";
 s = "overflow: scroll;" +
-  "position: absolute;";
+  "position: absolute;";
+
+s = "h. 0" + "h"
+s = ((("h. 0"))) + (("h")) + ("h")