[WIP] Add localtofield refactoring based on rope functionality

build/existingFiles.json

@@ -528,6 +528,7 @@
     "src/test/pythonFiles/formatting/dummy.ts",
     "src/test/refactor/extension.refactor.extract.method.test.ts",
     "src/test/refactor/extension.refactor.extract.var.test.ts",
+    "src/test/refactor/extension.refactor.local.to.field.test.ts",
     "src/test/refactor/rename.test.ts",
     "src/test/serviceRegistry.ts",
     "src/test/signature/signature.jedi.test.ts",

package.json

@@ -171,6 +171,11 @@
                 "title": "%python.command.python.refactorExtractMethod.title%",
                 "category": "Python Refactor"
             },
+            {
+                "command": "python.refactorLocalToField",
+                "title": "%python.command.python.refactorLocalToField.title%",
+                "category": "Python Refactor"
+            },
             {
                 "command": "python.viewTestOutput",
                 "title": "%python.command.python.viewTestOutput.title%",
@@ -342,6 +347,12 @@
                     "group": "Refactor",
                     "when": "editorHasSelection && editorLangId == python"
                 },
+                {
+                    "command": "python.refactorLocalToField",
+                    "title": "Refactor: Convert Local to Field",
+                    "group": "Refactor",
+                    "when": "editorHasSelection && editorLangId == python"
+                },
                 {
                     "command": "python.sortImports",
                     "title": "Refactor: Sort Imports",

package.nls.json

@@ -10,6 +10,7 @@
     "python.command.python.updateSparkLibrary.title": "Update Workspace PySpark Libraries",
     "python.command.python.refactorExtractVariable.title": "Extract Variable",
     "python.command.python.refactorExtractMethod.title": "Extract Method",
+    "python.command.python.refactorLocalToField.title": "Convert Local to Field",
     "python.command.python.viewTestOutput.title": "Show Unit Test Output",
     "python.command.python.selectAndRunTestMethod.title": "Run Unit Test Method ...",
     "python.command.python.selectAndDebugTestMethod.title": "Debug Unit Test Method ...",

pythonFiles/refactor.py

@@ -14,6 +14,7 @@
     from rope.base import libutils
     from rope.refactor.rename import Rename
     from rope.refactor.extract import ExtractMethod, ExtractVariable
+    from rope.refactor.localtofield import LocalToField
     import rope.base.project
     import rope.base.taskhandle
 except:
@@ -188,6 +189,24 @@ def onRefactor(self):
                 raise Exception('Unknown Change')
 
 
+class LocalToFieldRefactor(BaseRefactoring):
+
+    def __init__(self, project, resource, name="Local to Field", progressCallback=None, startOffset=None):
+        BaseRefactoring.__init__(self, project, resource,
+                                 name, progressCallback)
+        self.startOffset = startOffset
+
+    def onRefactor(self):
+        field = LocalToField(self.project, self.resource, self.startOffset)
+        changes = field.get_changes()
+        for item in changes.changes:
+            if isinstance(item, rope.base.change.ChangeContents):
+                self.changes.append(
+                    Change(item.resource.real_path, ChangeType.EDIT, get_diff(item)))
+            else:
+                raise Exception('Unknown Change')
+
+
 class RopeRefactoring(object):
 
     def __init__(self):
@@ -245,6 +264,23 @@ def _extractMethod(self, filePath, start, end, newName, indent_size):
             valueToReturn.append({'diff': change.diff})
         return valueToReturn
 
+    def _localToField(self, filePath, start, indent_size):
+        """
+        Extracts a method
+        """
+        project = rope.base.project.Project(
+            WORKSPACE_ROOT, ropefolder=ROPE_PROJECT_FOLDER, save_history=False, indent_size=indent_size)
+        resourceToRefactor = libutils.path_to_resource(project, filePath)
+        refactor = LocalToFieldRefactor(
+            project, resourceToRefactor, startOffset=start)
+        refactor.refactor()
+        changes = refactor.changes
+        project.close()
+        valueToReturn = []
+        for change in changes:
+            valueToReturn.append({'diff': change.diff})
+        return valueToReturn
+
     def _serialize(self, identifier, results):
         """
         Serializes the refactor results
@@ -282,6 +318,10 @@ def _process_request(self, request):
             changes = self._extractMethod(request['file'], int(
                 request['start']), int(request['end']), request['name'], int(request['indent_size']))
             return self._write_response(self._serialize(request['id'], changes))
+        elif lookup == 'local_to_field':
+            changes = self._localToField(request['file'], int(
+                request['start']), int(request['indent_size']))
+            return self._write_response(self._serialize(request['id'], changes))
 
     def _write_response(self, response):
         sys.stdout.write(response + '\n')

src/client/common/constants.ts

@@ -30,6 +30,7 @@ export namespace Commands {
     export const Tests_Run_Current_File = 'python.runCurrentTestFile';
     export const Refactor_Extract_Variable = 'python.refactorExtractVariable';
     export const Refaactor_Extract_Method = 'python.refactorExtractMethod';
+    export const Refactor_Local_To_Field = 'python.refactorLocalToField';
     export const Update_SparkLibrary = 'python.updateSparkLibrary';
     export const Build_Workspace_Symbols = 'python.buildWorkspaceSymbols';
     export const Start_REPL = 'python.startREPL';

src/client/providers/simpleRefactorProvider.ts

@@ -5,7 +5,7 @@ import { StopWatch } from '../common/utils/stopWatch';
 import { IServiceContainer } from '../ioc/types';
 import { RefactorProxy } from '../refactor/proxy';
 import { sendTelemetryWhenDone } from '../telemetry';
-import { REFACTOR_EXTRACT_FUNCTION, REFACTOR_EXTRACT_VAR } from '../telemetry/constants';
+import { REFACTOR_EXTRACT_FUNCTION, REFACTOR_EXTRACT_VAR, REFACTOR_LOCAL_TO_FIELD } from '../telemetry/constants';
 
 type RenameResponse = {
     results: [{ diff: string }];
@@ -36,6 +36,17 @@ export function activateSimplePythonRefactorProvider(context: vscode.ExtensionCo
         sendTelemetryWhenDone(REFACTOR_EXTRACT_FUNCTION, promise, stopWatch);
     });
     context.subscriptions.push(disposable);
+
+    disposable = vscode.commands.registerCommand('python.refactorLocalToField', () => {
+        const stopWatch = new StopWatch();
+        const promise = localToField(context.extensionPath,
+            vscode.window.activeTextEditor!,
+            vscode.window.activeTextEditor!.selection,
+            // tslint:disable-next-line:no-empty
+            outputChannel, serviceContainer).catch(() => { });
+        sendTelemetryWhenDone(REFACTOR_LOCAL_TO_FIELD, promise, stopWatch);
+    });
+    context.subscriptions.push(disposable);
 }
 
 // Exported for unit testing
@@ -84,6 +95,29 @@ export function extractMethod(extensionDir: string, textEditor: vscode.TextEdito
     });
 }
 
+// Exported for unit testing
+export function localToField(extensionDir: string, textEditor: vscode.TextEditor, range: vscode.Range,
+    // tslint:disable-next-line:no-any
+    outputChannel: vscode.OutputChannel, serviceContainer: IServiceContainer): Promise<any> {
+
+    let workspaceFolder = vscode.workspace.getWorkspaceFolder(textEditor.document.uri);
+    if (!workspaceFolder && Array.isArray(vscode.workspace.workspaceFolders) && vscode.workspace.workspaceFolders.length > 0) {
+        workspaceFolder = vscode.workspace.workspaceFolders[0];
+    }
+    const workspaceRoot = workspaceFolder ? workspaceFolder.uri.fsPath : __dirname;
+    const pythonSettings = serviceContainer.get<IConfigurationService>(IConfigurationService).getSettings(workspaceFolder ? workspaceFolder.uri : undefined);
+
+    return validateDocumentForRefactor(textEditor).then(() => {
+        const newName = `newmethod${new Date().getMilliseconds().toString()}`;
+        const proxy = new RefactorProxy(extensionDir, pythonSettings, workspaceRoot, serviceContainer);
+        const rename = proxy.localToField<RenameResponse>(textEditor.document, newName, textEditor.document.uri.fsPath, range, textEditor.options).then(response => {
+            return response.results[0].diff;
+        });
+
+        return extractName(extensionDir, textEditor, range, newName, rename, outputChannel);
+    });
+}
+
 // tslint:disable-next-line:no-any
 function validateDocumentForRefactor(textEditor: vscode.TextEditor): Promise<any> {
     if (!textEditor.document.isDirty) {

src/client/refactor/proxy.ts

@@ -96,6 +96,24 @@ export class RefactorProxy extends Disposable {
         };
         return this.sendCommand<T>(JSON.stringify(command));
     }
+    public localToField<T>(document: TextDocument, name: string, filePath: string, range: Range, options?: TextEditorOptions): Promise<T> {
+        if (!options) {
+            options = window.activeTextEditor!.options;
+        }
+        // Ensure last line is an empty line
+        if (!document.lineAt(document.lineCount - 1).isEmptyOrWhitespace && range.start.line === document.lineCount - 1) {
+            return Promise.reject<T>('Missing blank line at the end of document (PEP8).');
+        }
+        const command = {
+            lookup: 'local_to_field',
+            file: filePath,
+            start: this.getOffsetAt(document, range.start).toString(),
+            id: '1',
+            name: name,
+            indent_size: options.tabSize
+        };
+        return this.sendCommand<T>(JSON.stringify(command));
+    }
     private sendCommand<T>(command: string, telemetryEvent?: string): Promise<T> {
         return this.initialize(this.pythonSettings.pythonPath).then(() => {
             // tslint:disable-next-line:promise-must-complete

src/client/telemetry/constants.ts

@@ -20,6 +20,7 @@ export const UPDATE_PYSPARK_LIBRARY = 'UPDATE_PYSPARK_LIBRARY';
 export const REFACTOR_RENAME = 'REFACTOR_RENAME';
 export const REFACTOR_EXTRACT_VAR = 'REFACTOR_EXTRACT_VAR';
 export const REFACTOR_EXTRACT_FUNCTION = 'REFACTOR_EXTRACT_FUNCTION';
+export const REFACTOR_LOCAL_TO_FIELD = 'REFACTOR_LOCAL_TO_FIELD';
 export const REPL = 'REPL';
 export const PYTHON_INTERPRETER = 'PYTHON_INTERPRETER';
 export const WORKSPACE_SYMBOLS_BUILD = 'WORKSPACE_SYMBOLS.BUILD';

src/test/refactor/extension.refactor.local.to.field.test.ts

@@ -0,0 +1,144 @@
+// tslint:disable:interface-name no-any max-func-body-length estrict-plus-operands no-empty
+
+import * as assert from 'assert';
+import * as fs from 'fs-extra';
+import * as path from 'path';
+import { commands, Position, Range, Selection, TextEditorCursorStyle, TextEditorLineNumbersStyle, TextEditorOptions, Uri, window, workspace } from 'vscode';
+import { getTextEditsFromPatch } from '../../client/common/editor';
+import { localToField } from '../../client/providers/simpleRefactorProvider';
+import { RefactorProxy } from '../../client/refactor/proxy';
+import { getExtensionSettings, isPythonVersion } from '../common';
+import { UnitTestIocContainer } from '../unittests/serviceRegistry';
+import { closeActiveWindows, initialize, initializeTest, IS_CI_SERVER } from './../initialize';
+import { MockOutputChannel } from './../mockClasses';
+
+const EXTENSION_DIR = path.join(__dirname, '..', '..', '..');
+const refactorSourceFile = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'refactoring', 'standAlone', 'refactor.py');
+const refactorTargetFileDir = path.join(__dirname, '..', '..', '..', 'out', 'test', 'pythonFiles', 'refactoring', 'standAlone');
+
+interface RenameResponse {
+    results: [{ diff: string }];
+}
+
+suite('Local to Field Refactor', () => {
+    // Hack hac hack
+    const oldExecuteCommand = commands.executeCommand;
+    const options: TextEditorOptions = { cursorStyle: TextEditorCursorStyle.Line, insertSpaces: true, lineNumbers: TextEditorLineNumbersStyle.Off, tabSize: 4 };
+    let refactorTargetFile = '';
+    let ioc: UnitTestIocContainer;
+    suiteSetup(initialize);
+    suiteTeardown(() => {
+        commands.executeCommand = oldExecuteCommand;
+        return closeActiveWindows();
+    });
+    setup(async () => {
+        initializeDI();
+        refactorTargetFile = path.join(refactorTargetFileDir, `refactor${new Date().getTime()}.py`);
+        fs.copySync(refactorSourceFile, refactorTargetFile, { overwrite: true });
+        await initializeTest();
+        (<any>commands).executeCommand = (cmd) => Promise.resolve();
+    });
+    teardown(async () => {
+        commands.executeCommand = oldExecuteCommand;
+        try {
+            await fs.unlink(refactorTargetFile);
+        } catch { }
+        await closeActiveWindows();
+    });
+
+    function initializeDI() {
+        ioc = new UnitTestIocContainer();
+        ioc.registerCommonTypes();
+        ioc.registerProcessTypes();
+        ioc.registerVariableTypes();
+    }
+
+    async function testingFieldToLocal(shouldError: boolean, startPos: Position, endPos: Position): Promise<void> {
+        const pythonSettings = getExtensionSettings(Uri.file(refactorTargetFile));
+        const rangeOfTextToExtract = new Range(startPos, endPos);
+        const proxy = new RefactorProxy(EXTENSION_DIR, pythonSettings, path.dirname(refactorTargetFile), ioc.serviceContainer);
+
+        const DIFF = '--- a/refactor.py\n+++ b/refactor.py\n@@ -232,7 +232,8 @@\n         sys.stdout.flush()\n \n     def watch(self):\n-        self._write_response("STARTED")\n+        myNewVariable = "STARTED"\n+        self._write_response(myNewVariable)\n         while True:\n             try:\n                 self._process_request(self._input.readline())\n';
+        const mockTextDoc = await workspace.openTextDocument(refactorTargetFile);
+        const expectedTextEdits = getTextEditsFromPatch(mockTextDoc.getText(), DIFF);
+        try {
+            const response = await proxy.localToField<RenameResponse>(mockTextDoc, 'myNewVariable', refactorTargetFile, rangeOfTextToExtract, options);
+            if (shouldError) {
+                assert.fail('No error', 'Error', 'Extraction should fail with an error', '');
+            }
+            const textEdits = getTextEditsFromPatch(mockTextDoc.getText(), DIFF);
+            assert.equal(response.results.length, 1, 'Invalid number of items in response');
+            assert.equal(textEdits.length, expectedTextEdits.length, 'Invalid number of Text Edits');
+            textEdits.forEach(edit => {
+                const foundEdit = expectedTextEdits.filter(item => item.newText === edit.newText && item.range.isEqual(edit.range));
+                assert.equal(foundEdit.length, 1, 'Edit not found');
+            });
+        } catch (error) {
+            if (!shouldError) {
+                assert.equal('Error', 'No error', `${error}`);
+            }
+        }
+    }
+
+    // tslint:disable-next-line:no-function-expression
+    test('Local to Field', async function () {
+        if (isPythonVersion('3.7')) {
+            // tslint:disable-next-line:no-invalid-this
+            return this.skip();
+        } else {
+            const startPos = new Position(234, 29);
+            const endPos = new Position(234, 38);
+            await testingFieldToLocal(false, startPos, endPos);
+        }
+    });
+
+    test('Local to Field fails if whole string not selected', async () => {
+        const startPos = new Position(234, 20);
+        const endPos = new Position(234, 38);
+        await testingFieldToLocal(true, startPos, endPos);
+    });
+
+    async function testingFieldToLocalEndToEnd(shouldError: boolean, startPos: Position, endPos: Position): Promise<void> {
+        const ch = new MockOutputChannel('Python');
+        const rangeOfTextToExtract = new Range(startPos, endPos);
+
+        const textDocument = await workspace.openTextDocument(refactorTargetFile);
+        const editor = await window.showTextDocument(textDocument);
+
+        editor.selections = [new Selection(rangeOfTextToExtract.start, rangeOfTextToExtract.end)];
+        editor.selection = new Selection(rangeOfTextToExtract.start, rangeOfTextToExtract.end);
+        try {
+            await localToField(EXTENSION_DIR, editor, rangeOfTextToExtract, ch, ioc.serviceContainer);
+            if (shouldError) {
+                assert.fail('No error', 'Error', 'Local to Field should fail with an error', '');
+            }
+            assert.equal(ch.output.length, 0, 'Output channel is not empty');
+
+            const newVarDefLine = textDocument.lineAt(editor.selection.start);
+            const newVarRefLine = textDocument.lineAt(newVarDefLine.lineNumber + 1);
+
+            assert.equal(newVarDefLine.text.trim().indexOf('newvariable'), 0, 'New Variable not created');
+            assert.equal(newVarDefLine.text.trim().endsWith('= "STARTED"'), true, 'Started Text Assigned to variable');
+            assert.equal(newVarRefLine.text.indexOf('(newvariable') >= 0, true, 'New Variable not being used');
+        } catch (error) {
+            if (!shouldError) {
+                assert.fail('Error', 'No error', `${error}`);
+            }
+        }
+    }
+
+    // This test fails on linux (text document not getting updated in time)
+    if (!IS_CI_SERVER) {
+        test('Local to Field (end to end)', async () => {
+            const startPos = new Position(234, 29);
+            const endPos = new Position(234, 38);
+            await testingFieldToLocalEndToEnd(false, startPos, endPos);
+        });
+    }
+
+    test('Local to Field fails if whole string not selected (end to end)', async () => {
+        const startPos = new Position(234, 20);
+        const endPos = new Position(234, 38);
+        await testingFieldToLocalEndToEnd(true, startPos, endPos);
+    });
+});