hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-16 16:53:25 +01:00
Code Issues Packages Projects Releases Wiki Activity

JavaScript: Add new query UnusedIndexVariable.

Browse Source
This commit is contained in:
Max Schaefer
2019-09-09 17:01:21 +01:00
parent cdcc716675
commit 500cde68c3
17 changed files with 170 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
change-notes/1.23/analysis-javascript.md
View File

@@ -11,6 +11,7 @@
| **Query** | **Tags** | **Purpose** |
|---------------------------------------------------------------------------|-------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Unused index variable (`js/unused-index-variable`) | correctness | Highlights loops that iterate over an array, but do not use the index variable to access array elements, indicating a possible typo or logic error. |
## Changes to existing queries

1
javascript/config/suites/javascript/correctness-core
View File

@@ -28,6 +28,7 @@
+ semmlecode-javascript-queries/LanguageFeatures/SyntaxError.ql: /Correctness/Language Features
+ semmlecode-javascript-queries/LanguageFeatures/TemplateSyntaxInStringLiteral.ql: /Correctness/Language Features
+ semmlecode-javascript-queries/LanguageFeatures/ThisBeforeSuper.ql: /Correctness/Language Features
+ semmlecode-javascript-queries/LanguageFeatures/UnusedIndexVariable.ql: /Correctness/Language Features
+ semmlecode-javascript-queries/RegExp/BackrefBeforeGroup.ql: /Correctness/Regular Expressions
+ semmlecode-javascript-queries/RegExp/BackrefIntoNegativeLookahead.ql: /Correctness/Regular Expressions
+ semmlecode-javascript-queries/RegExp/DuplicateCharacterInCharacterClass.ql: /Correctness/Regular Expressions

3
javascript/ql/src/Declarations/UnusedVariable.qll
View File

@@ -3,6 +3,7 @@
*/
import javascript
import LanguageFeatures.UnusedIndexVariable
/**
* A local variable that is neither used nor exported, and is not a parameter
@@ -16,6 +17,8 @@ class UnusedLocal extends LocalVariable {
not exists(ClassExpr ce | this = ce.getVariable()) and
not exists(ExportDeclaration ed | ed.exportsAs(this, _)) and
not exists(LocalVarTypeAccess type | type.getVariable() = this) and
// avoid double reporting
not unusedIndexVariable(_, this, _) and
// common convention: variables with leading underscore are intentionally unused
getName().charAt(0) != "_"
}

45
javascript/ql/src/LanguageFeatures/UnusedIndexVariable.qhelp Normal file
View File

@@ -0,0 +1,45 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
If the loop variable of a <code>for</code> loop ranges over the indices of an array, that variable
would normally be used as an array index in the body of the loop. If, instead, the loop body only
refers to array elements at constant indices, this may indicate a logic error or leftover testing
code.
</p>
</overview>
<recommendation>
<p>
Examine the loop carefully to ensure it is behaving as expected. You may want to consider using
a <code>for</code>-<code>of</code> loop to iterate over all elements of an array without the need
for error-prone index manipulations.
</p>
</recommendation>
<example>
<p>
The following example shows a function that is intended to sum up the elements of an array
<code>xs</code>. The loop variable <code>i</code> is counted up from zero to
<code>xs.length-1</code>, but instead of adding <code>xs[i]</code> to the running sum
<code>res</code>, the code adds <code>xs[0]</code>, the first element of <code>xs</code>,
to it, which is likely a mistake:
</p>
<sample src="examples/UnusedIndexVariable.js"/>
<p>
The problem can be fixed by adding <code>xs[i]</code> instead:
</p>
<sample src="examples/UnusedIndexVariableGood.js"/>
<p>
Alternatively, the function can be written more succinctly using a <code>for</code>-<code>of</code>
loop:
</p>
<sample src="examples/UnusedIndexVariableGood2.js"/>
</example>
<references>
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for">for</a></li>
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of">for...of</a></li>
</references>
</qhelp>

17
javascript/ql/src/LanguageFeatures/UnusedIndexVariable.ql Normal file
View File

@@ -0,0 +1,17 @@
/**
* @name Unused index variable
* @description Iterating over an array but not using the index variable to access array elements
* may indicate a typo or logic error.
* @kind problem
* @problem.severity warning
* @id js/unused-index-variable
* @precision high
* @tags correctness
*/
import javascript
import UnusedIndexVariable
from RelationalComparison rel, Variable idx, Variable v
where unusedIndexVariable(rel, idx, v)
select rel, "Index variable " + idx + " is never used to access elements of " + v + "."

