Skip to content

Commit fa879ea

Browse files
harrisontharrisont
authored
Fix a bug where the ForEach loop variable did not appear on hover or on "go to document symbols". (#33)
Fixes #31.
1 parent 499a6cf commit fa879ea

File tree

5 files changed

+129
-37
lines changed

5 files changed

+129
-37
lines changed

‎CHANGELOG.md

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
1+
# v0.5.8
2+
3+
## Bug Fixes
4+
5+
* ([#31](https://github.com/harrisont/fastbuild-vscode/issues/4)) Fix a bug where the `ForEach` loop variable did not appear on hover or on "go to document symbols".
6+
17
# v0.5.7
28

39
## Changes
410

5-
* [#32] Stop returning duplicates for "get document/workspace symbols". This makes the results more usable. Additionally, in experiments on a large code base, this makes "get workspace symbols" twice as fast.
11+
* ([#28](https://github.com/harrisont/fastbuild-vscode/issues/28)) Stop returning duplicates for "get document/workspace symbols". This makes the results more usable. Additionally, in experiments on a large code base, this makes "get workspace symbols" twice as fast.
612

713
# v0.5.6
814

915
## Bug fixes
1016

11-
* [#27] Fix bug where the document symbols are missing data from the last change.
17+
* ([#27](https://github.com/harrisont/fastbuild-vscode/issues/27)) Fix bug where the document symbols are missing data from the last change.
1218

1319
# v0.5.5
1420

@@ -42,7 +48,7 @@
4248

4349
## Bug Fixes
4450

45-
* `#if exists(...)` now correctly evaluates if the environment variable exists instead of always being false (#23).
51+
* ([#23](https://github.com/harrisont/fastbuild-vscode/issues/23)) `#if exists(...)` now correctly evaluates if the environment variable exists instead of always being false.
4652

4753
# v0.5.0
4854

@@ -82,11 +88,11 @@
8288

8389
# v0.2.0
8490

85-
* [#4](https://github.com/harrisont/fastbuild-vscode/issues/4) Support listing document and workspace symbols.
91+
* ([#4](https://github.com/harrisont/fastbuild-vscode/issues/4)) Support listing document and workspace symbols.
8692

8793
# v0.1.9
8894

89-
* [#6](https://github.com/harrisont/fastbuild-vscode/issues/6) Support `ForEach` iterating over multiple arrays at a time (single array iterating already supported). This completes support for the full FASTBuild language.
95+
* ([#6](https://github.com/harrisont/fastbuild-vscode/issues/6)) Support `ForEach` iterating over multiple arrays at a time (single array iterating already supported). This completes support for the full FASTBuild language.
9096

9197
# v0.1.8
9298

@@ -99,7 +105,7 @@
99105

100106
# v0.1.6
101107

102-
* [#7](https://github.com/harrisont/fastbuild-vscode/issues/7) Wait for a delay before updating (debounce), to improve performance.
108+
* ([#7](https://github.com/harrisont/fastbuild-vscode/issues/7)) Wait for a delay before updating (debounce), to improve performance.
103109

104110
# v0.1.5
105111

‎CONTRIBUTING.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
1. [Build and run/debug the extension](#build-and-rundebug-the-extension).
1212
2. Make your change, including:
1313
1. The change itself.
14-
2. Update the `version` in [package.json](package.json).
15-
3. Update [CHANGELOG.md](CHANGELOG.md) with the release notes for the new version.
14+
2. A [test](server/src/test), if appropriate.
15+
3. Update the `version` in [package.json](package.json).
16+
4. Update [CHANGELOG.md](CHANGELOG.md) with the release notes for the new version.
1617
3. [Run/debug tests](#rundebug-tests).
1718
4. Submit a [pull request](https://github.com/harrisont/fastbuild-vscode/pulls) with the change for review.
1819
5. Once the PR is accepted, create a new release, using a new release tag for the new version. Include the release notes from [CHANGELOG.md](CHANGELOG.md). This will automatically publish the extension to the VS Code Marketplace via the GitHub Action.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "fastbuild-support",
33
"displayName": "FASTBuild Support",
44
"description": "FASTBuild language support. Includes go-to definition, find references, variable evaluation, syntax errors, etc.",
5-
"version": "0.5.7",
5+
"version": "0.5.8",
66
"preview": true,
77
"publisher": "HarrisonT",
88
"author": {

‎server/src/evaluator.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,10 +1136,8 @@ function evaluateStatements(statements: Statement[], context: EvaluationContext)
11361136
} else if (isParsedStatementForEach(statement)) {
11371137
// Evaluate the iterators (array to loop over plus the loop-variable)
11381138
interface ForEachIterator {
1139+
loopVariable: ScopeVariable;
11391140
arrayItems: Value[];
1140-
evaluatedLoopVarNameValue: string;
1141-
loopVarRange: SourceRange;
1142-
loopVarDefinition: VariableDefinition;
11431141
}
11441142
const iterators: ForEachIterator[] = [];
11451143
for (const iterator of statement.iterators) {
@@ -1179,11 +1177,18 @@ function evaluateStatements(statements: Statement[], context: EvaluationContext)
11791177

11801178
const loopVarDefinition = context.scopeStack.createVariableDefinition(loopVarRange, evaluatedLoopVarNameValue);
11811179

1180+
// The loop variable is a definition and a reference.
1181+
context.evaluatedData.variableDefinitions.push(loopVarDefinition);
1182+
context.evaluatedData.variableReferences.push({
1183+
definition: loopVarDefinition,
1184+
range: loopVarRange,
1185+
});
1186+
1187+
// Set a variable in the current scope for each iterator's loop variable.
1188+
const loopVariable = context.scopeStack.setVariableInCurrentScope(evaluatedLoopVarNameValue, 0, loopVarDefinition);
11821189
iterators.push({
1190+
loopVariable,
11831191
arrayItems,
1184-
evaluatedLoopVarNameValue,
1185-
loopVarRange,
1186-
loopVarDefinition,
11871192
});
11881193
}
11891194

@@ -1193,15 +1198,14 @@ function evaluateStatements(statements: Statement[], context: EvaluationContext)
11931198
const arrayItemsLength = iterators[0].arrayItems.length;
11941199
for (let arrayItemIndex = 0; arrayItemIndex < arrayItemsLength; arrayItemIndex++) {
11951200
context.scopeStack.withScope(() => {
1196-
// Set a variable in the current scope for each iterator's loop variable.
1201+
// Update the loop variables' values and add evaluated-variables for them.
11971202
for (const iterator of iterators) {
11981203
const arrayItem = iterator.arrayItems[arrayItemIndex];
1199-
const loopVariable = context.scopeStack.setVariableInCurrentScope(iterator.evaluatedLoopVarNameValue, arrayItem, iterator.loopVarDefinition);
1204+
iterator.loopVariable.value = arrayItem;
12001205

1201-
// The loop variable is a variable reference.
1202-
context.evaluatedData.variableReferences.push({
1203-
definition: loopVariable.definition,
1204-
range: iterator.loopVarRange,
1206+
context.evaluatedData.evaluatedVariables.push({
1207+
value: arrayItem,
1208+
range: iterator.loopVariable.definition.range,
12051209
});
12061210
}
12071211

‎server/src/test/2-evaluator.test.ts

Lines changed: 97 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,9 +1049,9 @@ describe('evaluator', () => {
10491049
['a', 'b', 'c'],
10501050
'Base',
10511051
['a', 'b', 'c'],
1052-
'a', 'Base-a',
1053-
'b', 'Base-b',
1054-
'c', 'Base-c',
1052+
'a', 'a', 'Base-a',
1053+
'b', 'b', 'Base-b',
1054+
'c', 'c', 'Base-c',
10551055
]);
10561056
});
10571057

@@ -2261,9 +2261,9 @@ describe('evaluator', () => {
22612261
assertEvaluatedVariablesValueEqual(input, [
22622262
['a', 'b', 'c'],
22632263
['a', 'b', 'c'],
2264-
'a',
2265-
'b',
2266-
'c'
2264+
'a', 'a',
2265+
'b', 'b',
2266+
'c', 'c',
22672267
]);
22682268
});
22692269

@@ -2294,9 +2294,9 @@ describe('evaluator', () => {
22942294
assertEvaluatedVariablesValueEqual(input, [
22952295
['a', 'b', 'c'],
22962296
['a', 'b', 'c'],
2297-
'a',
2298-
'b',
2299-
'c'
2297+
'a', 'a',
2298+
'b', 'b',
2299+
'c', 'c',
23002300
]);
23012301
});
23022302

@@ -2334,10 +2334,15 @@ describe('evaluator', () => {
23342334
myStruct1,
23352335
myStruct2,
23362336
[myStruct1, myStruct2],
2337-
//ForEach( .Item in .MyArray )
2337+
// in .MyArray )
23382338
[myStruct1, myStruct2],
2339-
// Print( .Item )
2339+
// `ForEach( .Item...` iteration 1
23402340
myStruct1,
2341+
// `Print( .Item )` iteration 1
2342+
myStruct1,
2343+
// `ForEach( .Item...` iteration 2
2344+
myStruct2,
2345+
// `Print( .Item )` iteration 2
23412346
myStruct2,
23422347
]);
23432348
});
@@ -2364,9 +2369,17 @@ describe('evaluator', () => {
23642369
['a1', 'b1', 'c1'],
23652370
['a2', 'b2', 'c2'],
23662371
['a3', 'b3', 'c3'],
2367-
// Print( '$Item1$-$Item2$-$Item3$' )
2372+
// Loop variables iteration 1
2373+
'a1', 'a2', 'a3',
2374+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 1
23682375
'a1', 'a2', 'a3',
2376+
// Loop variables iteration 2
23692377
'b1', 'b2', 'b3',
2378+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 2
2379+
'b1', 'b2', 'b3',
2380+
// Loop variables iteration 3
2381+
'c1', 'c2', 'c3',
2382+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 3
23702383
'c1', 'c2', 'c3',
23712384
]);
23722385
});
@@ -2394,9 +2407,17 @@ describe('evaluator', () => {
23942407
['a1', 'b1', 'c1'],
23952408
['a2', 'b2', 'c2'],
23962409
['a3', 'b3', 'c3'],
2397-
// Print( '$Item1$-$Item2$-$Item3$' )
2410+
// Loop variables iteration 1
2411+
'a1', 'a2', 'a3',
2412+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 1
23982413
'a1', 'a2', 'a3',
2414+
// Loop variables iteration 2
23992415
'b1', 'b2', 'b3',
2416+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 2
2417+
'b1', 'b2', 'b3',
2418+
// Loop variables iteration 3
2419+
'c1', 'c2', 'c3',
2420+
// `Print( '$Item1$-$Item2$-$Item3$' )` iteration 3
24002421
'c1', 'c2', 'c3',
24012422
]);
24022423
});
@@ -2425,9 +2446,9 @@ describe('evaluator', () => {
24252446
// ForEach( ... )
24262447
['a', 'b', 'c'],
24272448
// Print( .Item )
2428-
'a',
2429-
'b',
2430-
'c'
2449+
'a', 'a',
2450+
'b', 'b',
2451+
'c', 'c',
24312452
]);
24322453
});
24332454

@@ -2441,6 +2462,66 @@ describe('evaluator', () => {
24412462
const expectedErrorMessage = `'ForEach' variable to loop over must be an Array, but instead is an Integer`;
24422463
assertEvaluationError(input, expectedErrorMessage, createParseRange(2, 34, 2, 42));
24432464
});
2465+
2466+
it('has the expected variable definitions and references', () => {
2467+
const input = `
2468+
.MyArray = {'a', 'b'}
2469+
ForEach( .Item in .MyArray )
2470+
{
2471+
Print( .Item )
2472+
}
2473+
`;
2474+
2475+
const result = evaluateInput(input, true /*enableDiagnostics*/);
2476+
2477+
// `.MyArray = {'a', 'b'}`
2478+
const expectedDefinitionMyArray: VariableDefinition = {
2479+
id: 1,
2480+
range: createRange(1, 16, 1, 24),
2481+
name: 'MyArray',
2482+
};
2483+
2484+
// `ForEach( .Item...`
2485+
const expectedDefinitionItem: VariableDefinition = {
2486+
id: 2,
2487+
range: createRange(2, 25, 2, 30),
2488+
name: 'Item',
2489+
};
2490+
2491+
assert.deepStrictEqual(result.variableDefinitions, [
2492+
expectedDefinitionMyArray,
2493+
expectedDefinitionItem,
2494+
]);
2495+
2496+
const expectedReferences: VariableReference[] = [
2497+
// `.MyArray = {'a', 'b'}`
2498+
{
2499+
definition: expectedDefinitionMyArray,
2500+
range: expectedDefinitionMyArray.range,
2501+
},
2502+
// `...in .MyArray`
2503+
{
2504+
definition: expectedDefinitionMyArray,
2505+
range: createRange(2, 34, 2, 42),
2506+
},
2507+
// // `ForEach( .Item...`
2508+
{
2509+
definition: expectedDefinitionItem,
2510+
range: expectedDefinitionItem.range,
2511+
},
2512+
// `Print( .Item )` for the 1st loop iteration
2513+
{
2514+
definition: expectedDefinitionItem,
2515+
range: createRange(4, 27, 4, 32),
2516+
},
2517+
// `Print( .Item )` for the 2nd loop iteration
2518+
{
2519+
definition: expectedDefinitionItem,
2520+
range: createRange(4, 27, 4, 32),
2521+
},
2522+
];
2523+
assert.deepStrictEqual(result.variableReferences, expectedReferences);
2524+
});
24442525
});
24452526

24462527
// Functions that all we handle are registering their target and evaluating their statements.

0 commit comments

Comments
 (0)