Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit afcbff0

Browse files
niteskumboopeshmahendran
niteskum
authored and
boopeshmahendran
committed
JSRefactoring Bugs Fix (#14455)
* JSRefactoring Bugs Fix * Added Fix For TRY-CATCH Wraaper * Corrected the typo * Addressed Review Comments * Addressed Review Comments * Addressed Review Comments
1 parent 7c29cc5 commit afcbff0

5 files changed

Lines changed: 97 additions & 30 deletions

File tree

‎src/extensions/default/JavaScriptRefactoring/ExtractToVariable.js‎

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,24 @@ define(function(require, exports, module) {
3737
* Does the actual extraction. i.e Replacing the text, Creating a variable
3838
* and multi select variable names
3939
*/
40-
function extract(scopes, parentStatement, expns, text) {
40+
function extract(scopes, parentStatement, expns, text, insertPosition) {
4141
var varType = "var",
4242
varName = RefactoringUtils.getUniqueIdentifierName(scopes, "extracted"),
4343
varDeclaration = varType + " " + varName + " = " + text + ";\n",
44-
insertStartPos = session.editor.posFromIndex(parentStatement.start),
44+
parentStatementStartPos = session.editor.posFromIndex(parentStatement.start),
45+
insertStartPos = insertPosition || parentStatementStartPos,
4546
selections = [],
4647
doc = session.editor.document,
4748
replaceExpnIndex = 0,
48-
posToIndent;
49+
posToIndent,
50+
edits = [];
4951

5052
// If parent statement is expression statement, then just append var declaration
5153
// Ex: "add(1, 2)" will become "var extracted = add(1, 2)"
52-
if (parentStatement.type === "ExpressionStatement" && RefactoringUtils.isEqual(parentStatement.expression, expns[0])) {
54+
if (parentStatement.type === "ExpressionStatement" &&
55+
RefactoringUtils.isEqual(parentStatement.expression, expns[0]) &&
56+
insertStartPos.line === parentStatementStartPos.line &&
57+
insertStartPos.ch === parentStatementStartPos.ch) {
5358
varDeclaration = varType + " " + varName + " = ";
5459
replaceExpnIndex = 1;
5560
}
@@ -63,25 +68,29 @@ define(function(require, exports, module) {
6368
expns[i].start = doc.adjustPosForChange(expns[i].start, varDeclaration.split("\n"), insertStartPos, insertStartPos);
6469
expns[i].end = doc.adjustPosForChange(expns[i].end, varDeclaration.split("\n"), insertStartPos, insertStartPos);
6570

66-
selections.push({
67-
start: expns[i].start,
68-
end: {line: expns[i].start.line, ch: expns[i].start.ch + varName.length}
71+
edits.push({
72+
edit: {
73+
text: varName,
74+
start: expns[i].start,
75+
end: expns[i].end
76+
},
77+
selection: {
78+
start: expns[i].start,
79+
end: {line: expns[i].start.line, ch: expns[i].start.ch + varName.length}
80+
}
6981
});
7082
}
7183

7284
// Replace and multi-select
7385
doc.batchOperation(function() {
7486
doc.replaceRange(varDeclaration, insertStartPos);
7587

76-
for (var i = replaceExpnIndex; i < expns.length; ++i) {
77-
doc.replaceRange(varName, expns[i].start, expns[i].end);
78-
}
88+
selections = doc.doMultipleEdits(edits);
7989
selections.push({
8090
start: {line: insertStartPos.line, ch: insertStartPos.ch + varType.length + 1},
8191
end: {line: insertStartPos.line, ch: insertStartPos.ch + varType.length + varName.length + 1},
8292
primary: true
8393
});
84-
8594
session.editor.setSelections(selections);
8695
session.editor._codeMirror.indentLine(posToIndent.line, "smart");
8796
});
@@ -163,6 +172,7 @@ define(function(require, exports, module) {
163172
*/
164173
function extractToVariable(ast, start, end, text, scopes) {
165174
var doc = session.editor.document,
175+
editor = EditorManager.getActiveEditor(),
166176
parentExpn = RefactoringUtils.getExpression(ast, start, end, doc.getText()),
167177
expns = [],
168178
parentBlockStatement,
@@ -178,8 +188,26 @@ define(function(require, exports, module) {
178188
if (doc.getText().substr(parentExpn.start, parentExpn.end - parentExpn.start) === text) {
179189
parentBlockStatement = RefactoringUtils.findSurroundASTNode(ast, parentExpn, ["BlockStatement", "Program"]);
180190
expns = findAllExpressions(parentBlockStatement, parentExpn, text);
181-
parentStatement = RefactoringUtils.findSurroundASTNode(ast, expns[0], ["Statement"]);
182-
extract(scopes, parentStatement, expns, text);
191+
192+
RefactoringUtils.getScopeData(session, editor.posFromIndex(expns[0].start)).done(function(scope) {
193+
var firstExpnsScopes = RefactoringUtils.getAllScopes(ast, scope, doc.getText()),
194+
insertPostion;
195+
parentStatement = RefactoringUtils.findSurroundASTNode(ast, expns[0], ["Statement"]);
196+
if (scopes.length < firstExpnsScopes.length) {
197+
var parentScope;
198+
if (expns[0].body && expns[0].body.type === "BlockStatement") {
199+
parentScope = firstExpnsScopes[firstExpnsScopes.length - scopes.length];
200+
} else {
201+
parentScope = firstExpnsScopes[firstExpnsScopes.length - scopes.length - 1];
202+
}
203+
204+
var insertNode = RefactoringUtils.findSurroundASTNode(ast, parentScope.originNode, ["Statement"]);
205+
if (insertNode) {
206+
insertPostion = session.editor.posFromIndex(insertNode.start);
207+
}
208+
}
209+
extract(scopes, parentStatement, expns, text, insertPostion);
210+
});
183211
} else {
184212
parentStatement = RefactoringUtils.findSurroundASTNode(ast, parentExpn, ["Statement"]);
185213
extract(scopes, parentStatement, [{ start: start, end: end }], text);
@@ -212,6 +240,16 @@ define(function(require, exports, module) {
212240
expns,
213241
inlineMenu;
214242

243+
function callExtractToVariable(startPos, endPos, value) {
244+
RefactoringUtils.getScopeData(session, editor.posFromIndex(startPos))
245+
.done(function(expnscope) {
246+
scopes = RefactoringUtils.getAllScopes(ast, expnscope, doc.getText());
247+
extractToVariable(ast, startPos, endPos, value, scopes);
248+
}).fail(function() {
249+
editor.displayErrorMessageAtCursor(Strings.ERROR_TERN_FAILED);
250+
});
251+
}
252+
215253
RefactoringUtils.getScopeData(session, editor.posFromIndex(start)).done(function(scope) {
216254
ast = RefactoringUtils.getAST(doc.getText());
217255
scopes = RefactoringUtils.getAllScopes(ast, scope, doc.getText());
@@ -253,7 +291,7 @@ define(function(require, exports, module) {
253291

254292
// If only one surround expression, extract
255293
if (expns.length === 1) {
256-
extractToVariable(ast, expns[0].start, expns[0].end, expns[0].value, scopes);
294+
callExtractToVariable(expns[0].start, expns[0].end, expns[0].value);
257295
return;
258296
}
259297

@@ -271,7 +309,7 @@ define(function(require, exports, module) {
271309
inlineMenu.open(expns);
272310

273311
inlineMenu.onSelect(function (expnId) {
274-
extractToVariable(ast, expns[expnId].start, expns[expnId].end, expns[expnId].value, scopes);
312+
callExtractToVariable(expns[expnId].start, expns[expnId].end, expns[expnId].value);
275313
inlineMenu.close();
276314
});
277315

‎src/extensions/default/JavaScriptRefactoring/RefactoringUtils.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,14 @@ define(function (require, exports, module) {
467467
* @return {Object} - Ast of current opened doc
468468
*/
469469
RefactoringSession.prototype.createAstOfCurrentDoc = function () {
470-
return AcornLoose.parse_dammit(this.document.getText());
470+
var ast,
471+
text = this.document.getText();
472+
try {
473+
ast = Acorn.parse(text);
474+
} catch(e) {
475+
ast = Acorn.parse_dammit(text);
476+
}
477+
return ast;
471478
};
472479

473480
/**

‎src/extensions/default/JavaScriptRefactoring/RenameIdentifier.js‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,23 @@ define(function (require, exports, module) {
127127
}
128128
}
129129

130-
if (type === "local") {
131-
editor.setSelections(refs);
132-
} else {
133-
editor.setSelections(refs.filter(function(element) {
130+
var currentPosition = editor.posFromIndex(refsResp.offset),
131+
refsArray = refs;
132+
if (type !== "local") {
133+
refsArray = refs.filter(function (element) {
134134
return isInSameFile(element, refsResp);
135-
}));
135+
});
136136
}
137+
138+
// Finding the Primary Reference in Array
139+
var primaryRef = refsArray.find(function (element) {
140+
return ((element.start.line === currentPosition.line || element.end.line === currentPosition.line)
141+
&& currentPosition.ch <= element.end.ch && currentPosition.ch >= element.start.ch);
142+
});
143+
// Setting the primary flag of Primary Refence to true
144+
primaryRef.primary = true;
145+
146+
editor.setSelections(refsArray);
137147
}
138148

139149
/**

‎src/extensions/default/JavaScriptRefactoring/WrapSelection.js‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ define(function (require, exports, module) {
9797
});
9898

9999
if (wrapperName === TRY_CATCH) {
100-
var cursorLine = current.editor.getSelection().start.line - 1,
100+
var cursorLine = current.editor.getSelection().end.line - 1,
101101
startCursorCh = current.document.getLine(cursorLine).indexOf("\/\/"),
102102
endCursorCh = current.document.getLine(cursorLine).length;
103103

@@ -247,24 +247,36 @@ define(function (require, exports, module) {
247247

248248
var token = TokenUtils.getTokenAt(current.cm, current.cm.posFromIndex(endIndex)),
249249
isLastNode,
250-
lineEndPos,
251-
templateParams;
250+
templateParams,
251+
parentNode,
252+
propertyEndPos;
252253

253254
//Create getters and setters only if selected reference is a property
254255
if (token.type !== "property") {
255256
current.editor.displayErrorMessageAtCursor(Strings.ERROR_GETTERS_SETTERS);
256257
return;
257258
}
258259

260+
parentNode = current.getParentNode(current.ast, endIndex);
259261
// Check if selected propery is child of a object expression
260-
if (!current.getParentNode(current.ast, endIndex)) {
262+
if (!parentNode || !parentNode.properties) {
261263
current.editor.displayErrorMessageAtCursor(Strings.ERROR_GETTERS_SETTERS);
262264
return;
263265
}
264266

267+
268+
var propertyNodeArray = parentNode.properties;
269+
// Find the last Propery Node before endIndex
270+
var properyEndNode = propertyNodeArray.find(function (element) {
271+
return (endIndex >= element.start && endIndex < element.end);
272+
});
273+
274+
//Get Current Selected Property End Index;
275+
propertyEndPos = editor.posFromIndex(properyEndNode.end);
276+
277+
265278
//We have to add ',' so we need to find position of current property selected
266279
isLastNode = current.isLastNodeInScope(current.ast, endIndex);
267-
lineEndPos = current.lineEndPosition(current.startPos.line);
268280
templateParams = {
269281
"getName": token.string,
270282
"setName": token.string,
@@ -276,11 +288,11 @@ define(function (require, exports, module) {
276288
current.document.batchOperation(function() {
277289
if (isLastNode) {
278290
//Add ',' in the end of current line
279-
current.document.replaceRange(",", lineEndPos, lineEndPos);
280-
lineEndPos.ch++;
291+
current.document.replaceRange(",", propertyEndPos, propertyEndPos);
281292
}
293+
propertyEndPos.ch++;
282294

283-
current.editor.setSelection(lineEndPos); //Selection on line end
295+
current.editor.setSelection(propertyEndPos); //Selection on line end
284296

285297
// Add getters and setters for given token using template at current cursor position
286298
current.replaceTextFromTemplate(GETTERS_SETTERS, templateParams);

‎src/nls/root/strings.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ define({
689689
"ERROR_EXTRACTTO_VARIABLE_MULTICURSORS" : "Extract to Variable does not work in multicursors",
690690
"ERROR_EXTRACTTO_FUNCTION_MULTICURSORS" : "Extract to Function does not work in multicursors",
691691
"EXTRACTTO_FUNCTION_SELECT_SCOPE" : "Choose destination scope",
692-
"EXTRACTTO_VARIABLE_SELECT_EXPRESSION" : "Select a expression",
692+
"EXTRACTTO_VARIABLE_SELECT_EXPRESSION" : "Select an expression",
693693
"CMD_REFACTORING_RENAME" : "Rename",
694694
"CMD_REFACTORING_TRY_CATCH" : "Wrap in Try Catch",
695695
"CMD_REFACTORING_CONDITION" : "Wrap in Condition",

0 commit comments

Comments
 (0)
⚡