怎么做好团队的CR和代码治理？

╰+攻爆jí腚メ 2024-04-29 19:15 64阅读 0赞

### 前言 ###

我们团队共有20+前端同学，负责前端应用高达300+（使用微前端架构），前端仓库代码总量粗略估计已达100W+行，所以代码质量一直是我们很大的一个问题，虽然公司的发布系统在发布之前有强制的CR流程，但是在很多时候，没有人仔细审核代码，没有人提出问题，CR逐渐沦为了一个形式。后来我在团队内制定了`CR评论率`（评论数/CR次数）的指标，强制大家去发现一些问题并进行评论，并将这些问题整理成bad case集合，随着时间的沉淀，我们整理了很多相关案例，最后基于这些案例我们开始了对代码治理的一系列探索，并取得了相当可观的成果。

### 问题整理 ###

首先我还是基于我们之前通过CR沉淀的所有错误案例，整理了一下我们经常出现的一些问题类型：

#### 1. 常量复用 ####

![image.png][]

多个地方使用同一个值，没有定义变量/常量进行复用。修改为：

复制代码

`const MAX_PAGE_COUNT = 5; // 抽离常量复用 && 通过命名更有语义`

#### 2. 域名编码 ####

复制代码

`const onClick = () => { window.open('https://xxxx.com/?p=sas'); };`

这里写死的域名，可能在不同的环境（预发/线上/站点等）值也是不一样的，需要抽离为配置。修改为：

复制代码

