Use importlib.util.find_spec when checking if the given appModule exists to workaround issues in find_module and for forward compatibility.

[lukaszgo1]

Link to issue number:

None, discussion in PR #13814

Summary of the issue:

As mentioned in #13814 (comment) FileFinder.find_module is deprecated. find_spec is the recommended replacement method.
As noted in #13814 (comment), using either find_spec or find_module with pkgutil.iter_importers results in paths containing "." being incorrectly treated as python packages, causing #13813

Description of development approach

Description of user facing changes

No user facing change

Description of development approach

When checking if the given appModule exists, instead use importlib.util.find_spec, which does not have the same problematic behaviour as pkgutil.iter_importers.

Testing strategy:

• Made sure that Regression: Attempt to import wrong appModule #13813 has not been reintroduced
• Made sure that alias appModules are not imported
• Made sure that right module is imported for Firefox and Notepad++

Known issues with pull request:

None known

Change log entries:

None needed - internal change

Code Review Checklist:

• Pull Request description:
  • description is up to date
  • change log entries
• Testing:
  • Unit tests
  • System (end to end) tests
  • Manual testing
• API is compatible with existing add-ons.
• Documentation:
  • User Documentation
  • Developer / Technical Documentation
  • Context sensitive help for GUI changes
• UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English

[seanbudd]

@lukaszgo1 - the PR description is misleading, notably there is no bug with Python. Suggested changes:

Summary of the issue:

As mentioned in #13814 (comment) FileFinder.find_module is deprecated. find_spec is the recommended replacement method.
As noted in #13814 (comment), using either find_spec or find_module with pkgutil.iter_importers results in paths containing "." being incorrectly treated as python packages, causing #13813

Description of development approach

When checking if the given appModule exists, instead use importlib.util.find_spec, which does not have the same problematic behaviour as pkgutil.iter_importers.

[seanbudd]

Should this PR also remove the introduced change in #13814, which added the replacement of periods to underscores: . to _?

https://github.com/nvaccess/nvda/blob/7e85396/source/appModuleHandler.py#L159-L164

[seanbudd]

Could you also consider changing the other usage of the deprecated find_module?

nvda/source/addonHandler/__init__.py

Line 486 in ca41d22

loader = importer.find_module(name)

[lukaszgo1]

the PR description is misleading, notably there is no bug with Python.

I'm not sure I agree - if the method responsible for checking if the given module exists returns True for nonexistent module that is a bug to me.
To quote from the documentation:

If fullname contains a '.', the finders will be for the package
containing fullname, ...

In our case the finders should be for the package appModules. To use the example from #13813 since appModules has no sub-package 6 the fact that find_spec returns True seems to be a bug to me.

Should this PR also remove the introduced change in #13814, which added the replacement of periods to underscores: . to _?

No, it should not. This change was introduced in #5323 and some add-ons may rely on it. We should not try to deprecate this behavior as that would make it unnecessarily difficult for add-on authors to target both old and new releases of NVDA. #13814 just moved the code to a different (more appropriate IMHO) place. It also has additional benefit namely that add-on authors can add app modules for hosting processes whose binaries contain dots in their names.

Could you also consider changing the other usage of the deprecated find_module?

I'd rather do this in a follow up. addonHandler uses pkgutil.ImpImporter which is also deprecated so that would be much bigger change.

[seanbudd]

I'd argue that the behaviour is documented in pkgutil.iter_importers, and the consumer find_module/find_spec is not at fault.

Without a confirmed/reported python bug, I'd rather be agnostic in the language here.
The description I suggested covers what the problematic behaviour is, whether or not it is intended/documented.
It also fixes up the references to the description of the issue - which isn't explained in #13813.

To avoid bike shedding this PR, I'm going to update the description and merge as-is.

Feel free to update / edit it further.

[seanbudd]

note I have added that this behaviour is incorrect.

As noted in #13814 (comment), using either find_spec or find_module with pkgutil.iter_importers results in paths containing "." being incorrectly treated as python packages, causing #13813