Fix #250, strings vs expressions boundary cases (#259)

grammars/arm-expression-string.tmLanguage.json

@@ -2,14 +2,21 @@
 	"name": "ARM Template",
 	"scopeName": "source.tle.arm",
 	"uuid": "3ADA43CD-2258-4907-8477-169A7FDDF216",
-	"$preprocessComment1": "Items in the preprocess section get replaced during gulp build-grammars",
-	"$preprocessComment2": "The 'builtin' key/value are special - they're created during build from our function metadata",
-	"$preprocessComment3": "Currently you need to run 'build grammars' after changing this file",
 	"preprocess": {
+		"$preprocessComment1": "Items in the preprocess section get replaced during gulp build-grammars",
+		"$preprocessComment2": "Items beginning with $ are ignored",
+		"$preprocessComment3": "---",
+		"$preprocessComment4": "A preprocess item with the key 'builtin-functions' is automatically created during build from our function metadata",
+		"$preprocessComment6": "---",
+		"$preprocessComment7": "IMPORTANT!  You need to run 'build grammars' after changing this file, F5 currently doesn't handle that",
+		"$1": "--------------- Regular expressions ---------------",
+		"escaped-json-character": "(?:\\\\(?:[\"\\\\/bfnrt]|u[0-9a-fA-F]{4}))",
+		"escaped-apostrophe": "''",
 		"idchar": "[_$[:alnum:]]",
 		"id": "(?:[_$[:alpha:]]{{idchar}}*)",
 		"ns-userfunc": "(?:({{id}})\\s*(\\.)\\s*({{id}}))",
 		"logical": "(?:if|and|or|not)",
+		"$2": "--------------- Scope Names ---------------",
 		"scope-expression-start": "support.function.expression.begin.tle.arm",
 		"scope-expression-end": "support.function.expression.end.tle.arm",
 		"scope-builtin": "support.function.builtin.tle.arm",
@@ -42,8 +49,10 @@
 	"repository": {
 		"expressionstring": {
 			"name": "meta.expression.tle.arm",
-			"begin": "\"\\[(?!\\[)",
-			"$beginComment": "A string that starts with [[ is not an expression",
+			"begin": "(?x)  \"\\[(?!\\[)  (?= (?:{{escaped-apostrophe}}|{{escaped-json-character}}|[^\"])* ( \\]\"|$ ) )",
+			"$beginComment": "An expression must start with '[' (no whitespace before), not start with '[[', and end with ']' (no whitespace after)",
+			"$beginComment2": "Since we can't check if an expression ends with ']' for multi-line strings, we also assume a string which starts with '[' and",
+			"$beginComment3": "   isn't terminated on the first line is an expression",
 			"end": "\\]\"",
 			"beginCaptures": {
 				"0": {
@@ -123,13 +132,13 @@
 			"patterns": [
 				{
 					"$comment": "Escaped JSON string characters",
-					"match": "(?x)                # turn on extended mode\n  \\\\                # a literal backslash\n  (?:               # ...followed by...\n    [\"\\\\/bfnrt]     # one of these characters\n    |               # ...or...\n    u               # a u\n    [0-9a-fA-F]{4}) # and four hex digits",
+					"match": "{{escaped-json-character}}",
 					"name": "{{scope-json-escape-chars}}"
 				},
 				{
 					"$comment": "Escaped apostrophe (interpreted by ARM backend)",
 					"name": "{{scope-escapedapostrophe}}",
-					"match": "['][']"
+					"match": "{{escaped-apostrophe}}"
 				},
 				{
 					"match": "\\\\.",
@@ -186,7 +195,7 @@
 			}
 		},
 		"functionname": {
-			"match": "(?x) \\s* (?: {{ns-userfunc}} | (parameters) | (variables) | ({{logical}}) | ({{builtin}}) | ({{id}}) ) (?!{{idchar}} (?# Make sure we don't match a well-known name like 'add' inside something like 'add2'))",
+			"match": "(?x) \\s* (?: {{ns-userfunc}} | (parameters) | (variables) | ({{logical}}) | ({{builtin-functions}}) | ({{id}}) ) (?!{{idchar}} (?# Make sure we don't match a well-known name like 'add' inside something like 'add2'))",
 			"captures": {
 				"1": {
 					"$comment": "user namespace (capturing group inside ns-userfunc)",

gulpfile.ts

@@ -26,7 +26,7 @@ export const tleGrammarBuiltPath: string = path.resolve('dist/grammars/arm-expre
 
 export interface IGrammar {
     preprocess?: {
-        builtin: string;
+        "builtin-functions": string;
         [key: string]: string;
     };
     [key: string]: unknown;
@@ -55,15 +55,19 @@ function buildTLEGrammar(): void {
     let builtinFunctions: string[] = expressionMetadata.functionSignatures.map(sig => sig.name);
     let grammarAsObject = <IGrammar>JSON.parse(grammar);
     grammarAsObject.preprocess = {
-        builtin: `(?:(?i)${builtinFunctions.join('|')})`,
+        "builtin-functions": `(?:(?i)${builtinFunctions.join('|')})`,
         ... (grammarAsObject.preprocess || {})
     };
+
     grammarAsObject = {
-        $comment: `DO NOT EDIT - This file was built from ${path.relative(__dirname, tleGrammarBuiltPath)}`,
+        $comment: `DO NOT EDIT - This file was built from ${path.relative(__dirname, tleGrammarSourcePath)}`,
         ...grammarAsObject
     };
+
     grammar = JSON.stringify(grammarAsObject, null, 4);
-    const replacementKeys = Object.getOwnPropertyNames((<IGrammar>JSON.parse(grammar)).preprocess);
+
+    // Get the replacement keys from the preprocess section (ignore those that start with "$")
+    const replacementKeys = Object.getOwnPropertyNames((<IGrammar>JSON.parse(grammar)).preprocess).filter(key => !key.startsWith("$"));
 
     // Build grammar: Make replacements specified
     for (let key of replacementKeys) {
@@ -74,11 +78,16 @@ function buildTLEGrammar(): void {
         // remove quotes
         valueString = valueString.slice(1, valueString.length - 1);
         if (!sourceGrammar.includes(replacementKey)) {
-            console.log(`WARNING: Preprocess key ${replacementKey} not found in ${tleGrammarSourcePath}`);
+            console.log(`WARNING: Preprocess key ${replacementKey} is never referenced in ${tleGrammarSourcePath}`);
         }
         grammar = grammar.replace(new RegExp(replacementKey, "g"), valueString);
     }
 
+    // Remove preprocess section from the output grammar file
+    let outputGrammarAsObject = (<IGrammar>JSON.parse(grammar));
+    delete outputGrammarAsObject.preprocess;
+    grammar = JSON.stringify(outputGrammarAsObject, null, 4);
+
     fs.writeFileSync(tleGrammarBuiltPath, grammar);
     console.log(`Built ${tleGrammarBuiltPath}`);
 

test/colorization/inputs/expr-vs-string.is-expression.jsonc

@@ -0,0 +1,16 @@
+{
+    "$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#",
+    //
+    // Handle escaped characters - this should be an expression with a literal string inside it
+    //
+    "$TEST1": "['I said Hi! He''s here']",
+    "$TEST2": "['I said \"Hi! He''s here!\"']",
+    //
+    // Multi-line expressions - the colorization can't peek onto another line to determine whether the
+    // string ends with "]" (and therefore know it's an expression and not a string), so assume it is an expression
+    // if a multi-line string starts with "["
+    "$TEST10": "[concat('This is a ', 1, '-line ', 'expression ', 4, 'you!')]", //asdf
+    "$TEST11": "[concat('This is a ',
+        3, '-line ',
+        'expression ', 4, ' you!')]"
+}

test/colorization/inputs/expr-vs-string.is-string.NOT-EXPR.jsonc

@@ -0,0 +1,23 @@
+{
+    "$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#",
+    //
+    // Not an expression if it starts with [[
+    //
+    "$TEST1": "[[2]]",
+    "$TEST2": "[[[four]five]",
+    "$TEST3": "[['I said \"Hi! He''s here!\"]",
+    //
+    // Not an expression if it doesn't end with ]
+    //
+    "$TEST10": "[pre]post",
+    "$TEST11": "[[three]four",
+    //
+    // Not an expression if it doesn't start immediately with [ or the very last character is not ] (even if it's whitespace)
+    //
+    "$TEST20": " [starts with whitespace]",
+    "$TEST21": "[ends with whitespace] ",
+    //
+    // Real examples
+    //
+    "$TEST100": "[ChefInSpec]InstalledApplicationLinuxResource1;AttributesYmlContent"
+}

test/colorization/inputs/expr-vs-string.multiline.NOT-EXPR.jsonc

@@ -0,0 +1,15 @@
+{
+    "$schema": "https://schema.management.azure.com/schemas/2018-05-01/subscriptionDeploymentTemplate.json#",
+    //
+    // Not an expression if it starts with [[
+    //
+    "$TEST1": "[[2
+        ]]",
+    "$TEST2": "[[[four]
+        five]",
+    //
+    // Not an expression if it doesn't start immediately with [ or the very last character is not ] (even if it's whitespace)
+    //
+    "$TEST20": "
+[starts with whitespace]"
+}