Skip to content

Commit

Permalink
core: refine typing of isObject<T> (#12259)
Browse files Browse the repository at this point in the history
The template parameter for `isObject` is currently used to force cast
the value passed to the function, although there is no guarantee nor
check done to ensure that this is really the case.

This commit updates the typings so that the type passed to `isObject` is
used to guide TypeScript during subsequent field type checking,
assuming each field value is `unknown` and must also be type checked.

Example:

```ts
interface MyType {
    foo: string
}

const unknownValue: unknown = { foo: 2 };

if (isObject<MyType>(unknownValue)) {
    // We enter this branch because `unknownValue` is indeed an object,
    // and TypeScript now knows we might want to access the `foo` field
    // from our interface. But we can't yet assume to know the type of
    // `foo`, so it is still typed as `unknown` and we must check:
    if (isString(unknownValue.foo)) {
        // We won't get in this branch. But if we did, then `foo` is
        // now properly typed (and checked) as `string`.
    }
}
```
  • Loading branch information
paul-marechal authored Mar 9, 2023
1 parent 58248ac commit 610eb13
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 6 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/browser/shell/shell-layout-restorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,12 @@ export class ShellLayoutRestorer implements CommandContribution {
});
}
return widgets;
} else if (isObject<Record<string, WidgetDescription>>(value) && !Array.isArray(value)) {
} else if (isObject(value) && !Array.isArray(value)) {
const copy: Record<string, unknown> = {};
for (const p in value) {
if (this.isWidgetProperty(p)) {
parseContext.push(async context => {
copy[p] = await this.convertToWidget(value[p], context);
copy[p] = await this.convertToWidget(value[p] as WidgetDescription, context);
});
} else {
copy[p] = value[p];
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
export { ArrayUtils } from './array-utils';
export { Prioritizeable } from './prioritizeable';

type UnknownObject<T extends object> = Record<string | number | symbol, unknown> & { [K in keyof T]: unknown };

export type Deferred<T> = { [P in keyof T]: Promise<T[P]> };
export type MaybeArray<T> = T | T[];
export type MaybeNull<T> = { [P in keyof T]: T[P] | null };
Expand Down Expand Up @@ -54,7 +56,7 @@ export function isFunction<T extends (...args: unknown[]) => unknown>(value: unk
return typeof value === 'function';
}

export function isObject<T = Record<string | number | symbol, unknown>>(value: unknown): value is T {
export function isObject<T extends object>(value: unknown): value is UnknownObject<T> {
// eslint-disable-next-line no-null/no-null
return typeof value === 'object' && value !== null;
}
Expand Down
14 changes: 14 additions & 0 deletions packages/markers/src/common/marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
// *****************************************************************************

import { isObject, isString } from '@theia/core/lib/common/types';

/*
* A marker represents meta information for a given uri
*/
Expand All @@ -37,3 +39,15 @@ export interface Marker<T> {
*/
data: T;
}
export namespace Marker {
export function is(value: unknown): value is Marker<object>;
export function is<T>(value: unknown, subTypeCheck: (value: unknown) => value is T): value is Marker<T>;
export function is(value: unknown, subTypeCheck?: (value: unknown) => boolean): boolean {
subTypeCheck ??= isObject;
return isObject<Marker<object>>(value)
&& !Array.isArray(value)
&& subTypeCheck(value.data)
&& isString(value.uri)
&& isString(value.owner);
}
}
4 changes: 2 additions & 2 deletions packages/markers/src/common/problem-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface ProblemMarker extends Marker<Diagnostic> {
}

export namespace ProblemMarker {
export function is(node: Marker<object>): node is ProblemMarker {
return 'kind' in node && node.kind === PROBLEM_KIND;
export function is(node: unknown): node is ProblemMarker {
return Marker.is(node) && node.kind === PROBLEM_KIND;
}
}
5 changes: 4 additions & 1 deletion packages/plugin-ext/src/plugin/types-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,13 @@ export class Position {
return result!;
}

static isPosition(other: {}): other is Position {
static isPosition(other: unknown): other is Position {
if (!other) {
return false;
}
if (typeof other !== 'object' || Array.isArray(other)) {
return false;
}
if (other instanceof Position) {
return true;
}
Expand Down

0 comments on commit 610eb13

Please sign in to comment.