VS Codeのエージェントモードを拡張するには、を試してください！

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

2019年5月23日 マット・ビアナー (@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チェックを使用する場合、nullableになりうる型は、そのように注釈付けされなければなりません。

// 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エラーを修正するためにコードを分岐させることは、膨大なエンジニアリングのオーバーヘッドを追加したでしょう。そして、一体どこから始めるのでしょうか？エラーリストを上から下まで見ていく？さらに、ブランチでの変更はメイン（本流）に役立たず、チームの大多数はまだそこで作業していました。

私たちは、厳密な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
  ]
}

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

また、厳密なnullチェックされたプロジェクトにファイルを追加する際の繰り返し作業の一部を自動化するために、2つのシンプルなスクリプトも作成しました。最初のスクリプトは、厳密なnullチェックの対象となるファイルのリストを出力しました。ファイルは、自身が厳密なnullチェックされたファイルのみをインポートする場合に、対象と見なされます。2番目のスクリプトは、対象ファイルを厳密なnullプロジェクトに自動的に追加しようとしました。ファイルを追加してもコンパイルエラーが発生しない場合、それはtsconfig.strictNullChecks.jsonにコミットされました。

私たちは厳密なnull修正の一部を自動化することも検討しましたが、最終的にはこれに反対しました。厳密なnullエラーは、ソースコードがリファクタリングされるべきであるという良い兆候であることが多いからです。おそらく、型がnullableである理由が適切ではなかったのかもしれません。おそらく、実装者ではなく、呼び出し元がnullを処理すべきだったのかもしれません。これらのエラーを手動でレビューし修正することで、無理やり厳密なnull互換にするのではなく、コードを改善する機会が得られました。

計画の実行

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

一般的に、私たちはメインのコードベースでTypeScript の非nullアサーションを可能な限り使用しないように努めました。テストでは、テストコードでnullチェックが欠如しているために例外が発生すれば、いずれにせよテストは失敗するという考えから、より自由にそれを使用しました。

プロセス全体で気落ちする側面の一つは、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チェックされたのです！

まとめ

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

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

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

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

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

ハッピーコーディング、

マット・ビアナー、VS Code チームメンバー @mattbierner