|
| 1 | +# 代码风格检查工具 Flake8 引入计划 |
| 2 | + |
| 3 | +| 任务名称 | 代码风格检查工具 Flake8 引入计划 | |
| 4 | +| ------------ | -------------------------------- | |
| 5 | +| 提交作者 | Nyakku Shigure(@SigureMo) | |
| 6 | +| 提交时间 | 2022-09-27 | |
| 7 | +| 版本号 | v1.0 | |
| 8 | +| 依赖飞桨版本 | develop | |
| 9 | +| 文件名 | 20220927_introducing_flake8.md | |
| 10 | + |
| 11 | +## 一、概述 |
| 12 | + |
| 13 | +### 1、相关背景 |
| 14 | + |
| 15 | +Linter 是代码开发过程中的一个重要工具,它可以帮助开发者在编写代码时就发现一些格式、语法甚至一些逻辑错误。由于 Python 是一门解释型语言,并没有编译时来对一些错误进行检查,很多问题可能留到运行时才能暴露出来,而 Linter 则可以将这些问题提前到开发时,使得开发者能够及时发现并修复问题,提高代码质量。 |
| 16 | + |
| 17 | +Paddle 目前在 C++ 端代码使用 cpplint 作为 Linter,Python 端却并没有一个 Linter 来规范代码(虽然目前有使用 pylint 但仅仅用于检查 docstring)。 |
| 18 | + |
| 19 | +Flake8 是一个被广泛使用的 Python Linter,它利用插件系统集成了多个 Linter 工具,默认的插件包含了 pycodestyle、pyflakes、mccabe 三个工具,分别用于检查代码风格、语法问题和循环复杂度。此外 Flake8 还支持安装第三方插件,以对代码进行更全面的检查。 |
| 20 | + |
| 21 | +本任务来源于 [Call for contribution - Flake8](../../pfcc/call-for-contributions/code_style_flake8.md) |
| 22 | + |
| 23 | +### 2、功能目标 |
| 24 | + |
| 25 | +修复 Paddle 当前代码中 Flake8 存量问题,并在 pre-commit 和 CI 中增加 Flake8 检查项。 |
| 26 | + |
| 27 | +### 3、意义 |
| 28 | + |
| 29 | +规范代码风格,提高代码质量,并使开发者能够在开发时发现一些潜在的逻辑问题。 |
| 30 | + |
| 31 | +## 二、实现方案 |
| 32 | + |
| 33 | +### 阻止增量问题 |
| 34 | + |
| 35 | +1. 添加 Flake8 的配置文件 `.flake8`,在 pre-commit 中添加 flake8 hook(由 pre-commit clone repo) |
| 36 | +2. 将 flake8 修改为 local hook,添加相应的 hook 脚本,并在 Docker 镜像中提前安装 flake8 |
| 37 | +3. 更新代码风格检查指南文档,添加 flake8 |
| 38 | + |
| 39 | +### 修复存量问题 |
| 40 | + |
| 41 | +Flake8 默认启用的三个工具(pycodestyle、pyflakes、mccabe)共包含 132 个错误码,在 Flake8 引入任务启动前 Paddle 共存在 64 个错误码。 |
| 42 | + |
| 43 | +Flake8 的 132 个错误码中有 11 个由于未被广泛认可而默认未启用的,此外还有一些错误码(如 E203)还被认为过分严苛也没有被社区广泛认可,这些错误码可以在 Flake8 配置文件中排除掉,不作考虑。 |
| 44 | + |
| 45 | +对于其余错误码,先在 Flake8 配置文件中排除掉,之后每修复一个错误码的存量问题后在配置中移除该错误码,使 pre-commit 能够阻止该错误码增量。 |
| 46 | + |
| 47 | +以下是在完成 trailing whitespace(W291、W293)相关修复后的统计 |
| 48 | + |
| 49 | +```text |
| 50 | +Type: E (26468) |
| 51 | +E101 11 |
| 52 | +E121 8 |
| 53 | +E122 81 |
| 54 | +E123 12 |
| 55 | +E125 168 |
| 56 | +E126 723 |
| 57 | +E127 140 |
| 58 | +E128 207 |
| 59 | +E129 9 |
| 60 | +E131 45 |
| 61 | +E201 29 |
| 62 | +E202 11 |
| 63 | +E203 32 |
| 64 | +E225 61 |
| 65 | +E226 93 |
| 66 | +E228 3 |
| 67 | +E231 60 |
| 68 | +E241 2 |
| 69 | +E251 109 |
| 70 | +E261 11 |
| 71 | +E262 238 |
| 72 | +E265 925 |
| 73 | +E266 116 |
| 74 | +E271 4 |
| 75 | +E272 1 |
| 76 | +E301 7 |
| 77 | +E302 3 |
| 78 | +E303 7 |
| 79 | +E305 2 |
| 80 | +E306 1 |
| 81 | +E401 19 |
| 82 | +E402 2666 |
| 83 | +E501 19252 |
| 84 | +E502 400 |
| 85 | +E701 108 |
| 86 | +E711 166 |
| 87 | +E712 340 |
| 88 | +E713 22 |
| 89 | +E714 4 |
| 90 | +E721 8 |
| 91 | +E722 149 |
| 92 | +E731 62 |
| 93 | +E741 153 |
| 94 | +
|
| 95 | +Type: F (9895) |
| 96 | +F401 6750 |
| 97 | +F402 1 |
| 98 | +F403 57 |
| 99 | +F405 556 |
| 100 | +F522 1 |
| 101 | +F524 1 |
| 102 | +F541 33 |
| 103 | +F601 7 |
| 104 | +F631 2 |
| 105 | +F632 18 |
| 106 | +F811 177 |
| 107 | +F821 88 |
| 108 | +F841 2204 |
| 109 | +
|
| 110 | +Type: W (1414) |
| 111 | +W191 11 |
| 112 | +W503 279 |
| 113 | +W504 949 |
| 114 | +W601 3 |
| 115 | +W605 172 |
| 116 | +``` |
| 117 | + |
| 118 | +> **Note** |
| 119 | +> |
| 120 | +> - E、W 错误码详情见:[pycodestyle Error Code](https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes) |
| 121 | +> - F 错误码详情见:[Flake8 Error Code](https://flake8.pycqa.org/en/latest/user/error-codes.html) |
| 122 | +
|
| 123 | +其中 trailing whitespace 相关问题(W291、W293)已经被 pre-commit hook [trailing-whitespace](https://github.com/pre-commit/pre-commit-hooks#trailing-whitespace) 解决,Tabs 相关问题(E101、W191)已经被 pre-commit hook [remove-tabs](https://github.com/Lucas-C/pre-commit-hooks#usage) 解决。 |
| 124 | + |
| 125 | +此外存在一个语法错误(E999)已经在之前的 [#45287](https://github.com/PaddlePaddle/Paddle/pull/45287) 解决。 |
| 126 | + |
| 127 | +black(Formatter)能自动解决大多数格式上的问题(E 错误码),如果引入 black 则可以解决 E121、E122 等大多数 E 错误码。 |
| 128 | + |
| 129 | +其余错误码需要根据情况来处理: |
| 130 | + |
| 131 | +- 存量较少的错误码(低于 30):手动修复即可 |
| 132 | +- 存量较大的错误码: |
| 133 | + - 可利用 autoflake 修复部分 F 错误码 |
| 134 | + - 可利用 autopep8 修复部分 black 剩余的 E 错误码 |
| 135 | + - 上述两个工具无法修复的且存量较大的错误码需要编写脚本修复或者手动修复 |
| 136 | + |
| 137 | +以下是一些错误码的具体修复方案 |
| 138 | + |
| 139 | +#### 少量存量,手动修复的错误码 |
| 140 | + |
| 141 | +对于一些存量较少的错误码,直接手动修复是效率最高且最安全的修复方式,这样的错误码主要包含: |
| 142 | + |
| 143 | +- E713: test for membership should be ‘not in’ |
| 144 | +- E714: test for object identity should be ‘is not’ |
| 145 | +- F402: import `module` from line `N` shadowed by loop variable |
| 146 | +- F522: `.format(...)` unused named arguments |
| 147 | +- F524: `.format(...)` missing argument |
| 148 | +- F541: f-string without any placeholders |
| 149 | +- F601: dictionary key `name` repeated with different values |
| 150 | +- F632: use `==`/`!=` to compare `str`, `bytes`, and `int` literals |
| 151 | +- F631: assertion test is a tuple, which is always `True` |
| 152 | +- W601: `.has_key()` is deprecated, use ‘in’ |
| 153 | +- W605: invalid escape sequence ‘x’ |
| 154 | +- 还有一些需要在 black 引入后再修复的,会在之后继续添加 |
| 155 | + |
| 156 | +#### 使用/编写工具,自动修复的错误码 |
| 157 | + |
| 158 | +对于一些存量不是特别大,但手动修复比较麻烦的错误码,建议使用现有的工具或者编写脚本来进行修复,这样的错误码主要包含: |
| 159 | + |
| 160 | +- E711: comparison to None should be ‘if cond is None:’ |
| 161 | +- E712: comparison to True should be ‘if cond is True:’ or ‘if cond:’ |
| 162 | +- 可能还有些没考虑到的,需要之后进一步确定修复方案 |
| 163 | + |
| 164 | +#### F401(逐目录修复) |
| 165 | + |
| 166 | +F401 为 import 了未使用的模块,该问题存量非常大,因此使用 autoflake 来进行自动修复 |
| 167 | + |
| 168 | +由于在[尝试全量一次性修复](https://github.com/PaddlePaddle/Paddle/pull/45252/)后发生了难以定位的错误,因此该错误码需要逐目录来做,优先修复单测和 tools 这种不会被其他模块 import 的目录,之后逐渐向较为核心的模块进行修复。 |
| 169 | + |
| 170 | +以 `python/paddle/fluid/tests/unittests/asp/` 为例,在 Paddle 根目录执行以下命令即可一次性清除该目录下全部 F401 问题 |
| 171 | + |
| 172 | +```bash |
| 173 | +autoflake --in-place --remove-all-unused-imports --exclude=__init__.py --ignore-pass-after-docstring --recursive ./python/paddle/fluid/tests/unittests/asp/ |
| 174 | +``` |
| 175 | + |
| 176 | +如果清除后发现 CI 无法通过,需要根据情况判断问题,如果该 import 是必要的,应当在该 import 处添加 `# noqa: F401` 以 disable 该处报错。 |
| 177 | + |
| 178 | +## 三、任务分工和排期规划 |
| 179 | + |
| 180 | +由 Flake8 小组自行认领任务,每人负责部分错误码的部分目录。由于任务尚未开始,具体排期尚无法确定。 |
| 181 | + |
| 182 | +预计 2022 年底应该能完成绝大多数错误码的修复。 |
| 183 | + |
| 184 | +目前 Flake8 小组成员如下: |
| 185 | + |
| 186 | +- [Nyakku Shigure](https://github.com/SigureMo) |
| 187 | +- [Shuangchi He](https://github.com/Yulv-git) |
| 188 | +- [Tony Cao](https://github.com/caolonghao) |
| 189 | +- [Zheng_Bicheng](https://github.com/Zheng-Bicheng) |
| 190 | +- [Infinity_lee](https://github.com/enkilee) |
| 191 | + |
| 192 | +## 四、其他注意事项及风险评估 |
| 193 | + |
| 194 | +由于 fluid 预计将会在 Paddle 2.5 中移除,因此 flake8 忽略 `python/paddle/fluid/` 目录下的错误码,但其中的 `python/paddle/fluid/tests/` 目录仍需要进行修复。 |
| 195 | + |
| 196 | +## 五、影响面 |
| 197 | + |
| 198 | +开发人员在后续开发过程中需要遵守 Flake8 的规范,否则无法通过 pre-commit 和 CI,能够极大提高 Paddle Python 代码的质量。 |
| 199 | + |
| 200 | +## 名词解释 |
| 201 | + |
| 202 | +- Linter:代码检查工具(含代码风格、语法、逻辑等等) |
| 203 | +- Formatter:代码格式化工具 |
| 204 | + |
| 205 | +## 附件及参考资料 |
| 206 | + |
| 207 | +- [Flake8 Error Code](https://flake8.pycqa.org/en/latest/user/error-codes.html) |
| 208 | +- [pycodestyle Error Code](https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes) |
0 commit comments