diff --git a/change-notes/1.22/analysis-javascript.md b/change-notes/1.22/analysis-javascript.md index fd211197157..fd09522fc25 100644 --- a/change-notes/1.22/analysis-javascript.md +++ b/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 diff --git a/javascript/ql/src/Expressions/MissingSpaceInAppend.ql b/javascript/ql/src/Expressions/MissingSpaceInAppend.ql index 4870927fd51..bd744109675 100644 --- a/javascript/ql/src/Expressions/MissingSpaceInAppend.ql +++ b/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 + "'." diff --git a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/MissingSpaceInAppend.expected b/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/MissingSpaceInAppend.expected index 5be6f307c43..20e61738e7b 100644 --- a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/MissingSpaceInAppend.expected +++ b/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'. | diff --git a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/missing.js b/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/missing.js index 58b72d2da51..c97f6be724e 100644 --- a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/missing.js +++ b/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" diff --git a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/ok.js b/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/ok.js index b4791701528..f0adfd8254a 100644 --- a/javascript/ql/test/query-tests/Expressions/MissingSpaceInAppend/ok.js +++ b/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;"; \ No newline at end of file + "position: absolute;"; + +s = "h. 0" + "h" +s = ((("h. 0"))) + (("h")) + ("h") \ No newline at end of file