Skip to content

Commit b8d1e67

Browse files
CopilotDaanV2
andauthored
feat: support variable parameter counts with type validation in molang queries (#79)
## Issue/Feature Request/Bug report - Fixes #68 ## Changes Molang queries like `query.is_owner_identifier_any` and `query.equipped_item_all_tags` accept variable numbers of arguments (n ≥ minimum), but the diagnoser was enforcing exact parameter counts and not validating parameter types. **Core data model** (`packages/molang/src/data/molang-function.ts`): - Added `repeatable?: boolean` field to `MolangParameter` interface to mark parameters that can be repeated indefinitely - Removed complex `minParams` and `repeatableParam` fields from `MolangFunction` interface in favor of simpler per-parameter approach **Query definitions** (`packages/molang/src/data/general.ts`): - Updated 16 queries with type annotations and `repeatable: true` on the last parameter: - Entity queries: `is_owner_identifier_any`, `is_name_any` (type: string, repeatable) - Equipment: `equipped_item_all_tags`, `equipped_item_any_tag` (slot: string, tag: string repeatable) - Block position + tags: `block_has_all_tags`, `block_neighbor_has_all_tags`, etc (xyz: float, tag: string repeatable) - Client: `graphics_mode_is_any`, `last_input_mode_is_any` (mode: string repeatable) - Item: `is_item_name_any` (slot: string, index: float, item: string repeatable) - Biome: `entity_biome_has_all_tags`, `entity_biome_has_any_tags`, etc (tag/identifier: string repeatable) **Diagnostics** (`packages/bedrock-diagnoser/src/diagnostics/molang/expressions.ts`): - Updated validation to detect if the last parameter has `repeatable: true` - When repeatable parameter exists, validates minimum parameter count equals `parameters.length` - Added parameter type validation for both fixed and repeatable parameters - Validates argument types match expected types (string, float, boolean) - Provides detailed error messages: "wrong argument type at position X, expected Y but got Z" - Falls back to exact count check when no repeatable parameter exists (backward compatible) ## Examples **Before:** ```javascript q.is_owner_identifier_any('zombie', 'skeleton', 'creeper') // ❌ Error: expected 2 arguments but got 3 q.block_has_all_tags(0, 0, 0, 2, 1) // ✅ No validation error (incorrect!) ``` **After:** ```javascript q.is_owner_identifier_any('zombie', 'skeleton', 'creeper') // ✅ q.equipped_item_all_tags('slot.weapon.mainhand', 'tag1', 'tag2', 'tag3') // ✅ q.block_has_all_tags(0, 0, 0, 'stone', 'solid') // ✅ q.block_has_all_tags(0, 0, 0, 2, 1) // ❌ Error: wrong argument type at position 4, expected string but got float ``` **New structure:** ```typescript { id: 'equipped_item_all_tags', parameters: [ { id: 'slot_name', type: 'string' }, { id: 'tag', type: 'string', repeatable: true } ] } ``` ## Checklist - [x] It can build - [x] The tests are passing (530 tests across packages) - [x] Code has been reviewed <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>feat: additional molang parameter support</issue_title> > <issue_description>https://github.com/Xterionix/minecraft-bedrock-language-server/blob/main/packages/molang/src/data/general.ts#L1199 > > I'm thinking the solution could be either optional args or, the better option, allowing a query to essentially take n >= c amount of a certain argument. > > `query.equipped_item_all_tags` follows a similar format > > ``` > query.equipped_item_all_tags('slot.name', tag1, tag2, tag3, tag4 ...) > ```</issue_description> > > <agent_instructions>Can you update the molang data structure to allows for specifiying the minimum amount of parameters, and then update the diagnoser to check this value? so we can support parameters that can have an inifite amount</agent_instructions> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@DaanV2</author><body> > Think that a parameter on the query would be good enough for this 👍 easy to check in the diagnoser</body></comment_new> > <comment_new><author>@DaanV2</author><body> > Im going to do something bold (I want to test it), going to see if I can have copilot do it</body></comment_new> > </comments> > </details> - Fixes #68 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: DaanV2 <[email protected]>
1 parent 7fdda98 commit b8d1e67

File tree

5 files changed

+213
-22
lines changed

5 files changed

+213
-22
lines changed

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.1";
1+
export const Version = "9.0.5";

packages/bedrock-diagnoser/src/diagnostics/molang/expressions.ts

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
FunctionCallNode,
66
MolangData,
77
MolangFunction,
8+
MolangParameter,
89
MolangSet,
910
MolangSyntaxError,
1011
NodeType,
@@ -223,13 +224,80 @@ export function diagnose_molang_function(fn: FunctionCallNode, diagnoser: Diagno
223224
}
224225

225226
if (fnData.parameters) {
226-
if (fnData.parameters.length != fn.arguments.length) {
227-
diagnoser.add(
228-
OffsetWord.create(`${fn.scope}.${fn.names.join('.')}`, fn.position),
229-
`wrong amount of arguments, expected ${fnData.parameters.length} but got ${fn.arguments.length}`,
230-
DiagnosticSeverity.error,
231-
'molang.function.arguments',
232-
);
227+
// Check if the last parameter is repeatable
228+
const lastParam = fnData.parameters[fnData.parameters.length - 1];
229+
const hasRepeatableParam = lastParam?.repeatable === true;
230+
const minRequiredParams = fnData.parameters.length;
231+
232+
// Validate parameter count
233+
if (hasRepeatableParam) {
234+
// With repeatable parameter, we need at least the minimum parameters
235+
if (fn.arguments.length < minRequiredParams) {
236+
diagnoser.add(
237+
OffsetWord.create(`${fn.scope}.${fn.names.join('.')}`, fn.position),
238+
`wrong amount of arguments, expected at least ${minRequiredParams} but got ${fn.arguments.length}`,
239+
DiagnosticSeverity.error,
240+
'molang.function.arguments',
241+
);
242+
}
243+
} else {
244+
// Without repeatable parameter, we need exact match
245+
if (fnData.parameters.length != fn.arguments.length) {
246+
diagnoser.add(
247+
OffsetWord.create(`${fn.scope}.${fn.names.join('.')}`, fn.position),
248+
`wrong amount of arguments, expected ${fnData.parameters.length} but got ${fn.arguments.length}`,
249+
DiagnosticSeverity.error,
250+
'molang.function.arguments',
251+
);
252+
}
233253
}
254+
255+
// Validate parameter types
256+
for (let i = 0; i < fn.arguments.length; i++) {
257+
const arg = fn.arguments[i];
258+
let expectedParam: MolangParameter | undefined;
259+
260+
// Determine which parameter definition to use
261+
if (i < fnData.parameters.length) {
262+
// Use the parameter at this index
263+
expectedParam = fnData.parameters[i];
264+
} else if (hasRepeatableParam) {
265+
// Use the last (repeatable) parameter for additional arguments
266+
expectedParam = lastParam;
267+
}
268+
269+
// Validate type if specified
270+
if (expectedParam?.type) {
271+
const actualType = getArgumentType(arg);
272+
if (actualType && actualType !== expectedParam.type) {
273+
diagnoser.add(
274+
OffsetWord.create(`${fn.scope}.${fn.names.join('.')}`, fn.position),
275+
`wrong argument type at position ${i + 1}, expected ${expectedParam.type} but got ${actualType}`,
276+
DiagnosticSeverity.error,
277+
'molang.function.arguments.type',
278+
);
279+
}
280+
}
281+
}
282+
}
283+
}
284+
285+
/**
286+
* Helper function to determine the type of an argument node
287+
*/
288+
function getArgumentType(arg: ExpressionNode): 'string' | 'float' | 'boolean' | undefined {
289+
switch (arg.type) {
290+
case NodeType.StringLiteral:
291+
return 'string';
292+
case NodeType.Literal:
293+
// Check if it's a boolean literal (true/false) or numeric
294+
const value = (arg as any).value?.toLowerCase();
295+
if (value === 'true' || value === 'false') {
296+
return 'boolean';
297+
}
298+
return 'float';
299+
default:
300+
// For complex expressions, we can't determine the type statically
301+
return undefined;
234302
}
235303
}

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,30 @@ describe("Molang Syntax", () => {
4141
name: "Some complex",
4242
data: "v.temp_outfit!=q.property('foo:bar')+q.property('foo:bar')+q.property('foo:bar')",
4343
},
44+
{
45+
name: "Variable parameters: is_owner_identifier_any with 1 arg",
46+
data: "q.is_owner_identifier_any('minecraft:zombie')",
47+
},
48+
{
49+
name: "Variable parameters: is_owner_identifier_any with 3 args",
50+
data: "q.is_owner_identifier_any('minecraft:zombie', 'minecraft:skeleton', 'minecraft:creeper')",
51+
},
52+
{
53+
name: "Variable parameters: equipped_item_all_tags with 2 args",
54+
data: "q.equipped_item_all_tags('slot.weapon.mainhand', 'tag1')",
55+
},
56+
{
57+
name: "Variable parameters: equipped_item_all_tags with 4 args",
58+
data: "q.equipped_item_all_tags('slot.weapon.mainhand', 'tag1', 'tag2', 'tag3')",
59+
},
60+
{
61+
name: "Variable parameters: is_name_any with multiple args",
62+
data: "q.is_name_any('entity1', 'entity2', 'entity3', 'entity4')",
63+
},
64+
{
65+
name: "Variable parameters: block_has_all_tags with 5 args",
66+
data: "q.block_has_all_tags(0, 0, 0, 'tag1', 'tag2')",
67+
},
4468
];
4569

4670
for (const test of no_errors_tests) {
@@ -51,4 +75,44 @@ describe("Molang Syntax", () => {
5175
diagnoser.expectEmpty();
5276
});
5377
}
78+
79+
const error_tests: TestCase[] = [
80+
{
81+
name: "Variable parameters: is_owner_identifier_any with 0 args",
82+
data: "q.is_owner_identifier_any()",
83+
},
84+
{
85+
name: "Variable parameters: equipped_item_all_tags with 0 args",
86+
data: "q.equipped_item_all_tags()",
87+
},
88+
{
89+
name: "Variable parameters: equipped_item_all_tags with 1 arg (needs at least 2)",
90+
data: "q.equipped_item_all_tags('slot.weapon.mainhand')",
91+
},
92+
{
93+
name: "Variable parameters: block_has_all_tags with 3 args (needs at least 4)",
94+
data: "q.block_has_all_tags(0, 0, 0)",
95+
},
96+
{
97+
name: "Type validation: block_has_all_tags with wrong type for tag (number instead of string)",
98+
data: "q.block_has_all_tags(0, 0, 0, 2, 1)",
99+
},
100+
{
101+
name: "Type validation: equipped_item_all_tags with wrong type for tag (number instead of string)",
102+
data: "q.equipped_item_all_tags('slot.weapon.mainhand', 123)",
103+
},
104+
{
105+
name: "Type validation: is_name_any with wrong type (number instead of string)",
106+
data: "q.is_name_any(123)",
107+
},
108+
];
109+
110+
for (const test of error_tests) {
111+
it(`error test: ${test.name}`, () => {
112+
const diagnoser = new TestDiagnoser();
113+
diagnose_molang_syntax_text("", diagnoser, test.data);
114+
115+
diagnoser.expectAny();
116+
});
117+
}
54118
});

