[lldb] Print a useful error message when trying to import modules with dots or dashes

[JDevlieghere]

Summary

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 names: foo-bar

rdar://74263511

[JDevlieghere]

module specification -> module names

[JDevlieghere]

FWIW I played around with __import__ and importlib.import_module which does allow you to import modules with dashes, but unlike import it doesn't add them to the globals. I'm sure there's a way around that, but it didn't seem worth the additional complexity.

[mib]

LGTM!

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

if (!syspath_retval) {
       return llvm::make_error<llvm::StringError>(
           "Python sys.path handling failed", llvm::inconvertibleErrorCode());
     }
 
     return llvm::Error::success();
   };
 
   std::string module_name(pathname);
+  bool possible_package = false;
 
   if (extra_search_dir) {
     if (llvm::Error e = ExtendSysPath(extra_search_dir.GetPath())) {
       error = std::move(e);
       return false;
     }
   } else {
     FileSpec module_file(pathname);
     FileSystem::Instance().Resolve(module_file);
     FileSystem::Instance().Collect(module_file);
 
     fs::file_status st;
     std::error_code ec = status(module_file.GetPath(), st);
 
     if (ec || st.type() == fs::file_type::status_error ||
         st.type() == fs::file_type::type_unknown ||
         st.type() == fs::file_type::file_not_found) {
       // if not a valid file of any sort, check if it might be a filename still
       // dot can't be used but / and \ can, and if either is found, reject
       if (strchr(pathname, '\\') || strchr(pathname, '/')) {
         error.SetErrorString("invalid pathname");
         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");
         return false;
       }
       if (llvm::Error e =
               ExtendSysPath(module_file.GetDirectory().GetCString())) {
         error = std::move(e);
bool ScriptInterpreterPythonImpl::LoadScriptingModule(
   llvm::StringRef extension = llvm::sys::path::extension(module_name);
   if (!extension.empty()) {
     if (extension == ".py")
       module_name.resize(module_name.length() - 3);
     else if (extension == ".pyc")
       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 names: %s", module_name.c_str());
+    return false;
+  }
+
+  if (module_name.find('-') != llvm::StringRef::npos) {
+    error.SetErrorStringWithFormat(
+        "Python discourages dashes in module names: %s", module_name.c_str());
+    return false;
+  }
+
   // check if the module is already import-ed
   StreamString command_stream;
   command_stream.Clear();
   command_stream.Printf("sys.modules.__contains__('%s')", module_name.c_str());
   bool does_contain = false;
   // this call will succeed if the module was ever imported in any Debugger
   // in the lifetime of the process in which this LLDB framework is living
   bool was_imported_globally =

lldb/test/Shell/ScriptInterpreter/Python/command_import.test

+# 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 names: 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 names: foo.bar