Check api compatible (#34119)

* print the signatures of the apis

* some faults, and a test case

* use the FullArgSpec as the ArgSpec, repr>str

为了延续脚本历史，把FullArgsSpec替换为ArgSpec；在逆转回对象时，记得加上inpect.FullArgSpec对象

* script form @zhiboniu

* parsearg

* read argspec from file

* add comments

* refactor check_compatible and its testcases

* skip the empty ArgSepc()

* update the function, get inspect.FullArgSpec instances directly.

* typo

* for the functions using inner-defined VarType or other Class Type not accessable, use the string comparements.

* typo

* logging added

* 若变量和默认值都增加了，用负数索引需要再处理下这个增加的内容

* defaults itself may be NoneType

* defaults added 2 items when args only added 1 item

* same as the previous comment

* enable the check_api_compatible.py

* run test_check_api_compatible.py

* sort the output of print_signatures.py

tools/check_api_approvals.sh

@@ -37,8 +37,9 @@ function add_failed(){
 }
 
 
+api_params_diff=`python ${PADDLE_ROOT}/tools/check_api_compatible.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec  ${PADDLE_ROOT}/paddle/fluid/API_PR.spec` 
 api_spec_diff=`python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec.api  ${PADDLE_ROOT}/paddle/fluid/API_PR.spec.api` 
-if [ "$api_spec_diff" != "" ]; then
+if [ "$api_spec_diff" != "" -o "${api_params_diff}" != "" ]; then
     echo_line="You must have one RD (XiaoguangHu01 or lanxianghit) approval for API change.\n"
     echo_line="${echo_line} and one TPM approval for API change: \n"
     echo_line="${echo_line} jzhang533/ZhangJun, dingjiaweiww/DingJiaWei, Heeenrrry/LiKunLun, TCChenlong/ChenLong for general APIs\n"
@@ -87,6 +88,7 @@ if [ "${ADDED_OP_USE_DEFAULT_GRAD_MAKER}" != "" ]; then
   check_approval 1 6888866 7913861
 fi
 
+
 if [ -n "${echo_list}" ];then
   echo "****************"
   echo -e "${echo_list[@]}"
@@ -97,6 +99,9 @@ if [ -n "${echo_list}" ];then
   if [ "${api_spec_diff}" != "" -o "${api_doc_spec_diff}" != "" ] ; then
     python ${PADDLE_ROOT}/tools/diff_api.py ${PADDLE_ROOT}/paddle/fluid/API_DEV.spec  ${PADDLE_ROOT}/paddle/fluid/API_PR.spec
   fi
+  if [ "${api_params_diff}" != "" ] ; then
+    echo "api_params_diff: ${api_params_diff}"
+  fi 
   if [ "${op_type_spec_diff}" != "" ] ; then
     echo "op_type_spec_diff: ${op_type_spec_diff}"
   fi 

tools/check_api_compatible.py

+# Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.
+# 
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import argparse
+import inspect
+import sys
+import re
+import logging
+
+logger = logging.getLogger()
+if logger.handlers:
+    # we assume the first handler is the one we want to configure
+    console = logger.handlers[0]
+else:
+    console = logging.StreamHandler(sys.stderr)
+    logger.addHandler(console)
+console.setFormatter(
+    logging.Formatter(
+        "%(asctime)s - %(funcName)s:%(lineno)d - %(levelname)s - %(message)s"))
+
+
+def _check_compatible(args_o, args_n, defaults_o, defaults_n):
+    # 如果参数减少了，需要提醒关注
+    if len(args_o) > len(args_n):
+        logger.debug("args num less then previous: %s vs %s", args_o, args_n)
+        return False
+    # 参数改名了，也要提醒关注
+    for idx in range(min(len(args_o), len(args_n))):
+        if args_o[idx] != args_n[idx]:
+            logger.debug("args's %d parameter diff with previous: %s vs %s",
+                         idx, args_o, args_n)
+            return False
+    # 新增加了参数，必须提供默认值。以及不能减少默认值数量
+    if (len(args_n) - len(defaults_n)) > (len(args_o) - len(defaults_o)):
+        logger.debug("defaults num less then previous: %s vs %s", defaults_o,
+                     defaults_n)
+        return False
+    # 默认值必须相等
+    for idx in range(min(len(defaults_o), len(defaults_n))):
+        nidx_o = -1 - idx
+        nidx_n = -1 - idx - (len(args_n) - len(args_o))
+        if (defaults_o[nidx_o] != defaults_n[nidx_n]):
+            logger.debug("defaults's %d value diff with previous: %s vs %s",
+                         nidx_n, defaults_o, defaults_n)
+            return False
+    return True
+
+
+def check_compatible(old_api_spec, new_api_spec):
+    """
+    check compatible, FullArgSpec
+    """
+    if not (isinstance(old_api_spec, inspect.FullArgSpec) and isinstance(
+            new_api_spec, inspect.FullArgSpec)):
+        logger.warning(
+            "new_api_spec or old_api_spec is not instance of inspect.FullArgSpec"
+        )
+        return False
+    return _check_compatible(
+        old_api_spec.args, new_api_spec.args, []
+        if old_api_spec.defaults is None else old_api_spec.defaults, []
+        if new_api_spec.defaults is None else new_api_spec.defaults)
+
+
+def check_compatible_str(old_api_spec_str, new_api_spec_str):
+    patArgSpec = re.compile(
+        r'args=(.*), varargs=.*defaults=\((.*)\), kwonlyargs=.*')
+    mo_o = patArgSpec.search(old_api_spec_str)
+    mo_n = patArgSpec.search(new_api_spec_str)
+    if not (mo_o and mo_n):
+        # error
+        logger.warning("old_api_spec_str: %s", old_api_spec_str)
+        logger.warning("new_api_spec_str: %s", new_api_spec_str)
+        return False
+
+    args_o = eval(mo_o.group(1))
+    args_n = eval(mo_n.group(1))
+    defaults_o = mo_o.group(2).split(', ')
+    defaults_n = mo_n.group(2).split(', ')
+    return _check_compatible(args_o, args_n, defaults_o, defaults_n)
+
+
+def read_argspec_from_file(specfile):
+    """
+    read FullArgSpec from spec file
+    """
+    res_dict = {}
+    patArgSpec = re.compile(
+        r'^(paddle[^,]+)\s+\((ArgSpec.*),\s\(\'document\W*([0-9a-z]{32})')
+    fullargspec_prefix = 'inspect.Full'
+    for line in specfile.readlines():
+        mo = patArgSpec.search(line)
+        if mo and mo.group(2) != 'ArgSpec()':
+            logger.debug("%s argspec: %s", mo.group(1), mo.group(2))
+            try:
+                res_dict[mo.group(1)] = eval(fullargspec_prefix + mo.group(2))
+            except:  # SyntaxError, NameError:
+                res_dict[mo.group(1)] = fullargspec_prefix + mo.group(2)
+    return res_dict
+
+
+arguments = [
+    # flags, dest, type, default, help
+]
+
+
+def parse_args():
+    """
+    Parse input arguments
+    """
+    global arguments
+    parser = argparse.ArgumentParser(
+        description='check api compatible across versions')
+    parser.add_argument('--debug', dest='debug', action="store_true")
+    parser.add_argument(
+        'prev',
+        type=argparse.FileType('r'),
+        help='the previous version (the version from develop branch)')
+    parser.add_argument(
+        'post',
+        type=argparse.FileType('r'),
+        help='the post version (the version from PullRequest)')
+    for item in arguments:
+        parser.add_argument(
+            item[0], dest=item[1], help=item[4], type=item[2], default=item[3])
+
+    if len(sys.argv) < 2:
+        parser.print_help()
+        sys.exit(1)
+
+    args = parser.parse_args()
+    return args
+
+
+if __name__ == '__main__':
+    args = parse_args()
+    if args.debug:
+        logger.setLevel(logging.DEBUG)
+    else:
+        logger.setLevel(logging.INFO)
+    if args.prev and args.post:
+        prev_spec = read_argspec_from_file(args.prev)
+        post_spec = read_argspec_from_file(args.post)
+        diff_api_names = []
+        for as_post_name, as_post in post_spec.items():
+            as_prev = prev_spec.get(as_post_name)
+            if as_prev is None:  # the api is deleted
+                continue
+            if isinstance(as_prev, str) or isinstance(as_post, str):
+                as_prev_str = as_prev if isinstance(as_prev,
+                                                    str) else repr(as_prev)
+                as_post_str = as_post if isinstance(as_post,
+                                                    str) else repr(as_post)
+                if not check_compatible_str(as_prev_str, as_post_str):
+                    diff_api_names.append(as_post_name)
+            else:
+                if not check_compatible(as_prev, as_post):
+                    diff_api_names.append(as_post_name)
+        if diff_api_names:
+            print('\n'.join(diff_api_names))

tools/check_file_diff_approvals.sh

@@ -150,6 +150,9 @@ for API_FILE in ${API_FILES[*]}; do
       elif [ "${API_FILE}" == "tools/checkout_pr_approval.py" ];then
           echo_line="test_checkout_pr_approval.py will be executed for changed checkout_pr_approval.py.\n"
           run_tools_test test_checkout_pr_approval.py
+      elif [ "${API_FILE}" == "tools/checkout_api_compatible.py" ];then
+          echo_line="test_checkout_api_compatible.py will be executed for changed checkout_api_compatible.py.\n"
+          run_tools_test test_checkout_api_compatible.py
       elif [ "${API_FILE}" == "python/paddle/distributed/fleet/__init__.py" ]; then
 	      echo_line="You must have (fuyinno4 (Recommend), raindrops2sea) approval for ${API_FILE} changes"
 	      check_approval 1 35824027 38231817

tools/print_signatures.py

@@ -160,14 +160,16 @@ def insert_api_into_dict(full_name, gen_doc_anno=None):
     Return:
         api_info object or None
     """
+    import paddle
     try:
         obj = eval(full_name)
         fc_id = id(obj)
     except AttributeError:
         logger.warning("AttributeError occurred when `id(eval(%s))`", full_name)
         return None
-    except:
-        logger.warning("Exception occurred when `id(eval(%s))`", full_name)
+    except Exception as e:
+        logger.warning("Exception(%s) occurred when `id(eval(%s))`",
+                       str(e), full_name)
         return None
     else:
         logger.debug("adding %s to api_info_dict.", full_name)
@@ -186,6 +188,10 @@ def insert_api_into_dict(full_name, gen_doc_anno=None):
                 api_info_dict[fc_id]["docstring"] = inspect.cleandoc(docstr)
             if gen_doc_anno:
                 api_info_dict[fc_id]["gen_doc_anno"] = gen_doc_anno
+            if inspect.isfunction(obj):
+                api_info_dict[fc_id]["signature"] = repr(
+                    inspect.getfullargspec(obj)).replace('FullArgSpec',
+                                                         'ArgSpec', 1)
         return api_info_dict[fc_id]
 
 
@@ -311,7 +317,7 @@ def parse_args():
         '--method',
         dest='method',
         type=str,
-        default='from_modulelist',
+        default='get_all_api',
         help="using get_all_api or from_modulelist")
     parser.add_argument(
         'module', type=str, help='module', default='paddle')  # not used
@@ -334,10 +340,20 @@ if __name__ == '__main__':
         for name in member_dict:
             print(name, member_dict[name])
     elif args.method == 'get_all_api':
-        api_signs = get_all_api()
-        for api_sign in api_signs:
-            print("{0} ({0}, ('document', '{1}'))".format(api_sign[0], api_sign[
-                1]))
+        get_all_api()
+        all_api_names_to_k = {}
+        for k, api_info in api_info_dict.items():
+            # 1. the shortest suggested_name may be renamed;
+            # 2. some api's fullname is not accessable, the module name of it is overrided by the function with the same name;
+            api_name = sorted(list(api_info['all_names']))[0]
+            all_api_names_to_k[api_name] = k
+        all_api_names_sorted = sorted(all_api_names_to_k.keys())
+        for api_name in all_api_names_sorted:
+            api_info = api_info_dict[all_api_names_to_k[api_name]]
+            print("{0} ({2}, ('document', '{1}'))".format(
+                api_name,
+                md5(api_info['docstring']), api_info['signature']
+                if 'signature' in api_info else 'ArgSpec()'))
 
     if len(ErrorSet) == 0:
         sys.exit(0)

tools/test_check_api_compatible.py

+#! /usr/bin/env python
+
+# Copyright (c) 2021 PaddlePaddle Authors. All Rights Reserved.
+# 
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""
+TestCases for check_api_compatible.py
+"""
+import unittest
+import sys
+import os
+import tempfile
+import inspect
+
+from check_api_compatible import read_argspec_from_file
+from check_api_compatible import check_compatible
+from check_api_compatible import check_compatible_str
+
+
+class Test_check_compatible(unittest.TestCase):
+    def setUp(self) -> None:
+        self.fullargspec_prefix = 'inspect.Full'
+        self.argspec_str_o = self.fullargspec_prefix + '''ArgSpec(args=['shape', 'dtype', 'name'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        return super().setUp()
+
+    def test_normal_not_changed(self):
+        argspec_o = eval(self.argspec_str_o)
+        argspec_n = eval(self.argspec_str_o)
+        self.assertTrue(check_compatible(argspec_o, argspec_n))
+
+    def test_args_added(self):
+        argspec_str_n = '''ArgSpec(args=['shape', 'dtype', 'name', 'arg4'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_o = eval(self.argspec_str_o)
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertFalse(check_compatible(argspec_o, argspec_n))
+
+        argspec_str_n = '''ArgSpec(args=['shape', 'dtype', 'name', 'arg4'], varargs=None, varkw=None, defaults=(None, None, 1), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertTrue(check_compatible(argspec_o, argspec_n))
+
+        argspec_str_n = '''ArgSpec(args=['shape', 'dtype', 'name', 'arg4'], varargs=None, varkw=None, defaults=(None, None, 1, True), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertFalse(check_compatible(argspec_o, argspec_n))
+
+        argspec_str_n = '''ArgSpec(args=['shape', 'dtype', 'name', 'arg4'], varargs=None, varkw=None, defaults=(True, None, None, 1), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertTrue(check_compatible(argspec_o, argspec_n))
+
+    def test_args_places_exchanged(self):
+        argspec_str_n = '''ArgSpec(args=['shape', 'name', 'dtype'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_o = eval(self.argspec_str_o)
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertFalse(check_compatible(argspec_o, argspec_n))
+
+    def test_args_reduced(self):
+        argspec_str_n = '''ArgSpec(args=['shape', 'name'], varargs=None, varkw=None, defaults=(None,), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        argspec_o = eval(self.argspec_str_o)
+        argspec_n = eval(self.fullargspec_prefix + argspec_str_n)
+        self.assertFalse(check_compatible(argspec_o, argspec_n))
+
+
+class Test_check_compatible_str(unittest.TestCase):
+    def setUp(self) -> None:
+        self.fullargspec_prefix = 'inspect.Full'
+        # paddle.fluid.layer_helper_base.LayerHelperBase.create_parameter
+        self.argspec_str_o = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient', 'type'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        return super().setUp()
+
+    def test_normal_not_changed(self):
+        argspec_o = self.argspec_str_o
+        argspec_n = self.argspec_str_o
+        self.assertTrue(check_compatible_str(argspec_o, argspec_n))
+
+    def test_args_added(self):
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient', 'type', 'argadded'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        argspec_o = self.argspec_str_o
+        self.assertFalse(check_compatible_str(argspec_o, argspec_n))
+
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient', 'type', 'argadded'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR, argadded), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        self.assertTrue(check_compatible_str(argspec_o, argspec_n))
+
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient', 'type', 'argadded'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR, argadded, 1), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        self.assertFalse(check_compatible_str(argspec_o, argspec_n))
+
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient', 'type', 'argadded'], varargs=None, varkw=None, defaults=(1, None, False, None, False, VarType.LOD_TENSOR, argadded), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        self.assertTrue(check_compatible_str(argspec_o, argspec_n))
+
+    def test_args_places_exchanged(self):
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'type', 'stop_gradient'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        argspec_o = self.argspec_str_o
+        self.assertFalse(check_compatible_str(argspec_o, argspec_n))
+
+    def test_args_reduced(self):
+        argspec_n = self.fullargspec_prefix + """ArgSpec(args=['self', 'attr', 'shape', 'dtype', 'is_bias', 'default_initializer', 'stop_gradient'], varargs=None, varkw=None, defaults=(None, False, None, False, VarType.LOD_TENSOR), kwonlyargs=[], kwonlydefaults=None, annotations={})"""
+        argspec_o = self.argspec_str_o
+        self.assertFalse(check_compatible_str(argspec_o, argspec_n))
+
+
+class Test_read_argspec_from_file(unittest.TestCase):
+    def setUp(self) -> None:
+        self.fullargspec_prefix = 'inspect.Full'
+        self.argspec_str_o = self.fullargspec_prefix + '''ArgSpec(args=['shape', 'dtype', 'name'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={})'''
+        self.api_spec_file = tempfile.TemporaryFile('w+t')
+        if self.api_spec_file:
+            self.api_spec_file.write("\n".join([
+                """paddle.ones (ArgSpec(args=['shape', 'dtype', 'name'], varargs=None, varkw=None, defaults=(None, None), kwonlyargs=[], kwonlydefaults=None, annotations={}), ('document', '50a3b3a77fa13bb2ae4337d8f9d091b7'))""",
+                # """paddle.four_plus_four (paddle.four_plus_four, ('document', 'ff0f188c95030158cc6398d2a6c5four'))""",
+                """paddle.five_plus_five (ArgSpec(), ('document', 'ff0f188c95030158cc6398d2a6c5five'))""",
+            ]))
+            self.api_spec_file.seek(0)
+        return super().setUp()
+
+    def tearDown(self):
+        if self.api_spec_file:
+            self.api_spec_file.close()
+
+    def test_case_normal(self):
+        if self.api_spec_file:
+            api_argspec_dict = read_argspec_from_file(self.api_spec_file)
+            argspec = eval(self.argspec_str_o)
+            self.assertEqual(
+                api_argspec_dict.get('paddle.ones').args, argspec.args)
+            self.assertEqual(
+                api_argspec_dict.get('paddle.ones').defaults, argspec.defaults)
+            self.assertIsNone(api_argspec_dict.get('paddle.five_plus_five'))
+        else:
+            self.fail('api_spec_file error')
+
+
+if __name__ == '__main__':
+    unittest.main()

tools/test_sampcd_processor.py

@@ -433,6 +433,7 @@ class Test_get_api_md5(unittest.TestCase):
                 """paddle.two_plus_two (ArgSpec(args=[], varargs=None, keywords=None, defaults=(,)), ('document', 'ff0f188c95030158cc6398d2a6c55two'))""",
                 """paddle.three_plus_three (ArgSpec(args=[], varargs=None, keywords=None, defaults=(,)), ('document', 'ff0f188c95030158cc6398d2a6cthree'))""",
                 """paddle.four_plus_four (paddle.four_plus_four, ('document', 'ff0f188c95030158cc6398d2a6c5four'))""",
+                """paddle.five_plus_five (ArgSpec(), ('document', 'ff0f188c95030158cc6398d2a6c5five'))""",
             ]))
 
     def tearDown(self):
@@ -449,6 +450,8 @@ class Test_get_api_md5(unittest.TestCase):
                          res['paddle.three_plus_three'])
         self.assertEqual("ff0f188c95030158cc6398d2a6c5four",
                          res['paddle.four_plus_four'])
+        self.assertEqual("ff0f188c95030158cc6398d2a6c5five",
+                         res['paddle.five_plus_five'])
 
 
 class Test_get_incrementapi(unittest.TestCase):