Strict Null Checking Part II: The Testing!

[mjbvz]

A sequel to October's hit: #60565, Strict Null Checking VS Code

Backstory

We are incrementally working to enable TypeScript's strict null checks for the core of VS Code: #60565. Strict null checks help to catch silly programming mistakes and generally enforce a more explicit and safer coding style. Enabling strict null checks should reduce the number of undefined access exceptions VS Code users hit and help VS Code contributors move more quickly

This time

With #60565, we have already migrated over 800 files to be strict null checked but have not yet enabled strict null for many of our unit tests. That's where you can help out.

This item tracks enabling strict null checks for the *.test.ts files in the core VS Code codebase (under ./src). This work can be done on a file-by-file basis and should be fairly straightforward. If you are interested in contributing to VS Code, enabling strict null checking for a few test files is a great way to help out and a great way to learn more about the codebase.

How to contribute

1. Setup a VS Code development environment if you have not already done so: https://github.com/Microsoft/vscode/wiki/How-to-Contribute

2. Make sure you are on the latest commit of VS Code from master.

3. Pick a strict null check eligible test file from the list posted below.

4. Add the file in the files section of the src/tsconfig.strictNullChecks.json file in the VS Code codebase.

   "files": [
       ...,
   
       // To avoid merge conflicts, try to add your file in roughly sorted order
       "./vs/base/test/common/keyCodes.test.ts",
   
       ...
   ]

5. Run yarn strict-null-check, or yarn strict-null-check -- --watch to run the null checks in watch mode

6. Fix null check related errors. See the guidelines below for more details. Some files may only have a single error while others may have hundreds. We welcome all incremental fixes

7. Run the tests to make sure they still pass

8. Submit a PR with your change. Try to keep each PR to converting a single file

General guidelines for fixing strict null errors in tests

• Annotate nullable types and fix simple type errors

• Use the ! not null assertion to skip null checking when testing APIs that may return undefined. For example, if we were testing the divide function:

  function divide(a: number, b: number): { result: number } | undefined {
      return b === 0 ? undefined : { result: a / b };
  }
  
  test('div tests', () => {
      // Normally strict null checks would complain about accessing `.result` since
      // `divide` may return undefined. Use the ! not null assertion to suppress this
      // since the test will fail anyways if we try accessing `undefined.result`. 
      assert.strictEquals(divide(1, 2)!.result, 0.5);
  });

❗️ If you aren't sure about how to fix a given error, just ask or leave it unfixed. We can only merge PRs that have a passing build so if you do skip any errors, make sure your PR does not actually add the file to src/tsconfig.strictNullChecks.json.

(Also please do not close this issue in your PR. This issue will be left open until we have enable strict null checking for all tests)

[mjbvz]

List of eligible test files

Updated: December 24, 2018

• "./vs/editor/contrib/find/test/findModel.test.ts"
• "./vs/editor/contrib/suggest/test/suggestMemory.test.ts"
• "./vs/platform/commands/test/commands.test.ts"
• "./vs/platform/keybinding/test/common/abstractKeybindingService.test.ts"
• "./vs/workbench/parts/search/test/common/searchModel.test.ts"
• "./vs/workbench/parts/search/test/common/searchResult.test.ts"
• "./vs/workbench/parts/tasks/test/electron-browser/configuration.test.ts"
• "./vs/workbench/services/files/node/watcher/unix/test/chockidarWatcherService.test.ts"
• "./vs/workbench/services/keybinding/test/macLinuxKeyboardMapper.test.ts"
• "./vs/workbench/services/search/test/node/search.test.ts"