Merge pull request #141 from microsoft/user/winstonliu/expression-key-must-be-scalar

Only check for expressions when keys are scalar

Author: winstliu

language-service/src/parser/yamlParser.ts

@@ -184,9 +184,7 @@ function recursivelyBuildAst(parent: ASTNode, node: Yaml.YAMLNode): ASTNode {
 // and remove the expression from the parsed YAML.
 function addPropertiesToObjectNode(node: ObjectASTNode, properties: Yaml.YAMLMapping[]): void {
 	for (const property of properties) {
-		// The endsWith check is necessary because you can have dynamically-generated variables;
-		// for example, ${{ environment }}Release: true.
-		if (property.key.value.startsWith("${{") && property.key.value.endsWith("}}")) {
+		if (isCompileTimeExpression(property)) {
 			// Ensure we have a value (object) _and_ that the value has mappings (properties).
 			if (property.value?.mappings !== undefined) {
 				addPropertiesToObjectNode(node, property.value.mappings);
@@ -214,9 +212,7 @@ function addItemsToArrayNode(node: ArrayASTNode, items: Yaml.YAMLNode[]): void {
 			// Hoisted expressions must be the first (and only) property in an object,
 			// so we can safely check only the first key.
 			// TODO: Confirm the above statement.
-			if (item.kind === Yaml.Kind.MAP &&
-				item.mappings[0].key.value.startsWith("${{") &&
-				item.mappings[0].key.value.endsWith("}}")) {
+			if (item.kind === Yaml.Kind.MAP && isCompileTimeExpression(item.mappings[0])) {
 				const value = item.mappings[0].value;
 				if (value === null) {
 					// Incomplete object: they're still working on the value :).
@@ -274,6 +270,12 @@ function addItemsToArrayNode(node: ArrayASTNode, items: Yaml.YAMLNode[]): void {
 	}
 }
 
+function isCompileTimeExpression(node: Yaml.YAMLNode): boolean {
+	return node.key.kind === Yaml.Kind.SCALAR &&
+		node.key.value.startsWith("${{") &&
+		node.key.value.endsWith("}}");
+}
+
 function convertError(e: Yaml.YAMLException): YAMLError {
 	return { getMessage: () => e.reason, start: e.mark.position, end: e.mark.position + e.mark.column };
 }

language-service/test/pipelinesTests/yamlvalidation.test.ts

@@ -119,6 +119,22 @@ steps:
 `);
       assert.equal(diagnostics.length, 2);
     });
+
+    it('validates incorrectly-indented pipelines that look like they have an array property', async function () {
+      // Note: the real purpose of this test is to ensure we don't throw,
+      // but I can't figure out how to assert that yet.
+      // diagnostics.length can be whatever, as long as we get to that point :).
+      const diagnostics = await runValidationTest(`
+steps:
+- task: PowerShellOnTargetMachines@3
+  inputs:
+    Machines: EXAMPLE
+    InlineScript: |
+    [System.Reflection.Assembly]::LoadWithPartialName("System.IO.Compression.FileSystem") | Out-Null
+    CommunicationProtocol: Http);
+`);
+      assert.equal(diagnostics.length, 4);
+    });
 });
 
 const workspaceContext = {