Skip to content

Commit 9480ca5

Browse files
CopilotDaanV2
andauthored
fix: correct math.pi constant rewriting and code action range in Molang optimizations
- In `string.ts`: Don't append `()` for zero-argument FunctionCall nodes (e.g. `math.pi`) so that constants are serialized correctly without parentheses. - In `nodes.ts`: Fix infinite loop in `getLastEndPosition` for FunctionCall with 0 args by returning early with the correct identifier length. - In `nodes.ts`: Add `ExpressionNode.getStartPosition()` helper that finds the minimum (leftmost) source offset among all descendants of an expression node. - In `registry.ts`: Use `OffsetWord` with the full expression range (from `getStartPosition` to `getLastEndPosition`) when adding optimization diagnostics, so code actions replace the correct text instead of just the operator and adjacent identifier. - In `optimizations.test.ts`: Add regression test asserting that `5*3*math.pi` produces a replacement containing `math.pi` and not `math.pi()`. Agent-Logs-Url: https://github.com/Blockception/minecraft-bedrock-language-server/sessions/c46da7a3-c138-4dfb-a355-ca41b9f597bc Co-authored-by: DaanV2 <2393905+DaanV2@users.noreply.github.com>
1 parent 320dd69 commit 9480ca5

7 files changed

Lines changed: 61 additions & 4 deletions

File tree

ide/vscode/src/version.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
export const Version = "9.0.32";
1+
export const Version = "9.1.3";

package-lock.json

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13549,7 +13549,8 @@
1354913549
"devDependencies": {
1355013550
"@eslint/js": "^10.0.1",
1355113551
"@types/eslint__js": "^9.14.0",
13552-
"@types/node": "^25.6.2",
13552+
"@types/jest": "^30.0.0",
13553+
"@types/node": "^25.7.0",
1355313554
"eslint": "^10.3.0",
1355413555
"rimraf": "^6.1.3",
1355513556
"ts-loader": "^9.5.0",
@@ -13558,6 +13559,23 @@
1355813559
"typescript-eslint": "^8.59.2"
1355913560
}
1356013561
},
13562+
"packages/molang/node_modules/@types/node": {
13563+
"version": "25.7.0",
13564+
"resolved": "https://registry.npmjs.org/@types/node/-/node-25.7.0.tgz",
13565+
"integrity": "sha512-z+pdZyxE+RTQE9AcboAZCb4otwcrvgHD+GlBpPgn0emDVt0ohrTMhAwlr2Wd9nZ+nihhYFxO2pThz3C5qSu2Eg==",
13566+
"dev": true,
13567+
"license": "MIT",
13568+
"dependencies": {
13569+
"undici-types": "~7.21.0"
13570+
}
13571+
},
13572+
"packages/molang/node_modules/undici-types": {
13573+
"version": "7.21.0",
13574+
"resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.21.0.tgz",
13575+
"integrity": "sha512-w9IMgQrz4O0YN1LtB7K5P63vhlIOvC7opSmouCJ+ZywlPAlO9gIkJ+otk6LvGpAs2wg4econaCz3TvQ9xPoyuQ==",
13576+
"dev": true,
13577+
"license": "MIT"
13578+
},
1356113579
"packages/project": {
1356213580
"name": "bc-minecraft-project",
1356313581
"version": "1.22.2-1",

packages/bedrock-diagnoser/src/diagnostics/molang/optimizations/registry.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ExpressionNode, walk } from 'bc-minecraft-molang';
2+
import { OffsetWord } from 'bc-minecraft-bedrock-shared';
23
import { DiagnosticsBuilder } from '../../../types';
34
import { OptimizationCategory, OptimizationRule } from './framework';
45
import { createConstantConditionCategory, createConstantFoldingCategory, createConstantResultCategory, createDivisionByZeroCategory, createDoubleNegationCategory, createIdentityOperationsCategory, createRedundantComparisonCategory, createRedundantUnaryCategory, createSelfCancellationCategory, createSelfDivisionCategory } from './rules';
@@ -63,10 +64,17 @@ export class OptimizationRegistry {
6364
result = [result];
6465
}
6566

