Swap pkgutil.ImpImporter for importlib when loading custom modules from add-ons

source/addonHandler/__init__.py

@@ -1,5 +1,5 @@
 # A part of NonVisual Desktop Access (NVDA)
-# Copyright (C) 2012-2022 Rui Batista, NV Access Limited, Noelia Ruiz Martínez,
+# Copyright (C) 2012-2023 Rui Batista, NV Access Limited, Noelia Ruiz Martínez,
 # Joseph Lee, Babbage B.V., Arnold Loubriat, Łukasz Golonka, Leonard de Ruijter
 # This file is covered by the GNU General Public License.
 # See the file COPYING for more details.
@@ -11,7 +11,6 @@
 import inspect
 import itertools
 import collections
-import pkgutil
 import shutil
 from io import StringIO
 import pickle
@@ -29,6 +28,9 @@
 import addonAPIVersion
 from . import addonVersionCheck
 from .addonVersionCheck import isAddonCompatible
+from .packaging import isModuleName
+import importlib
+from types import ModuleType
 import extensionPoints
 
 
@@ -475,25 +477,45 @@ def _getPathForInclusionInPackage(self, package):
 		extension_path = os.path.join(self.path, package.__name__)
 		return extension_path
 
-	def loadModule(self, name):
+	def loadModule(self, name: str) -> ModuleType:
 		""" loads a python module from the addon directory
 		@param name: the module name
-		@type name: string
-		@returns the python module with C{name}
-		@rtype python module
+		@raises: Any exception that can be raised when importing a module,
+			such as NameError, AttributeError, ImportError, etc.
+			a ValueError is raised when the module name is invalid.
 		"""
-		log.debug("Importing module %s from plugin %s", name, self.name)
-		importer = pkgutil.ImpImporter(self.path)
-		loader = importer.find_module(name)
-		if not loader:
-			return None
+		if not isModuleName(name):
+			raise ValueError(f"{name} is an invalid python module name")
+		log.debug(f"Importing module {name} from plugin {self!r}")
 		# Create a qualified full name to avoid modules with the same name on sys.modules.
-		fullname = "addons.%s.%s" % (self.name, name)
-		try:
-			return loader.load_module(fullname)
-		except ImportError:
-			# in this case return None, any other error throw to be handled elsewhere
-			return None
+		fullName = f"addons.{self.name}.{name}"
+		# If the given name contains dots (i.e. it is a submodule import),
+		# ensure the module at the top of the hierarchy is created correctly.
+		# After that, the import mechanism will be able to resolve the submodule automatically.
+		splitName = name.split('.')
+		fullNameTop = f"addons.{self.name}.{splitName[0]}"
+		if fullNameTop in sys.modules:
+			# The module can safely be imported, since the top level module is known.
+			return importlib.import_module(fullName)
+		# Ensure the new module is resolvable by the import system.
+		# For this, all packages in the tree have to be available in sys.modules.
+		# We add mock modules for the addons package and the addon itself.
+		# If we don't do this, namespace packages can't be imported correctly.
+		for parentName in ("addons", f"addons.{self.name}"):
+			if parentName in sys.modules:
+				# Parent package already initialized
+				continue
+			parentSpec = importlib._bootstrap.ModuleSpec(parentName, None, is_package=True)
+			parentModule = importlib.util.module_from_spec(parentSpec)
+			sys.modules[parentModule.__name__] = parentModule
+		spec = importlib.machinery.PathFinder.find_spec(fullNameTop, [self.path])
+		if not spec:
+			raise ModuleNotFoundError(f"No module named {name!r}", name=name)
+		mod = importlib.util.module_from_spec(spec)
+		sys.modules[fullNameTop] = mod
+		if spec.loader:
+			spec.loader.exec_module(mod)
+		return mod if fullNameTop == fullName else importlib.import_module(fullName)
 
 	def getTranslationsInstance(self, domain='nvda'):
 		""" Gets the gettext translation instance for this add-on.
@@ -511,7 +533,11 @@ def runInstallTask(self,taskName,*args,**kwargs):
 		in the add-on's installTasks module if it exists.
 		"""
 		if not hasattr(self,'_installTasksModule'):
-			self._installTasksModule=self.loadModule('installTasks')
+			try:
+				installTasksModule = self.loadModule('installTasks')
+			except ModuleNotFoundError:
+				installTasksModule = None
+			self._installTasksModule = installTasksModule
 		if self._installTasksModule:
 			func=getattr(self._installTasksModule,taskName,None)
 			if func:

source/addonHandler/packaging.py

@@ -1,5 +1,5 @@
 # A part of NonVisual Desktop Access (NVDA)
-# Copyright (C) 2009-2022 NV Access Limited, Rui Batista, Zahari Yurukov, Leonard de Ruijter
+# Copyright (C) 2009-2023 NV Access Limited, Rui Batista, Zahari Yurukov, Leonard de Ruijter
 # This file is covered by the GNU General Public License.
 # See the file COPYING for more details.
 
@@ -11,6 +11,7 @@
 from types import ModuleType
 import globalVars
 import config
+from keyword import iskeyword
 
 
 def initializeModulePackagePaths():
@@ -60,3 +61,16 @@ def addDirsToPythonPackagePath(module: ModuleType, subdir: Optional[str] = None)
 	pathList = [fullPath]
 	pathList.extend(module.__path__)
 	module.__path__ = pathList
+
+
+def isModuleName(name: str) -> bool:
+	"""When adding a module to sys.modules, it is important to check module name validity.
+	the L{str.isidentifier} method checks whether a string is a valid python identifier,
+	however this includes identifiers like 'def' and 'class', which are definitely invalid module names.
+	Therefore a valid module name should be an identifier but not a keyword.
+	A valid module name can also contain dots, but a dot is considered invalid in identifiers.
+	Therefore, use dot as a split separator and check all the name parts independently.
+	@param moduleName: De module name to check for naming conventions.
+	@returns: Whether the module name is valid.
+	"""
+	return all(n.isidentifier() and not iskeyword(n) for n in name.split("."))

user_docs/en/changes.t2t

@@ -134,6 +134,11 @@ To disable the braille handler programatically, register a handler to ``braille.
 - It is no longer possible to update the display size of the handler by setting ``braille.handler.displaySize``.
 To update the displaySize programatically, register a handler to ``braille.handler.filter_displaySize``.
 Refer to ``brailleViewer`` for an example on how to do this. (#14503)
+- There have been changes to the usage of ``addonHandler.Addon.loadModule``. (#14481)
+  - ``loadModule`` now expects dot as a separator, rather than backslash.
+  For example "lib.example" instead of "lib\example".
+  - ``loadModule`` now raises an exception when a module can't be loaded or has errors, instead of silently returning ``None`` without giving information about the cause.
+  -
 -