From 00f60fbfa19f79bf43074efce7b13f2106b7a974 Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Sat, 4 Mar 2023 00:28:12 -0500 Subject: [PATCH 1/5] core: refine typing of `isObject` 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 +++- .../src/browser/problem/problem-selection.ts | 2 +- packages/markers/src/common/marker.ts | 14 ++++++++++++++ packages/plugin-ext/src/plugin/types-impl.ts | 2 ++ 5 files changed, 22 insertions(+), 4 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..55f3e295853b3 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; + 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 & { [K in keyof T]: unknown } { // eslint-disable-next-line no-null/no-null return typeof value === 'object' && value !== null; } diff --git a/packages/markers/src/browser/problem/problem-selection.ts b/packages/markers/src/browser/problem/problem-selection.ts index 0651e5e0fb682..853b7674158a4 100644 --- a/packages/markers/src/browser/problem/problem-selection.ts +++ b/packages/markers/src/browser/problem/problem-selection.ts @@ -25,7 +25,7 @@ export interface ProblemSelection { } export namespace ProblemSelection { export function is(arg: unknown): arg is ProblemSelection { - return isObject(arg) && ProblemMarker.is(arg.marker); + return isObject(arg) && Marker.is(arg.marker) && ProblemMarker.is(arg.marker); } export class CommandHandler extends SelectionCommandHandler { 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/plugin-ext/src/plugin/types-impl.ts b/packages/plugin-ext/src/plugin/types-impl.ts index d3e50b97221fd..6fa74bed148db 100644 --- a/packages/plugin-ext/src/plugin/types-impl.ts +++ b/packages/plugin-ext/src/plugin/types-impl.ts @@ -546,6 +546,8 @@ export class Range { return true; } return isObject(arg) + && isObject(arg.start) + && isObject(arg.end) && Position.isPosition(arg.start) && Position.isPosition(arg.end); } From bfe4d71a916c89d77917d1e7d2809d3eef3f0cbe Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Mon, 6 Mar 2023 10:23:26 -0500 Subject: [PATCH 2/5] simplify typings --- packages/core/src/common/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/common/types.ts b/packages/core/src/common/types.ts index 55f3e295853b3..6fc5a93d326fe 100644 --- a/packages/core/src/common/types.ts +++ b/packages/core/src/common/types.ts @@ -17,7 +17,7 @@ export { ArrayUtils } from './array-utils'; export { Prioritizeable } from './prioritizeable'; -type UnknownObject = Record; +type UnknownObject = Record & { [K in keyof T]: unknown }; export type Deferred = { [P in keyof T]: Promise }; export type MaybeArray = T | T[]; @@ -56,7 +56,7 @@ export function isFunction unknown>(value: unk return typeof value === 'function'; } -export function isObject(value: unknown): value is UnknownObject & { [K in keyof T]: unknown } { +export function isObject(value: unknown): value is UnknownObject { // eslint-disable-next-line no-null/no-null return typeof value === 'object' && value !== null; } From 2ae368818ebc3b50b5ef3c87149cbdbdf62dc4dd Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Mon, 6 Mar 2023 10:33:42 -0500 Subject: [PATCH 3/5] retype ProblemMarker.is --- packages/markers/src/browser/problem/problem-selection.ts | 2 +- packages/markers/src/common/problem-marker.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/markers/src/browser/problem/problem-selection.ts b/packages/markers/src/browser/problem/problem-selection.ts index 853b7674158a4..0651e5e0fb682 100644 --- a/packages/markers/src/browser/problem/problem-selection.ts +++ b/packages/markers/src/browser/problem/problem-selection.ts @@ -25,7 +25,7 @@ export interface ProblemSelection { } export namespace ProblemSelection { export function is(arg: unknown): arg is ProblemSelection { - return isObject(arg) && Marker.is(arg.marker) && ProblemMarker.is(arg.marker); + return isObject(arg) && ProblemMarker.is(arg.marker); } export class CommandHandler extends SelectionCommandHandler { 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; } } From e29178987220f9434e6ac6e25989e8a415d399f1 Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Mon, 6 Mar 2023 11:27:13 -0500 Subject: [PATCH 4/5] retype Position.isPosition --- packages/plugin-ext/src/plugin/types-impl.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/plugin-ext/src/plugin/types-impl.ts b/packages/plugin-ext/src/plugin/types-impl.ts index 6fa74bed148db..c9f862d33bb46 100644 --- a/packages/plugin-ext/src/plugin/types-impl.ts +++ b/packages/plugin-ext/src/plugin/types-impl.ts @@ -404,7 +404,7 @@ export class Position { return result!; } - static isPosition(other: {}): other is Position { + static isPosition(other: unknown): other is Position { if (!other) { return false; } @@ -546,8 +546,6 @@ export class Range { return true; } return isObject(arg) - && isObject(arg.start) - && isObject(arg.end) && Position.isPosition(arg.start) && Position.isPosition(arg.end); } From 62c89923819c68bd6380837da77b1d7806d3c2d2 Mon Sep 17 00:00:00 2001 From: Paul Marechal Date: Mon, 6 Mar 2023 12:49:21 -0500 Subject: [PATCH 5/5] make isPosition more robust --- packages/plugin-ext/src/plugin/types-impl.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/plugin-ext/src/plugin/types-impl.ts b/packages/plugin-ext/src/plugin/types-impl.ts index c9f862d33bb46..ea589b4d176d7 100644 --- a/packages/plugin-ext/src/plugin/types-impl.ts +++ b/packages/plugin-ext/src/plugin/types-impl.ts @@ -408,6 +408,9 @@ export class Position { if (!other) { return false; } + if (typeof other !== 'object' || Array.isArray(other)) { + return false; + } if (other instanceof Position) { return true; }