FilesExpand file tree

 main

/

UselessConditional.ql

Copy path

Blame

More file actions

Blame

More file actions

Latest commit

 

History

History

History

166 lines (155 loc) · 5.19 KB

 main

/

UselessConditional.ql

Copy path

Top

File metadata and controls

• Code
• Blame

166 lines (155 loc) · 5.19 KB

Raw

Copy raw file

Download raw file

Open symbols panel

Edit and raw actions

1

2

3

4

5

6

7

8

9

10

11

12

13

14

15

16

17

18

19

20

21

22

23

24

25

26

27

28

29

30

31

32

33

34

35

36

37

38

39

40

41

42

43

44

45

46

47

48

49

50

51

52

53

54

55

56

57

58

59

60

61

62

63

64

65

66

67

68

69

70

71

72

73

74

75

76

77

78

79

80

81

82

83

84

85

86

87

88

89

90

91

92

93

94

95

96

97

98

99

100

101

102

103

104

105

106

107

108

109

110

111

112

113

114

115

116

117

118

119

120

121

122

123

124

125

126

127

128

129

130

131

132

133

134

135

136

137

138

139

140

141

142

143

144

145

146

147

148

149

150

151

152

153

154

155

156

157

158

159

160

161

162

163

164

165

166

/**

* @name Useless conditional

* @description If a conditional expression always evaluates to true or always

* evaluates to false, this suggests incomplete code or a logic

* error.

* @kind problem

* @problem.severity warning

* @id js/trivial-conditional

* @tags quality

* reliability

* correctness

* external/cwe/cwe-570

* external/cwe/cwe-571

* @precision very-high

*/

import javascript

import semmle.javascript.RestrictedLocations

import semmle.javascript.dataflow.Refinements

import semmle.javascript.DefensiveProgramming

import UselessConditional

/**

* Gets the unique definition of `v`.

*

* If `v` has no definitions or more than one, or if its single definition

* is a destructuring assignment, this predicate is undefined.

*/

VarDef getSingleDef(Variable v) {

strictcount(VarDef vd | vd.getAVariable() = v) = 1 and

result.getTarget() = v.getAReference()

}

/**

* Holds if variable `v` looks like a symbolic constant, that is, it is assigned

* exactly once, either in a `const` declaration or with a constant initializer.

*

* We do not consider conditionals to be useless if they check a symbolic constant.

*/

predicate isSymbolicConstant(Variable v) {

exists(VarDef vd | vd = getSingleDef(v) |

vd.(VariableDeclarator).getDeclStmt() instanceof ConstDeclStmt

or

vd.(VariableDeclarator).getDeclStmt() instanceof UsingDeclStmt

or

isConstant(vd.getSource())

)

}

/**

* Holds if `e` is a literal constant or a reference to a symbolic constant.

*/

predicate isConstant(Expr e) {

e instanceof Literal or

isSymbolicConstant(e.(VarAccess).getVariable())

}

/**

* Holds if `e` directly uses a parameter's negated or initial value as passed in from the caller.

*/

predicate isInitialParameterUse(Expr e) {

// unlike `SimpleParameter.getAnInitialUse` this will not include uses we have refinement information for

exists(SimpleParameter p, SsaExplicitDefinition ssa |

ssa.getDef() = p and

ssa.getVariable().getAUse() = e and

not p.isRestParameter()

)

or

// same as above, but for captured variables

exists(SimpleParameter p, LocalVariable var |

var = p.getVariable() and

var.isCaptured() and

e = var.getAnAccess() and

not p.isRestParameter()

)

or

isInitialParameterUse(e.(LogNotExpr).getOperand())

}

/**

* Holds if `e` directly uses the returned value from functions that return constant boolean values.

*/

predicate isConstantBooleanReturnValue(Expr e) {

// unlike `SourceNode.flowsTo` this will not include uses we have refinement information for

exists(string b | (b = "true" or b = "false") |

forex(DataFlow::CallNode call, Expr ret |

ret = call.getACallee().getAReturnedExpr() and

(

e = call.asExpr()

or

// also support return values that are assigned to variables

exists(SsaExplicitDefinition ssa |

ssa.getDef().getSource() = call.asExpr() and

ssa.getVariable().getAUse() = e

)

)

|

ret.(BooleanLiteral).getValue() = b

)

)

or

isConstantBooleanReturnValue(e.(LogNotExpr).getOperand())

}

private Expr maybeStripLogNot(Expr e) {

result = maybeStripLogNot(e.(LogNotExpr).getOperand()) or

result = e

}

/**

* Holds if `e` is a defensive expression with a fixed outcome.

*/

predicate isConstantDefensive(Expr e) {

exists(DefensiveExpressionTest defensive |

maybeStripLogNot(defensive.asExpr()) = e and

exists(defensive.getTheTestResult())

)

}

/**

* Holds if `e` is an expression that should not be flagged as a useless condition.

*

* We currently whitelist these kinds of expressions:

*

* - constants (including references to literal constants);

* - negations of constants;

* - defensive checks;

* - expressions that rely on inter-procedural parameter information;

* - constant boolean returned values.

*/

predicate whitelist(Expr e) {

isConstant(e) or

isConstant(e.(LogNotExpr).getOperand()) or

isConstantDefensive(e) or // flagged by js/useless-defensive-code

isInitialParameterUse(e) or

isConstantBooleanReturnValue(e)

}

/**

* Holds if `e` is part of a conditional node `cond` that evaluates

* `e` and checks its value for truthiness.

*

* The return value of `e` may have other uses besides the truthiness check,

* but if the truthiness check always goes one way, it still indicates an error.

*/

predicate isConditional(AstNode cond, Expr e) {

isExplicitConditional(cond, e) or

e = cond.(LogicalBinaryExpr).getLeftOperand()

}

from AstNode cond, DataFlow::AnalyzedNode op, boolean cv, AstNode sel, string msg

where

isConditional(cond, op.asExpr()) and

cv = op.getTheBooleanValue() and

not whitelist(op.asExpr()) and

// if `cond` is of the form `<non-trivial truthy expr> && <something>`,

// we suggest replacing it with `<non-trivial truthy expr>, <something>`

if cond.(LogAndExpr).getLeftOperand() = op.asExpr() and cv = true and not op.asExpr().isPure()

then (

sel = cond and msg = "This logical 'and' expression can be replaced with a comma expression."

) else (

// otherwise we just report that `op` always evaluates to `cv`

sel = op.asExpr().stripParens() and

msg = "This " + describeExpression(sel) + " always evaluates to " + cv + "."

)

select sel, msg