Python: Don't flag return procedure_call() in __init__ as error

RasmusWL · RasmusWL · commit d2739740457a · 2019-09-23T11:22:55.000+02:00
This commit fixes the results for https://lgtm.com/projects/b/jjburton/cgmtools/snapshot/0d8a429b7ea17854a5e5341df98b1cbd54d7fe6c/files/mayaTools/cgm/lib/classes/AttrFactory.py?sort=name&dir=ASC&mode=heatmap#L90 ``` def __init__(...): if error_case: return guiFactory.warning(...) ``` that was wrongly reporting _Explicit return in __init__ method._ as an error.
diff --git a/python/ql/src/Functions/ExplicitReturnInInit.ql b/python/ql/src/Functions/ExplicitReturnInInit.ql
@@ -12,11 +12,12 @@
 
 import python
 
-from Return r
+from Return r, Expr rv
 where
-    exists(Function init | init.isInitMethod() and r.getScope() = init and exists(r.getValue())) and
-    not r.getValue() instanceof None and
-    not exists(FunctionValue f | f.getACall() = r.getValue().getAFlowNode() | f.neverReturns()) and
+    exists(Function init | init.isInitMethod() and r.getScope() = init) and
+    r.getValue() = rv and
+    not rv.pointsTo(Value::none_()) and
+    not exists(FunctionValue f | f.getACall() = rv.getAFlowNode() | f.neverReturns()) and
     // to avoid double reporting, don't trigger if returning result from other __init__ function
-    not exists(Attribute meth | meth = r.getValue().(Call).getFunc() | meth.getName() = "__init__")
+    not exists(Attribute meth | meth = rv.(Call).getFunc() | meth.getName() = "__init__")
 select r, "Explicit return in __init__ method."
diff --git a/python/ql/test/query-tests/Functions/general/ExplicitReturnInInit.expected b/python/ql/test/query-tests/Functions/general/ExplicitReturnInInit.expected
@@ -1 +1,2 @@
 | explicit_return_in_init.py:4:9:4:19 | Return | Explicit return in __init__ method. |
+| explicit_return_in_init.py:102:9:102:18 | Return | Explicit return in __init__ method. |
diff --git a/python/ql/test/query-tests/Functions/general/explicit_return_in_init.py b/python/ql/test/query-tests/Functions/general/explicit_return_in_init.py
@@ -3,7 +3,7 @@ class ExplicitReturnInInit(object):
     def __init__(self):
         return self
 
-#These are OK
+# These are OK
 class ExplicitReturnNoneInInit(object):
 
     def __init__(self):
@@ -32,7 +32,7 @@ class InitIsGenerator(object):
     def __init__(self):
         yield self
 
-#OK as it returns result of a call to super().__init__()
+# OK as it returns result of a call to super().__init__()
 class InitCallsInit(InitCallsError):
 
     def __init__(self):
@@ -43,3 +43,60 @@ class InitCallsBadInit(ExplicitReturnInInit):
 
     def __init__(self):
         return ExplicitReturnInInit.__init__(self)
+
+# OK as procedure implicitly returns None
+#
+# this was seen in the wild: https://lgtm.com/projects/b/jjburton/cgmtools/snapshot/0d8a429b7ea17854a5e5341df98b1cbd54d7fe6c/files/mayaTools/cgm/lib/classes/AttrFactory.py?sort=name&dir=ASC&mode=heatmap#L90
+# using a pattern of `return procedure_that_logs_error()`
+
+def procedure():
+    pass
+
+def explicit_none():
+    return None
+
+def explicit_none_nested():
+    return explicit_none()
+
+class InitReturnsCallResult1(object):
+
+    def __init__(self):
+        return procedure()
+
+class InitReturnsCallResult2(object):
+
+    def __init__(self):
+        return explicit_none()
+
+class InitReturnsCallResult3(object):
+
+    def __init__(self):
+        return explicit_none_nested()
+
+class InitReturnsCallResult4(object):
+
+    def __init__(self, b):
+        if b:
+            p = procedure
+        else:
+            p = explicit_none
+        return p()
+
+class InitReturnsCallResult5(object):
+
+    def __init__(self, b):
+        return procedure() if b else explicit_none()
+
+# Not OK
+
+def not_ok():
+    return 42
+
+class InitReturnsCallResult6(object):
+
+    def __init__(self, b):
+        if b:
+            p = procedure_implicit_none()
+        else:
+            p = not_ok
+        return p()

Original file line number	Diff line number	Diff line change
`@@ -1 +1,2 @@`
`1`	`1`	`\| explicit_return_in_init.py:4:9:4:19 \| Return \| Explicit return in __init__ method. \|`
	`2`	`+\| explicit_return_in_init.py:102:9:102:18 \| Return \| Explicit return in __init__ method. \|`