Pvb/codeaction/api #10185

paulvanbrenk · 2016-08-05T23:42:06Z

Add CodeAction API

vladima · 2016-08-05T23:43:10Z

src/compiler/core.ts

@@ -115,6 +115,15 @@ namespace ts {
        return -1;
    }

+    export function firstOrUndefined<T>(array: T[], predicate: (x: T) => boolean): T {


forEach is slightly different. Also this is usually called find.

vladima · 2016-08-05T23:47:22Z

src/services/codefixes/codeFixProvider.ts

+                const fixes = codeFixes[context.errorCode];
+                let allActions: CodeAction[] = [];
+
+                Debug.assert(fixes && fixes.length > 0, "No fixes found for error: '${errorCode}'.");


isn't it too strict?

…eScript into pvb/codeaction/api

mhegazy · 2016-08-29T23:29:12Z

src/services/codefixes/superFixes.ts

+            const superCall = findSuperCall((<ConstructorDeclaration>constructor).body);
+            Debug.assert(!!superCall, "Failed to find super call.");
+
+            const newPosition = getOpenBraceEnd(<ConstructorDeclaration>constructor, sourceFile);


can you add a test for the case where this is an argument to super

super(this.x);

mhegazy · 2016-09-01T22:44:41Z

Please add this to the tsserver, you will need to define a new request and response kinds in server\protocol.d.ts, and add a handler for it in server\session.ts

mhegazy · 2016-09-01T22:45:51Z

src/services/codefixes/references.ts

@@ -0,0 +1,3 @@
+///<reference path='..\services.ts' />


we stoped using references for a while. they should be picked up by the tsconfig.json in this folder any ways. so please remove this.

# Conflicts: # src/compiler/diagnosticMessages.json # src/services/services.ts

…eScript into pvb/codeaction/api

# Conflicts: # src/services/types.ts

mhegazy · 2016-10-07T00:08:35Z

src/server/protocol.d.ts

@@ -204,6 +204,53 @@ declare namespace ts.server.protocol {
    }

    /**
+      * Request for the available codefixed at a specific position.


nit Code Fixes instead of codefixed

mhegazy · 2016-10-07T00:14:36Z

src/server/protocol.d.ts

+        textChanges: CodeEdit[];
+    }
+
+    export interface CodeActionResponse extends Response {


CodeFixResponse

mhegazy · 2016-10-07T00:14:53Z

src/server/protocol.d.ts

+        /** Description of the code action to display in the UI of the editor */
+        description: string;
+        /** Text changes to apply to each file as part of the code action */
+        changes: FileCodeEdits[];


CodeAction [] ?

mhegazy · 2016-10-07T00:15:12Z

src/server/protocol.d.ts

+        /**
+          * Changes to apply to each file as part of the code action.
+          */
+        changes: FileTextChanges[];


CodeEdit [] instead.

see formatting for example.

mhegazy · 2016-10-07T00:15:52Z

src/server/protocol.d.ts

+        textChanges: TextChange[];
+    }
+
+    export class TextChange {


not needed.

mhegazy · 2016-10-07T00:19:00Z

src/server/session.ts

+                    description: source.description,
+                    changes: source.changes.map(change => ({
+                        fileName: change.fileName,
+                        textChanges: change.textChanges.map(textChange => ({


extract the common part between here and getFormattingEditsForRange in convertTextChangeToCodeEdit

mhegazy · 2016-10-07T00:19:52Z

src/server/client.ts

@@ -630,6 +630,10 @@ namespace ts.server {
            throw new Error("Not Implemented Yet.");
        }

+        getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[]): ts.CodeAction[] {
+            throw new Error("Not Implemented Yet.");


why? add implementation here, and add a test in tests\cases\fourslash\server

mhegazy · 2016-10-07T00:21:35Z

src/services/shims.ts

@@ -251,6 +251,8 @@ namespace ts {
         */
        isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): string;

+        getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: string): string;


we do not need to update the shims, since we moved to tsserver

mhegazy · 2016-10-07T00:24:11Z

src/services/services.ts

+                    newLineCharacter: newLineChar
+                };
+
+                const fixes = codefix.getFixes(context);


check the cancellation token for every iteration.

mhegazy · 2016-10-07T00:29:16Z

src/services/codefixes/codeFixProvider.ts

+namespace ts {
+    export interface CodeFix {
+        errorCodes: number[];
+        getCodeActions(context: CodeFixContext): CodeAction[];


getCodeActions(context: CodeFixContext): CodeAction[] | undefined to allow strictNullChecks to work for users implementing it

mhegazy · 2016-10-12T00:11:25Z

src/server/protocol.d.ts

@@ -1524,4 +1592,16 @@ declare namespace ts.server.protocol {
    export interface NavBarResponse extends Response {
        body?: NavigationBarItem[];
    }
+
+    export interface CodeAction {


this is already defined above.

mhegazy · 2016-10-12T00:16:19Z

src/server/session.ts

@@ -134,6 +135,9 @@ namespace ts.server {
        export const NameOrDottedNameSpan = "nameOrDottedNameSpan";
        export const BreakpointStatement = "breakpointStatement";
        export const CompilerOptionsForInferredProjects = "compilerOptionsForInferredProjects";
+        export const GetCodeFixes = "getCodeFixes";
+        export const GetCodeFixesFull = "getCodeFixes-full";
+        export const GetSupportedCodeFixes = "getSupportedCodeFixes";


i do not see the definition of the request for this command in protocol.d.ts

can you also add a test for it.

There are no arguments for the command so no need for a request

ghost · 2016-10-12T16:08:30Z

src/services/codefixes/codeFixProvider.ts

+            });
+        }
+
+        export function getSupportedErrorCodes() {


Any reason this returns a string[] and not a number[]? Error codes are always numbers, so you could map this with Number.parseInt.

Yes, while we currently only return numbers for errorcodes, in the future e.g. a linter extension could prepend the errorcodes they return with a string. That way they can be differentiated and handled by separate 'fixers', even when they have the same error number.

Maybe the definitions of errorCode and errorCodes above (in CodeFixContent and CodeFix) should use string then?

You make a good point, I'll update that.

Paul van Brenk added 3 commits August 5, 2016 16:20

Api Changes and simple superfixes

94ea825

Fourslash support

466d26f

SuperFix fourslash tests

6f4fb06

msftclas added the cla-not-required label Aug 5, 2016

paulvanbrenk mentioned this pull request Aug 5, 2016

Pvb/discover code actions #9304

Closed

vladima reviewed Aug 5, 2016
View reviewed changes

Clean up

e0b73c4

vladima reviewed Aug 5, 2016
View reviewed changes

Paul van Brenk added 4 commits August 26, 2016 14:32

PR Feedback

124e05f

Merge branch 'master' into pvb/codeaction/api

730f31f

Merge branch 'pvb/codeaction/api' of https://github.com/Microsoft/Typ…

9a26da1

…eScript into pvb/codeaction/api

Fix build break caused by merge from master

f931e60

mhegazy reviewed Aug 29, 2016
View reviewed changes

Merge branch 'master' into pvb/codeaction/api

66e1c92

mhegazy reviewed Sep 1, 2016
View reviewed changes

Paul van Brenk added 12 commits September 9, 2016 14:30

PR feedback

49b65c7

Merge branch 'master' into pvb/codeaction/api

3df79d5

# Conflicts: # src/compiler/diagnosticMessages.json # src/services/services.ts

PR feedback

682f812

Merge branch 'pvb/codeaction/api' of https://github.com/Microsoft/Typ…

6e8eb1d

…eScript into pvb/codeaction/api

Merge branch 'master' into pvb/codeaction/api

20dea29

# Conflicts: # src/services/types.ts

Implement codefixes in tsserver

4f404ad

PR feedback

1cc9732

Merge branch 'master' into pvb/codeaction/api

cf4e300

Error span moved from constructor to this keyword.

ebcfce4

Handle case where this access is inside the supercall

163e758

Use just the errorcode, without the TS prefix

75e1b80

Fix linting issues

1e9b653

mhegazy suggested changes Oct 7, 2016

View reviewed changes

Paul van Brenk added 6 commits October 7, 2016 10:31

PR feedback

c29e9b7

rename of the response body

77610f6

Codefixes in client for testing the server.

a83a2b0

Merge branch 'master' into pvb/codeaction/api

a88ceb5

Deal with buildbreak

f310310

Fix test failure

7c96c28

mhegazy suggested changes Oct 12, 2016

View reviewed changes

mhegazy approved these changes Oct 12, 2016

View reviewed changes

Paul van Brenk added 2 commits October 11, 2016 17:30

Remove duplicate interface

dc516c0

Merge branch 'master' into pvb/codeaction/api

9b98d00

paulvanbrenk merged commit 9f73ae5 into master Oct 12, 2016

paulvanbrenk deleted the pvb/codeaction/api branch October 12, 2016 02:51

ghost reviewed Oct 12, 2016

View reviewed changes

vvakame mentioned this pull request Nov 10, 2016

Quick Fix support #6943

Closed

mkusher mentioned this pull request Nov 14, 2016

Support code fixes api Quramy/tsuquyomi#97

Closed

jimmytheneutrino mentioned this pull request Mar 6, 2017

Rewrite atom-typescript to use tsserver API TypeStrong/atom-typescript#1166

Merged

23 tasks

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pvb/codeaction/api #10185

Pvb/codeaction/api #10185

paulvanbrenk commented Aug 5, 2016

vladima Aug 5, 2016

rbuckton Aug 20, 2016

vladima Aug 5, 2016

mhegazy Aug 29, 2016

mhegazy commented Sep 1, 2016

mhegazy Sep 1, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016 •

edited

Loading

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 7, 2016

mhegazy Oct 7, 2016

mhegazy Oct 7, 2016

mhegazy Oct 7, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 7, 2016

paulvanbrenk Oct 7, 2016

mhegazy Oct 12, 2016

mhegazy Oct 12, 2016

mhegazy Oct 12, 2016

paulvanbrenk Oct 12, 2016

ghost Oct 12, 2016

paulvanbrenk Oct 12, 2016

ghost Oct 12, 2016

paulvanbrenk Oct 12, 2016

Pvb/codeaction/api #10185

Pvb/codeaction/api #10185

Conversation

paulvanbrenk commented Aug 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy commented Sep 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulvanbrenk Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulvanbrenk Oct 7, 2016 •

edited

Loading