Visual Studio Codeの厳密なnullチェック

2019年5月23日 by Matt Bierner, @mattbierner

安全性がスピードを可能にする

素早く動くことは楽しいことです。新しい機能をリリースし、ユーザーを喜ばせ、コードベースを改善するのは楽しいことです。しかし、同時に、バグのある製品をリリースするのは楽しいことではありません。問題報告を受けたり、午前3時にインシデントで起こされたりするのを好む人はいません。

素早く動くことと安定したコードを出荷することは、しばしば相容れないものとして提示されますが、そうであるべきではありません。多くの場合、コードを脆弱でバグだらけにする要因は、開発を遅らせる要因と同じです。結局のところ、常に何かを壊すことを心配していたら、どうして素早く動けるでしょうか？

この記事では、VS Codeチームが最近完了した主要なエンジニアリングの取り組み、つまり、TypeScriptの厳密なnullチェックをコードベースで有効にすることについて共有したいと思います。私たちは、この作業によって、より速く動き、より安定した製品を出荷できるようになると信じています。厳密なnullチェックを有効にした動機は、バグを孤立したイベントとしてではなく、ソースコード内のより大きな危険の兆候として理解することにありました。厳密なnullチェックをケーススタディとして、私たちの作業の動機、問題に対処するための段階的なアプローチをどのように考案したか、そして修正をどのように実装していったかについて説明します。この危険を特定し、削減するための同じ一般的なアプローチは、どんなソフトウェアプロジェクトにも適用できます。

例

厳密なnullチェックを有効にする前に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の呼び出しを導入するすべての変更は、シニアエンジニアによって承認されることを必須としましょう。これで、これらの厄介なバグは永久に止まるはずです...

そして、今回は次のクラッシュまでにもう数日、もしかしたら数ヶ月持つかもしれません。しかし、私たちのコードが二度と変更されない限り、バグは発生するでしょう。この特定の関数でなくても、コードベースのどこか別の場所で。

さらに悪いことに、すべての変更には、undefinedの防御的なチェック、テストの変更または新しいテストの追加、そしてチームの承認が必要になります。どういうことでしょう？私たちは皆、自分の役割を果たしているのに、まだバグがあるのです！もっと良い方法があるはずです。

危険の特定

上記の例のバグは明白に見えるかもしれませんが、VS Codeの開発中に同じ種類の問題に遭遇していました。イテレーションごとに、予期しないundefinedに関連するバグを修正していました。そして、テストを追加しました。そして、より良いエンジニアになることを誓いました。それらはすべて伝統的な対応ですが、次のイテレーションで、それはまた起こりました。これは一部のユーザーにVS Codeで不快な体験をさせていただけでなく、これらのバグとそれに対する私たちの対応は、新機能に取り組んだり、既存のソースコードを変更したりする際の作業を遅らせていました。

私たちは、バグを新しい方法で理解し始める必要があることに気づきました。孤立したイベントとしてではなく、より大きな問題の症状/兆候としてです。これらのバグに対する私たちの対応や、迅速に動けないことへの不満もまた症状でした。これらの症状の根本原因について議論し始めたとき、私たちはいくつかの共通点を見つけました。

nullやundefinedのプロパティへのアクセスなど、単純なプログラミングミスをキャッチできないこと。
インターフェースの仕様が不十分であること。どのパラメータがundefinedやnullになり得るのか、どの関数がundefinedやnullを返す可能性があるのか？多くの場合、関数の実装者は呼び出し側とは異なる前提で作業していました。
型の奇妙さ。undefined 対 null。undefined 対 false。undefined 対空文字列。
コードを信頼できず、安全にリファクタリングできないと感じること。

根本原因を特定することは良い第一歩でしたが、私たちはさらに深く掘り下げたいと考えました。これらすべてのケースで、善意のエンジニアがそもそもバグを混入させてしまった危険は何だったのでしょうか？そして、私たちはすぐに、これらすべての問題に共通する明白な危険を特定しました。それは、VS Codeのコードベースにおける厳密なnullチェックの欠如です。

厳密なnullチェックを理解するには、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で作業する上で特定した4つの根本原因（その他多数）の中心でもありました。

しかし幸いなことに、TypeScriptには厳密なnullチェックと呼ばれるオプションがあり、これによりundefinedとnullが別個の型として扱われます。厳密なnullチェックを使用する場合、null許容である可能性のある型は、そのように注釈を付ける必要があります。

// With "strictNullCheck": true, all of these produce compile errors

getStatus(undefined); // Error
getStatus(null); // Error
getStatus({ id: undefined }); // Error

コードの孤立した行を修正したり、テストを追加したりすることは、特定のバグのみを修正する対症療法的な解決策でした。厳密なnullチェックを有効にすることは、私たちが毎月報告されていたバグを修正するだけでなく、これらのバグのクラス全体が将来発生するのを防ぐ予防的な解決策です。オプショナルなプロパティに値があるかどうかをチェックし忘れることはもうありません。関数がnullを返すことができるかどうかを疑問に思うことももうありません。その利点は明らかでした。

段階的な計画の考案

問題は、コンパイラフラグを有効にするだけで、すべてが魔法のように修正されるわけではないということでした。VS Codeの中核となるコードベースには約1800のTypeScriptファイルがあり、50万行以上で構成されています。これを"strictNullChecks": trueでコンパイルすると、約4500のエラーが発生しました。うへぇ！

さらに、VS Codeは少人数のコアチームで構成されており、私たちは迅速に動くことを好みます。その4500件の厳密なnullエラーを修正するためにコードをブランチすると、膨大な量のエンジニアリングオーバーヘッドが追加されたでしょう。そして、どこから始めればよいのでしょうか？エラーのリストを上から順に見ていく？さらに、ブランチでの変更は、チームの大部分がまだ作業しているmainブランチの助けにはなりません。

私たちは、チームのすべてのエンジニアに、すぐに厳密なnullチェックの利点を段階的にもたらす計画を望んでいました。そうすれば、作業を管理可能な変更に分割し、それぞれの小さな変更でコードを少しずつ安全にすることができます。

これを実現するために、私たちはtsconfig.strictNullChecks.jsonという新しいTypeScriptプロジェクトファイルを作成しました。これは厳密なnullチェックを有効にし、最初はゼロ個のファイルで構成されていました。次に、個々のファイルを選択的にこのプロジェクトに追加し、それらのファイル内の厳密なnullエラーを修正し、その変更をチェックインしました。インポートがないか、または既に厳密なnullチェック済みのファイルのみをインポートするファイルを追加する限り、各イテレーションで修正する必要があるエラーは少数でした。

{
  "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
  ]
}

この計画は妥当に見えましたが、1つの問題は、mainで作業しているエンジニアが通常、VS Codeの厳密なnullチェック済みのサブセットをコンパイルしないことでした。既に厳密なnullチェック済みのファイルへの意図しないリグレッションを防ぐために、tsconfig.strictNullChecks.jsonをコンパイルする継続的インテグレーションのステップを追加しました。これにより、厳密なnullチェックをリグレッションさせるチェックインがビルドを壊すことが保証されました。

また、厳密なnullチェック済みプロジェクトにファイルを追加する際に関連するいくつかの反復的なタスクを自動化するために、2つの簡単なスクリプトを用意しました。最初のスクリプトは、厳密なnullチェックの対象となるファイルのリストを出力しました。ファイルは、それ自体が厳密なnullチェック済みであるファイルのみをインポートする場合に対象と見なされます。2番目のスクリプトは、対象となるファイルを厳密なnullプロジェクトに自動的に追加しようとしました。ファイルを追加してもコンパイルエラーが発生しない場合は、tsconfig.strictNullChecks.jsonにコミットされました。

厳密なnull修正自体の一部を自動化することも検討しましたが、最終的にはこれを採用しませんでした。厳密なnullエラーは、多くの場合、ソースコードをリファクタリングすべきだという良いシグナルです。型がnull許容であることに正当な理由がなかったのかもしれません。実装者ではなく、呼び出し側がnullを処理すべきだったのかもしれません。これらのエラーを手動でレビューして修正することで、無理やり厳密なnull互換にするのではなく、コードをより良くする機会が得られました。

計画の実行

次の数ヶ月にわたって、私たちは厳密なnullチェック済みのファイルの数をゆっくりと増やしていきました。これはしばしば退屈な作業でした。ほとんどの厳密なnullエラーは単純なもので、nullのアノテーションを追加するだけでした。他のものについては、コードの意図を理解するのが困難でした。値が意図的に初期化されていないのか、それとも実際にプログラミングミスがあるのか？

一般的に、私たちはメインのコードベースではTypeScriptの非nullアサーションをできるだけ使用しないように努めました。テストではより自由に使用しましたが、その理由は、テストコードでのnullチェックの欠如が例外を引き起こす場合、テストはいずれにせよ失敗するからです。

