Add infrastructure for refactors by zhengbli · Pull Request #14624 · microsoft/TypeScript · GitHub

This PR adds the infrastructure for refactors.

The PR adds one new kind of code fixes as well as refactors:

"diagnostic-generating code fixes": a special kind of code fixes that would generate non-error diagnostics. After normal syntactic check and semantic check, the language service will do an extra pass of the AST to get the non-error diagnostics generated by registered "diagnostic-generating code fixes". Then when the user interacts with the new diagnostics in code, a query for corresponding code actions will be triggered, just like with other code fixes.
"refactors": to get a refactor it is a two-step procedure. First, the editor will ask about all "applicable refactor info" at a given location / range, the returned "applicable refactor info" would only contain basic information like refactor names, it wouldn't compute the code actions yet. Then after the user chose a particular refactor, then the editor will send another request to compute the corresponding code actions.

"messages" should be "message"

… refactor_only

Does this crash if range === undefined ?

Here the range is not supposed to be nullable, as we only use it to test the session API GetRefactorsForRange. Will update

Can you name the first two members line and offset to more closely coincide with FileLocationRequestArgs andLocation? Or use a Location for the start and end?

Maybe FileRangeRequestArgs would fit better. But using location for start and end would be an unnecessary breaking change for editors already consuming this type.

Do you want to save the result in args.startPosition (resp. args.endPosition)?

should this have a property canBeSuggested: boolean; ?

Can you add a comment explaining what bound and nonBound means?

Would it be right to name this getSuggestable...? That would be more consistent with the naming of the interfaces.

Do we have a convention for when we split function parameters across multiple lines?

I don't think so, though normally i do that when I have to use the horizontal scroll bar on my laptop if putting all in one line.

… refactor_only

do not use an array. make it an map.

we do not use namespaces anywhere else.

i would call this name and make it string.

i would make this an object instead of an array.

probally just one refactoring to ask for code actions for

do not think we need this, it should be covered by getCodeActions

… refactor_only

position may be undefined, is this intended? Initialize at line 1450 if so

This is really confusing, can we just use a normal if here

This member is declared as optional, but isn't checked for undefined in this method

in line 41 it already checked the existence of createCodeFixDiagnosticIfApplicable

👍

Comment defining what "Light" means

Why does this return an empty array vs getApplicableRefactors returns undefined if the array would have been empty?

Should we add a trivial refactor with this just so we can have some testcases?

Sure, I'm adding one now will update soon

I added two things:

a diagnostic-generating code fix: converting functions that has prototype assignment to ES6 classes
a sample refactor: add Async suffix to async function declarations

Mohamed Hegazy (@mhegazy) any other comments?

closing in favor of #15569

Add infrastructure for refactors

e38b98a

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Mar 13, 2017

Arthur Ozga (aozgaa) reviewed Mar 14, 2017

View reviewed changes

zhengbli added 2 commits March 15, 2017 16:23

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

6cbc258

… refactor_only

Grammar

3011d16

Arthur Ozga (aozgaa) reviewed Mar 17, 2017

View reviewed changes

Zhengbo Li added 2 commits March 20, 2017 16:14

cr feedbacks

fa5a483

Redesign APIs

b357841

Zhengbo Li (zhengbli) force-pushed the refactor_only branch from 132051b to b357841 Compare March 26, 2017 09:00

Zhengbo Li added 2 commits March 27, 2017 00:31

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

e92d016

… refactor_only

fix linting issues

675cf03

Mohamed Hegazy (mhegazy) suggested changes Mar 28, 2017

View reviewed changes

Zhengbo Li added 5 commits March 28, 2017 16:25

replace the 'suggestable' refactors with code fixes

f108fd7

Remove namespace in protocol

b86d40a

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

909b436

… refactor_only

Change the args shape to be more compatible

0965223

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

ba6e22c

… refactor_only

Ryan Cavanaugh (RyanCavanaugh) reviewed Apr 18, 2017

View reviewed changes

Zhengbo Li added 2 commits April 18, 2017 14:38

address CR feedback

683a0a0

add codefix to convert function to es6 class

dfa2506

Zhengbo Li (zhengbli) force-pushed the refactor_only branch from a6f34a9 to dfa2506 Compare April 19, 2017 01:33

Zhengbo Li added 2 commits April 18, 2017 22:24

improve tests and force synchronizeHosts before codeFix diag

0aeff72

Add a simple refactor as a sample

2e8f6c6

Ryan Cavanaugh (RyanCavanaugh) self-assigned this Apr 24, 2017

This was referenced May 1, 2017

Support refactoring commands provided by TS Server microsoft/vscode#25739

Closed

Refactoring support #15569

