From 49eb53b44db0c91e3bf749a14f32ea1fc301634e Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 25 Sep 2018 10:31:41 -0500 Subject: [PATCH] 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 | 106 +++++++++++++++++++++++ hacking/deprecated_issue_template.md | 34 ++++++++ lib/ansible/plugins/loader.py | 2 +- test/sanity/pylint/config/default | 1 + test/sanity/pylint/plugins/deprecated.py | 97 +++++++++++++++++++++ 5 files changed, 239 insertions(+), 1 deletion(-) create mode 100644 hacking/create_deprecated_issues.py create mode 100644 hacking/deprecated_issue_template.md create mode 100644 test/sanity/pylint/plugins/deprecated.py diff --git a/hacking/create_deprecated_issues.py b/hacking/create_deprecated_issues.py new file mode 100644 index 00000000000..f5e3f309035 --- /dev/null +++ b/hacking/create_deprecated_issues.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# (c) 2017, Matt Martz +# +# 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 . + +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) diff --git a/hacking/deprecated_issue_template.md b/hacking/deprecated_issue_template.md new file mode 100644 index 00000000000..e319fba7947 --- /dev/null +++ b/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 diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py index 1c22b634d27..04f8f95440a 100644 --- a/lib/ansible/plugins/loader.py +++ b/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] diff --git a/test/sanity/pylint/config/default b/test/sanity/pylint/config/default index 70759d04f60..1f5e0f6117d 100644 --- a/test/sanity/pylint/config/default +++ b/test/sanity/pylint/config/default @@ -1,6 +1,7 @@ [MESSAGES CONTROL] disable= + ansible-deprecated-version, abstract-method, access-member-before-definition, arguments-differ, diff --git a/test/sanity/pylint/plugins/deprecated.py b/test/sanity/pylint/plugins/deprecated.py new file mode 100644 index 00000000000..c61392e61f2 --- /dev/null +++ b/test/sanity/pylint/plugins/deprecated.py @@ -0,0 +1,97 @@ +# (c) 2018, Matt Martz +# 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))