Skip to content
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.

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

Remove suppressImplicitAnyIndexErrors #76442

Closed
22 of 35 tasks
mjbvz opened this issue Jul 2, 2019 · 10 comments
Closed
22 of 35 tasks

Remove suppressImplicitAnyIndexErrors #76442

mjbvz opened this issue Jul 2, 2019 · 10 comments
Assignees
Labels
debt Code quality issues
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 2, 2019

Although VS Code is now strict null checked, we currently set "suppressImplicitAnyIndexErrors": true. This setting allows using bracket accessors on values, even if the value's type does not specifically have an index access signature.

This is bad because it can easily end up hiding errors that TypeScript could have caught:

const map = new Map()
map.set('property', true);

...

if (map['property']) { // This is not an TS error but it does not do what you want!
    doImportantStuff();
} 

Fix

The good news is that there are only around 240 errors total and these errors are typically not too difficult to fix. Here's the process:

  1. In ./src/tsconfig.base.json, set "suppressImplicitAnyIndexErrors": false

  2. Restart the compile (if using watch)

  3. Open the file you wish to fix errors in and fix the errors:

    Potential fixes include:

    • Use a Map or Set instead of an object literal.

    • Add an index signature to the type

    • Cast to a type with an index signature (or any)

Here's the complete list of files with errors. I've assigned a few potential owners but feel free to claim others:

  • vs/base/common/console.ts @mjbvz
  • vs/base/parts/storage/node/storage.ts @mjbvz
  • vs/code/electron-browser/issue/issueReporterMain.ts @RMacfarlane
  • vs/code/electron-main/window.ts
  • vs/editor/standalone/browser/standaloneLanguages.ts @mjbvz
  • vs/platform/configuration/common/configuration.ts @sandy081
  • vs/platform/configuration/common/configurationModels.ts @sandy081
  • vs/platform/configuration/common/configurationRegistry.ts@sandy081
  • vs/platform/environment/node/environmentService.ts @sandy081
  • vs/platform/extensionManagement/node/extensionGalleryService.ts @sandy081
  • vs/platform/instantiation/common/instantiationService.ts @mjbvz
  • vs/platform/windows/electron-main/windowsService.ts @bpasero
  • vs/workbench/api/browser/mainThreadTreeViews.ts @sandy081
  • vs/workbench/api/common/extHostTreeViews.ts @sandy081
  • vs/workbench/browser/editor.ts @bpasero
  • vs/workbench/browser/parts/editor/editorStatus.ts @mjbvz
  • vs/workbench/browser/parts/views/viewsViewlet.ts @sandy081
  • vs/workbench/contrib/comments/browser/commentsEditorContribution.ts
  • vs/workbench/contrib/debug/browser/rawDebugSession.ts @isidorn
  • vs/workbench/contrib/debug/test/node/debugger.test.ts @isidorn
  • vs/workbench/contrib/experiments/electron-browser/experimentService.ts
  • vs/workbench/contrib/extensions/common/extensionQuery.ts @mjbvz
  • vs/workbench/contrib/extensions/electron-browser/extensionEditor.ts @sandy081
  • vs/workbench/contrib/localizations/browser/localizations.contribution.ts @sandy081
  • vs/workbench/contrib/preferences/browser/keybindingsEditor.ts @sandy081
  • vs/workbench/contrib/preferences/browser/settingsEditor2.ts @roblourens
  • vs/workbench/contrib/preferences/browser/settingsTree.ts @roblourens
  • vs/workbench/contrib/preferences/electron-browser/preferencesSearch.ts @roblourens
  • vs/workbench/contrib/quickopen/browser/viewPickerHandler.ts
  • vs/workbench/contrib/tasks/common/taskConfiguration.ts @alexr00
  • vs/workbench/electron-browser/window.ts @bpasero
  • vs/workbench/services/extensions/common/abstractExtensionService.ts @alexandrudima
  • vs/workbench/services/extensions/common/extensionsRegistry.ts @alexandrudima
  • vs/workbench/services/keybinding/common/keybindingEditing.ts @sandy081
  • vs/workbench/services/workspace/electron-browser/workspaceEditingService.ts @sandy081
@mjbvz mjbvz added the debt Code quality issues label Jul 2, 2019
@mjbvz mjbvz self-assigned this Jul 2, 2019
sbatten added a commit that referenced this issue Jul 2, 2019
mjbvz added a commit that referenced this issue Jul 2, 2019
alexr00 added a commit that referenced this issue Jul 2, 2019
jrieken added a commit that referenced this issue Jul 2, 2019
mjbvz added a commit that referenced this issue Jul 2, 2019
mjbvz added a commit that referenced this issue Jul 2, 2019
mjbvz added a commit that referenced this issue Jul 2, 2019
mjbvz added a commit that referenced this issue Jul 2, 2019
@joaomoreno joaomoreno added this to the July 2019 milestone Jul 3, 2019
joaomoreno added a commit that referenced this issue Jul 3, 2019
@aeschli
Copy link
Contributor

aeschli commented Jul 3, 2019

TypeScript seems to not understand iteration with in. @mjbvz What's the suggested worksaround?

interface MyObject {
   a: string;
   b: string;
}
const myObject : MyObject = { a: "val1", b: "val2" };
for (const key in myObject) {
    console.log(myObject[key])  // typescipt doesnt like this, why?
}

@aeschli
Copy link
Contributor

aeschli commented Jul 3, 2019

I started to use the following workaround to be able to use for ... in ...

interface MyObject {
   a: string;
   b: string;
}
const myObject : MyObject = { a: "val1", b: "val2" };
for (const objectKey in myObject) {
    const key = <keyof typeof myObject> objectKey;
    console.log(myObject[key]) 
}

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 3, 2019

@aeschli See this comment for more info about the issue: microsoft/TypeScript#31087 (comment)

roblourens added a commit that referenced this issue Jul 3, 2019
@sandy081 sandy081 self-assigned this Jul 4, 2019
mjbvz added a commit that referenced this issue Jul 9, 2019
mjbvz added a commit that referenced this issue Jul 9, 2019
@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 9, 2019

Only around 85 errors left. I've updated the list and assigned some initial owners

mjbvz added a commit that referenced this issue Jul 9, 2019
mjbvz added a commit that referenced this issue Jul 9, 2019
sandy081 added a commit that referenced this issue Jul 9, 2019
@sandy081 sandy081 removed their assignment Jul 9, 2019
alexr00 added a commit that referenced this issue Jul 10, 2019
bpasero added a commit that referenced this issue Jul 10, 2019
@mjbvz mjbvz closed this as completed in c228362 Jul 10, 2019
@dbaeumer
Copy link
Member

@mjbvz I compiled VS Code today using yarn run gulp compile and I receive a ton of compile errors. Looking at the list in the description not everyone seems to have the work done. Any reason why the compiler flag got already removed.

@bpasero
Copy link
Member

bpasero commented Jul 11, 2019

@dbaeumer run gulp mixin-server and try again.

@dbaeumer
Copy link
Member

Thanks.

@dbaeumer
Copy link
Member

@bpasero can I also 'unmix' ?

@aeschli
Copy link
Contributor

aeschli commented Jul 11, 2019

@dbaeumer git clean -xdf. Of course don't do this while you have uncommited changes.

@dbaeumer
Copy link
Member

@aeschli Thanks

@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

6 participants