40
javascript/ql/src/LanguageFeatures/UnusedIndexVariable.qll Normal file
View File

@@ -0,0 +1,40 @@
/**
* Provides a predicate for identifying unused index variables in loops.
*/
import javascript
/**
* Holds if `arr` is of the form `base[idx]` and is nested inside loop `fs`.
*/
private predicate arrayIndexInLoop(IndexExpr arr, Variable base, Expr idx, ForStmt fs) {
arr.getEnclosingStmt().getParentStmt*() = fs.getBody() and
arr.getBase() = base.getAnAccess() and
arr.getIndex() = idx
}
/**
* Gets `e` or a sub-expression `s` resulting from `e` by peeling off unary and binary
* operators, increments, decrements, type assertions, parentheses, sequence expressions,
* and assignments.
*/
private Expr unwrap(Expr e) {
result = e or
result = unwrap(e.(UpdateExpr).getOperand()) or
result = unwrap(e.(UnaryExpr).getOperand()) or
result = unwrap(e.(BinaryExpr).getAnOperand()) or
result = unwrap(e.getUnderlyingValue())
}
/**
* Holds if `rel` is a for-loop condition of the form `idx <= v.length`, but all array
* indices `v[c]` inside the loop body (of which there must be at least one) use a constant
* index `c` instead of an index based on `idx`.
*/
predicate unusedIndexVariable(RelationalComparison rel, Variable idx, Variable v) {
exists(ForStmt fs | fs.getTest() = rel |
unwrap(rel.getLesserOperand()) = idx.getAnAccess() and
rel.getGreaterOperand().(PropAccess).accesses(v.getAnAccess(), "length") and
forex(IndexExpr arr, Expr e | arrayIndexInLoop(arr, v, e, fs) | exists(e.getIntValue()))
)
}

6
javascript/ql/src/LanguageFeatures/examples/UnusedIndexVariable.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var i=0; i<xs.length; ++i)
res += xs[0]; // BAD: should be xs[i]
return res;
}

6
javascript/ql/src/LanguageFeatures/examples/UnusedIndexVariableGood.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var i=0; i<xs.length; ++i)
res += xs[i];
return res;
}

6
javascript/ql/src/LanguageFeatures/examples/UnusedIndexVariableGood2.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var x of xs)
res += x;
return res;
}

6
javascript/ql/test/query-tests/Declarations/UnusedVariable/UnusedIndexVariable.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs, i) {
var res = 0;
for(;i++<xs.length;) // NOT OK, but flagged by js/unused-index-variable
res += xs[0];
return res;
}

2
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariable.expected Normal file
View File

@@ -0,0 +1,2 @@
| UnusedIndexVariable2.js:3:8:3:20 | i++<xs.length | Index variable i is never used to access elements of xs. |
| UnusedIndexVariable.js:3:16:3:26 | i<xs.length | Index variable i is never used to access elements of xs. |

6
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariable.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var i=0; i<xs.length; ++i)
res += xs[0]; // BAD: should be xs[i]
return res;
}

1
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariable.qlref Normal file
View File

@@ -0,0 +1 @@
LanguageFeatures/UnusedIndexVariable.ql

6
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariable2.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs, i) {
var res = 0;
for(;i++<xs.length;)
res += xs[0]; // BAD: should be xs[i]
return res;
}

6
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariableGood.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var i=0; i<xs.length; ++i)
res += xs[i];
return res;
}

6
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/UnusedIndexVariableGood2.js Normal file
View File

@@ -0,0 +1,6 @@
function sum(xs) {
var res = 0;
for(var x of xs)
res += x;
return res;
}

12
javascript/ql/test/query-tests/LanguageFeatures/UnusedIndexVariable/tst.js Normal file
View File

@@ -0,0 +1,12 @@
function isEmpty(xs) {
for(var i=0; i<xs.length; ++i)
return false;
return true;
}
function desk(xs) {
for(var i=0; i<xs.length; ++i)
if(xs[i] < xs[0])
return "yellow";
return [];
}