mirror of
https://github.com/github/codeql.git
synced 2026-04-29 10:45:15 +02:00
JavaScript: Introduce new query UnclearOperatorPrecedence.
This commit is contained in:
@@ -0,0 +1,46 @@
|
||||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
Nested expressions that rely on less well-known operator precedence rules can be
|
||||
hard to read and understand. They could even indicate a bug where the author of the
|
||||
code misunderstood the precedence rules.
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>
|
||||
Use parentheses or additional whitespace to clarify grouping.
|
||||
</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>
|
||||
Consider the following snippet of code:
|
||||
</p>
|
||||
|
||||
<sample src="examples/UnclearOperatorPrecedence.js" />
|
||||
|
||||
<p>
|
||||
It might look like this tests whether <code>x</code> and <code>y</code> have any bits in
|
||||
common, but in fact <code>==</code> binds more tightly than <code>&</code>, so the test
|
||||
is equivalent to <code>x & (y == 0)</code>.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
If this is the intended interpretation, parentheses should be used to clarify this. You could
|
||||
also consider adding extra whitespace around <code>&</code> or removing whitespace
|
||||
around <code>==</code> to make it visually apparent that it binds less tightly:
|
||||
<code>x & y==0</code>.
|
||||
</p>
|
||||
<p>
|
||||
Probably the best approach in this case, though, would be to use the <code>&&</code>
|
||||
operator instead to clarify the intended interpretation: <code>x && y == 0</code>.
|
||||
</p>
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>Mozilla Developer Network, <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence">Operator precedence</a>.</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
29
javascript/ql/src/Expressions/UnclearOperatorPrecedence.ql
Normal file
29
javascript/ql/src/Expressions/UnclearOperatorPrecedence.ql
Normal file
@@ -0,0 +1,29 @@
|
||||
/**
|
||||
* @name Unclear precedence of nested operators
|
||||
* @description Nested expressions involving binary bitwise operators and comparisons are easy
|
||||
* to misunderstand without additional disambiguating parentheses or whitespace.
|
||||
* @kind problem
|
||||
* @problem.severity recommendation
|
||||
* @id js/unclear-operator-precedence
|
||||
* @tags maintainability
|
||||
* correctness
|
||||
* statistical
|
||||
* non-attributable
|
||||
* external/cwe/cwe-783
|
||||
* @precision very-high
|
||||
*/
|
||||
|
||||
import javascript
|
||||
|
||||
from BitwiseBinaryExpr bit, Comparison rel, Expr other
|
||||
where bit.hasOperands(rel, other) and
|
||||
// only flag if whitespace doesn't clarify the nesting (note that if `bit` has less
|
||||
// whitespace than `rel`, it will be reported by `js/whitespace-contradicts-precedence`)
|
||||
bit.getWhitespaceAroundOperator() = rel.getWhitespaceAroundOperator() and
|
||||
// don't flag if the other operand is itself a comparison,
|
||||
// since the nesting tends to be visually more obvious in such cases
|
||||
not other instanceof Comparison and
|
||||
// don't flag occurrences in minified code
|
||||
not rel.getTopLevel().isMinified()
|
||||
select rel, "The '" + rel.getOperator() + "' operator binds more tightly than " +
|
||||
"'" + bit.getOperator() + "', which may not be obvious in this case."
|
||||
@@ -0,0 +1,3 @@
|
||||
if (x & y == 0) {
|
||||
// ...
|
||||
}
|
||||
Reference in New Issue
Block a user