Split out from #28.

First I need to confirm that this duplicated data isn't necessary.

• Update the contributor-docs with a high-level architecture (lexer, parser, evaluator).
• Update the contributor-docs with a file-layout overview.

• Support ForEach iterating over multiple arrays at a time (single array iterating already supported) (docs). This is low priority, since I have never seen it used.

VS Code docs:

• Show all Symbol Definitions Within a Document
• Show all Symbol Definitions in Folder

The symbol list should include target and variable definitions.

Repro

"Go to references" on the .A of the Print( .A ) statement in:

.MyStruct1 = [
    .A = 1
]

.MyStruct2 = [
    .A = 2
]

.MyStructs = {
    .MyStruct1
    .MyStruct2
}

ForEach( .MyStruct in .MyStructs )
{
    Using( .MyStruct )
    Print( .A )
}

Expected

4 references:

• .A = 1
• .A = 2
• Using( .MyStruct )
• Print( .A )

Actual

3 references:

• .A = 1
• Using( .MyStruct )
• Print( .A )

Specifically by deduplicating the following fields in evaluated data:

• variableReferences with the same range
• variableDefinitions returned by getDocumentSymbols/getWorkspaceSymbols with the same name and range

Repro case

.Message = 'h"i'

Expected

The string in the hover is colored the same.

Actual

The string in the hover is colored differently after the ".

#if exists(...) (docs) now checks the actual environment variable instead of always evaluating to false.

Code snippet:

            } else if (isParsedDirectiveIfConditionTermEnvVarExists(term)) {
                // The language server cannot know what environment variables will exist when FASTBuild is run,
                // so always assume "exists(...)" evaluates to false.
                evaulatedTerm = false;
            }

Note that both the target's name and the target's output file reference a target.

Places that can define targets:

• Target names
• Copy's .Dest (be sure to correctly handle both file and directory values)
• CopyDir's .Dest?
• ...
• Exec's .ExecOutput
• ...

Places that can reference targets:

• Targets' .PreBuildDependencies
• Copy's .Source
• CopyDir's .SourcePaths?
• ...
• Exec's .ExecInput
• ...
• (unsure) Places that set one the following (defined in FASTBuild's src\Code\Tools\FBuild\FBuildCore\Graph\Node.h):
  • Node::m_PreBuildDependencies
  • Node::m_StaticDependencies
  • Node::m_DynamicDependencies

This will make it so that users don't need to also get another extension (e.g. RoscoP.fastbuild) for syntax highlighting.

Docs for VS Code grammar
Docs for grammar names

Right now the results are in evaluation order.

Repro

1. Create the following file contents:
   fbuild.bff:

   .A = 1
   #include 'other-file.bff'
   

   other-file.bff:

   Print( .A )
   

2. Open other-file.bff and ensure that fbuild.bff is closed.
3. Hold down Ctrl and hover over the .A in Print( .A ).

Expected Behavior

• The extension does not re-parse or re-evaluate, because no content changed.

Actual Behavior

• VS Code sends the notifications that fbuild.bff opened, changed, and closed.
  • Note that VS Code only has trace logs for the opened and closed notifications. The changed notification can be seen either by breakpointing the body of state.documents.onDidChangeContent in server.ts or by adding a log there.
• The extension re-parses and re-evaluates.

Verbose Trace logs (trimmed):

[Trace - ...] Sending request 'textDocument/definition - (...)'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/Users/hting/Desktop/harrison.bff"
    },
    "position": {
        "line": 0,
        "character": 8
    }
}

[Trace - ...] Received response 'textDocument/definition - (...)' in 2ms.
Result: [
    {
        ...
        "targetUri": "file:///c%3A/Users/hting/Desktop/fbuild.bff",
        ...
    }
]

[Trace - ...] Sending notification 'textDocument/didOpen'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/Users/hting/Desktop/fbuild.bff",
        ...
    }
}

[Trace - ...] Sending notification 'textDocument/didClose'.
Params: {
    "textDocument": {
        "uri": "file:///c%3A/Users/hting/Desktop/fbuild.bff"
    }
}

parse-duration: 0.751ms
evaluation-duration: 0.173ms

Notes

• VS Code seemingly incorrectly triggers an onDidChangeContent notification even though nothing changed.

Docs

For example,

Exec('MyAlias')
{
}

should generate a non-fatal error like "Exec() - Missing required property '.ExecOutput'".

Repro

"Go to definition" on the .A of the Print( .A ) statement in:

.MyStruct1 = [
    .A = 1
]

.MyStruct2 = [
    .A = 2
]

.MyStructs = {
    .MyStruct1
    .MyStruct2
}

ForEach( .MyStruct in .MyStructs )
{
    Using( .MyStruct )
    Print( .A )
}

Expected

It shows the following definitions:

• Using( .MyStruct )
• .A = 1
• .A = 2

Actual

It only shows the Using( .MyStruct ) definition.

Repo

.MyVar = { 'a' }
ForEach(.Item in .MyVar) {}
Print(.Item)

Expected behavior

