Skip to content

Commit a0569d0

Browse files
kammeowskammeows
committed
fixes on renaming logic for classes and methods along with other minor bug fixes
1 parent 842c46b commit a0569d0

File tree

5 files changed

+66
-75
lines changed

5 files changed

+66
-75
lines changed

‎client/modules/IDE/components/Editor/index.jsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,9 @@ class Editor extends React.Component {
227227
}
228228

229229
this._cm.on('keydown', (_cm, e) => {
230-
// Show hint
230+
if ((e.ctrlKey || e.metaKey) && e.key === 'v') {
231+
return;
232+
}
231233
const mode = this._cm.getOption('mode');
232234
if (/^[a-z]$/i.test(e.key) && (mode === 'css' || mode === 'javascript')) {
233235
this.showHint(_cm);

‎client/modules/IDE/components/p5CodeAstAnalyzer.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ Object.entries(classMap).forEach(([className, classData]) => {
1717
});
1818
});
1919

20-
// Cache to store last valid result
2120
let lastValidResult = {
2221
variableToP5ClassMap: {},
2322
scopeToDeclaredVarsMap: {},
@@ -90,7 +89,6 @@ function _p5CodeAstAnalyzer(_cm) {
9089
const methodName = element.key.name;
9190

9291
if (element.kind === 'constructor') {
93-
// constructor params
9492
element.params.forEach((param) => {
9593
if (param.type === 'Identifier') {
9694
classInfo.constructor_params.push(param.name);
@@ -114,7 +112,6 @@ function _p5CodeAstAnalyzer(_cm) {
114112
}
115113
});
116114

117-
// collect constructor locals
118115
traverse(
119116
element,
120117
{
@@ -132,7 +129,6 @@ function _p5CodeAstAnalyzer(_cm) {
132129
} else {
133130
classInfo.methods.push(methodName);
134131

135-
// collect local vars inside method
136132
const localVars = [];
137133
element.body.body.forEach((stmt) => {
138134
if (stmt.type === 'VariableDeclaration') {
@@ -147,7 +143,6 @@ function _p5CodeAstAnalyzer(_cm) {
147143
classInfo.methodVars[methodName] = localVars;
148144
}
149145

150-
// ✅ Collect this.* assignments and this.* calls in *all* methods (incl constructor)
151146
traverse(
152147
element,
153148
{
@@ -258,7 +253,6 @@ function _p5CodeAstAnalyzer(_cm) {
258253
})
259254
.filter(Boolean);
260255

261-
// Store function metadata for hinting
262256
userDefinedFunctionMetadata[fnName] = {
263257
text: fnName,
264258
type: 'fun',
@@ -287,11 +281,9 @@ function _p5CodeAstAnalyzer(_cm) {
287281
};
288282

289283
lastValidResult = result;
290-
console.log(result);
291284
return result;
292285
}
293286

294-
// Export a debounced version
295287
export default debounce(_p5CodeAstAnalyzer, 200, {
296288
leading: true,
297289
trailing: true,

‎client/modules/IDE/components/rename-variable.js

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,11 @@ export function handleRename(fromPos, oldName, newName, cm) {
3030
function startRenaming(cm, ast, fromPos, newName, oldName) {
3131
const code = cm.getValue();
3232
const fromIndex = cm.indexFromPos(fromPos);
33-
const { scopeToDeclaredVarsMap = {}, userDefinedClassMetadata = {} } =
34-
p5CodeAstAnalyzer(cm) || {};
33+
const {
34+
scopeToDeclaredVarsMap = {},
35+
userDefinedClassMetadata = {},
36+
userDefinedFunctionMetadata = {}
37+
} = p5CodeAstAnalyzer(cm) || {};
3538

3639
const baseContext = getContext(
3740
cm,
@@ -40,23 +43,19 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
4043
scopeToDeclaredVarsMap,
4144
userDefinedClassMetadata
4245
);
43-
const isInsideClassContext = baseContext in userDefinedClassMetadata;
44-
45-
const bc = getClassContext(cm, ast, fromPos);
4646

47+
const isInsideClassContext = baseContext in userDefinedClassMetadata;
48+
const baseClassContext = getClassContext(cm, ast, fromPos);
4749
const isBaseThis = isThisReference(cm, ast, fromPos, oldName);
4850
const isGlobal = isGlobalReference(
4951
oldName,
5052
baseContext,
5153
scopeToDeclaredVarsMap,
5254
userDefinedClassMetadata,
53-
bc,
55+
baseClassContext,
5456
isBaseThis
5557
);
5658

57-
console.log('Clicked identifier isThisReference? →', isBaseThis);
58-
console.log(fromPos, isGlobal, 'iscc=', isInsideClassContext);
59-
6059
const replacements = [];
6160

6261
traverse(ast, {
@@ -77,11 +76,8 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
7776
parent.type === 'ArrowFunctionExpression') &&
7877
parent.params.some((p) => p.type === 'Identifier' && p.name === oldName)
7978
) {
80-
// If the node *is* one of the parameters being declared, allow renaming
8179
if (parent.params.includes(node)) {
82-
// parameter declaration → mark for renaming
8380
} else {
84-
// usage inside param list → skip
8581
return;
8682
}
8783
}
@@ -109,7 +105,6 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
109105
const isThis = isThisReference(cm, ast, pos, oldName);
110106

111107
if (isInsideClassContext) {
112-
// Case: inside a class
113108
const classContext = getClassContext(cm, ast, fromPos);
114109
let baseMethodName = null;
115110
if (classContext && classContext.includes('.')) {
@@ -130,31 +125,23 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
130125
}
131126
}
132127

133-
// 1. Constructor parameter renaming
134-
console.log(baseMethodName, isThis, isBaseThis);
135128
if (
136129
baseMethodName === 'constructor' &&
137130
classMeta.constructor_params.includes(oldName) &&
138131
isThis === isBaseThis &&
139132
baseContext === thisContext
140133
) {
141-
// Only rename inside the constructor body
142-
console.log(pos, 'constructor=', thisContext, baseContext);
143-
144134
for (const [methodName, vars] of Object.entries(
145135
classMeta.methodVars || {}
146136
)) {
147-
// console.log(pos, !vars.includes(oldName), thisContext, methodName);
148137
if (!vars.includes(oldName) && thisContext === methodName) {
149138
shouldRenameMethodVar = true;
150139
}
151140
}
152141
shouldRename =
153142
thisContext === baseContext &&
154143
(currentMethodName == 'constructor' || shouldRenameGlobalVar);
155-
}
156-
// 2. Local variable inside a method (methodVars)
157-
else {
144+
} else {
158145
for (const [methodName, vars] of Object.entries(
159146
classMeta.methodVars || {}
160147
)) {
@@ -192,6 +179,24 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
192179
!isInBaseScope && thisScopeVars.hasOwnProperty(oldName);
193180
const isDeclaredInBaseScope = baseScopeVars.hasOwnProperty(oldName);
194181
const isDeclaredGlobally = globalScopeVars.hasOwnProperty(oldName);
182+
const isThisGlobal = isGlobalReference(
183+
oldName,
184+
thisContext,
185+
scopeToDeclaredVarsMap,
186+
userDefinedClassMetadata,
187+
baseClassContext,
188+
isBaseThis
189+
);
190+
191+
if (
192+
isThisGlobal &&
193+
thisContext in userDefinedFunctionMetadata &&
194+
userDefinedFunctionMetadata[thisContext].params.some(
195+
(param) => param.p === oldName
196+
)
197+
) {
198+
return;
199+
}
195200

196201
const methodPath = path.findParent((p) => p.isClassMethod());
197202
let currentMethodName = null;
@@ -206,15 +211,14 @@ function startRenaming(cm, ast, fromPos, newName, oldName) {
206211
}
207212
}
208213

209-
// ✅ NEW: If we're in a class constructor and "oldName" is a parameter, skip
210214
if (
211215
thisContext in userDefinedClassMetadata &&
212216
currentMethodName === 'constructor' &&
213217
userDefinedClassMetadata[thisContext]?.constructor_params?.includes(
214218
oldName
215219
)
216220
) {
217-
return; //don’t rename constructor param or its usages
221+
return;
218222
}
219223

220224
if (

‎client/modules/IDE/utils/ScreenReaderHelper.jsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ export default function announceToScreenReader(message, assertive = false) {
55
// liveRegion.setAttribute('aria-live', assertive ? 'assertive' : 'polite');
66
liveRegion.setAttribute('aria-live', 'assertive');
77

8-
// Clear first to ensure repeated messages are read
98
liveRegion.textContent = '';
109
setTimeout(() => {
1110
liveRegion.textContent = message;

‎client/modules/IDE/utils/renameVariableHelper.jsx

Lines changed: 35 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,34 @@ export function isGlobalReference(
99
context,
1010
scopeToDeclaredVarsMap,
1111
userDefinedClassMetadata,
12-
bc,
12+
baseClassContext,
1313
isBaseThis
1414
) {
15-
console.log(context, bc, varName);
16-
// First check: is this declared global at all?
1715
const globalVars = scopeToDeclaredVarsMap['global'] || {};
1816
if (!globalVars.hasOwnProperty(varName)) {
19-
return false; // not a global
17+
return false;
2018
}
2119

22-
// Next: check if shadowed in local scope (block, function, etc.)
2320
const localVars = scopeToDeclaredVarsMap[context] || {};
24-
if (localVars.hasOwnProperty(varName)) {
25-
return false; // locally declared → not global
21+
if (!(context === 'global') && localVars.hasOwnProperty(varName)) {
22+
return false;
2623
}
2724

28-
// If inside a class context
29-
if (bc) {
30-
// const [className, methodName] = context.split('.');
25+
if (baseClassContext) {
3126
const className = context;
3227
let methodName = null;
33-
if (bc && bc.includes('.')) {
34-
methodName = bc.split('.').pop();
28+
if (baseClassContext && baseClassContext.includes('.')) {
29+
methodName = baseClassContext.split('.').pop();
3530
}
36-
console.log('meow', className, methodName);
3731
const classMeta = userDefinedClassMetadata[className];
3832
if (classMeta) {
39-
// Shadowing inside constructor
4033
if (
4134
methodName === 'constructor' &&
4235
classMeta.constructor_params?.includes(varName)
4336
) {
4437
return false;
4538
}
4639

47-
// Shadowing inside method vars
4840
if (
4941
methodName &&
5042
classMeta.methodVars?.[methodName] &&
@@ -53,14 +45,12 @@ export function isGlobalReference(
5345
return false;
5446
}
5547

56-
// Shadowing by instance fields declared in class (like "this.a = ...")
5748
if (classMeta.fields?.includes(varName) && isBaseThis) {
5849
return false;
5950
}
6051
}
6152
}
6253

63-
// If we got here, it's not shadowed → must be a reference to the global
6454
return true;
6555
}
6656

@@ -73,16 +63,15 @@ export function isThisReference(cm, ast, fromPos, oldName) {
7363
const { node } = path;
7464
if (!node.loc) return;
7565

76-
// check if cursor is inside this property name
7766
if (offset >= node.start && offset <= node.end) {
7867
if (
7968
node.object.type === 'ThisExpression' &&
8069
node.property.type === 'Identifier' &&
8170
node.property.name === oldName &&
82-
!node.computed // skip this["foo"]
71+
!node.computed
8372
) {
8473
isThisRef = true;
85-
path.stop(); // found it, stop traversal
74+
path.stop();
8675
}
8776
}
8877
}
@@ -105,30 +94,37 @@ export function getContext(
10594
enter(path) {
10695
const { node } = path;
10796
if (!node.loc) return;
108-
if (offset < node.start || offset > node.end) return;
97+
if (offset < node.start || offset > node.end) {
98+
return;
99+
}
109100

110101
if (
111-
(node.type === 'FunctionDeclaration' ||
112-
node.type === 'FunctionExpression' ||
113-
node.type === 'ArrowFunctionExpression') &&
114-
node.body &&
115-
offset >= node.body.start &&
116-
offset <= node.body.end
102+
node.type === 'FunctionDeclaration' ||
103+
node.type === 'FunctionExpression' ||
104+
node.type === 'ArrowFunctionExpression'
117105
) {
118-
if (node.id?.name) {
119-
context = node.id.name;
120-
} else {
121-
const parent = path.parentPath?.node;
122-
if (
123-
parent?.type === 'VariableDeclarator' &&
124-
parent.id?.type === 'Identifier'
125-
) {
126-
context = parent.id.name;
106+
const inBody =
107+
node.body && offset >= node.body.start && offset <= node.body.end;
108+
const inParams = node.params?.some(
109+
(p) => offset >= p.start && offset <= p.end
110+
);
111+
112+
if (inBody || inParams) {
113+
if (node.id?.name) {
114+
context = node.id.name;
127115
} else {
128-
context = '(anonymous)';
116+
const parent = path.parentPath?.node;
117+
if (
118+
parent?.type === 'VariableDeclarator' &&
119+
parent.id?.type === 'Identifier'
120+
) {
121+
context = parent.id.name;
122+
} else {
123+
context = '(anonymous)';
124+
}
129125
}
126+
path.skip();
130127
}
131-
path.skip();
132128
}
133129

134130
if (
@@ -172,7 +168,6 @@ export function getClassContext(cm, ast, fromPos) {
172168
if (!node.loc) return;
173169
if (offset < node.start || offset > node.end) return;
174170

175-
// Check if we're inside a class
176171
if (
177172
(node.type === 'ClassDeclaration' || node.type === 'ClassExpression') &&
178173
offset >= node.start &&
@@ -181,7 +176,6 @@ export function getClassContext(cm, ast, fromPos) {
181176
const className = node.id?.name || '(anonymous class)';
182177
classContext = className;
183178

184-
// Look for the method containing the offset
185179
const methodPath = path.get('body.body').find((p) => {
186180
return (
187181
(p.node.type === 'ClassMethod' ||
@@ -211,5 +205,5 @@ export function getClassContext(cm, ast, fromPos) {
211205
}
212206
});
213207

214-
return classContext; // null if not inside a class
208+
return classContext;
215209
}

0 commit comments

Comments
 (0)