プロセス全体で意気消沈する側面の1つは、VS Codeのコードベースにおける厳密なnullエラーの総数が決して減少しないように見えたことでした。どちらかといえば、VS Code全体を厳密なnullチェックを有効にしてコンパイルすると、私たちの厳密なnullの作業はすべて、実際にはエラーの総数を増やしているように見えました！これは、厳密なnull修正がしばしば連鎖的な影響を及ぼすためです。関数がundefinedを返す可能性があることを正しく注釈付けすると、その関数のすべてのコンシューマに対して厳密なnullエラーが発生する可能性があります。残りのエラーの総数を心配するのではなく、私たちはすでに厳密なnullチェック済みのファイルの数に焦点を当て、この合計を決して後退させないように努めました。

厳密なnullチェックを有効にしても、厳密なnull関連の例外が二度と発生しないように魔法のように防ぐわけではないことに注意することも重要です。たとえば、any型や不正なキャストは、厳密なnullチェックを簡単に迂回できます。

// 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における厳密なnullエラーを100%排除することではありませんでした。それは不可能ではないにしても、非常に困難でしょう。目的は、一般的な厳密なnull関連エラーの大部分を防ぐことでした。また、コードをクリーンアップし、リファクタリングをより安全にする良い機会でもありました。95%まで達成できれば、私たちにとっては許容範囲でした。

私たちの厳密なnullチェック計画全体とその実行は、GitHubでご覧いただけます。VS Codeチームの全メンバーと多くの外部コントリビューターがこの取り組みに参加しました。この作業の推進者として、私は最も多くの厳密なnull関連の修正を行いましたが、それでも私のエンジニアリング時間の約4分の1を費やしただけでした。途中には確かにいくつかの苦労があり、多くの厳密なnullのリグレッションがチェックイン後に継続的インテグレーションによってのみ検出されることへの苛立ちもありました。厳密なnullの作業はいくつかの新しいバグももたらしました。しかし、変更されたコードの量を考えると、物事は驚くほどうまく進みました。

最終的にVS Codeのコードベース全体で厳密なnullチェックを有効にした変更は、むしろあっけないものでした。いくつかのコードエラーを修正し、tsconfig.strictNullChecks.jsonを削除し、メインのtsconfigで"strictNullChecks": trueを設定しました。ドラマがなかったのは、まさに計画通りでした。そして、それをもって、VS Codeは厳密なnullチェック済みになりました！

まとめ

このプロジェクトについて人々に話すときによく聞かれる質問の1つは、「それで、いくつのバグを修正したのですか？」です。私はその質問はあまり意味がないと思います。VS Codeでは、厳密なnullチェックの欠如に関連するバグの修正に問題があったことはありませんでした。通常、条件文と、場合によっては1つか2つのテストを追加するだけでした。しかし、私たちは同じ種類のバグを何度も何度も目にし続けました。これらのバグを修正することは、私たちの速度を不必要に落とし、コードを完全に信頼できないことを意味していました。コードベースに厳密なnullチェックがなかったことは危険であり、バグはその危険の症状に過ぎませんでした。厳密なnullチェックを有効にすることで、私たちはバグのクラス全体を防ぐための重要な作業を行っただけでなく、コードベースと作業スタイルに他の多くの利点をもたらしました。

この記事の目的は、大規模なコードベースで厳密なnullチェックを有効にするためのチュートリアルではありません。もしこの問題があなたに当てはまるなら、魔法を使わずにまともな方法でそれが可能であることがお分かりいただけたと思います。（新しいTypeScriptプロジェクトを始めるなら、将来の自分のために、デフォルトで"strict": trueから始めることを付け加えておきます。）

私が皆さんに持ち帰ってほしいのは、あまりにも多くの場合、バグへの対応はテストを追加するか、誰かを非難するかのどちらかであるということです。「ボブは当然、そのプロパティにアクセスする前にundefinedをチェックすべきだった」。人々は善意で行動しますが、間違いを犯します。テストは有用ですが、コストもかかり、私たちが書いたテストしか行いません。

代わりに、バグやあなたを遅くさせている何かに遭遇したときは、急いで修正して次の問題に移るのではなく、一瞬立ち止まって、何がそれを引き起こしたのかを本当に探求してください。その根本原因は何でしたか？それはどのような危険を明らかにしますか？例えば、ソースコードに危険なコーディングパターンが含まれていて、リファクタリングが必要かもしれません。そして、その影響に見合った方法で危険に対処するために作業してください。すべてを書き直す必要はありません。必要な最小限の先行作業を行い、意味がある場合は自動化してください。危険を減らし、今日の世界を段階的により良くしていきましょう。

私たちはVS Codeの厳密なnullチェックでこのアプローチを取りましたし、将来的には他の問題にも適用するでしょう。皆さんがどのような種類のプロジェクトに取り組んでいるかに関わらず、これが役立つことを願っています。

ハッピーコーディング、

Matt Bierner、VS Codeチームメンバー @mattbierner