From 610eb130d4a032d84ad64ee55dc5614aa069bbf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Thu, 9 Mar 2023 11:56:46 -0500 Subject: [PATCH] core: refine typing of `isObject` (#12259) 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(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`. } } ``` --- .../src/browser/shell/shell-layout-restorer.ts | 4 ++-- packages/core/src/common/types.ts | 4 +++- packages/markers/src/common/marker.ts | 14 ++++++++++++++ packages/markers/src/common/problem-marker.ts | 4 ++-- packages/plugin-ext/src/plugin/types-impl.ts | 5 ++++- 5 files changed, 25 insertions(+), 6 deletions(-) diff --git a/packages/core/src/browser/shell/shell-layout-restorer.ts b/packages/core/src/browser/shell/shell-layout-restorer.ts index e7db537a0d0ea..8b7d42df347d5 100644 --- a/packages/core/src/browser/shell/shell-layout-restorer.ts +++ b/packages/core/src/browser/shell/shell-layout-restorer.ts @@ -278,12 +278,12 @@ export class ShellLayoutRestorer implements CommandContribution { }); } return widgets; - } else if (isObject>(value) && !Array.isArray(value)) { + } else if (isObject(value) && !Array.isArray(value)) { const copy: Record = {}; 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]; diff --git a/packages/core/src/common/types.ts b/packages/core/src/common/types.ts index fff7231299fce..6fc5a93d326fe 100644 --- a/packages/core/src/common/types.ts +++ b/packages/core/src/common/types.ts @@ -17,6 +17,8 @@ export { ArrayUtils } from './array-utils'; export { Prioritizeable } from './prioritizeable'; +type UnknownObject = Record & { [K in keyof T]: unknown }; + export type Deferred = { [P in keyof T]: Promise }; export type MaybeArray = T | T[]; export type MaybeNull = { [P in keyof T]: T[P] | null }; @@ -54,7 +56,7 @@ export function isFunction unknown>(value: unk return typeof value === 'function'; } -export function isObject>(value: unknown): value is T { +export function isObject(value: unknown): value is UnknownObject { // eslint-disable-next-line no-null/no-null return typeof value === 'object' && value !== null; } diff --git a/packages/markers/src/common/marker.ts b/packages/markers/src/common/marker.ts index 55e3c2e9bdaa6..4a5bf25f717bc 100644 --- a/packages/markers/src/common/marker.ts +++ b/packages/markers/src/common/marker.ts @@ -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 */ @@ -37,3 +39,15 @@ export interface Marker { */ data: T; } +export namespace Marker { + export function is(value: unknown): value is Marker; + export function is(value: unknown, subTypeCheck: (value: unknown) => value is T): value is Marker; + export function is(value: unknown, subTypeCheck?: (value: unknown) => boolean): boolean { + subTypeCheck ??= isObject; + return isObject>(value) + && !Array.isArray(value) + && subTypeCheck(value.data) + && isString(value.uri) + && isString(value.owner); + } +} diff --git a/packages/markers/src/common/problem-marker.ts b/packages/markers/src/common/problem-marker.ts index 1c0493dbffb61..a9e56a4efbecb 100644 --- a/packages/markers/src/common/problem-marker.ts +++ b/packages/markers/src/common/problem-marker.ts @@ -24,7 +24,7 @@ export interface ProblemMarker extends Marker { } export namespace ProblemMarker { - export function is(node: Marker): 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; } } diff --git a/packages/plugin-ext/src/plugin/types-impl.ts b/packages/plugin-ext/src/plugin/types-impl.ts index d3e50b97221fd..ea589b4d176d7 100644 --- a/packages/plugin-ext/src/plugin/types-impl.ts +++ b/packages/plugin-ext/src/plugin/types-impl.ts @@ -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; }