``const onClick = () => { window.open(`${appConfig.yundunDomain}/?p=sas`); // 根据环境对域名进行动态配置 };``

#### 3. 中文硬编码 ####

复制代码

`const title = '配置管理';`

如果产品支持国际化，前端编码中就不应该出现中文字符，应该将文案进行配置化管理。修改为：

复制代码

`const title = intl('config.manage'); // 国际化处理`

#### 4. 类型转换/判断 ####

复制代码

`const result = !b; const result = a == b;`

第一行代码，我们要注意如果`!0`和`!undefined`的结果是一样的，所以在使用时要注意b的数据类型。第二行代码，我们要注意`==`与`===`的差异，比如`2=='2'`的结果是`true`。代码中尽量使用`===`。这两个问题虽然很简单，但在新同学中还是很常见的，CR时不能掉以轻心。

复制代码

`const title = intl('config.manage'); // 国际化处理`

#### 5. 取值不考虑兼容 ####

复制代码

`const data = info.component.spec;`

如果info.component为`undefined`，后面取值就会报错了，很常见的一种错误。修改为：

复制代码

`const data = info?.component?.spec; // 即便前面为undined，也不会报错`

#### 6. 复杂语法 ####

复制代码

`const result = a.x === variable1 || a.x === variable2 || a.x === variable3;`

语法冗余，完全可以合并和精简。修改为：

复制代码

`const result = [variable1,variable2,variable3].includes(a.x); // 逻辑简化，达到相同效果`

#### 7. 复杂语法 ####

复制代码

`if (type === 'type1') { // action1 } else if (type === 'type2') { // action2 } else if (type === 'type3') { // action3 }`

对于这样的条件判断语句，我们应该使用对象来映射条件和操作。修改为：

复制代码

`// 使用对象映射关系，可以清晰的知道对应的操作。 const actionMap = { 'type1': () => {/* action1 */}, 'type2': () => {/* action2 */}, 'type3': () => {/* action3 */} };`

#### 8. 复杂三元表达式 ####

复制代码

`const result = condition1 ? value1 : condition2 ? value2 : condition3 ? value3 : defaultValue;`

对于这种复杂多层的三元表达式，随着嵌套层数的增加，代码的可读性和可维护性会显著下降，应该进行重构，这里有几种常用方法进行优化。如果条件比较简单相似，优先使用对象映射的方式进行优化，对于比较复杂的条件，使用函数封装的方式进行优化，修改为：

复制代码

`function getResult() { if (condition1) return value1; if (condition2) return value2; if (condition3) return value3; return defaultValue; }`

#### 9. 过于简单的switch ####

复制代码

`switch (key) { case value1: return '1' break; case value2: return '2' break; }`

上面这个`switch`条件语句下面只有两个分支，我们也是不推荐的，这种简单的条件使用if-esle进行判断即可，没必要使用复杂的switch。

#### 10. 无默认分支的switch ####

复制代码

`switch (key) { case value: return '1' break; ... }`

尽管switch语法并没有强制要求`default`分支，但是通常情况下，default分支可以防止出现一些非预期的意外行为，还可以提升可读性和可维护性。在我们团队，default分支是必须的。

#### 11. import引用顺序 ####

复制代码

`import './index.scss'; import { useData } from './utils'; import utils from 'src/utils'; import lodash from lodash;`

虽然import语句对引用顺序没有明确规定，但还是可以在团队内部做一个简单的规范（三方包 -> 内部包 -> CSS）。我们团队使用eslint进行了规范，并实现自动格式化，保存文件时会自动修改为：

复制代码

`import lodash from lodash; import utils from 'src/utils'; import { useData } from './utils'; import './index.scss';`

#### 12. 遗留的console.log ####

为了不把`console.log`等调试代码发布到线上，我们可以通过几种方式：

1.  eslint配置规则不允许使用。
2.  webpack插件在构建时自动去除。

#### 13. 重复代码 ####

一旦有重复代码存在，阅读这些重复的代码时你就必须加倍仔细，留意其细微的差异，在修改时也很容易出现遗漏，所以如果多个地方使用同一实现逻辑，应该尽量把这个逻辑抽离进行复用。

#### 14. 混用forEach和map ####

复制代码

`let value; arr.map(item => { if (item.x) value = item.value; });`

数组的`map`方法，用以创建一个新的数组，新数组由原数组中的每个元素进行函数处理返回的元素组成，所以map函数的回调需要有返回值。上面这种纯循环数组的操作，应该改为`forEach`，修改为：

复制代码

`let value; arr.forEach(item => { if (item.x) value = item.value; });`

#### 15. 不必要的串行promise ####

复制代码

`const result1 = await getResult1(); const result2 = await getResult2();`

这种没有依赖关系的`promise`，应该使用并发调用。修改为：

复制代码

`const [result1, result2] = await Promise.all([ getResult1(), getResult2() ]);`

这里需要注意的是`Promise.all`方法，getResult1请求失败，将导致result1和result2均获取不到值，需要根据业务进行评估使用。顺便可以了解一下`Promise.allsettled`，[传送门][Link 1]。

#### 16. 函数形参过多 ####

复制代码

`const formatPriceInfo = (info, versionInfo, saleInfo, config, params) => { // ... }`

对于这样的参数过多的函数，非常影响扩展性和可维护性，这种传参方式要求使用的地方要和形参一一严格对应，稍有不慎，就可能引发严重的BUG，应该使用对象的形式进行传参。修改为：

复制代码

`// 使用对象形式的形参，使用方无需关心属性顺序 const formatPriceInfo = ({ info, versionInfo, saleInfo, config, params }) => { // ... }`

#### 17. 函数内修改传入参数 ####

复制代码

`const formatPriceInfo = (info) => { info.items.froEach(d => { if (d.id === 1) { d.count = 10; } }); };`

上面的函数，将传进来的`info.items`属性进行了循环，并可能修改部分数组项的count属性，这是一个典型的具有副作用的函数：执行函数之后，可能会影响info的值，如果这个info在很多地方调用，就很容易造成不可控的问题出现。代码中的函数都应该是`纯函数`（传入相同的参数永远都返回相同的值）。

#### 18. 函数过于复杂（过长） ####

函数内容越长，就越难理解，你需要花很多的时间才能明白这个函数是在干什么。而更好的阐释力、更易于理解和分享的功能，往往是由小函数来支持的。所以对于过于复杂的函数，我们应该将其中的部分功能抽离为小函数，并为其语义化命名，以提升可读性和维护性。

#### 19. 不建议使用类组件 ####

已经使用React Hooks的团队，建议都使用函数组件进行开发，不要再使用类组件，我们团队已经做了相应规约。

#### 20. 文件行数过大 ####

和函数内容过长类似，大文件内容不便于我们理解和共享，我们应该将文件部分内容通过函数进行拆分，提升可维护性。

#### 21. 临时注释忘记撤回 ####

复制代码

`// const canTry = info.canTry; 通过临时注释，方便本地测试，但是发布的时候忘记了撤回代码 const canTry = true;`

比如上述代码，便于调试临时给cantry变量赋值为true，但是很有可能就忘记了这段注释直接发到了线上，我们之前就出现过这种情况。这种问题工具不太好检查，所以需要人工更仔细的代码审核。

> 针对这个问题我有个设想，编辑器在注释代码的时候，通过插件给一个提示，是否要加TODO的标签，如果加了之后，在代码检查环节对TODO的注释进行阻断提示，不知道是否可行。

#### 22. 大段注释代码不删除 ####

大量注释代码不删除，虽然没有打包构建等问题，但还是会给我们造成一些困惑，比如：

1.  增加维护成本：为什么只是注释，是不是还有一些其他逻辑存在。
2.  降低编译效率：虽然影响很小，但确实会增加构建编译的性能成本。
3.  增加文件行数：增加了文件行数，如果有代码检查工具，会影响它们的判断。
4.  影响代码搜索：比如我要搜索一个代码关键词，恰好存在于注释片段，会影响搜索结果。

所以，如果有大段代码被注释，那就放心的删除掉，如果需要找回，借助`git`即可。

#### 23. 无用的依赖包/文件/函数 ####

项目经过长时间的迭代，`package.json`文件中的`dependencies`配置，可能会存在一些没有使用的依赖包，应该进行及时清理。还有文件、函数、css选择器，没有使用到的同样需要进行定时清理，这方面通过人肉也不太好排查，所以需要借助工具。我们团队是有相应工具去治理的，后面会提到。

#### 24. CSS属性顺序 ####

css属性一般遵从以下先后顺序：

1.  定位： position / z-index 等
2.  布局（盒模型）：display / width/ height / margin / padding 等
3.  文本属性： color / font / text-align / background 等
4.  其它属性： cursor / shadow / opacity / animation 等
5.  伪类属性：:before / :after / :hover 等 顺便说一句，`stylelint`中有一个插件[stylelint-order][]可以用来自动格式化属性顺序。我们团队也使用此工具进行了规约，后面会提到。

#### 25. CSS内联样式 ####

代码中应该尽量少的使用内联样式，内联样式有两个问题：

1.  缺少语义，不知道为什么会设置这些属性。
2.  在list场景下，会重复渲染这些样式代码。

#### 26. CSS滥用float属性 ####

当元素使用float属性后，该元素就会脱离文档流，这可能导致父容器无法正确识别子容器的高度，要解决这个问题就要使用清除浮动，给代码增加了不必要的复杂度。随着Flexbox和CSS Grid的出现，我们应该禁止在css中使用float属性，正好可以通过`stylelint`的`declaration-property-value-disallowed-list`规则进行禁用float属性。

#### 27. 逻辑 || 和 ?? ####

复制代码

`const a = 0; console.log(a || false); // false console.log(a ?? false); // 0`

注意这两个逻辑的区别，应该在哪个场景使用对应的逻辑，需要好好审核。

### 治理手段 ###

上面提到的这些问题，就是我们平时有意无意可能会发生的问题，如果你想纯靠人工CR来发现这些问题，几乎是不可能的，因为成本实在是太大了，聪明的程序员善于借助工具，看看我们都做了什么？我们首先对这些问题一条一条进行梳理，过滤出能够通过工具检测的问题，然后建设了本地检查+错误拦截+远程扫描三个环节，共同形成了团队项目代码治理的工具链路。

#### 本地检查 ####

代码本地检查环节，我们基于`ESlint`整理了一个统一的配置规则包`eslint-config-yd`，用以实现对JS文件的检查，并基于`stylelint`开发了`stylelint-config-yd`，这两个包没有对外开放权限，我贴一部分源码：

eslint规则配置：

复制代码

`module.exports = { extends: [ 'eslint-config-ali/react', ], parser: '@babel/eslint-parser', plugins: [ 'react', 'react-hooks', 'sonarjs', 'jsx-a11y' ], rules: { // import 顺序 "import/order": ["error", { "groups": ["builtin", "external", "internal", "parent", "sibling", "index"] }], // react 'react/jsx-closing-tag-location': 0, // 不要想着标签严格缩进匹配 'react/jsx-filename-extension': [1, { extensions: ['.js', '.jsx', '.ts', '.tsx'] }], // 只允许在JS/JSX出现JSX 'react/no-unused-state': 2, // 不要出现没有使用的 state 'react/prop-types': 0, // 关闭 prop-types 'react/no-array-index-key': 0, // 允许使用数组索引 'react/no-danger': 2, // react hooks 'react-hooks/rules-of-hooks': 2, 'react-hooks/exhaustive-deps': 0, // sonar 的 code smell detection 'sonarjs/no-redundant-boolean': 2, 'sonarjs/no-duplicated-branches': 2, 'sonarjs/prefer-immediate-return': 2, 'sonarjs/no-small-switch': 2, 'sonarjs/max-switch-cases': [2, 20], // switch检查 'sonarjs/cognitive-complexity': [1, 30], // 函数复杂度 // Custom 'eqeqeq': 2, // 严格等号逻辑 'no-unused-expressions': 0, 'dot-notation': 0, // 获取对象属性不是必须用点 'no-console': 1, // 警告 console.log 使用 semi: [1, 'always'], // 双引号 'arrow-parens': [0, 'always'], // 箭头函数参数应该始终包含括号 indent: [1, 2, { SwitchCase: 1 }], // 2空格缩进，否则警告 'no-unused-vars': 1, // 未使用的变量进行警告 'no-script-url': 2, // 允许 javascript:void(0) 的写法 'arrow-body-style': [1, 'as-needed'], // 箭头函数可以去掉括号就去掉括号 radix: 0, // parseInt 不强制要求第二个参数 'array-callback-return': 1, // 应该return的没有return的进行警告 'no-restricted-globals': 2, // 不要直接使用 global 的东西 'no-nested-ternary': 0, // 不允许嵌套三元运算 }, };`

然后配合VSCODE编辑器的ESLint插件，即可实现保存文件时对一些错误语法进行自动修复，看看一些检查效果：

switch语句分支太少：

![image.png][image.png 1]

不允许出现域名硬编码：

![image.png][image.png 2]

函数形参不能超过4个：

![image.png][image.png 3]

链式取值未做兼容：

![image.png][image.png 4]

#### 错误拦截 ####

除了本地静态检查和自动修复以外，我们也会在项目构建之前对代码进行检查并尝试自动修复，如果仍然存在无法自动修复的严重错误则会提示出来并阻止发布，必须全部修复才能继续发布，比如：

![image.png][image.png 5]

#### 代码扫描 ####

我相信，通过`静态检查-自动修复`和`错误拦截-手动修复`两个环节，我们代码中的严重问题就几乎不存在了，但对于一些警告类型的问题或工具不好检查的问题，就会被允许发到线上了，所以我们为了彻底做好代码治理，畅想一下还有没有更进一步的解决方案？

答案是有的，试想一下，当项目代码合并到master分支时，我们对最新代码进行扫描，并将扫描出来的问题进行上报，最后在一个平台将这些问题通过任务的形式展示出来，推动所有成员进行持续治理和重构，真正将代码治理的流程闭环掉，做到极致。

### 总结 ###

本文主要介绍了我在工作过程中整理的一些常见编码问题，以及一个成熟的团队如果想要做好代码治理，可以通过哪几个环节和方式来进行规约。没有太多的源代码，更多的是一种思路，如果大家有疑问，欢迎随时私信我交流。

其实基于上面代码扫描环节的结论继续畅想，如果真的建设了这样一个治理系统将代码治理流转了起来，你就会发现，除了代码治理之外，我们还可以做更多维度的治理，比如慢接口、重复接口、死链文档、错误文档等等，如果你使用微前端架构，还可以监控微应用的加载时间并进行治理。我们甚至还可以将这些问题形成扣分项，就可以给每个代码库产出一个代码质量分数，这样在优化的时候会更有说服力。但是除了目前治理的这些问题之外，其实还有很多无法通过工具发现的问题，需要时刻提醒我们在CR的时候做的更好。

[image.png]: https://img-blog.csdnimg.cn/img_convert/803abd3f19de11d30b3f59ecd4d0c86a.webp?x-oss-process=image/format,png
[Link 1]: https://link.juejin.cn/?target=https%3A%2F%2Fdeveloper.mozilla.org%2Fzh-CN%2Fdocs%2FWeb%2FJavaScript%2FReference%2FGlobal_Objects%2FPromise%2FallSettled
[stylelint-order]: https://link.juejin.cn/?target=https%3A%2F%2Fwww.npmjs.com%2Fpackage%2Fstylelint-order
[image.png 1]: https://img-blog.csdnimg.cn/img_convert/cc57ccbae3a40d51b15ee6be7840946a.webp?x-oss-process=image/format,png
[image.png 2]: https://img-blog.csdnimg.cn/img_convert/5dd211aecf71f9d377b9ca143a5f6d4a.webp?x-oss-process=image/format,png
[image.png 3]: https://img-blog.csdnimg.cn/img_convert/0f006ccff6d02d46dfadaa506b9cf275.webp?x-oss-process=image/format,png
[image.png 4]: https://img-blog.csdnimg.cn/img_convert/7d36a0f29ebc071affdb74028e4dfbe5.webp?x-oss-process=image/format,png
[image.png 5]: https://img-blog.csdnimg.cn/img_convert/6bf508eedfc652c4765c0b0be5bfb40e.webp?x-oss-process=image/format,png