Visual Studio Code 的严格空检查
2019 年 5 月 23 日,作者:Matt Bierner,@mattbierner
安全带来速度
快速行动很有趣。发布新功能、让用户满意、改进我们的代码库都很有趣。但是,与此同时,发布有 bug 的产品并不有趣。没有人喜欢遇到问题,或者在凌晨三点因为突发事件被叫醒。
尽管快速行动和发布稳定代码经常被认为是互不兼容的,但事实并非如此。很多时候,导致代码脆弱和有 bug 的因素也正是拖慢开发速度的因素。毕竟,如果我们总是担心会破坏东西,又如何能快速行动呢?
在这篇文章中,我想分享 VS Code 团队最近完成的一项重大工程工作:在我们的代码库中启用 TypeScript 的严格空检查。我们相信这项工作将使我们能够更快地行动并发布更稳定的产品。启用严格空检查的动机是,将 bug 视为源代码中更大隐患的症状,而不是孤立的事件。我将以严格空检查作为案例研究,讨论我们工作的动机,我们如何提出增量解决问题的方法,以及我们如何实施修复。这种识别和减少隐患的通用方法可以应用于任何软件项目。
一个例子
为了说明 VS Code 在启用严格空检查之前面临的问题,我们来看一个简单的 TypeScript 库。如果您是 TypeScript 新手,请不用担心;具体细节并不重要。这个虚构的例子只是为了说明我们在 VS Code 代码库中遇到的问题类型,并提及一些针对此类问题的传统应对措施。
我们的示例库包含一个简单的 getStatus
函数,它从一个假想网站的后端获取给定用户的状态
export interface User {
readonly id: string;
}
/**
* Get the status of a user
*/
export async function getStatus(user: User): Promise<string> {
const id = user.id;
const result = await fetch(`/api/v0/${id}/status`);
const json = await result.json();
return json.status;
}
看起来很合理。发布吧!
但在部署我们的新代码后,我们发现崩溃事件激增。从调用堆栈来看,崩溃似乎发生在我们的 getStatus
函数中。糟糕!
再往前追溯一点,似乎我们的一位同事在尝试获取当前用户状态时误导性地调用了 getStatus(undefined)
。当代码尝试访问 undefined.id
时,这导致了一个异常。一个简单的错误。现在我们知道了原因,让我们修复它!
所以我们更新了调用代码,更新了 getStatus
以处理 undefined
,并在我们的文档注释中添加了一个有用的警告
/**
* Get the status of a user
*
* Don't call this with undefined or null!
*/
export async function getStatus(user: User): Promise<string> {
if (!user) {
return '';
}
const id = user.id;
const result = await fetch(`/api/v0/${id}/status`);
const json = await result.json();
return json.status;
}
因为我们是真正的工程师,我们还编写了一个测试
it('should return empty status for undefined user', async () => {
assert.equals(getStatus(undefined), '');
});
太棒了!没有更多的崩溃。我们甚至又回到了 100% 的测试覆盖率!我们的代码现在一定是完美的。
几天过去了,然后:砰!有人在我们的日志中发现了一些奇怪的东西,大量的请求指向 /api/v0/undefined/status
。这是一个奇怪的用户名...
所以我们再次调查,再次修复代码,添加更多测试。也许还给那些调用 getStatus({ id: undefined })
的人发一封消极攻击性邮件。
/**
* Get the status of a user
*
* !!!
* WARNING: Don't call this with undefined or null, or with a user without an id
* !!!
*/
export async function getStatus(user: User): Promise<string> {
if (!user) {
return '';
}
const id = user.id;
if (typeof id !== 'string') {
return '';
}
const result = await fetch(`/api/v0/${id}/status`);
const json = await result.json();
return json.status;
}
完美。但是,为了确保万无一失,我们要求所有引入 getStatus
调用的更改都必须由高级工程师批准。这应该能永久阻止这些恼人的 bug...
也许这次我们会在下一次崩溃之前多过几天。甚至几个月。但是,除非我们的代码再也不改变,否则它总会发生。如果不是在这个特定的函数中,那么也会在我们的代码库的其他地方。
更糟糕的是,现在每一次更改都需要:防御性地检查 undefined
,更改测试或添加新测试,并获得团队批准。怎么回事?我们都尽力了,但仍然有 bug!一定有更好的方法。
识别隐患
虽然上面例子中的 bug 可能看起来很明显,但我们在开发 VS Code 时也遇到了同样类型的问题。每一次迭代,我们都会修复与意外的 undefined
相关的 bug。我们会添加测试。我们会发誓要做更好的工程师。这些都是传统的应对措施,但在下一次迭代中,它会再次发生。这不仅导致一些用户在使用 VS Code 时体验不佳,这些 bug 和我们对它们的应对也拖慢了我们开发新功能或更改现有源代码的速度。
我们意识到,我们需要以一种新的方式来理解我们的 bug,不是将它们视为孤立事件,而是将其视为更大问题的症状/信号。我们对这些 bug 的反应以及我们对无法快速行动的沮丧情绪也是症状。当我们开始讨论这些症状的根本原因时,我们发现了一些常见的现象
- 未能捕获简单的编程错误,例如访问
null
或undefined
上的属性。 - 接口定义不明确。哪些参数可以是
undefined
或null
,哪些函数可能返回undefined
或null
?通常,函数的实现者与调用者基于不同的假设集工作。 - 类型怪异之处。
undefined
vsnull
。undefined
vsfalse
。undefined
vs 空字符串。 - 感觉我们无法信任代码或安全地重构它。
识别根本原因是一个很好的第一步,但我们想更深入。在所有这些情况下,是什么隐患让一个善意的工程师首先引入了 bug?我们很快就发现了一个所有这些问题共有的明显隐患:VS Code 代码库中缺乏严格的空检查。
要理解严格空检查,您必须记住 TypeScript 的目标是为 JavaScript 添加类型。TypeScript 的 JavaScript 传统导致的一个结果是,默认情况下,TypeScript 允许将 undefined
和 null
用于任何值
// Without strict null checking, all of these calls are valid
getStatus(undefined); // Ok
getStatus(null); // Ok
getStatus({ id: undefined }); // Ok
虽然这种灵活性使得从 JavaScript 迁移到 TypeScript 变得更简单,但我们假想网站的示例库表明它也是一个隐患。这个隐患也是我们在 VS Code 上工作时发现的四个根本原因(以及许多其他原因)的核心。
值得庆幸的是,TypeScript 提供了一个名为严格空检查的选项,它将 undefined
和 null
视为不同的类型。使用严格空检查时,任何可能为空的类型都必须进行相应的标注
// With "strictNullCheck": true, all of these produce compile errors
getStatus(undefined); // Error
getStatus(null); // Error
getStatus({ id: undefined }); // Error
修复孤立的代码行或添加测试是一种被动解决方案,它只修复了那些特定的 bug。启用严格空检查是一种主动解决方案,它不仅可以修复我们每月报告的 bug,还可以防止未来发生此类 bug。不再忘记检查可选属性是否有值。不再质疑函数是否可以返回 null。好处显而易见。
制定增量计划
问题在于我们不能简单地启用编译器标志,然后一切都会神奇地修复。核心 VS Code 代码库有大约 1800 个 TypeScript 文件,包含超过 50 万行代码。用 "strictNullChecks": true
编译会产生大约 4500 个错误。啊!
此外,VS Code 是由一个小型核心团队组成的,我们喜欢快速行动。为了修复这 4500 个严格空错误而分支代码会增加大量的工程开销。而且从哪里开始呢?从上到下浏览错误列表?此外,分支中的更改对主线没有帮助,团队的大多数成员仍然会在主线工作。
我们希望制定一个计划,立即开始逐步为团队中的所有工程师带来严格空检查的好处。这样,我们可以将工作分解为可管理的更改,每次小小的更改都会使代码更加安全。
为此,我们创建了一个新的 TypeScript 项目文件,名为 tsconfig.strictNullChecks.json
,它启用了严格空检查,最初不包含任何文件。然后,我们有选择地将单个文件添加到该项目,修复这些文件中的严格空错误,然后提交更改。只要我们添加的文件要么没有导入,要么只导入其他已经进行严格空检查的文件,我们每次迭代只需修复少量错误。
{
"extends": "./tsconfig.base.json", // Shared configuration with our main `tsconfig.json`
"compilerOptions": {
"noEmit": true, // Don't output any javascript
"strictNullChecks": true
},
"files": [
// Slowly growing list of strict null check files goes here
]
}
虽然这个计划看起来合理,但一个问题是,在主线工作的工程师通常不会编译 VS Code 的严格空检查子集。为了防止意外地回滚到已经进行严格空检查的文件,我们添加了一个持续集成步骤,该步骤编译 tsconfig.strictNullChecks.json
。这确保了如果提交回滚了严格空检查,构建就会失败。
我们还编写了两个简单的脚本,以自动化与将文件添加到严格空检查项目相关的一些重复性任务。第一个脚本打印出符合严格空检查条件的文件列表。如果一个文件只导入了本身已经进行严格空检查的文件,则认为它符合条件。第二个脚本尝试自动将符合条件的文件添加到严格空项目。如果添加文件没有导致编译错误,则将其提交到 tsconfig.strictNullChecks.json
。
我们还考虑过自动化一些严格空修复本身,但最终我们放弃了。严格空错误通常是一个很好的信号,表明源代码应该重构。也许一个类型是可空的并没有很好的理由。也许调用者应该处理 null 而不是实现者。手动审查和修复这些错误让我们有机会改进代码,而不是强制使其与严格空检查兼容。
执行计划
在接下来的几个月里,我们慢慢扩大了进行严格空检查的文件数量。这通常是一项枯燥的工作。大多数严格空错误都很简单:只需添加空注解。对于其他错误,很难理解代码的意图。一个值是故意未初始化,还是实际上存在编程错误?
一般来说,我们尽量避免在主代码库中使用TypeScript 的非空断言。我们在测试中更自由地使用它,理由是如果测试代码中缺乏空检查会导致异常,那么测试无论如何都会失败。
整个过程中令人沮丧的一个方面是,VS Code 代码库中严格空错误的总体数量似乎从未减少。如果说有什么变化的话,那就是如果你在启用严格空检查的情况下编译所有 VS Code,我们所有的严格空工作似乎实际上导致错误总数增加了!这是因为严格空修复通常会产生连锁反应。正确地注解一个函数可以返回 undefined
可能会为该函数的所有消费者引入严格空错误。我们没有担心剩余错误的总体数量,而是专注于已经进行严格空检查的文件数量,并努力确保我们从未回滚这个总数。
同样重要的是要注意,启用严格空检查并不能神奇地阻止与严格空相关的异常发生。例如,any
类型或错误的类型转换可以轻易绕过严格空检查
// strictNullCheck: true
function double(x: number): number {
return x * 2;
}
double(undefined as any); // not an error
就像访问数组中越界元素一样
// strictNullCheck: true
function double(x: number): number {
return x * 2;
}
const arr = [1, 2, 3];
double(arr[5]); // not an error
此外,除非您还启用 TypeScript 的严格属性初始化,否则如果访问尚未初始化的成员,编译器不会报错
// strictNullCheck: true
class Value {
public x: number;
public setValue(x: number) {
this.x = x;
}
public double(): number {
return this.x * 2; // not an error even though `x` will be `undefined` if `setValue` has not been called yet
}
}
这项工作的目的绝不是要消除 VS Code 中 100% 的严格空错误——这即使不是不可能,也将极其困难——而是要防止绝大多数常见的严格空相关错误。这也是一个清理代码并使其更安全地重构的好机会。达到 95% 的目标对我们来说是可以接受的。
您可以在GitHub上找到我们完整的严格空检查计划及其执行情况。VS Code 团队的所有成员以及许多外部贡献者都参与了这项工作。作为这项工作的推动者,我进行了最多的严格空相关修复,但这只占用了我大约四分之一的工程时间。在此过程中肯定有一些痛苦,包括一些恼人的问题,即许多严格空回归只在提交后通过持续集成捕获。严格空工作也引入了一些新的 bug。然而,考虑到代码更改的数量,事情进展得异常顺利。
最终为整个 VS Code 代码库启用严格空检查的更改相当平淡:它修复了一些额外的代码错误,删除了 tsconfig.strictNullChecks.json
,并在我们的主 tsconfig
中设置了 "strictNullChecks": true
。缺乏戏剧性正是按计划进行的。就这样,VS Code 进行了严格空检查!
结论
当我向人们讲述这个项目时,我常听到的一个问题是:那它修复了多少 bug?我认为这个问题没有实际意义。在 VS Code 中,我们从未遇到过修复与缺乏严格空检查相关的 bug 的问题。通常,这涉及添加一个条件,也许还有一两个测试。但我们一遍又一遍地看到相同类型的 bug。修复这些 bug 不必要地拖慢了我们的速度,这意味着我们无法完全信任我们的代码。我们代码库中缺乏严格空检查是一个隐患,而 bug 只是这个隐患的症状。通过启用严格空检查,我们做了大量工作来防止一整类 bug,此外还为我们的代码库和工作方式带来了许多其他好处。
这篇文章的目的不是作为一个在大型代码库中启用严格空检查的教程。如果这个问题适用于您,希望您能看到以一种理智的方式实现它是可能的,而且不需要任何魔法。(我还要补充一点,如果您正在开始一个新的 TypeScript 项目,请为您的未来着想,默认使用 "strict": true
。)
我希望您能从中汲取教训,那就是,很多时候,对 bug 的反应要么是添加测试,要么是指责。“当然,Bob 在访问那个属性之前应该知道检查 undefined。”人们是好意的,但会犯错误。测试很有用,但也有成本,而且只测试我们编写它们来测试的东西。
相反,当您遇到 bug 或其他拖慢您速度的问题时,不要急于修复并转到下一个问题,而是停下来真正探究其原因。它的根本原因是什么?它揭示了哪些隐患?例如,也许您的源代码包含危险的编码模式,并且需要重构。然后以与影响成比例的方式解决隐患。您不需要重写所有内容。进行最少量的前期工作,并在有意义时进行自动化。减少隐患,让世界今天就逐步变得更好。
我们在 VS Code 的严格空检查方面采用了这种方法,将来也会将其应用于其他问题。我希望您也能发现它有用,无论您正在处理什么类型的项目。
编程愉快,
Matt Bierner,VS Code 团队成员 @mattbierner