mirror of
https://github.com/github/codeql.git
synced 2025-12-25 13:16:33 +01:00
5e62da7690
This was brought up on the LGTM.com forums here: https://discuss.lgtm.com/t/warn-when-always-failing-assert-is-reachable-rather-than-unreachable/2436 Essentially, in a complex chain of `elif` statements, like ```python if x < 0: ... elif x >= 0: ... else: ... ``` the `else` clause is redundant, since the preceding conditions completely exhaust the possible values for `x` (assuming `x` is an integer). Rather than promoting the final `elif` clause to an `else` clause, it is common to instead raise an explicit exception in the `else` clause. During execution, this exception will never actually be raised, but its presence indicates that the preceding conditions are intended to cover all possible cases. I think it's a fair point. This is a clear instance where the alert, even if it is technically correct, is not useful for the end user. Also, I decided to make the exclusion fairly restrictive: it only applies if the unreachable statement is an `assert False, ...` or `raise ...`, and only if said statement is the first in the `else` block. Any other statements will still be reported.
65 lines
1.7 KiB
Plaintext
65 lines
1.7 KiB
Plaintext
/**
|
|
* @name Unreachable code
|
|
* @description Code is unreachable
|
|
* @kind problem
|
|
* @tags maintainability
|
|
* useless-code
|
|
* external/cwe/cwe-561
|
|
* @problem.severity warning
|
|
* @sub-severity low
|
|
* @precision very-high
|
|
* @id py/unreachable-statement
|
|
*/
|
|
|
|
import python
|
|
|
|
predicate typing_import(ImportingStmt is) {
|
|
exists(Module m |
|
|
is.getScope() = m and
|
|
exists(TypeHintComment tc | tc.getLocation().getFile() = m.getFile())
|
|
)
|
|
}
|
|
|
|
/** Holds if `s` contains the only `yield` in scope */
|
|
predicate unique_yield(Stmt s) {
|
|
exists(Yield y | s.contains(y)) and
|
|
exists(Function f |
|
|
f = s.getScope() and
|
|
strictcount(Yield y | f.containsInScope(y)) = 1
|
|
)
|
|
}
|
|
|
|
/** Holds if `contextlib.suppress` may be used in the same scope as `s` */
|
|
predicate suppression_in_scope(Stmt s) {
|
|
exists(With w |
|
|
w.getContextExpr().(Call).getFunc().pointsTo(Value::named("contextlib.suppress")) and
|
|
w.getScope() = s.getScope()
|
|
)
|
|
}
|
|
|
|
/** Holds if `s` is a statement that raises an exception at the end of an if-elif-else chain. */
|
|
predicate marks_an_impossible_else_branch(Stmt s) {
|
|
exists(If i | i.getOrelse().getItem(0) = s |
|
|
s.(Assert).getTest() instanceof False
|
|
or
|
|
s instanceof Raise
|
|
)
|
|
}
|
|
|
|
predicate reportable_unreachable(Stmt s) {
|
|
s.isUnreachable() and
|
|
not typing_import(s) and
|
|
not suppression_in_scope(s) and
|
|
not exists(Stmt other | other.isUnreachable() |
|
|
other.contains(s)
|
|
or
|
|
exists(StmtList l, int i, int j | l.getItem(i) = other and l.getItem(j) = s and i < j)
|
|
) and
|
|
not unique_yield(s) and
|
|
not marks_an_impossible_else_branch(s)
|
|
}
|
|
|
|
from Stmt s
|
|
where reportable_unreachable(s)
|
|
select s, "Unreachable statement."
|