Skip to content

vscode extension: migrate from any to unknown where possible #2989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 2, 2020
Merged

vscode extension: migrate from any to unknown where possible #2989

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12d0970
vscode extension: migrate from any to unknown where possible
Feb 2, 2020
5411d65
vscode: fix, fallback to any for cmd type
Feb 2, 2020
2fd7af2
vscode: use void where possible
Feb 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion editors/code/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ PATH=${process.env.PATH}
// This also requires considering our settings strategy, which is work which needs doing
// @ts-ignore The tracer is private to vscode-languageclient, but we need access to it to not log publishDecorations requests
res._tracer = {
log: (messageOrDataObject: string | any, data?: string) => {
log: (messageOrDataObject: string | unknown, data?: string) => {
if (typeof messageOrDataObject === 'string') {
if (
messageOrDataObject.includes(
Expand Down
2 changes: 1 addition & 1 deletion editors/code/src/color_theme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class ColorTheme {
}

function loadThemeNamed(themeName: string): ColorTheme {
function isTheme(extension: vscode.Extension<any>): boolean {
function isTheme(extension: vscode.Extension<unknown>): boolean {
return (
extension.extensionKind === vscode.ExtensionKind.UI &&
extension.packageJSON.contributes &&
Expand Down
2 changes: 1 addition & 1 deletion editors/code/src/commands/syntax_tree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function syntaxTree(ctx: Ctx): Cmd {

// We need to order this after LS updates, but there's no API for that.
// Hence, good old setTimeout.
function afterLs(f: () => any) {
function afterLs(f: () => unknown) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this maybe should be => void, or whats the TypeScript alternative for -> ()?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this is just a horrible hack which needs to die in flames...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let me revisit this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, void is semantically -> (), but not the unit type itself (). It is quite a special language item that is meant to be used only for the function return type and denotes that the value should be discarded (also function with void return type is covariant to other functions with different return type, in which way void type is different from undefined type).

I would say that the concept of unit type the way it is present in Rust is not implemented in TypeScripts type system.

setTimeout(f, 10);
}

Expand Down
12 changes: 6 additions & 6 deletions editors/code/src/ctx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ export class Ctx {
overrideCommand(name: string, factory: (ctx: Ctx) => Cmd) {
const defaultCmd = `default:${name}`;
const override = factory(this);
const original = (...args: any[]) =>
const original = (...args: unknown[]) =>
vscode.commands.executeCommand(defaultCmd, ...args);
try {
const d = vscode.commands.registerCommand(
name,
async (...args: any[]) => {
async (...args: unknown[]) => {
if (!(await override(...args))) {
return await original(...args);
}
Expand All @@ -73,11 +73,11 @@ export class Ctx {
}
}

get subscriptions(): { dispose(): any }[] {
get subscriptions(): { dispose(): unknown }[] {
return this.extCtx.subscriptions;
}

pushCleanup(d: { dispose(): any }) {
pushCleanup(d: { dispose(): unknown }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this wants to be a : void?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, yes you are right, now when I am looking at it, this seems reasonable

this.extCtx.subscriptions.push(d);
}

Expand All @@ -86,12 +86,12 @@ export class Ctx {
}
}

export type Cmd = (...args: any[]) => any;
export type Cmd = (...args: any[]) => unknown;

export async function sendRequestWithRetry<R>(
client: lc.LanguageClient,
method: string,
param: any,
param: unknown,
token?: vscode.CancellationToken,
): Promise<R> {
for (const delay of [2, 4, 6, 8, 10, null]) {
Expand Down