Skip to content

Commit 0906d85

Browse files
authored
Merge pull request #19726 from Napalys/js/quality/string_interpolation
JS: Promote `js/template-syntax-in-string-literal` to the Code Quality suite.
2 parents ad64e04 + d7ad625 commit 0906d85

File tree

5 files changed

+69
-5
lines changed

5 files changed

+69
-5
lines changed

javascript/ql/integration-tests/query-suite/javascript-code-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ ql/javascript/ql/src/Declarations/SuspiciousMethodNameDeclaration.ql
33
ql/javascript/ql/src/Expressions/ExprHasNoEffect.ql
44
ql/javascript/ql/src/Expressions/MissingAwait.ql
55
ql/javascript/ql/src/LanguageFeatures/SpuriousArguments.ql
6+
ql/javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql
67
ql/javascript/ql/src/Quality/UnhandledErrorInStreamPipeline.ql
78
ql/javascript/ql/src/RegExp/DuplicateCharacterInCharacterClass.ql
89
ql/javascript/ql/src/RegExp/RegExpAlwaysMatches.ql

javascript/ql/src/LanguageFeatures/TemplateSyntaxInStringLiteral.ql

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
* @problem.severity warning
66
* @id js/template-syntax-in-string-literal
77
* @precision high
8-
* @tags correctness
8+
* @tags quality
9+
* reliability
10+
* correctness
11+
* language-features
912
*/
1013

1114
import javascript
@@ -74,8 +77,8 @@ class CandidateStringLiteral extends StringLiteral {
7477
*/
7578
predicate hasObjectProvidingTemplateVariables(CandidateStringLiteral lit) {
7679
exists(DataFlow::CallNode call, DataFlow::ObjectLiteralNode obj |
77-
call.getAnArgument().getALocalSource() = obj and
78-
call.getAnArgument().asExpr() = lit and
80+
call.getAnArgument() = [lit.flow(), StringConcatenation::getRoot(lit.flow())] and
81+
obj.flowsTo(call.getAnArgument()) and
7982
forex(string name | name = lit.getAReferencedVariable() | exists(obj.getAPropertyWrite(name)))
8083
)
8184
}
@@ -91,12 +94,38 @@ VarDecl getDeclIn(Variable v, Scope scope, string name, CandidateTopLevel tl) {
9194
result.getTopLevel() = tl
9295
}
9396

97+
/**
98+
* Tracks data flow from a string literal that may flow to a replace operation.
99+
*/
100+
DataFlow::SourceNode trackStringWithTemplateSyntax(
101+
CandidateStringLiteral lit, DataFlow::TypeTracker t
102+
) {
103+
t.start() and result = lit.flow() and exists(lit.getAReferencedVariable())
104+
or
105+
exists(DataFlow::TypeTracker t2 | result = trackStringWithTemplateSyntax(lit, t2).track(t2, t))
106+
}
107+
108+
/**
109+
* Gets a string literal that flows to a replace operation.
110+
*/
111+
DataFlow::SourceNode trackStringWithTemplateSyntax(CandidateStringLiteral lit) {
112+
result = trackStringWithTemplateSyntax(lit, DataFlow::TypeTracker::end())
113+
}
114+
115+
/**
116+
* Holds if the string literal flows to a replace method call.
117+
*/
118+
predicate hasReplaceMethodCall(CandidateStringLiteral lit) {
119+
trackStringWithTemplateSyntax(lit).getAMethodCall() instanceof StringReplaceCall
120+
}
121+
94122
from CandidateStringLiteral lit, Variable v, Scope s, string name, VarDecl decl
95123
where
96124
decl = getDeclIn(v, s, name, lit.getTopLevel()) and
97125
lit.getAReferencedVariable() = name and
98126
lit.isInScope(s) and
99127
not hasObjectProvidingTemplateVariables(lit) and
100-
not lit.getStringValue() = "${" + name + "}"
128+
not lit.getStringValue() = "${" + name + "}" and
129+
not hasReplaceMethodCall(lit)
101130
select lit, "This string is not a template literal, but appears to reference the variable $@.",
102131
decl, v.getName()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Fixed false positives in the `js/template-syntax-in-string-literal` query where template syntax in string concatenation and "manual string interpolation" patterns were incorrectly flagged.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: queryMetadata
3+
---
4+
* Added `reliability` and `language-features` tags to the `js/template-syntax-in-string-literal` query.

javascript/ql/test/query-tests/LanguageFeatures/TemplateSyntaxInStringLiteral/TemplateSyntaxInStringLiteral.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,30 @@ function foo1() {
4040
writer.emit("Name: ${name}, Date: ${date}.", data);
4141

4242
writer.emit("Name: ${name}, Date: ${date}, ${foobar}", data); // $ Alert - `foobar` is not in `data`.
43-
}
43+
}
44+
45+
function a(actual, expected, description) {
46+
assert(false, "a", description, "expected (" +
47+
typeof expected + ") ${expected} but got (" + typeof actual + ") ${actual}", {
48+
expected: expected,
49+
actual: actual
50+
});
51+
}
52+
53+
function replacer(str, name) {
54+
return str.replace("${name}", name);
55+
}
56+
57+
function replacerAll(str, name) {
58+
return str.replaceAll("${name}", name);
59+
}
60+
61+
function manualInterpolation(name) {
62+
let str = "Name: ${name}";
63+
let result1 = replacer(str, name);
64+
console.log(result1);
65+
66+
str = "Name: ${name} and again: ${name}";
67+
let result2 = replacerAll(str, name);
68+
console.log(result2);
69+
}

0 commit comments

Comments
 (0)