Add hook: recipe linting

README.md

@@ -8,7 +8,15 @@
 
 Repository to develop **experimental** [Conan](https://conan.io) hooks for Conan >= 1.8.
 
-**WARNING**: Hooks were originally named "Plugins"
+ * [Conan Center](#conan-center)
+ * [Attribute checker](#attribute-checker)
+ * [Bintray updater](#bintray-updater)
+ * [Binary linter](#binary-linter)
+ * [Github updater](#github-updater)
+ * [Member typo checker](#members-typo-checker)
+ * [SPDX checker](#spdx-checker)
+ * [Recipe linter](#recipe-linter)
+
 
 ## Hook setup
 
@@ -160,6 +168,21 @@ Use `pip install spdx_lookup` in order to install required dependency.
 
 The hook is automatically called when *export* command is executed.
 
+### [Recipe linter](hooks/recipe_linter.py)
+
+This hook runs [Pylint](https://www.pylint.org/) over the recipes before exporting
+them (it runs in the `pre_export` hook), it can be really useful to check for
+typos, code flaws or company standards.
+
+There several environment variables you can use to configure it:
+ * `CONAN_PYLINTRC`: path to a configuration file to fully customize Pylint behavior.
+ * `CONAN_PYLINT_WERR`: if set, linting errors will trigger a `ConanException`.
+ * `CONAN_PYLINT_RECIPE_PLUGINS`: list of modules (comma separated list) to load. They are used to register additional checker or dynamic fields before running
+ the linter. By default it points to the `conans.pylint_plugin` module distributed
+ together with Conan, this file contains the declaration of some extra fields that are valid in the `ConanFile` class.
+
+This hook requires additional dependencies to work: `pip install pylint astroid`.
+
 ## License
 
 [MIT License](LICENSE)

hooks/recipe_linter.py

@@ -0,0 +1,81 @@
+# coding=utf-8
+
+import json
+import os
+import platform
+import subprocess
+import sys
+import re
+
+from conans.errors import ConanException
+from conans.tools import logger
+
+try:
+    import astroid  # Conan 'pylint_plugin.py' uses astroid
+    from pylint import epylint as lint
+except ImportError as e:
+    sys.stderr.write("Install pylint to use 'recipe_linter' hook: 'pip install pylint astroid'")
+    sys.exit(1)
+
+
+CONAN_HOOK_PYLINT_RCFILE = "CONAN_PYLINTRC"
+CONAN_HOOK_PYLINT_WERR = "CONAN_PYLINT_WERR"
+CONAN_HOOK_PYLINT_RECIPE_PLUGINS = "CONAN_PYLINT_RECIPE_PLUGINS"
+
+
+def pre_export(output, conanfile_path, *args, **kwargs):
+    output.info("Lint recipe '{}'".format(conanfile_path))
+    conanfile_dirname = os.path.dirname(conanfile_path)
+
+    lint_args = ['--output-format=json',  # JSON output fails in Windows (parsing)
+                 '--py3k',
+                 '--enable=all',
+                 '--reports=no',
+                 '--disable=no-absolute-import',
+                 '--persistent=no',
+                 # These were disabled in linter that was inside Conan
+                 # '--disable=W0702',  # No exception type(s) specified (bare-except)
+                 # '--disable=W0703',  # Catching too general exception Exception (broad-except)
+                 '--init-hook="import sys;sys.path.extend([\'{}\',])"'.format(conanfile_dirname.replace('\\', '\\\\'))
+                 ]
+
+    pylint_plugins = os.getenv(CONAN_HOOK_PYLINT_RECIPE_PLUGINS, 'conans.pylint_plugin')
+    if pylint_plugins:
+        lint_args += ['--load-plugins={}'.format(pylint_plugins)]
+
+    rc_file = os.getenv(CONAN_HOOK_PYLINT_RCFILE)
+    if rc_file:
+        lint_args += ['--rcfile', rc_file]
+
+    try:
+        command = ['pylint'] + lint_args + ['"{}"'.format(conanfile_path).replace('\\', '\\\\')]
+        command = " ".join(command)
+        shell = bool(platform.system() != "Windows")
+        p = subprocess.Popen(command, shell=shell, bufsize=10,
+                             stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        pylint_stdout, pylint_stderr = p.communicate()
+        # Remove ANSI escape sequences
+        ansi_escape = re.compile(r'\x1B\[[0-?]*[ -/]*[@-~]')
+        pylint_stdout = ansi_escape.sub('', pylint_stdout.decode('utf-8'))
+    except Exception as exc:
+        output.error("Unexpected error running linter: {}".format(exc))
+    else:
+        try:
+            messages = json.loads(pylint_stdout)
+        except Exception as exc:
+            output.error("Error parsing JSON output: {}".format(exc))
+            logger.error(
+                "Error parsing linter output for recipe '{}': {}".format(conanfile_path, exc))
+            logger.error(" - linter arguments: {}".format(lint_args))
+            logger.error(" - output: {}".format(pylint_stdout.getvalue()))
+            logger.error(" - stderr: {}".format(pylint_stderr.getvalue()))
+        else:
+            errors = 0
+            for msg in messages:
+                line = "{path}:{line}:{column}: {message-id}: {message} ({symbol})".format(**msg)
+                output.info(line)
+                errors += int(msg["type"] == "error")
+
+            output.info("Linter detected '{}' errors".format(errors))
+            if os.getenv(CONAN_HOOK_PYLINT_WERR) and errors:
+                raise ConanException("Package recipe has linter errors. Please fix them.")

tests/requirements_test.txt

@@ -2,3 +2,5 @@ pytest>=3.6
 parameterized
 responses
 pluggy==0.11.0
+pylint
+astroid

tests/test_hooks/test_recipe_linter.py

@@ -0,0 +1,210 @@
+# coding=utf-8
+
+import os
+import textwrap
+import unittest
+
+import six
+from packaging import version
+from parameterized import parameterized
+
+from conans import __version__ as conan_version
+from conans import tools
+from conans.client.command import ERROR_GENERAL, SUCCESS
+from conans.tools import environment_append
+from tests.utils.test_cases.conan_client import ConanClientTestCase
+
+
+class RecipeLinterTests(ConanClientTestCase):
+    conanfile = textwrap.dedent(r"""
+        from conans import ConanFile, tools
+
+        class TestConan(ConanFile):
+            name = "name"
+            version = "version"
+
+            def build(self):
+                print("Hello world")    
+                for k, v in {}.iteritems():
+                    pass
+                tools.msvc_build_command(self.settings, "path")
+        """)
+
+    def _get_environ(self, **kwargs):
+        kwargs = super(RecipeLinterTests, self)._get_environ(**kwargs)
+        kwargs.update({'CONAN_HOOKS': os.path.join(os.path.dirname(
+            __file__), '..', '..', 'hooks', 'recipe_linter')})
+        return kwargs
+
+    @parameterized.expand([(False, ), (True, )])
+    def test_basic(self, pylint_werr):
+        tools.save('conanfile.py', content=self.conanfile)
+        pylint_werr_value = "1" if pylint_werr else None
+        with environment_append({"CONAN_PYLINT_WERR": pylint_werr_value}):
+            return_code = ERROR_GENERAL if pylint_werr else SUCCESS
+            output = self.conan(['export', '.', 'name/version@'], expected_return_code=return_code)
+
+            if pylint_werr:
+                self.assertIn("pre_export(): Package recipe has linter errors."
+                              " Please fix them.", output)
+
+            if six.PY2:
+                self.assertIn("pre_export(): conanfile.py:9:8:"
+                              " E1601: print statement used (print-statement)", output)
+            else:
+                self.assertIn("pre_export(): conanfile.py:10:20:"
+                              " E1101: Instance of 'dict' has no 'iteritems' member (no-member)",
+                              output)
+
+            self.assertIn("pre_export(): conanfile.py:10:20:"
+                          " W1620: Calling a dict.iter*() method (dict-iter-method)", output)
+            self.assertIn("pre_export(): conanfile.py:10:12:"
+                          " W0612: Unused variable 'k' (unused-variable)", output)
+            self.assertIn("pre_export(): conanfile.py:10:15:"
+                          " W0612: Unused variable 'v' (unused-variable)", output)
+
+    def test_path_with_spaces(self):
+        conanfile = textwrap.dedent(r"""
+            from conans import ConanFile
+
+            class Recipe(ConanFile):
+                def build(self):
+                    pass
+        """)
+        tools.save(os.path.join("path spaces", "conanfile.py"), content=conanfile)
+        output = self.conan(['export', 'path spaces/conanfile.py', 'name/version@'])
+        recipe_path = os.path.join(os.getcwd(), "path spaces", "conanfile.py")
+        self.assertIn("pre_export(): Lint recipe '{}'".format(recipe_path), output)
+        self.assertIn("pre_export(): Linter detected '0' errors", output)
+
+    def test_custom_rcfile(self):
+        tools.save('conanfile.py', content=self.conanfile)
+        tools.save('pylintrc', content="[FORMAT]\nindent-string='  '")
+
+        with environment_append({"CONAN_PYLINTRC": os.path.join(os.getcwd(), "pylintrc")}):
+            output = self.conan(['export', '.', 'name/version@'])
+        self.assertIn("pre_export(): conanfile.py:5:0: "
+                      "W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)", output)
+
+    def test_custom_plugin(self):
+        conanfile = textwrap.dedent(r"""
+            from conans import ConanFile
+
+            class Recipe(ConanFile):
+                def build(self):
+                    self.output.info(self.source_folder)
+        """)
+        tools.save('conanfile.py', content=conanfile)
+        with environment_append({"CONAN_PYLINT_WERR": "1"}):
+            # With the default 'python_plugin' it doesn't raise
+            with environment_append({"CONAN_PYLINT_RECIPE_PLUGINS": None}):
+                output = self.conan(['export', '.', 'consumer/version@'])
+                self.assertIn("pre_export(): Lint recipe", output)  # Hook run without errors
+                self.assertIn("pre_export(): Linter detected '0' errors", output)
+
+            # With a custom one, it should fail
+            tools.save("plugin_empty.py", content="def register(_):\n\tpass")
+            with environment_append({"CONAN_PYLINT_RECIPE_PLUGINS": "plugin_empty"}):
+                output = self.conan(['export', '.', 'consumer/other@'], expected_return_code=ERROR_GENERAL)
+                self.assertIn("pre_export(): Package recipe has linter errors."
+                              " Please fix them.", output)
+
+    def test_dynamic_fields(self):
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+
+            class TestConan(ConanFile):
+                name = "consumer"
+                version = "version"
+
+                def build(self):
+                    self.output.info(self.source_folder)
+                    self.output.info(self.package_folder)
+                    self.output.info(self.build_folder)
+                    self.output.info(self.install_folder)
+
+                def package(self):
+                    self.copy("*")
+
+                def package_id(self):
+                    self.info.header_only()
+
+                def build_id(self):
+                    self.output.info(str(self.info_build))
+
+                def build_requirements(self):
+                    self.build_requires("name/version")
+
+                def requirements(self):
+                    self.requires("name/version")
+
+                def deploy(self):
+                    self.copy_deps("*.dll")
+            """)
+        tools.save('consumer.py', content=conanfile)
+        with environment_append({"CONAN_PYLINT_WERR": "1"}):
+            output = self.conan(['export', 'consumer.py', 'consumer/version@'])
+            self.assertIn("pre_export(): Lint recipe", output)  # Hook run without errors
+            self.assertIn("pre_export(): Linter detected '0' errors", output)
+            self.assertNotIn("(no-member)", output)
+
+    def test_catch_them_all(self):
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+            class BaseConan(ConanFile):
+
+                def source(self):
+                    try:
+                        raise Exception("Pikaaaaa!!")
+                    except:
+                        pass
+                    try:
+                        raise Exception("Pikaaaaa!!")
+                    except Exception:
+                        pass
+            """)
+
+        tools.save('conanfile.py', content=conanfile)
+        with environment_append({"CONAN_PYLINT_WERR": "1"}):
+            output = self.conan(['export', '.', 'consumer/version@'])
+            self.assertIn("pre_export(): Lint recipe", output)  # Hook run without errors
+            self.assertIn("pre_export(): Linter detected '0' errors", output)
+            self.assertNotIn("no-member", output)
+
+    def test_conan_data(self):
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile
+
+            class ExampleConan(ConanFile):
+
+                def build(self):
+                    _ = self.conan_data["sources"][float(self.version)]
+            """)
+        tools.save('conanfile.py', content=conanfile)
+        with environment_append({"CONAN_PYLINT_WERR": "1"}):
+            output = self.conan(['export', '.', 'consumer/version@'])
+            self.assertIn("pre_export(): Lint recipe", output)  # Hook run without errors
+            self.assertIn("pre_export(): Linter detected '0' errors", output)
+            self.assertNotIn("no-member", output)
+
+    @unittest.skipUnless(version.parse(conan_version) >= version.parse("1.21.0"), "Need python_version")
+    def test_python_requires(self):
+        """ python_requires were not added to the 'pylint_plugin' until 1.21 """
+        conanfile = textwrap.dedent("""
+            from conans import ConanFile, python_requires
+
+            base = python_requires("name/version")
+
+            class TestConan(ConanFile):
+                name = "consumer"
+                version = "version"
+            """)
+        tools.save('require.py', self.conanfile)
+        self.conan(['export', 'require.py', 'name/version@'])
+
+        tools.save('consumer.py', content=conanfile)
+        with environment_append({"CONAN_PYLINT_WERR": "1"}):
+            output = self.conan(['export', 'consumer.py', 'consumer/version@'])
+            self.assertIn("pre_export(): Lint recipe", output)  # Hook run without errors
+            self.assertIn("pre_export(): Linter detected '0' errors", output)
+            self.assertNotIn("(no-name-in-module)", output)