pylint plugin to catch due/past-due deprecated calls (#44143)

* Start of work on pylint plugin to catch due/past-due deprecated calls

* Improve deprecated pylint plugin

* Catch call to AnsibleModule.deprecate also

* Skip splatted kwargs, we can't infer that info

* Add error for invalid version in deprecation

* Skip version if it's a reference to a var

* Disable ansible-deprecated-no-version for displaying deprecated module info

* fix comments

* is None

* Force specifying a version, this can be disabled on a per case basis

* Disable ansible-deprecated-version by default

* Remove to look for 2.8 deprecated

* Revert "Remove to look for 2.8 deprecated"

This reverts commit 4e84034fd104879f429f0262ff0b2317e3d08deb.

* Add script and template used for creating issues for deprecated issues

* Fix underscore var

hacking/create_deprecated_issues.py

@@ -0,0 +1,106 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+# (c) 2017, Matt Martz <matt@sivel.net>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+import argparse
+import os
+import time
+
+from collections import defaultdict
+
+from ansible.release import __version__ as ansible_version
+
+ansible_major_version = '.'.join(ansible_version.split('.')[:2])
+
+try:
+    from github3 import GitHub
+except ImportError:
+    raise SystemExit(
+        'This script needs the github3.py library installed to work'
+    )
+
+if not os.getenv('GITHUB_TOKEN'):
+    raise SystemExit(
+        'Please set the GITHUB_TOKEN env var with your github oauth token'
+    )
+
+deprecated = defaultdict(list)
+
+
+parser = argparse.ArgumentParser()
+parser.add_argument('--template', default='deprecated_issue_template.md',
+                    type=argparse.FileType('r'),
+                    help='Path to markdown file template to be used for issue '
+                         'body. Default: %(default)s')
+parser.add_argument('problems', nargs=1, type=argparse.FileType('r'),
+                    help='Path to file containing pylint output for the '
+                         'ansible-deprecated-version check')
+args = parser.parse_args()
+
+
+body_tmpl = args.template.read()
+args.template.close()
+
+text = args.problems.read()
+args.problems.close()
+
+
+for line in text.splitlines():
+    path, line, column, msg = line.split(':')
+    msg = msg.strip()
+    if path.endswith('__init__.py'):
+        component = os.path.basename(os.path.dirname(path))
+    else:
+        component, ext_ = os.path.splitext(os.path.basename(path).lstrip('_'))
+
+    title = (
+        '%s contains deprecated call to be removed in %s' %
+        (component, ansible_major_version)
+    )
+    deprecated[component].append(
+        dict(title=title, msg=msg, path=path, line=line, column=column)
+    )
+
+
+g = GitHub(token=os.getenv('GITHUB_TOKEN'))
+repo = g.repository('ansible', 'ansible')
+
+# Not enabled by default, this fetches the column of a project,
+# so that we can later add the issue to a project column
+# You will need the project and column IDs for this to work
+# and then update the below lines
+# project = repo.project(1749241)
+# column = project.column(3314029)
+
+for component, items in deprecated.items():
+    title = items[0]['title']
+    msg = '\n'.join(i['msg'] for i in items)
+    path = '\n'.join(i['path'] for i in items)
+    body = body_tmpl % dict(component=component, msg=msg, path=path,
+                            line=line, column=column,
+                            version=ansible_major_version)
+
+    issue = repo.create_issue(title, body=body, labels=['deprecated'])
+    print(issue)
+    # Sleep a little, so that the API doesn't block us
+    time.sleep(0.5)
+    # Uncomment the next 2 lines if you want to add issues to a project
+    # Needs to be done in combination with the above code for selecting
+    # the project/column
+    # column.create_card_with_issue(issue)
+    # time.sleep(0.5)

hacking/deprecated_issue_template.md

@@ -0,0 +1,34 @@
+##### SUMMARY
+%(component)s contains call to Display.deprecated or AnsibleModule.deprecate and is scheduled for removal
+
+```
+%(path)s:%(line)s:%(column)s: %(msg)s
+```
+
+##### ISSUE TYPE
+ - Bug Report
+
+##### COMPONENT NAME
+```
+%(path)s
+```
+
+##### ANSIBLE VERSION
+```
+%(version)s
+```
+
+##### CONFIGURATION
+N/A
+
+##### OS / ENVIRONMENT
+N/A
+
+##### STEPS TO REPRODUCE
+N/A
+
+##### EXPECTED RESULTS
+N/A
+
+##### ACTUAL RESULTS
+N/A

lib/ansible/plugins/loader.py

@@ -315,7 +315,7 @@ class PluginLoader:
             if alias_name in pull_cache:
                 if not ignore_deprecated and not os.path.islink(pull_cache[alias_name]):
                     # FIXME: this is not always the case, some are just aliases
-                    display.deprecated('%s is kept for backwards compatibility but usage is discouraged. '
+                    display.deprecated('%s is kept for backwards compatibility but usage is discouraged. '  # pylint: disable=ansible-deprecated-no-version
                                        'The module documentation details page may explain more about this rationale.' % name.lstrip('_'))
                 return pull_cache[alias_name]
 

test/sanity/pylint/config/default

@@ -1,6 +1,7 @@
 [MESSAGES CONTROL]
 
 disable=
+    ansible-deprecated-version,
     abstract-method,
     access-member-before-definition,
     arguments-differ,

test/sanity/pylint/plugins/deprecated.py

@@ -0,0 +1,97 @@
+# (c) 2018, Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+# -*- coding: utf-8 -*-
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from distutils.version import StrictVersion
+
+import astroid
+
+from pylint.interfaces import IAstroidChecker
+from pylint.checkers import BaseChecker
+from pylint.checkers.utils import check_messages
+
+from ansible.release import __version__ as ansible_version_raw
+
+MSGS = {
+    'E9501': ("Deprecated version (%r) found in call to Display.deprecated "
+              "or AnsibleModule.deprecate",
+              "ansible-deprecated-version",
+              "Used when a call to Display.deprecated specifies a version "
+              "less than or equal to the current version of Ansible",
+              {'minversion': (2, 6)}),
+    'E9502': ("Display.deprecated call without a version",
+              "ansible-deprecated-no-version",
+              "Used when a call to Display.deprecated does not specify a "
+              "version",
+              {'minversion': (2, 6)}),
+    'E9503': ("Invalid deprecated version (%r) found in call to "
+              "Display.deprecated or AnsibleModule.deprecate",
+              "ansible-invalid-deprecated-version",
+              "Used when a call to Display.deprecated specifies an invalid "
+              "version number",
+              {'minversion': (2, 6)}),
+}
+
+
+ANSIBLE_VERSION = StrictVersion('.'.join(ansible_version_raw.split('.')[:3]))
+
+
+def _get_expr_name(node):
+    """Funciton to get either ``attrname`` or ``name`` from ``node.func.expr``
+
+    Created specifically for the case of ``display.deprecated`` or ``self._display.deprecated``
+    """
+    try:
+        return node.func.expr.attrname
+    except AttributeError:
+        # If this fails too, we'll let it raise, the caller should catch it
+        return node.func.expr.name
+
+
+class AnsibleDeprecatedChecker(BaseChecker):
+    """Checks for Display.deprecated calls to ensure that the ``version``
+    has not passed or met the time for removal
+    """
+
+    __implements__ = (IAstroidChecker,)
+    name = 'deprecated'
+    msgs = MSGS
+
+    @check_messages(*(MSGS.keys()))
+    def visit_call(self, node):
+        version = None
+        try:
+            if (node.func.attrname == 'deprecated' and 'display' in _get_expr_name(node) or
+                    node.func.attrname == 'deprecate' and 'module' in _get_expr_name(node)):
+                if node.keywords:
+                    for keyword in node.keywords:
+                        if len(node.keywords) == 1 and keyword.arg is None:
+                            # This is likely a **kwargs splat
+                            return
+                        elif keyword.arg == 'version':
+                            if isinstance(keyword.value.value, astroid.Name):
+                                # This is likely a variable
+                                return
+                            version = keyword.value.value
+                if not version:
+                    try:
+                        version = node.args[1].value
+                    except IndexError:
+                        self.add_message('ansible-deprecated-no-version', node=node)
+                        return
+
+                try:
+                    if ANSIBLE_VERSION >= StrictVersion(str(version)):
+                        self.add_message('ansible-deprecated-version', node=node, args=(version,))
+                except ValueError:
+                    self.add_message('ansible-invalid-deprecated-version', node=node, args=(version,))
+        except AttributeError:
+            # Not the type of node we are interested in
+            pass
+
+
+def register(linter):
+    """required method to auto register this checker """
+    linter.register_checker(AnsibleDeprecatedChecker(linter))