Merged

Mohamed Hegazy (mhegazy) closed this May 19, 2017

Microsoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018

+                              // the semanticDiag message
                               host.runQueuedImmediateCallbacks();
-                              assert.equal(host.getOutput().length, 2, "expect 2 messages");
+                              assert.equal(host.getOutput().length, 1, "expect 1 messages");

-                    * Instances of this interface specify errorcodes on a specific location in a sourcefile.
-                    */
-                  export interface CodeFixRequestArgs extends FileRequestArgs {
+                  export interface TextRangeRequestArgs extends FileRequestArgs {

+              /* @internal */
+              namespace ts {
+                  interface BaseRefactor {
+                      /** Description of the refactor to display in the UI of the editor */

+                          /* The kind of the applicable refactor */
+                          refactorKinds?: number[];
+                          /* The diagnostic code of a refactor diagnostic */
+                          diagnosticCodes?: number[];

+                      export function getCodeFixDiagnosticsForNode(context: CodeFixDiagnoseContext, node: Node): Diagnostic[] | undefined {
+                          let result: Diagnostic[];
+                          for (const codeFix of diagnosticGeneratingCodeFixes) {
+                              const newDiag = codeFix.createCodeFixDiagnosticIfApplicable(node, context);

                       }
+                      getRefactorDiagnostics(fileName: string, range?: TextRange): RefactorDiagnostic[] {
+                          const startLineOffset = this.positionToOneBasedLineOffset(fileName, range.pos);

+                          return { startPosition, endPosition };
+                          function getStartPosition() {
+                              return args.startPosition !== undefined ? args.startPosition : scriptInfo.lineOffsetToPosition(args.startLine, args.startOffset);

+                  export type Refactor = SuggestableRefactor | NonSuggestableRefactor;
+                  export interface LightRefactorContext {
+                      nonBoundSourceFile: SourceFile;

+                          return results;
+                      }
+                      export function getSuggestedRefactorDiagnosticsForNode(node: Node, context: RefactorContext) {

+                          }
+                      }
+                      function getCodeActionsForRefactorAtPosition(

+                      refactorCode: number;
+                  }
+                  export interface GetCodeActionsForRefactorResponse extends Response {

+                      position?: number;
+                  }
+                  export namespace Refactor {

+                      }
+                      export interface ApplicableRefactorInfo {
+                          refactorKind: number;

+                      }
+                      export interface GetApplicableRefactorsResponse extends Response {
+                          body?: ApplicableRefactorInfo[];

+                              const { startPosition, endPosition } = this.getStartAndEndPosition(args, scriptInfo);
+                              textRange = { pos: startPosition, end: endPosition };
+                          }
+                          return { position, textRange };

-                              return args.endPosition !== undefined ? args.endPosition : scriptInfo.lineOffsetToPosition(args.endLine, args.endOffset);
-                          }
+                      private getStartAndEndPosition(args: protocol.FileRangeRequestArgs, scriptInfo: ScriptInfo) {
+                          const startPosition = args.startPosition !== undefined

Add infrastructure for refactors by zhengbli · Pull Request #14624 · microsoft/TypeScript · GitHub

Uh oh!

Add infrastructure for refactors - #14624

Add infrastructure for refactors#14624Zhengbo Li (zhengbli) wants to merge 16 commits intomicrosoft:mastermicrosoft/TypeScript:masterfrom zhengbli:refactor_onlyzhengbli/TypeScript:refactor_onlyCopy head branch name to clipboard

Conversation

Zhengbo Li (zhengbli) commented Mar 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Arthur Ozga (aozgaa) Mar 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zhengbo Li (zhengbli) Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryan Cavanaugh (RyanCavanaugh) Apr 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryan Cavanaugh (RyanCavanaugh) commented Apr 18, 2017

Uh oh!

Zhengbo Li (zhengbli) commented Apr 18, 2017

Uh oh!

Zhengbo Li (zhengbli) commented Apr 19, 2017

Uh oh!

Ryan Cavanaugh (RyanCavanaugh) commented Apr 19, 2017

Uh oh!

Mohamed Hegazy (mhegazy) commented May 19, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Add infrastructure for refactors#14624
Zhengbo Li (zhengbli) wants to merge 16 commits into
microsoft:masterfrom
zhengbli:refactor_only

Zhengbo Li (zhengbli) commented Mar 13, 2017 •
edited
Loading

Arthur Ozga (aozgaa) Mar 17, 2017 •
edited
Loading

Zhengbo Li (zhengbli) Mar 20, 2017 •
edited
Loading

Ryan Cavanaugh (RyanCavanaugh) Apr 18, 2017 •
edited
Loading