diff --git a/news/2 Fixes/695.md b/news/2 Fixes/695.md new file mode 100644 index 000000000000..bb5b48569222 --- /dev/null +++ b/news/2 Fixes/695.md @@ -0,0 +1 @@ +Resoves rename refactor issue that remvoes the last line of the source file when the line is being refactored and source does not end with an EOL. \ No newline at end of file diff --git a/pythonFiles/refactor.py b/pythonFiles/refactor.py index dd4b23ee7727..b782eee26ffb 100644 --- a/pythonFiles/refactor.py +++ b/pythonFiles/refactor.py @@ -2,9 +2,11 @@ # 1. Working directory. # 2. Rope folder +import difflib import io -import sys import json +import os +import sys import traceback try: @@ -55,6 +57,27 @@ def __init__(self, filePath, fileMode=ChangeType.EDIT, diff=""): self.diff = diff self.fileMode = fileMode +def get_diff(changeset): + """This is a copy of the code form the ChangeSet.get_description method found in Rope.""" + new = changeset.new_contents + old = changeset.old_contents + if old is None: + if changeset.resource.exists(): + old = changeset.resource.read() + else: + old = '' + + # Ensure code has a trailing empty lines, before generating a diff. + # https://github.com/Microsoft/vscode-python/issues/695. + old_lines = old.splitlines(True) + if not old_lines[-1].endswith('\n'): + old_lines[-1] = old_lines[-1] + os.linesep + new = new + os.linesep + + result = difflib.unified_diff( + old_lines, new.splitlines(True), + 'a/' + changeset.resource.path, 'b/' + changeset.resource.path) + return ''.join(list(result)) class BaseRefactoring(object): """ @@ -117,7 +140,7 @@ def onRefactor(self): for item in changes.changes: if isinstance(item, rope.base.change.ChangeContents): self.changes.append( - Change(item.resource.real_path, ChangeType.EDIT, item.get_description())) + Change(item.resource.real_path, ChangeType.EDIT, get_diff(item))) else: raise Exception('Unknown Change') @@ -141,7 +164,7 @@ def onRefactor(self): for item in changes.changes: if isinstance(item, rope.base.change.ChangeContents): self.changes.append( - Change(item.resource.real_path, ChangeType.EDIT, item.get_description())) + Change(item.resource.real_path, ChangeType.EDIT, get_diff(item))) else: raise Exception('Unknown Change') @@ -160,7 +183,7 @@ def onRefactor(self): for item in changes.changes: if isinstance(item, rope.base.change.ChangeContents): self.changes.append( - Change(item.resource.real_path, ChangeType.EDIT, item.get_description())) + Change(item.resource.real_path, ChangeType.EDIT, get_diff(item))) else: raise Exception('Unknown Change') diff --git a/src/test/pythonFiles/refactoring/source folder/with empty line.py b/src/test/pythonFiles/refactoring/source folder/with empty line.py new file mode 100644 index 000000000000..01ed75727900 --- /dev/null +++ b/src/test/pythonFiles/refactoring/source folder/with empty line.py @@ -0,0 +1,8 @@ +import os + +def one(): + return True + +def two(): + if one(): + print("A" + one()) diff --git a/src/test/pythonFiles/refactoring/source folder/without empty line.py b/src/test/pythonFiles/refactoring/source folder/without empty line.py new file mode 100644 index 000000000000..a449eb106f5c --- /dev/null +++ b/src/test/pythonFiles/refactoring/source folder/without empty line.py @@ -0,0 +1,8 @@ +import os + +def one(): + return True + +def two(): + if one(): + print("A" + one()) \ No newline at end of file diff --git a/src/test/refactor/rename.test.ts b/src/test/refactor/rename.test.ts new file mode 100644 index 000000000000..cbc4f641ddf7 --- /dev/null +++ b/src/test/refactor/rename.test.ts @@ -0,0 +1,72 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { expect } from 'chai'; +import { EOL } from 'os'; +import * as path from 'path'; +import * as typeMoq from 'typemoq'; +import { Range, TextEditorCursorStyle, TextEditorLineNumbersStyle, TextEditorOptions, window, workspace } from 'vscode'; +import { EXTENSION_ROOT_DIR } from '../../client/common/constants'; +import { BufferDecoder } from '../../client/common/process/decoder'; +import { ProcessService } from '../../client/common/process/proc'; +import { PythonExecutionFactory } from '../../client/common/process/pythonExecutionFactory'; +import { IProcessServiceFactory, IPythonExecutionFactory } from '../../client/common/process/types'; +import { IConfigurationService, IPythonSettings } from '../../client/common/types'; +import { IServiceContainer } from '../../client/ioc/types'; +import { RefactorProxy } from '../../client/refactor/proxy'; +import { PYTHON_PATH } from '../common'; +import { closeActiveWindows, initialize, initializeTest } from './../initialize'; + +type RenameResponse = { + results: [{ diff: string }]; +}; + +suite('Refactor Rename', () => { + const options: TextEditorOptions = { cursorStyle: TextEditorCursorStyle.Line, insertSpaces: true, lineNumbers: TextEditorLineNumbersStyle.Off, tabSize: 4 }; + let pythonSettings: typeMoq.IMock; + let serviceContainer: typeMoq.IMock; + suiteSetup(initialize); + setup(async () => { + pythonSettings = typeMoq.Mock.ofType(); + pythonSettings.setup(p => p.pythonPath).returns(() => PYTHON_PATH); + const configService = typeMoq.Mock.ofType(); + configService.setup(c => c.getSettings(typeMoq.It.isAny())).returns(() => pythonSettings.object); + const processServiceFactory = typeMoq.Mock.ofType(); + processServiceFactory.setup(p => p.create(typeMoq.It.isAny())).returns(() => Promise.resolve(new ProcessService(new BufferDecoder()))); + + serviceContainer = typeMoq.Mock.ofType(); + serviceContainer.setup(s => s.get(typeMoq.It.isValue(IConfigurationService), typeMoq.It.isAny())).returns(() => configService.object); + serviceContainer.setup(s => s.get(typeMoq.It.isValue(IProcessServiceFactory), typeMoq.It.isAny())).returns(() => processServiceFactory.object); + serviceContainer.setup(s => s.get(typeMoq.It.isValue(IPythonExecutionFactory), typeMoq.It.isAny())).returns(() => new PythonExecutionFactory(serviceContainer.object)); + await initializeTest(); + }); + teardown(closeActiveWindows); + suiteTeardown(closeActiveWindows); + + test('Rename function in source without a trailing empty line', async () => { + const sourceFile = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'pythonFiles', 'refactoring', 'source folder', 'without empty line.py'); + const expectedDiff = `--- a/${path.basename(sourceFile)}${EOL}+++ b/${path.basename(sourceFile)}${EOL}@@ -1,8 +1,8 @@${EOL} import os${EOL} ${EOL}-def one():${EOL}+def three():${EOL} return True${EOL} ${EOL} def two():${EOL}- if one():${EOL}- print(\"A\" + one())${EOL}+ if three():${EOL}+ print(\"A\" + three())${EOL}`; + + const proxy = new RefactorProxy(EXTENSION_ROOT_DIR, pythonSettings.object, path.dirname(sourceFile), serviceContainer.object); + const textDocument = await workspace.openTextDocument(sourceFile); + await window.showTextDocument(textDocument); + + const response = await proxy.rename(textDocument, 'three', sourceFile, new Range(7, 20, 7, 23), options); + expect(response.results).to.be.lengthOf(1); + expect(response.results[0].diff).to.be.equal(expectedDiff); + }); + test('Rename function in source with a trailing empty line', async () => { + const sourceFile = path.join(EXTENSION_ROOT_DIR, 'src', 'test', 'pythonFiles', 'refactoring', 'source folder', 'with empty line.py'); + const expectedDiff = `--- a/${path.basename(sourceFile)}${EOL}+++ b/${path.basename(sourceFile)}${EOL}@@ -1,8 +1,8 @@${EOL} import os${EOL} ${EOL}-def one():${EOL}+def three():${EOL} return True${EOL} ${EOL} def two():${EOL}- if one():${EOL}- print(\"A\" + one())${EOL}+ if three():${EOL}+ print(\"A\" + three())${EOL}`; + + const proxy = new RefactorProxy(EXTENSION_ROOT_DIR, pythonSettings.object, path.dirname(sourceFile), serviceContainer.object); + const textDocument = await workspace.openTextDocument(sourceFile); + await window.showTextDocument(textDocument); + + const response = await proxy.rename(textDocument, 'three', sourceFile, new Range(7, 20, 7, 23), options); + expect(response.results).to.be.lengthOf(1); + expect(response.results[0].diff).to.be.equal(expectedDiff); + }); +});