The reference to .Item in Print(.Item) generates an error, since it should not be defined.

Actual behavior

No error is generated and hovering over the .Item in Print(.Item) shows the value from the loop.

Initial debugging shows that getWorkspaceSymbols returns the symbols and the Received response 'workspace/symbol' message happens, but VS Code is stuck showing that they are still loading.

• Potential solution 1: When a document-update event occurs while an existing one is still being processed, cancel the existing processing. Do so by: pass through a cancellation token, intermittently check if it has been canceled, and either return or raise an exception if so.
• Potential solution 2: Add a "debounce". Wait until X milliseconds have passed since the last document change before processing the change.
  • Example

Note that we already have a form of incremental parsing, in that we already only re-parse the changed files. This issue is specifically about incremental evaluation.

• Potential solution 1a: when editing a file, reuse the evaluation state that existed prior to importing the file.
  • Keep a cache that maps a file URI to the evaluation state (result and context) prior to first importing that file.
  • At least for the initial implementation, this cache has a size of 1, and we clear it before adding a new file.
  • When a document-update event occurs to a file that exists in the cache, bootstrap the evaluation with the cached evaluation state.
  • When evaluating an import statement, first call an onBeforeImportFile callback. The callback will check if the file is the same as the one being edited and if so, clear the cache and add the new file/state. It first clears the cache because the existing cached state might now be invalid.
• Potential solution 1b: when editing a file, reuse the evaluation state that existed prior to the second import of the file (or the final evaluation state if the file was only imported once), if the post-first-import evaluation state is the same as the previous evaluation state that existed prior to first importing the file.
  • Keep a cache that maps a file URI to the following.
    • The evaluation state (result and context) after first importing that file
    • The evaluation state (result and context) prior to the second import of the file (or the final evaluation state if the file was only imported once)
  • Strategy:
    1. Before evaluating, run through the statements to calculate the first and second import, if any, of the file being edited.
    2. After evaluating an import statement, call an onAfterImportFile callback.
    3. The callback will check if the file is the same as the one being edited.
       • If so, check if the cached state is the same as the new state.
         • If so, check if this is the first import of the file:
           • If so, return the following:
             • The cached evaluation state (result and context) prior to the second import of the file (or the final evaluation state if the file was only imported once).
             • A directive to skip evaluation ahead to just before the second import of the file (or the end if the file was only imported once).
           • Else:
             1. Clear the cached evaluation state (result and context) prior to the second import of the file. It first clears the cache because the existing cached state might now be invalid.
             2. Replace the evaluation state (result and context) after first importing that file.
         • Else, check if this is the second import of the file. If so:
           • Set the cached evaluation state (result and context) prior to the second import of the file.
       • Else, clear the cache and add the new file and evaluation state (result and context) after first importing that file. It first clears the cache because the existing cached state might now be invalid.
• Potential solution 2:
  • Keep running total of statement number
  • Use metric to decide whether to cache. Maybe number of statements? Maybe every scope? Maybe by file?
  • If cache, store (statement number, inputs, outputs), where inputs is the value of and variables read from outside the scope, and outputs is the value of variables modified outside the scope.
  • On processing scope, if cached and inputs are the same, set the outputs and skip processing the scope.

Repro case

.Message1 = "h^"i"
.Message2 = 'h^'i'
.Message3 = 'h^$i'

Expected

The strings are highlighted in the same color, and the Developer: Inspect Editor Tokens and Scopes's textmate scopes are string.quoted.single.fastbuild/string.quoted.double.fastbuild, source.fastbuild.

Actual

The ^ and the character after it are a different color, and have with textmate scopes constant.character.escape.fastbuild, string.quoted.single.fastbuild/string.quoted.double.fastbuild, source.fastbuild.

Repro

.Message = '^a'

Expected

An error message that indicates that it's an invalid escape sequence, and that only $, ^, ' (in single-quoted strings), and " (in double-quoted strings) need to be escaped.

Actual

The error message is:

Syntax error: Unexpected input.
Expecting to see one of the following:
 • single-quoted-String-end: "'"
 • String-literal (example: "abc")

Repro

Attempt to "go to references" of the 1 in:

.A = 1

Expected behavior

No references

Actual behavior

Getting references fails:

[Trace - 6:36:33 PM] Sending request 'textDocument/references - (34)'.
[Trace - 6:36:33 PM] Received response 'textDocument/references - (34)' in 60ms. Request failed: Request textDocument/references failed with message: Reduce of empty array with no initial value (-32603).
[Error - 6:36:33 PM] Request textDocument/references failed.
  Message: Request textDocument/references failed with message: Reduce of empty array with no initial value
  Code: -32603 

Internally, the error is:

TypeError: Reduce of empty array with no initial value
 at Array.reduce (<anonymous>)
 at getVariableReferences (server\src\features\referenceProvider.ts:94:10)
 at Object.getReferences (server\src\features\referenceProvider.ts:34:24)

I'm unsure if this is actually undesired behavior or not - I'll need to ponder it.

Repro

"Go to references" on the .A of the .A = 3 statement in:

