hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 03:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

JS: address review comments

Browse Source
This commit is contained in:
Esben Sparre Andreasen
2019-02-25 15:08:56 +01:00
parent 6c1b29e4b6
commit 047b69a4c2
6 changed files with 32 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/src/Declarations/UnusedParameter.qll
View File

@@ -1,5 +1,5 @@
/**
* This library contains the majority of the 'js/unused-parameter' query implementation.
* Provides classes and predicates for the 'js/unused-parameter' query.
*
* In order to suppress alerts that are similar to the 'js/unused-parameter' alerts,
* `isAnAccidentallyUnusedParameter` should be used since it holds iff that alert is active.

30
javascript/ql/src/Declarations/UnusedProperty.ql
View File

@@ -30,9 +30,12 @@ predicate hasUnknownPropertyRead(CapturedSource obj) {
exists(obj.getAPropertyRead("propertyIsEnumerable"))
}
predicate flowsToTypeRestrictedExpression(CapturedSource n) {
/**
* Holds if `obj` flows to an expression that must have a specific type.
*/
predicate flowsToTypeRestrictedExpression(CapturedSource obj) {
exists (Expr restricted, TypeExpr type |
n.flowsToExpr(restricted) and
obj.flowsToExpr(restricted) and
not type.isAny() |
exists (TypeAssertion assertion |
type = assertion.getTypeAnnotation() and
@@ -47,14 +50,15 @@ predicate flowsToTypeRestrictedExpression(CapturedSource n) {
)
}
from DataFlow::PropWrite w, CapturedSource n, string name
from DataFlow::PropWrite write, CapturedSource obj, string name
where
w = n.getAPropertyWrite(name) and
not exists(n.getAPropertyRead(name)) and
not w.getBase().analyze().getAValue() != n.analyze().getAValue() and
not hasUnknownPropertyRead(n) and
write = obj.getAPropertyWrite(name) and
not exists(obj.getAPropertyRead(name)) and
// `obj` is the only base object for the write: it is not spurious
not write.getBase().analyze().getAValue() != obj.analyze().getAValue() and
not hasUnknownPropertyRead(obj) and
// avoid reporting if the definition is unreachable
w.getAstNode().getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock and
write.getAstNode().getFirstControlFlowNode().getBasicBlock() instanceof ReachableBasicBlock and
// avoid implicitly read properties
not (
name = "toString" or
@@ -62,13 +66,13 @@ where
name.matches("@@%") // @@iterator, for example
) and
// avoid flagging properties that a type system requires
not flowsToTypeRestrictedExpression(n) and
not flowsToTypeRestrictedExpression(obj) and
// flagged by js/unused-local-variable
not exists(UnusedLocal l | l.getAnAssignedExpr().getUnderlyingValue().flow() = n) and
not exists(UnusedLocal l | l.getAnAssignedExpr().getUnderlyingValue().flow() = obj) and
// flagged by js/unused-parameter
not exists(Parameter p | isAnAccidentallyUnusedParameter(p) |
p.getDefault().getUnderlyingValue().flow() = n
p.getDefault().getUnderlyingValue().flow() = obj
) and
// flagged by js/useless-expression
not inVoidContext(n.asExpr())
select w, "Unused property " + name + "."
not inVoidContext(obj.asExpr())
select write, "Unused property " + name + "."

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

@@ -1,5 +1,5 @@
/**
* This library contains parts of the 'js/unused-local-variable' query implementation.
* Provides classes and predicates for the 'js/unused-local-variable' query.
*/
import javascript

2
javascript/ql/src/Expressions/ExprHasNoEffect.qll
View File

@@ -1,5 +1,5 @@
/**
* This library contains parts of the 'js/useless-expression' query implementation.
* Provides classes and predicates for the 'js/useless-expression' query.
*/
import javascript

4
javascript/ql/src/semmle/javascript/dataflow/CapturedNodes.qll
View File

@@ -42,7 +42,7 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
n.flowsToExpr(any(FunctionBindExpr bind).getObject())
or
// technically, the builtin prototypes could have a `this`-using function through which this node escapes, but we ignore that here
// (we also ignore `o['__' + 'proto__"] = ...`)
// (we also ignore `o['__' + 'proto__'] = ...`)
exists(n.getAPropertyWrite("__proto__"))
or
// could check the assigned value of all affected variables, but it is unlikely to matter in practice
@@ -51,7 +51,7 @@ private predicate exposedAsReceiver(DataFlow::SourceNode n) {
/**
* A source for which the flow is entirely captured by the dataflow library.
* All uses of the node is represented by `this.flowsTo(_)` and friends.
* All uses of the node are modeled by `this.flowsTo(_)` and related predicates.
*/
class CapturedSource extends DataFlow::SourceNode {
CapturedSource() {

18
javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
View File

@@ -256,20 +256,22 @@ private predicate hasDefiniteReceiver(
* Enables inter-procedural type inference for the return value of a
* method call to a flow-insensitively type-inferred callee.
*/
class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalAnalyzedReturnFlow {
private class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalAnalyzedReturnFlow {
DataFlow::FunctionNode fun;
TypeInferredMethodWithAnalyzedReturnFlow() {
exists(CapturedSource s, DataFlow::PropWrite w, string name |
exists(CapturedSource obj, DataFlow::PropWrite write, string name |
this.(DataFlow::MethodCallNode).getMethodName() = name and
s.hasOwnProperty(name) and
hasDefiniteReceiver(this, s) and
w = s.getAPropertyWrite() and
fun.flowsTo(w.getRhs()) and
obj.hasOwnProperty(name) and
hasDefiniteReceiver(this, obj) and
// include all potential callees
// by construction, there are no unknown methods on `obj`
write = obj.getAPropertyWrite() and
fun.flowsTo(write.getRhs()) and
(
not exists(w.getPropertyName())
not exists(write.getPropertyName())
or
w.getPropertyName() = name
write.getPropertyName() = name
)
)
}