Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: check for both path/foo.py and path/foo/__init__.py being present #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/check_wheel_contents/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,29 @@ def check_W010(self, contents: WheelContents) -> list[FailedCheck]:
else:
return []

def check_W011(self, contents: WheelContents) -> list[FailedCheck]:
"""
W011 — Wheel contains module and submodule with the same name

E.g: checks for both `path/foo.py` and `path/foo/__init__.py` being present
"""
conflicts: list[str] = []

def check_dir(tree: Directory) -> None:
for name, entry in tree.entries.items():
if isinstance(entry, Directory):
Comment on lines +311 to +312
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for name, entry in tree.entries.items():
if isinstance(entry, Directory):
for name, entry in tree.subdirectories.items():

if tree.files.get(name + ".py") is not None:
conflicts.append(entry.path or name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning a single failed check that just contains the conflicting directory paths, I feel you should instead return one FailedCheck for each foo.py + foo/__init__.py pair, and the FailedCheck should contains both of those paths.

Cf. the following paragraph from "Adding a New Check" in CONTRIBUTING.md:

Generally, a check method should return at most one FailedCheck containing all of the failing filepaths; only return multiple FailedChecks if the check applies to groups of files. For example, W002 ("Wheel contains duplicate files") returns a separate FailedCheck for each group of equal files.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
conflicts.append(entry.path or name)
# `entry` exists within a directory and thus is not the root directory, and so it has a path.
assert entry.path is not None
conflicts.append(entry.path)

check_dir(entry)

for tree in (contents.purelib_tree, contents.platlib_tree):
if isinstance(tree, Directory):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is redundant; contents.purelib_tree and contents.platlib_tree are always Directorys.

check_dir(tree)

if conflicts:
return [FailedCheck(Check.W011, conflicts)]
return []

def check_W101(self, contents: WheelContents) -> list[FailedCheck]:
"""
W101 — Wheel library is missing files in package tree
Expand Down
1 change: 1 addition & 0 deletions src/check_wheel_contents/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class Check(Enum):
W008 = "Wheel is empty"
W009 = "Wheel contains multiple toplevel library entries"
W010 = "Toplevel library directory contains no Python modules"
W011 = "Wheel contains module and submodule with the same name"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
W011 = "Wheel contains module and submodule with the same name"
W011 = "Wheel contains module and package with the same basename"

W101 = "Wheel library is missing files in package tree"
W102 = "Wheel library contains files not in package tree"
W201 = "Wheel library is missing specified toplevel entry"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"stdout": "module_file_folder_shadow-0.1.0-py3-none-any.whl: W011: Wheel library has multiple declared toplevel entries:\n module_file_folder_shadow/foo/\n module_file_folder_shadow/foo/bar/",
"stderr": "",
"rc": 1
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing with actual wheels is only for special cases, and there generally isn't a need to add more test wheels at this time. Instead, the new check should be tested with a test function in test/test_checking.py that just operates on lists of filepaths; cf. the tests already in that file and the "Adding a New Check" section of CONTRIBUTING.md.

Please remove this wheel from your PR's history before requesting a re-review.

Binary file not shown.