packages/molang/src/data/general.ts

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -446,18 +446,23 @@ export namespace General {
446446
{
447447
id: 'x',
448448
documentation: 'World-origin-relative position on the x axis',
449+
type: 'float',
449450
},
450451
{
451452
id: 'y',
452453
documentation: 'World-origin-relative position on the y axis',
454+
type: 'float',
453455
},
454456
{
455457
id: 'z',
456458
documentation: 'World-origin-relative position on the z axis',
459+
type: 'float',
457460
},
458461
{
459462
id: 'tag',
460-
documentation: 'The first tag',
463+
documentation: 'tag name to check',
464+
type: 'string',
465+
repeatable: true,
461466
},
462467
],
463468
},
@@ -469,18 +474,23 @@ export namespace General {
469474
{
470475
id: 'x',
471476
documentation: 'World-origin-relative position on the x axis',
477+
type: 'float',
472478
},
473479
{
474480
id: 'y',
475481
documentation: 'World-origin-relative position on the y axis',
482+
type: 'float',
476483
},
477484
{
478485
id: 'z',
479486
documentation: 'World-origin-relative position on the z axis',
487+
type: 'float',
480488
},
481489
{
482490
id: 'tag',
483-
documentation: 'The first tag',
491+
documentation: 'tag name to check',
492+
type: 'string',
493+
repeatable: true,
484494
},
485495
],
486496
},
@@ -492,18 +502,23 @@ export namespace General {
492502
{
493503
id: 'x',
494504
documentation: 'block-relative position on the x axis',
505+
type: 'float',
495506
},
496507
{
497508
id: 'y',
498509
documentation: 'block-relative position on the y axis',
510+
type: 'float',
499511
},
500512
{
501513
id: 'z',
502514
documentation: 'block-relative position on the z axis',
515+
type: 'float',
503516
},
504517
{
505518
id: 'tag',
506-
documentation: 'The first tag',
519+
documentation: 'tag name to check',
520+
type: 'string',
521+
repeatable: true,
507522
},
508523
],
509524
},
@@ -515,18 +530,23 @@ export namespace General {
515530
{
516531
id: 'x',
517532
documentation: 'block-relative position on the x axis',
533+
type: 'float',
518534
},
519535
{
520536
id: 'y',
521537
documentation: 'block-relative position on the y axis',
538+
type: 'float',
522539
},
523540
{
524541
id: 'z',
525542
documentation: 'block-relative position on the z axis',
543+
type: 'float',
526544
},
527545
{
528546
id: 'tag',
529-
documentation: 'The first tag',
547+
documentation: 'tag name to check',
548+
type: 'string',
549+
repeatable: true,
530550
},
531551
],
532552
},
@@ -731,11 +751,19 @@ export namespace General {
731751
id: 'equipped_item_all_tags',
732752
documentation:
733753
"takes a slot name followed by any tag you want to check for in the form of 'tag_name' and returns 1 if all of the tags are on that equipped item, 0 otherwise",
754+
parameters: [
755+
{ id: 'slot_name', documentation: 'equipment slot name', type: 'string' },
756+
{ id: 'tag', documentation: 'tag name to check', type: 'string', repeatable: true },
757+
],
734758
},
735759
{
736760
id: 'equipped_item_any_tag',
737761
documentation:
738762
"takes a slot name followed by any tag you want to check for in the form of 'tag_name' and returns 0 if none of the tags are on that equipped item or 1 if at least 1 tag exists",
763+
parameters: [
764+
{ id: 'slot_name', documentation: 'equipment slot name', type: 'string' },
765+
{ id: 'tag', documentation: 'tag name to check', type: 'string', repeatable: true },
766+
],
739767
},
740768
{
741769
id: 'equipped_item_is_attachable',
@@ -783,6 +811,9 @@ export namespace General {
783811
id: 'graphics_mode_is_any',
784812
documentation:
785813
"Takes in one or more arguments ('simple', 'fancy', 'deferred', 'raytraced'). If the graphics mode of the client matches any of the arguments, return 1.0. Available on the Client (Resource Packs) only.",
814+
parameters: [
815+
{ id: 'mode', documentation: "graphics mode ('simple', 'fancy', 'deferred', 'raytraced')", type: 'string', repeatable: true },
816+
],
786817
},
787818
{
788819
id: 'ground_speed',
@@ -1122,9 +1153,9 @@ export namespace General {
11221153
documentation:
11231154
"Takes an equipment slot name (see the replaceitem command) and an optional slot index value. After that, takes one or more full name (with 'namespace:') strings to check for. Returns 1.0 if an item in the specified slot has any of the specified names, otherwise returns 0.0. An empty string '' can be specified to check for an empty slot. Note that querying slot.enderchest, slot.saddle, slot.armor, or slot.chest will only work in behavior packs. A preferred query to query.get_equipped_item_name, as it can be adjusted by Mojang to avoid breaking content if names are changed.",
11241155
parameters: [
1125-
{ id: 'quipment slot name', documentation: 'quipment slot name' },
1126-
{ id: 'slot index', documentation: '' },
1127-
{ id: 'item', documentation: '' },
1156+
{ id: 'slot_name', documentation: 'equipment slot name', type: 'string' },
1157+
{ id: 'slot_index', documentation: 'optional slot index value', type: 'float' },
1158+
{ id: 'item', documentation: 'item name to check', type: 'string', repeatable: true },
11281159
],
11291160
},
11301161

@@ -1170,8 +1201,7 @@ export namespace General {
11701201
documentation:
11711202
"Takes one or more arguments. If the entity's name is any of the specified string values, returns 1.0. Otherwise returns 0.0. A preferred query to query.get_name, as it can be adjusted by Mojang to avoid breaking content if names are changed.",
11721203
parameters: [
1173-
{ id: 'name 1', documentation: 'possible entity name' },
1174-
{ id: 'name 2', documentation: 'possible entity name' },
1204+
{ id: 'name', documentation: 'possible entity name', type: 'string', repeatable: true },
11751205
],
11761206
},
11771207
{
@@ -1200,8 +1230,7 @@ export namespace General {
12001230
documentation:
12011231
'Takes one or more arguments. Returns whether the root actor identifier is any of the specified strings. A preferred query to query.owner_identifier, as it can be adjusted by Mojang to avoid breaking content if names are changed.',
12021232
parameters: [
1203-
{ id: 'name 1', documentation: 'possible entity name' },
1204-
{ id: 'name 2', documentation: 'possible entity name' },
1233+
{ id: 'name', documentation: 'possible entity name', type: 'string', repeatable: true },
12051234
],
12061235
},
12071236
{
@@ -1400,6 +1429,9 @@ export namespace General {
14001429
id: 'last_input_mode_is_any',
14011430
documentation:
14021431
"Takes one or more arguments ('keyboard_and_mouse', 'touch', or 'gamepad'). If the last input used is any of the specified string values, returns 1.0. Otherwise returns 0.0. Available on the Client (Resource Packs) only.",
1432+
parameters: [
1433+
{ id: 'mode', documentation: "input mode ('keyboard_and_mouse', 'touch', or 'gamepad')", type: 'string', repeatable: true },
1434+
],
14031435
},
14041436
{
14051437
id: 'lie_amount',
@@ -1527,11 +1559,23 @@ export namespace General {
15271559
id: 'relative_block_has_all_tags',
15281560
documentation:
15291561
'Takes an entity-relative position and one or more tag names, and returns either 0 or 1 based on if the block at that position has all of the tags provided.',
1562+
parameters: [
1563+
{ id: 'x', documentation: 'entity-relative position on the x axis', type: 'float' },
1564+
{ id: 'y', documentation: 'entity-relative position on the y axis', type: 'float' },
1565+
{ id: 'z', documentation: 'entity-relative position on the z axis', type: 'float' },
1566+
{ id: 'tag', documentation: 'tag name to check', type: 'string', repeatable: true },
1567+
],
15301568
},
15311569
{
15321570
id: 'relative_block_has_any_tag',
15331571
documentation:
15341572
'Takes an entity-relative position and one or more tag names, and returns either 0 or 1 based on if the block at that position has any of the tags provided.',
1573+
parameters: [
1574+
{ id: 'x', documentation: 'entity-relative position on the x axis', type: 'float' },
1575+
{ id: 'y', documentation: 'entity-relative position on the y axis', type: 'float' },
1576+
{ id: 'z', documentation: 'entity-relative position on the z axis', type: 'float' },
1577+
{ id: 'tag', documentation: 'tag name to check', type: 'string', repeatable: true },
1578+
],
15351579
},
15361580
{
15371581
id: 'ride_body_x_rotation',
@@ -1750,15 +1794,24 @@ export namespace General {
17501794
// Experimental
17511795
{
17521796
id: 'entity_biome_has_all_tags',
1753-
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more tag names, and returns either 0 or 1 based on if all of the tag names match. Only supported in resource packs (client-side).'
1797+
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more tag names, and returns either 0 or 1 based on if all of the tag names match. Only supported in resource packs (client-side).',
1798+
parameters: [
1799+
{ id: 'tag', documentation: 'biome tag name to check', type: 'string', repeatable: true },
1800+
],
17541801
},
17551802
{
17561803
id: 'entity_biome_has_any_identifier',
1757-
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more identifier names, and returns either 0 or 1 based on if any of the identifier names match. Only supported in resource packs (client-side).'
1804+
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more identifier names, and returns either 0 or 1 based on if any of the identifier names match. Only supported in resource packs (client-side).',
1805+
parameters: [
1806+
{ id: 'identifier', documentation: 'biome identifier to check', type: 'string', repeatable: true },
1807+
],
17581808
},
17591809
{
17601810
id: 'entity_biome_has_any_tags',
1761-
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more tag names, and returns either 0 or 1 based on if any of the tag names match. Only supported in resource packs (client-side).'
1811+
documentation: '(EXPERIMENTAL) Compares the biome the entity is standing in with one or more tag names, and returns either 0 or 1 based on if any of the tag names match. Only supported in resource packs (client-side).',
1812+
parameters: [
1813+
{ id: 'tag', documentation: 'biome tag name to check', type: 'string', repeatable: true },
1814+
],
17621815
},
17631816
{
17641817
id: 'get_pack_setting',

0 commit comments

Comments
 (0)