Python: fix alerts for py/import-deprecated-module

python/ql/src/Imports/DeprecatedModule.ql

@@ -12,9 +12,12 @@
 
 import python
 
-
+/**
+ * The module `name` was deprecated in Python version `major`.`minor`,
+ * and module `instead` should be used instead (or `instead = "no replacement"`)
+ */
 predicate deprecated_module(string name, string instead, int major, int minor) {
-    name = "posixfile" and instead = "email" and major = 1 and minor = 5
+    name = "posixfile" and instead = "fcntl" and major = 1 and minor = 5
     or
     name = "gopherlib" and instead = "no replacement" and major = 2 and minor = 5
     or
@@ -34,40 +37,49 @@ predicate deprecated_module(string name, string instead, int major, int minor) {
     or
     name = "rotor" and instead = "no replacement" and major = 2 and minor = 4
     or
-    name = "statcache"  and instead = "no replacement" and major = 2 and minor = 2
+    name = "statcache" and instead = "no replacement" and major = 2 and minor = 2
     or
-    name = "mpz"  and instead = "a third party" and major = 2 and minor = 2
+    name = "mpz" and instead = "a third party" and major = 2 and minor = 2
     or
     name = "xreadlines" and instead = "no replacement" and major = 2 and minor = 3
     or
     name = "multifile" and instead = "email" and major = 2 and minor = 5
     or
-    name = "sets"  and instead = "builtins" and major = 2 and minor = 6
+    name = "sets" and instead = "builtins" and major = 2 and minor = 6
     or
     name = "buildtools" and instead = "no replacement" and major = 2 and minor = 3
     or
-    name = "cfmfile" and instead = "no replacement" and major = 2  and minor = 4
+    name = "cfmfile" and instead = "no replacement" and major = 2 and minor = 4
     or
     name = "macfs" and instead = "no replacement" and major = 2 and minor = 3
     or
-    name = "md5"  and instead = "hashlib" and major = 2  and minor = 5
+    name = "md5" and instead = "hashlib" and major = 2 and minor = 5
     or
-    name = "sha"  and instead = "hashlib" and major = 2  and minor = 5
+    name = "sha" and instead = "hashlib" and major = 2 and minor = 5
 }
 
 string deprecation_message(string mod) {
-		exists(int major, int minor | deprecated_module(mod, _, major, minor) |
-			  	 result = "The " + mod  + " module was deprecated in version " + major.toString() + "." + minor.toString() + ".")
+    exists(int major, int minor | deprecated_module(mod, _, major, minor) |
+        result = "The " + mod + " module was deprecated in version " + major.toString() + "." +
+                minor.toString() + "."
+    )
 }
 
 string replacement_message(string mod) {
-		exists(string instead | deprecated_module(mod, instead, _, _) |
-		       result =  " Use " + instead + " module instead." and not instead = "no replacement"
-		       or
-		       result =  "" and instead = "no replacement"
-		)
+    exists(string instead | deprecated_module(mod, instead, _, _) |
+        result = " Use " + instead + " module instead." and not instead = "no replacement"
+        or
+        result = "" and instead = "no replacement"
+    )
 }
 
-from ImportExpr imp, Stmt s, Expr e
-where s.getASubExpression() = e and (e = imp or e.contains(imp))
-select s, deprecation_message(imp.getName()) + replacement_message(imp.getName())
+from ImportExpr imp, string name, string instead
+where
+    name = imp.getName() and
+    deprecated_module(name, instead, _, _) and
+    not exists(Try try, ExceptStmt except | except = try.getAHandler()
+    |
+        except.getType().pointsTo(ClassValue::importError()) and
+        except.containsInScope(imp)
+    )
+select imp, deprecation_message(name) + replacement_message(name)

python/ql/src/semmle/python/objects/ObjectAPI.qll

@@ -714,4 +714,9 @@ module ClassValue {
         result = TBuiltinClassObject(Builtin::builtin("NameError"))
     }
 
+    /** Get the `ClassValue` for the `ImportError` class. */
+    ClassValue importError() {
+        result = TBuiltinClassObject(Builtin::builtin("ImportError"))
+    }
+
 }

python/ql/test/query-tests/Imports/deprecated/DeprecatedModule.expected

@@ -1,2 +1,3 @@
-| test.py:5:1:5:13 | Import | The rfc822 module was deprecated in version 2.3. Use email module instead. |
-| test.py:6:1:6:16 | Import | The posixfile module was deprecated in version 1.5. Use email module instead. |
+| test.py:2:8:2:13 | ImportExpr | The rfc822 module was deprecated in version 2.3. Use email module instead. |
+| test.py:3:8:3:16 | ImportExpr | The posixfile module was deprecated in version 1.5. Use fcntl module instead. |
+| test.py:8:16:8:18 | ImportExpr | The md5 module was deprecated in version 2.5. Use hashlib module instead. |

python/ql/test/query-tests/Imports/deprecated/DeprecatedModule.qlref

@@ -1 +1 @@
-Imports/DeprecatedModule.ql
+Imports/DeprecatedModule.ql

python/ql/test/query-tests/Imports/deprecated/test.py

@@ -1,6 +1,14 @@
-
-
-
-#Some deprecated modules
+# Some deprecated modules
 import rfc822
 import posixfile
+
+# We should only report a bad import once
+class Foo(object):
+    def foo(self):
+        import md5
+
+# Backwards compatible code, should not report
+try:
+    from hashlib import md5
+except ImportError:
+    from md5 import md5