Skip to content

Custom methods must be protected #127

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

Open
wants to merge 7 commits 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
29 changes: 28 additions & 1 deletion hooks/conan-center.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@
import sys
from logging import WARNING, ERROR, INFO, DEBUG, NOTSET


from conans import tools, ConanFile
import yaml
from conans import tools
try:
from conans import Settings
except ImportError:
Expand Down Expand Up @@ -46,6 +47,7 @@
"KB-H031": "CONANDATA.YML REDUCE",
"KB-H032": "SYSTEM REQUIREMENTS",
"KB-H034": "TEST PACKAGE - NO IMPORTS()",
"KB-H036": "CUSTOM METHODS",
"KB-H037": "NO AUTHOR",
"KB-H040": "NO TARGET NAME",
"KB-H041": "NO FINAL ENDLINE",
Expand Down Expand Up @@ -421,6 +423,31 @@ def test(out):
if "def imports" in test_package_conanfile:
out.error("The method `imports` is not allowed in test_package/conanfile.py")

@run_test("KB-H036", output)
def test(out):
def get_methods(conanfile):
methods = []
for member in dir(conanfile):

try:
if callable(getattr(conanfile, member)):
methods.append(str(member))
except:
methods.append(str(member))
return methods

mock = ConanFile(conanfile.output, None)
valid_attrs = get_methods(mock)
current_attrs = get_methods(conanfile)
invalid_attrs = re.findall(r"def (__.*[^_])\s?\(", conanfile_content, re.MULTILINE)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get it, you query valid attrs and current attrs, why query invalid attrs with different method?
isn't result the same as current_attrs - valid_attrs?

Copy link
Member Author

Choose a reason for hiding this comment

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

private methods are not filtered by dir method. e.g. def __foo(self):

for attr in current_attrs:
if not attr.startswith("_") and attr not in valid_attrs:
invalid_attrs.append(attr)

if invalid_attrs:
out.error("Custom methods must be declared as protected. " \
"The follow methods are invalid: '{}'".format("', '".join(invalid_attrs)))

@run_test("KB-H037", output)
def test(out):
author = getattr(conanfile, "author", None)
Expand Down
39 changes: 39 additions & 0 deletions tests/test_hooks/conan-center/test_conan-center.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ def test_conanfile(self):
self.assertIn("ERROR: [CONAN CENTER INDEX URL (KB-H027)] The attribute 'url' should " \
"point to: https://github.com/conan-io/conan-center-index", output)
self.assertIn("[CMAKE MINIMUM VERSION (KB-H028)] OK", output)
self.assertIn("[CUSTOM METHODS (KB-H036)] OK", output)
self.assertIn("[SYSTEM REQUIREMENTS (KB-H032)] OK", output)
self.assertIn("[SINGLE REQUIRES (KB-H055)] OK", output)

Expand All @@ -141,6 +142,7 @@ def test_conanfile_header_only(self):
"recipe", output)
self.assertIn("[META LINES (KB-H025)] OK", output)
self.assertIn("[CMAKE MINIMUM VERSION (KB-H028)] OK", output)
self.assertIn("[CUSTOM METHODS (KB-H036)] OK", output)
self.assertIn("[SYSTEM REQUIREMENTS (KB-H032)] OK", output)

def test_conanfile_header_only_with_settings(self):
Expand All @@ -162,6 +164,7 @@ def test_conanfile_header_only_with_settings(self):
"recipe", output)
self.assertIn("[META LINES (KB-H025)] OK", output)
self.assertIn("[CMAKE MINIMUM VERSION (KB-H028)] OK", output)
self.assertIn("[CUSTOM METHODS (KB-H036)] OK", output)
self.assertIn("[SYSTEM REQUIREMENTS (KB-H032)] OK", output)

def test_conanfile_settings_clear_with_settings(self):
Expand Down Expand Up @@ -204,6 +207,7 @@ def test_conanfile_installer(self):
"recipe", output)
self.assertIn("[META LINES (KB-H025)] OK", output)
self.assertIn("[CMAKE MINIMUM VERSION (KB-H028)] OK", output)
self.assertIn("[CUSTOM METHODS (KB-H036)] OK", output)

def test_shebang(self):
conanfile = textwrap.dedent("""\
Expand Down Expand Up @@ -604,6 +608,41 @@ def system_requirements(self):
self.assertIn("[SYSTEM REQUIREMENTS (KB-H032)] 'libusb' is part of the allowlist.", output)
self.assertNotIn("ERROR: [SYSTEM REQUIREMENTS (KB-H032)]", output)

def test_invalid_recipe_methods(self):
conanfile = textwrap.dedent("""\
from conans import ConanFile
class AConan(ConanFile):
url = "fake_url.com"
license = "fake_license"
description = "whatever"
homepage = "homepage.com"
topics = ("fake_topic", "another_fake_topic")

def configure(self):
self.output.info("ok")

def barbarian(self):
self.output.info("Conan")

def __my_own_method(self):
self.output.info("foobar")

def __my_private_method(self):
self.output.info("foobar")

def __abs__(self):
return self.version

def _baz(self):
self.output.info("qux")

""")
tools.save('conanfile.py', content=conanfile)
output = self.conan(['create', '.', 'name/version@user/test'])
self.assertIn("ERROR: [CUSTOM METHODS (KB-H036)] Custom methods must be declared as "
Copy link
Contributor

Choose a reason for hiding this comment

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

might be too strict, as we may define some magic methods (__str__, __eq__, __new__, __init__, __enter__, __dir__, etc)?

Copy link
Member Author

Choose a reason for hiding this comment

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

but it's the exception you know. It sounds like cccl and other few recipes. We know that is not a regular case, but new contributors can use private instead of protected when writing a recipe, for instance.

"protected. The follow methods are invalid: '__my_own_method', "
"'__my_private_method', 'barbarian'", output)

def test_imports_not_allowed(self):
conanfile_tp = textwrap.dedent("""\
from conans import ConanFile, tools
Expand Down