67+
// Compute the full source range of the expression so the code action
68+
// replacement targets the correct text.
69+
const startOffset = ExpressionNode.getStartPosition(node);
70+
const endOffset = ExpressionNode.getLastEndPosition(node);
71+
const length = Math.max(1, endOffset - startOffset);
72+
const nodeRange = OffsetWord.create('x'.repeat(length), startOffset);
73+
6674
for (const optimizedNode of result) {
6775
const data = optimizedNode.replacement !== undefined ? { replacement: optimizedNode.replacement } : undefined;
6876
diagnoser.add(
69-
node.position,
77+
nodeRange,
7078
optimizedNode.message,
7179
optimizedNode.severity ?? rule.severity,
7280
optimizedNode.code ?? rule.code,

packages/bedrock-diagnoser/test/lib/diagnostics/molang/optimizations.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,5 +492,17 @@ describe('Molang Optimization Diagnostics', () => {
492492
expect(diag).toBeDefined();
493493
expect(diag?.data).toEqual({ replacement: '6' });
494494
});
495+
496+
it('should not add parentheses to math constants like math.pi in replacement (regression)', () => {
497+
const diagnoser = new TestDiagnoser();
498+
diagnose_molang_syntax_line('5*3*math.pi', diagnoser);
499+
500+
const diag = diagnoser.items.find((d) => d.code === 'molang.optimization.constant-folding');
501+
expect(diag).toBeDefined();
502+
const replacement = (diag?.data as { replacement: string })?.replacement;
503+
expect(replacement).toBeDefined();
504+
expect(replacement).toContain('math.pi');
505+
expect(replacement).not.toContain('math.pi()');
506+
});
495507
});
496508
});

packages/molang/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@
4242
"devDependencies": {
4343
"@eslint/js": "^10.0.1",
4444
"@types/eslint__js": "^9.14.0",
45-
"@types/node": "^25.6.2",
45+
"@types/jest": "^30.0.0",
46+
"@types/node": "^25.7.0",
4647
"eslint": "^10.3.0",
4748
"rimraf": "^6.1.3",
4849
"ts-loader": "^9.5.0",

packages/molang/src/molang/syntax/nodes.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,9 @@ export namespace ExpressionNode {
297297
node = max(node.right, node.left);
298298
break;
299299
case NodeType.FunctionCall:
300+
if (node.arguments.length === 0) {
301+
return node.position + getIdentifier(node).length;
302+
}
300303
const args = node.arguments;
301304
args.forEach((arg) => (node = max(node, arg)));
302305
break;
@@ -316,4 +319,18 @@ export namespace ExpressionNode {
316319
export function toString(node: ExpressionNode): string {
317320
return nodesToString(node);
318321
}
322+
323+
/**
324+
* Gets the minimum (leftmost) character offset of an expression node and all its descendants.
325+
* This is the true start of the expression in the source text.
326+
*/
327+
export function getStartPosition(node: ExpressionNode): number {
328+
let min = node.position;
329+
const children = getChildern(node);
330+
for (const child of children) {
331+
const childMin = getStartPosition(child);
332+
if (childMin < min) min = childMin;
333+
}
334+
return min;
335+
}
319336
}

packages/molang/src/molang/syntax/string.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export function nodesToString(node: ExpressionNode): string {
1212
return `(${nodesToString(node.condition)} ? ${nodesToString(node.trueExpression)} : ${nodesToString(node.falseExpression!)})`;
1313
case NodeType.FunctionCall:
1414
const id = ExpressionNode.getIdentifier(node);
15+
if (node.arguments.length === 0) return id;
1516
return `${id}(${node.arguments.map(nodesToString).join(', ')})`;
1617
case NodeType.Literal:
1718
return node.value;

0 commit comments

Comments
 (0)