Skip to content

Commit ccc561f

Browse files
committed
Fix lambda and function parameter not renaming
1 parent 433d552 commit ccc561f

File tree

5 files changed

+65
-25
lines changed

5 files changed

+65
-25
lines changed

server/src/ast.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ export class AST {
195195
}
196196
const param_declaration: ParameterSymbol = {
197197
name: name,
198-
scope: lambda.body.loc!,
198+
scope: lambda.loc!,
199199
meta: this.context.chapter == Chapter.SOURCE_1 || this.context.chapter === Chapter.SOURCE_2 ? "const" : "let",
200200
declarationKind: DeclarationKind.KIND_PARAM,
201201
range: sourceLocToRange(param.loc!),
@@ -241,7 +241,8 @@ export class AST {
241241
}
242242
const param_declaration: ParameterSymbol = {
243243
name: name,
244-
scope: child.body.loc!,
244+
// Set the scope to the function loc so that the parameter can be renamed
245+
scope: child.loc!,
245246
meta: this.context.chapter == Chapter.SOURCE_1 || this.context.chapter === Chapter.SOURCE_2 ? "const" : "let",
246247
declarationKind: DeclarationKind.KIND_PARAM,
247248
range: sourceLocToRange(loc),
@@ -351,12 +352,12 @@ export class AST {
351352
scopeFound = true;
352353
if (scopeFound && node.type === NODES.IDENTIFIER && node.name === identifier.name)
353354
ret.push(sourceLocToRange(node.loc!))
354-
355+
355356
getNodeChildren(node, true).forEach(node => {
356357
if (
357358
scopeFound
358359
// We want to ignore scopes where the variable was redeclared
359-
// This occurs when there is an inner scope in the scope of our declaration, and the child node belongs in that scope
360+
// This occurs when there is a declaration with a scope in the scope of our original declaration, and the child node belongs in that scope
360361
&& !this.declarations.get(identifier.name)?.some(x => !sourceLocEquals(x.scope, declaration.scope) && sourceLocInSourceLoc(x.scope, declaration.scope) && sourceLocInSourceLoc(node.loc!, x.scope))
361362
|| (!scopeFound && node.loc && sourceLocInSourceLoc(declaration.scope, node.loc))
362363
)
@@ -387,9 +388,11 @@ export class AST {
387388
// This leads to an error in the client, as there are two changes to the program that have overlapping ranges
388389
// For now, if the name that the user is trying to rename is an imported name, we don't allow renames
389390
// I will leave this for a future CP3108 student to fix :)
391+
390392
const identifier = this.findIdentifierNode(vsPosToEsPos(pos));
391393
if (!identifier) return null;
392394

395+
393396
const declaration = this.findDeclarationByName(identifier.name, identifier.loc!);
394397
if (!declaration || declaration.declarationKind === DeclarationKind.KIND_IMPORT) return null;
395398

server/src/test/featuresTest.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,56 @@ suite("Rename", () => {
9696
}
9797
});
9898
});
99+
100+
test("Rename Function Parameters", () => {
101+
const text = "function test(arg1) { return arg1; }";
102+
const doc = TextDocument.create("test://test/test.sourcejs", 'sourcejs', 0, text);
103+
const ast = new AST(text, context, doc.uri);
104+
const edits = ast.renameSymbol(
105+
{ line: 0, character: 15 },
106+
'y'
107+
)
108+
109+
assert.deepStrictEqual(edits, {
110+
changes: {
111+
[doc.uri]: [
112+
{
113+
"range": { "start": { "line": 0, "character": 14 }, "end": { "line": 0, "character": 18 } },
114+
"newText": "y"
115+
},
116+
{
117+
"range": { "start": { "line": 0, "character": 29 }, "end": { "line": 0, "character": 33 } },
118+
"newText": "y"
119+
}
120+
]
121+
}
122+
})
123+
})
124+
125+
test("Rename Lambda Parameters", () => {
126+
const text = "const test = (arg1) => { return arg1; }";
127+
const doc = TextDocument.create("test://test/test.sourcejs", 'sourcejs', 0, text);
128+
const ast = new AST(text, context, doc.uri);
129+
const edits = ast.renameSymbol(
130+
{ line: 0, character: 15 },
131+
'y'
132+
)
133+
134+
assert.deepStrictEqual(edits, {
135+
changes: {
136+
[doc.uri]: [
137+
{
138+
"range": { "start": { "line": 0, "character": 14 }, "end": { "line": 0, "character": 18 } },
139+
"newText": "y"
140+
},
141+
{
142+
"range": { "start": { "line": 0, "character": 32 }, "end": { "line": 0, "character": 36 } },
143+
"newText": "y"
144+
}
145+
]
146+
}
147+
})
148+
})
99149
});
100150

101151
suite("Document Symbols", () => {

server/src/utils.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ export function getNodeChildren(node: es.Node, allChildren = false): es.Node[] {
111111
return node.argument ? [node.argument] : []
112112
case 'FunctionDeclaration':
113113
const func_id: es.Node[] = (allChildren && node.id) ? [node.id] : []
114-
return func_id.concat([node.body])
114+
return func_id.concat(allChildren ? node.params : [], [node.body])
115115
case 'VariableDeclaration':
116116
return node.declarations.flatMap(x => getNodeChildren(x, allChildren))
117117
case 'VariableDeclarator':
@@ -123,7 +123,7 @@ export function getNodeChildren(node: es.Node, allChildren = false): es.Node[] {
123123
case 'ImportSpecifier':
124124
return [node.imported, node.local]
125125
case 'ArrowFunctionExpression':
126-
return [node.body]
126+
return (allChildren ? (node.params as es.Node[]) : []).concat(node.body)
127127
case 'FunctionExpression':
128128
return [node.body]
129129
case 'UnaryExpression':
@@ -252,7 +252,7 @@ export function esPosInSourceLoc(pos: es.Position, loc: es.SourceLocation) {
252252
}
253253

254254
export function sourceLocInSourceLoc(inner: es.SourceLocation, outer: es.SourceLocation) {
255-
return vsPosInSourceLoc({ line: inner.start.line - 1, character: inner.start.column }, outer) && vsPosInSourceLoc({ line: inner.end.line - 1, character: inner.end.column }, outer);
255+
return esPosInSourceLoc(inner.start, outer) && esPosInSourceLoc(inner.end, outer);
256256
}
257257

258258
export function sourceLocEquals(s1: es.SourceLocation, s2: es.SourceLocation) {

test_files/functions.source

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,4 @@
1-
// PREPENDED LINES, DO NOT EDIT
2-
import { black } from "rune";
3-
const foo = (x, y) => x*y;
4-
// END OF PREPEND
5-
function test(arg1, ...args) {
6-
7-
}
8-
9-
const x = () => return 0
10-
x();
11-
12-
let y = 1;
13-
y(); // Runtime error
14-
15-
foo(10, 5);
1+
const x = 1;
2+
function test(arg1) {
3+
return arg1;
4+
}

test_files/lambda.source

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
const test = (x) => {
22
return x;
3-
};
4-
5-
(x) => {};
3+
};

0 commit comments

Comments
 (0)