.A = 3
Print( 'Before $A$' )

.MyStruct = [
    .A = 1
]

Using( .MyStruct )
Print( 'After $A$' )

Expected

3 references:

• .A = 3
• Print( 'Before $A$' )
• Using( .MyStruct )

Actual

5 references:

• .A = 1
• .A = 3
• Print( 'Before $A$' )
• Using( .MyStruct )
• Print( 'After $A$' )

Right now if there is an evaluation error, it immediately stops the rest of the evaluation. In other words, all errors are "fatal" errors.

Support non-fatal errors by supporting generation and display of errors that do not stop the rest of the evaluation.

An example of a non-fatal error is a missing the required .ExecExecutable property when calling the Exec function.

• #define / #undef FASTBuild docs
• #if / #else / #endif FASTBuild docs

Ideas for context:

• Index (a number representing the iteration number that the value is from)
• The enclosing target name
• Other

Potential reason not to do this: without context we can deduplicate values across iterations. With context we would naturally show each iteration's value, even if they are the same. So it might be more usable to not show context and deduplicate values across iterations.

Right now it does not appear on hover or on "go to document symbols".

#13 added code completion of function properties. Adding code completion of available variables would be useful.

This will possibly require adding support for tracking the AST (abstract syntax tree), which doesn't currently happen.

• Function docs
• Built-in-variables docs
• Built-in-symbol docs

Repro

.MyVar = '^';'

Expected Behavior

No parse error

Actual Behavior

Syntax error: Unexpected input.
Expecting to see one of the following:
 • single-quoted-String-end: "'"
 • String-literal (example: "abc")

Docs

Example: .ProjectBuildCommand in T:\p4\Patcher\Build\MakeRunnableProjectConfig.bff

I'm guessing that VS Code is truncating the result.

Right now errors throw an exception and stop the evaluation. This issue is to instead continue the evaluating even after hitting non-fatal errors. This involves:

• Distinguish between fatal and non-fatal errors, and determine which each error is.
• Track the errors in the context.evaluatedData.
• Change diagnosticProvider.ts and server.ts to handle the multiple errors.

The evaluation completes after the textDocument/documentSymbol response is sent:

[Trace] Sending notification 'textDocument/didChange'.
[Trace] Received request 'workspace/configuration - (3)'.
[Trace] Sending response 'workspace/configuration - (3)'. Processing request took 0ms
[Trace] Sending request 'textDocument/documentSymbol - (9)'.
[Trace] Received response 'textDocument/documentSymbol - (9)' in 45ms.
parse-duration: 9.052ms
evaluation-duration: 832.045ms

I'm guessing that's because the textDocument/didChange notification queues an update because of the debounce delay, and then the client immediately requests textDocument/documentSymbol.

My guess is that the fix is to change the handling of textDocument/documentSymbol to first check if the document has a queued update, and wait to respond until it completes.

For example, TypeScript shows the container that the variable is in (the getReferences function is in the ReferenceProvider class):

Include the variable container as its context:

• A struct's variable's container is the struct name.
• A function's variable's container is the function name.

Implementation details

• Modify getDocumentSymbols/getWorkspaceSymbols to include the "children" symbols in the children field in the DocumentSymbols that it returns, and remove the "children" symbols from the top-level symbols (otherwise they'll be duplicated).

Repro

.'a.b' = 'Foo'
Print("$a.b$")

Expected Behavior

No syntax error

Actual

Syntax error: Unexpected input.
Expecting to see the following:
 • template-variable-end: "$"

Also, the template does not syntax highlight properly.

Examples:

• A file that is included in multiple locations that references a file defined in multiple locations.
• A ForEach looping over an array of structs, with a Using on that struct, referencing a variable in that struct in the loop has multiple definitions, one for each definition in the struct in the array.

Note: avoid returning multiple definitions that have the same location, since that's not helpful. For example, the definition of a loop variable will be the same for the whole loop.

Repro

.Message = '^a'

Expected

No error.

Actual

Errors:

Syntax error: Unexpected input.
Expecting to see one of the following:
 • single-quoted-String-end: "'"
 • String-literal (example: "abc")

Created as a result of #48.

Repro

1. Have a FASTBuild file with .MyVar = 1.
2. Delete the 1, which correctly generates a parse error (Syntax error: Unexpected end of file.).
3. Re-add the 1.

Expected Behavior

The error goes away.

Actual Behavior

The error stays.

Implementation notes

This is happening because the parse data for the document after step 1 of the repro is cached, along with the document content that generated that data. Step 2 does not change the cache. Step 3 then sees that the document content has not changed, so it early outs.

Docs

Repro

Exec('MyAlias')
{
}

Exec('MyAlias')
{
}

Actual error

Target name "MyAlias" already exists at "file:///c%3A/Users/hting/Desktop/fbuild.bff": 1:6 - 1:15.

Expected error

The same as the actual error, but with the existing definition split out into into a "related information", so the user can easily jump to it.

Implementation Notes

• publishDiagnostics docs
• relatedInformation docs.