feat: [DEVEX-1088] add warn when executor param has no default value (#203)

* feat: warn instead of err when executor param has no default

* test: add test for missing default executor parameter

Author: jvincent42

pkg/parser/validate/jobs.go

@@ -39,7 +39,18 @@ func (val Validate) validateSingleJob(job ast.Job) {
 
 			checkParam := func(executorDefault string, rng protocol.Range) {
 				isOrbExecutor, err := val.doesOrbExecutorExist(executorDefault, rng)
-				if val.Context.Api.UseDefaultInstance() && !val.Doc.DoesExecutorExist(executorDefault) &&
+				if !param.IsOptional() {
+					val.addDiagnostic(
+						protocol.Diagnostic{
+							Range: rng,
+							Message: fmt.Sprintf(
+								"No default value specified for parameter `%s`.",
+								paramName,
+							),
+							Severity: protocol.DiagnosticSeverityWarning,
+						},
+					)
+				} else if val.Context.Api.UseDefaultInstance() && !val.Doc.DoesExecutorExist(executorDefault) &&
 					(!isOrbExecutor && err == nil) {
 					// Error on the default value
 					val.addDiagnostic(

pkg/parser/validate/jobs_test.go

@@ -11,49 +11,84 @@ import (
 )
 
 func TestExecutorParam(t *testing.T) {
-	yamlData := `jobs:
+	testCases := []struct {
+		label        string
+		yamlData     string
+		expectedDiag protocol.Diagnostic
+	}{
+		{
+			label: "without default",
+			yamlData: `jobs:
+  test:
+    parameters:
+      os:
+        type: executor
+    executor: << parameters.os >>
+    steps:
+      - checkout`,
+			expectedDiag: protocol.Diagnostic{
+				Range: protocol.Range{
+					Start: protocol.Position{Line: 5, Character: 4},
+					End:   protocol.Position{Line: 5, Character: 33},
+				},
+				Severity: protocol.DiagnosticSeverityWarning,
+			},
+		},
+		{
+			label: "with unknown default",
+			yamlData: `jobs:
   test:
     parameters:
       os:
         type: executor
+        default: unknown
     executor: << parameters.os >>
     steps:
-      - checkout`
-	ctx := &utils.LsContext{
-		Api: utils.ApiContext{
-			Token:   "XXXXXXXXXXXX",
-			HostUrl: "https://circleci.com",
+      - checkout`,
+			expectedDiag: protocol.Diagnostic{
+				Range: protocol.Range{
+					Start: protocol.Position{Line: 6, Character: 4},
+					End:   protocol.Position{Line: 6, Character: 33},
+				},
+				Severity: protocol.DiagnosticSeverityError,
+			},
 		},
 	}
-	doc, err := parser.ParseFromContent(
-		[]byte(yamlData),
-		ctx,
-		uri.URI(""),
-		protocol.Position{},
-	)
-	assert.NoError(t, err, "invalid YAML data")
-	assert.Contains(t, doc.Jobs, "test")
 
-	val := Validate{
-		Context:     ctx,
-		Doc:         doc,
-		Diagnostics: &[]protocol.Diagnostic{},
-	}
-	val.validateSingleJob(doc.Jobs["test"])
+	for _, testCase := range testCases {
+		t.Run("executor parameter: "+testCase.label, func(t *testing.T) {
 
-	expectedDiag := protocol.Diagnostic{
-		Range: protocol.Range{
-			Start: protocol.Position{Line: 5, Character: 4},
-			End:   protocol.Position{Line: 5, Character: 33},
-		},
-		Severity: protocol.DiagnosticSeverityError,
-	}
-	for _, diag := range *val.Diagnostics {
-		if diag.Range == expectedDiag.Range && diag.Severity == expectedDiag.Severity {
-			return
-		}
+			ctx := &utils.LsContext{
+				Api: utils.ApiContext{
+					Token:   "XXXXXXXXXXXX",
+					HostUrl: "https://circleci.com",
+				},
+			}
+			doc, err := parser.ParseFromContent(
+				[]byte(testCase.yamlData),
+				ctx,
+				uri.URI(""),
+				protocol.Position{},
+			)
+			assert.NoError(t, err, "invalid YAML data")
+			assert.Contains(t, doc.Jobs, "test")
+
+			val := Validate{
+				Context:     ctx,
+				Doc:         doc,
+				Diagnostics: &[]protocol.Diagnostic{},
+			}
+			val.validateSingleJob(doc.Jobs["test"])
+
+			for _, diag := range *val.Diagnostics {
+				if diag.Range == testCase.expectedDiag.Range &&
+					diag.Severity == testCase.expectedDiag.Severity {
+					return
+				}
+			}
+			t.Fatalf(`missing "parameter as executor" diagnostic`)
+		})
 	}
-	t.Fatalf(`missing "parameter as executor" diagnostic`)
 }
 
 func TestResourceClass(t *testing.T) {