Skip to content

Clean up as any type assertions in the codebase #269213

@mjbvz

Description

@mjbvz

We've seen a few build breaks caused by as any type assertions in our codebase. This can hide errors and break after mangling. Some language models seem to be specially keen on adding as any assertions to work around problems. This can break our mangling and results in more fragile code

We'd like to remove these as any cast in as many

Plan

The first priority is preventing new code from using as any. We'll then start addressing existing usages. Once complete we can also consider adding even stricter rules for type assertions

  • @mjbvz add a new eslint rule for a as any and <any>.
  • @mjbvz Will add suppression comments for all existing breakers. There are around 900 or so
  • 🏃 @mjbvz Will update this issue with a list of all files that have breaks
  • 🏃 Assigned team member review and fix these errors during debt week

Once complete:

  • Evaluate if we also want to enabled the @typescript-eslint/no-unsafe-type-assertion option for even stricter rules

Fixing as any

To find places in your code that need fixing, just search for: // eslint-disable-next-line local/code-no-any-casts

Here are some general patterns you can use to replace as any in code:

Use real type guards

This test code was using as any to simplify writing the test:

const tokens = store.root as any;
assert.strictEqual(tokens.children[0].token, 1);

Ww can use a real type check with instanceof to get correct error reporting and Intellisense:

const tokens = store.root;
assert.ok(tokens instanceof ListNode);

assert.strictEqual(tokens.children[0].token, 1);

Use more correct types

This as any was added because we need to cast an string to a AXPropertyName (a string union):

function createAXProperty(name: string, value: any, type: AXValueType = 'string'): AXProperty {
	return {
		name: name as any,
		value: createAXValue(type, value)
	};
}

If we just use proper types, we don't need the assertion:

function createAXProperty(name: AXPropertyName, value: any, type: AXValueType = 'string'): AXProperty {
	return {
		name: name,
		value: createAXValue(type, value)
	};
}

Add better types

This code uses as any to access some internal types:

// HACK: xterm.js doesn't expose this by design as it's an internal core
// function an embedder could easily do damage with. Additionally, this
// can't really be upstreamed since the event relies on shell integration to
// verify the shifting is necessary.
(this._terminal as any)._core._bufferService.buffer.lines.onDeleteEmitter.fire({
	index: this._terminal.buffer.active.baseY,
	amount: potentialShiftedLineCount
});

A better approach is to declare the shape of the type we're accessing:

interface ITerminalInternals extends Terminal {
	_core: ...;
}
	
// HACK: xterm.js doesn't expose this by design as it's an internal core
// function an embedder could easily do damage with. Additionally, this
// can't really be upstreamed since the event relies on shell integration to
// verify the shifting is necessary.
(this._terminal as ITerminalInternals)._core._bufferService.buffer.lines.onDeleteEmitter.fire({
	index: this._terminal.buffer.active.baseY,
	amount: potentialShiftedLineCount
});

You can also consider writing a helper function that returns obj is ITerminalInternals:

function isTerminalInterals(obj: unknown): obj is ITerminalInternals {
  return !!(obj as ITerminalInternals)._core;
}
const term = this.terminal;
if (isTerminalInternals(term)) {
  // term will for sure be an ITerminalInternals
}

Use unknown

If you are dealing with a value whose type you don't care about, use unknown instead. This will prevent accessing properties on the value

Assignments

Please reassign if these are incorrect

Built-in extension

Build scripts (/build/**/*)

Workbench/contrib

Metadata

Metadata

Labels

debtCode quality issues

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions