getApplicableRefactors returns "Install missing types package" quickfix

TypeScript Version: 2.6.1-insiders.20171019

Code

In a empty project with a package.json that includes lodash but not @types/lodash. Try writing an import:

import _ from 'lodash'

Request quickfixs on 'lodash'

Bug

getApplicableRefactors is also returning a quick fix here for Install missing types package

Trace  - 11:13:01 AM] Sending request: getApplicableRefactors (51). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "/Users/matb/projects/san/src/test.ts",
    "startLine": 1,
    "startOffset": 15,
    "endLine": 1,
    "endOffset": 23
}
[Trace  - 11:13:01 AM] Response received: getApplicableRefactors (51). Request took 2 ms. Success: true 
Result: [
    {
        "name": "Install missing types package",
        "description": "Install missing types package",
        "actions": [
            {
                "description": "Install '@types/lodash'",
                "name": "install"
            }
        ]
    }
]
[Trace  - 11:13:01 AM] Sending request: getCodeFixes (52). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "/Users/matb/projects/san/src/test.ts",
    "startLine": 1,
    "startOffset": 15,
    "endLine": 1,
    "endOffset": 23,
    "errorCodes": [
        2307
    ]
}
[Trace  - 11:13:01 AM] Response received: getCodeFixes (52). Request took 2 ms. Success: true 
Result: [
    {
        "description": "Install '@types/lodash'",
        "changes": [],
        "commands": [
            {
                "type": "install package",
                "packageName": "@types/lodash"
            }
        ]
    }
]

Mohamed Hegazy (@mhegazy)

I've added a workaround for this on the VS Code side but believe this should be addressed for 2.6.1. Mohamed Hegazy (@mhegazy) and Andy (Andrewkraft) (@Andy-MS) Do you agree?

Yes, this causes us to show the same code action twice, once from the quickfix for the error and once from the refactoring.

I believe that this is a quickfix and should not be returned as a refactoring. We previously used to send compute quickFix requests to TS even if there were no errors in the currently selected code, but disabled this for performance reasons in TS 2.4. Should we look into enabling this again?

Yes that's what I'm trying to say. We currently don't even call getCodeFixes if there are no errors in the selected code:

	public async provideCodeActions(
		document: TextDocument,
		range: Range,
		context: CodeActionContext,
		token: CancellationToken
	): Promise<Command[]> {
		const file = this.client.normalizePath(document.uri);

		// This part was added when we picked up TS 2.4		
		const supportedActions = await this.getSupportedActionsForContext(context);
		if (!supportedActions.size) {
			return [];
		}

		const args: Proto.CodeFixRequestArgs = {
			...vsRangeToTsFileRange(file, range),
			errorCodes: Array.from(supportedActions)
		};
		const response = await this.client.execute('getCodeFixes', args, token);
		return (response.body || []).map(action => this.getCommandForAction(action, file));
	}

We can remove the size check so that we still call getCodeFixes even if there are no error codes.

If we want to support the no implicit any case, I propose we change the getCodeFix to support empty error code requests. If that change would be too complex for 2.6, we can ship with the no implicit any case not being supported yet and look into handling it in a future release. However I feel that we need to remove this quick fix from the getApplicableRefactors response for 2.6

Matt Bierner (@mjbvz)

Matt Bierner (@mjbvz) do you happen to have --strict in your project? or did you get this config file from tsc --init (which adds --strict)?

If so, i think i have a fix for this issue.

No, my tsconfig was:

{
    "compilerOptions": {
        "target": "ES6",
        "allowJs": true
    },
    "exclude": [
        "node_modules",
        "**/node_modules/*"
    ]
}

Andy (Andrewkraft) (@Andy-MS)

I am unable to reproduce this locally with the setup mentioned above with 1.18-insiders and latest TS.

Andy (Andrewkraft) (@Andy-MS) , seems from your comments, you understand the issue here. can you elaborate on how i can reproduce this locally?

aah.. now i see. why do we have the quick fix associated with Diagnostics.Cannot_find_module_0.code ? it should only be associated with Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type.code.. or am i missing something?

Sorry, was stuck trying to understand the issue and did not look at the change.

Fixed by #19394.

The original issue here was not addressed and the difference in semantics between quick fixes and refactorings is causing problems on our side (see microsoft/vscode#37288)

I still would like install Types (@types) to be returned only as a quick fix. See my comment on the PR for more info: #19394 (comment)

We are working on adding a new non-error diagnostic category, and moving all these refactoring to quick fix only. this should address the problem.

Navigation Menu

Repository navigation

getApplicableRefactors returns "Install missing types package" quickfix #19378

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

getApplicableRefactors returns "Install missing types package" quickfix #19378

Description

Activity

mjbvz commented on Oct 20, 2017

mjbvz commented on Oct 20, 2017

mjbvz commented on Oct 20, 2017

mjbvz commented on Oct 20, 2017

mhegazy commented on Oct 20, 2017

mjbvz commented on Oct 20, 2017

mhegazy commented on Oct 20, 2017

mhegazy commented on Oct 21, 2017

mhegazy commented on Oct 23, 2017

mhegazy commented on Oct 24, 2017

mjbvz commented on Dec 8, 2017

mhegazy commented on Jan 11, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions