Skip to content

Commit

Permalink
Correctly match package/module names in import hook (#144)
Browse files Browse the repository at this point in the history
The previously used regular expression tried to handle both exact
matches and and prefix matches in one go, using this approach:

    re.compile(r'^%s\.?' % pkg)

However, this is incorrect, since the literal dot is optional in the
pattern, causing longer matches to also get included. For example, ‘foo’
should match ‘foo’ and ‘foo.bar’, but it also incorrectly matches
‘foobar’:

    >>> re.compile(r'^foo\.?').match('foobar')
    <_sre.SRE_Match object; span=(0, 3), match='foo'>

In practice, a command like this (using the pytest plugin as an example)
is supposed to check the ‘flask’ package and any modules below it:

    pytest --typeguard-packages=flask

... but in reality it also checks other packages, such as
‘flask_sqlalchemy’ and ‘flask_redis’, if those happen to be installed.

This can be easily fixed by not using regular expression, but simple
string matching instead.
  • Loading branch information
wbolster committed Aug 22, 2020
1 parent 01a6fc4 commit 55fbcb5
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
19 changes: 18 additions & 1 deletion tests/test_importhook.py
Expand Up @@ -6,7 +6,7 @@

import pytest

from typeguard.importhook import install_import_hook
from typeguard.importhook import TypeguardFinder, install_import_hook

this_dir = Path(__file__).parent
dummy_module_path = this_dir / 'dummymodule.py'
Expand Down Expand Up @@ -99,3 +99,20 @@ def test_inner_class_classmethod(dummymodule):
def test_inner_class_staticmethod(dummymodule):
retval = dummymodule.Outer.create_inner_staticmethod()
assert retval.__class__.__qualname__ == 'Outer.Inner'


def test_package_name_matching():
"""
The path finder only matches configured (sub)packages.
"""
packages = ["ham", "spam.eggs"]
dummy_original_pathfinder = None
finder = TypeguardFinder(packages, dummy_original_pathfinder)

assert finder.should_instrument("ham")
assert finder.should_instrument("ham.eggs")
assert finder.should_instrument("spam.eggs")

assert not finder.should_instrument("spam")
assert not finder.should_instrument("ha")
assert not finder.should_instrument("spam_eggs")
7 changes: 3 additions & 4 deletions typeguard/importhook.py
@@ -1,5 +1,4 @@
import ast
import re
import sys
from importlib.machinery import SourceFileLoader
from importlib.abc import MetaPathFinder
Expand Down Expand Up @@ -94,7 +93,7 @@ class TypeguardFinder(MetaPathFinder):
"""

def __init__(self, packages, original_pathfinder):
self._package_exprs = [re.compile(r'^%s\.?' % pkg) for pkg in packages]
self.packages = packages
self._original_pathfinder = original_pathfinder

def find_spec(self, fullname, path=None, target=None):
Expand All @@ -113,8 +112,8 @@ def should_instrument(self, module_name: str) -> bool:
:param module_name: full name of the module that is about to be imported (e.g. ``xyz.abc``)
"""
for expr in self._package_exprs:
if expr.match(module_name):
for package in self.packages:
if module_name == package or module_name.startswith(package + '.'):
return True

return False
Expand Down

0 comments on commit 55fbcb5

Please sign in to comment.