[Lldb-commits] [PATCH] D96833: [lldb] Print a useful error message when trying to import modules with dots or dashes

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, kastiglione, mib.
JDevlieghere requested review of this revision.

LLDB does not like to import Python files with dashes or dots in their name. While the former are technically allowed, they're discouraged [1]. Dots are allowed for subpackages but not in module names. This patch improves the user experience by printing a useful error.

Currently lldb prints:

  error: module importing failed: SyntaxError('invalid syntax', ('<string>', 1, 11, 'import foo-bar\n'))

After this patch:

  error: module importing failed: Python discourages dashes in module specification: foo-bar


https://reviews.llvm.org/D96833

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/Shell/ScriptInterpreter/Python/command_import.test


Index: lldb/test/Shell/ScriptInterpreter/Python/command_import.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/command_import.test
@@ -0,0 +1,20 @@
+# REQUIRES: python
+
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: echo "print('Rene Magritte')" >> %t/foo.py
+# RUN: echo "print('Jan van Eyck')" >> %t/foo-bar.py
+# RUN: echo "print('Pieter Bruegel the Elder')" >> %t/foo.bar.py
+# RUN: echo "print('Pieter Bruegel the Younger')" >> %t/foo.bar
+
+# RUN: %lldb --script-language python -o 'command script import %t/foo.py' 2>&1 | FileCheck %s --check-prefix MAGRITTE
+# MAGRITTE: Rene Magritte
+
+# RUN: %lldb --script-language python -o 'command script import %t/foo-bar.py' 2>&1 | FileCheck %s --check-prefix VANEYCK
+# VANEYCK-NOT: Jan van Eyck
+# VANEYCK: module importing failed: Python discourages dashes in module specification: foo-bar
+
+# RUN: %lldb --script-language python -o 'command script import %t/foo.bar.py' 2>&1 | FileCheck %s --check-prefix BRUEGEL
+# RUN: %lldb --script-language python -o 'command script import %t/foo.bar' 2>&1 | FileCheck %s --check-prefix BRUEGEL
+# BRUEGEL-NOT: Pieter Bruegel the Elder
+# BRUEGEL-NOT: Pieter Bruegel the Younger
+# BRUEGEL: module importing failed: Python does not allow dots in module specification: foo.bar
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2781,6 +2781,7 @@
   };
 
   std::string module_name(pathname);
+  bool possible_package = false;
 
   if (extra_search_dir) {
     if (llvm::Error e = ExtendSysPath(extra_search_dir.GetPath())) {
@@ -2805,6 +2806,7 @@
         return false;
       }
       // Not a filename, probably a package of some sort, let it go through.
+      possible_package = true;
     } else if (is_directory(st) || is_regular_file(st)) {
       if (module_file.GetDirectory().IsEmpty()) {
         error.SetErrorString("invalid directory name");
@@ -2831,6 +2833,20 @@
       module_name.resize(module_name.length() - 4);
   }
 
+  if (!possible_package && module_name.find('.') != llvm::StringRef::npos) {
+    error.SetErrorStringWithFormat(
+        "Python does not allow dots in module specification: %s",
+        module_name.c_str());
+    return false;
+  }
+
+  if (module_name.find('-') != llvm::StringRef::npos) {
+    error.SetErrorStringWithFormat(
+        "Python discourages dashes in module specification: %s",
+        module_name.c_str());
+    return false;
+  }
+
   // check if the module is already import-ed
   StreamString command_stream;
   command_stream.Clear();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96833.324168.patch
Type: text/x-patch
Size: 2827 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210217/9a785463/attachment.bin>