Skip to content

Enable no-explicit-any throughout production code #3353

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
robertbrignull merged 7 commits into from
Feb 14, 2024
Merged

Enable no-explicit-any throughout production code #3353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
55c8601
Disallow use of any by default
robertbrignull Feb 6, 2024
e3a6678
Disable no-explicit-any in test directory
robertbrignull Feb 6, 2024
a4294e2
Disable no-explicit-any in gulpfile.ts directory
robertbrignull Feb 7, 2024
334dbf5
Ignore any in CommandManager.ts
robertbrignull Feb 13, 2024
ac707ff
Ignore any in useEffectEvent.ts
robertbrignull Feb 13, 2024
ed96cbb
Use as unknown as ResultsPaths in ResultsApp.tsx
robertbrignull Feb 13, 2024
93e6c53
Merge eslintrc files
robertbrignull Feb 14, 2024
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
8 changes: 7 additions & 1 deletion extensions/ql-vscode/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const baseConfig = {
ignoreRestSiblings: false,
},
],
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-explicit-any": "error",
"@typescript-eslint/no-floating-promises": ["error", { ignoreVoid: true }],
"@typescript-eslint/no-invalid-this": "off",
"@typescript-eslint/no-shadow": "off",
Expand Down Expand Up @@ -188,5 +188,11 @@ module.exports = {
"import/no-namespace": ["error", { ignore: ["react"] }],
},
},
{
files: ["test/**/*", "gulpfile.ts/**/*"],
rules: {
"@typescript-eslint/no-explicit-any": "off",
},
},
],
};
1 change: 1 addition & 0 deletions extensions/ql-vscode/src/packages/commands/CommandManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { Disposable } from "./Disposable";
/**
* A command function is a completely untyped command.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Copy link
Copy Markdown
Contributor Author

@robertbrignull robertbrignull Feb 13, 2024

Choose a reason for hiding this comment

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

I couldn't find a good way around this so ended up just ignoring the eslint error. Admittedly I didn't try super hard, but I didn't want to end up with a solution that was unnecessarily complex either.

Copy link
Copy Markdown
Member

@koesie10 koesie10 Feb 14, 2024

Choose a reason for hiding this comment

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

I agree that this is the best solution, this is the same error that I ran into yesterday on the CachedOperation, where seemingly ...args: unknown[] isn't well supported.

export type CommandFunction = (...args: any[]) => Promise<unknown>;

/**
Expand Down
1 change: 1 addition & 0 deletions extensions/ql-vscode/src/view/common/SuggestBox/useEffectEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useCallback, useInsertionEffect, useRef } from "react";
*
* @param callback The callback to call when the event is triggered.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Copy link
Copy Markdown
Contributor Author

@robertbrignull robertbrignull Feb 13, 2024

Choose a reason for hiding this comment

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

We could rewrite this function to avoid any but I'm hesitant because I don't fully understand what it's doing and it's copied verbatim from another source. We could alternatively rewrite this.

export function useEffectEvent<T extends (...args: any[]) => any>(callback: T) {
const ref = useRef<T>(callback);

Expand Down
2 changes: 1 addition & 1 deletion extensions/ql-vscode/src/view/results/ResultsApp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export function ResultsApp() {
},
selectedTable: tableName,
},
origResultsPaths: undefined as any, // FIXME: Not used for interpreted, refactor so this is not needed
origResultsPaths: undefined as unknown as ResultsPaths, // FIXME: Not used for interpreted, refactor so this is not needed
Copy link
Copy Markdown
Contributor Author

@robertbrignull robertbrignull Feb 13, 2024

Choose a reason for hiding this comment

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

This whole situation in ResultsApp.tsx is a bit unfortunate. There are a few arguments where we pass undefined or other "fake" values. I briefly looked at trying to do the refactoring that this comment asks for but I found this area of code very confusing. For now I think doing as unknown as ResultsPaths gets the same effect and isn't any worse than as any. 🤷🏼

sortedResultsMap: new Map(), // FIXME: Not used for interpreted, refactor so this is not needed
database: msg.database,
interpretation: msg.interpretation,
Expand Down