Skip to content

Commit 78dc619

Browse files
committed
[PIPE-5387] add "terminal" shorthand for upstream job requires array
Adds the ability to use "terminal" in place of [success, failure, canceled, not_run] when specifying the upstream job requirements. Also adds a small hint to users that they can do this, with a code action to do it for them.
1 parent ccc0070 commit 78dc619

File tree

8 files changed

+662
-14
lines changed

8 files changed

+662
-14
lines changed

pkg/ast/workflow.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,10 @@ type JobRef struct {
5252
}
5353

5454
type Require struct {
55-
Name string
56-
Status []string
57-
Range protocol.Range
55+
Name string
56+
Status []string
57+
Range protocol.Range
58+
StatusRange protocol.Range
5859
}
5960

6061
type WorkflowTrigger struct {

pkg/parser/validate/workflow.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"github.com/CircleCI-Public/circleci-yaml-language-server/pkg/utils"
1010
)
1111

12+
var TerminalJobStatuses = []string{"success", "failed", "canceled", "not_run"}
13+
1214
func (val Validate) ValidateWorkflows() {
1315
for _, workflow := range val.Doc.Workflows {
1416
val.validateSingleWorkflow(workflow)
@@ -56,6 +58,47 @@ func (val Validate) validateSingleWorkflow(workflow ast.Workflow) error {
5658
require.Range,
5759
fmt.Sprintf("Cannot find declaration for job reference %s", require.Name)))
5860
}
61+
62+
if requireHasAllTerminalStatuses(require.Status) {
63+
// Use " terminal" for multi-line arrays so there's a space after the colon.
64+
// "terminal" for inline arrays since we're replacing an array that is
65+
// already spaced after the colon. e.g.
66+
//
67+
// Inline:
68+
// Before: - job_name: [inline-array]
69+
// After: - job_name: terminal
70+
//
71+
// Vs multi-line:
72+
// Before:
73+
// - job_name:
74+
// - success
75+
//
76+
// After:
77+
// - job_name: terminal
78+
newText := "terminal"
79+
if require.StatusRange.Start.Line != require.StatusRange.End.Line {
80+
newText = " terminal"
81+
}
82+
codeAction := utils.CreateCodeActionTextEdit(
83+
"Simplify these statuses to 'terminal'",
84+
val.Doc.URI,
85+
[]protocol.TextEdit{
86+
{
87+
NewText: newText,
88+
Range: require.StatusRange,
89+
},
90+
},
91+
true, // preferred
92+
)
93+
val.addDiagnostic(
94+
protocol.Diagnostic{
95+
Range: require.StatusRange,
96+
Message: fmt.Sprintf("Statuses: '%v' can be simplified to just 'terminal'", require.Status),
97+
Severity: protocol.DiagnosticSeverityHint,
98+
Data: []protocol.CodeAction{codeAction},
99+
},
100+
)
101+
}
59102
}
60103

61104
if cachedFile := val.Cache.FileCache.GetFile(val.Doc.URI); val.Context.Api.Token != "" &&
@@ -142,3 +185,30 @@ func (val Validate) validateDAG(workflow ast.Workflow) {
142185
}
143186
}
144187
}
188+
189+
func requireHasAllTerminalStatuses(statuses []string) bool {
190+
if len(statuses) != len(TerminalJobStatuses) {
191+
return false
192+
}
193+
194+
terminalSet := make(map[string]bool)
195+
for _, status := range TerminalJobStatuses {
196+
terminalSet[status] = false
197+
}
198+
199+
for _, s := range statuses {
200+
if _, ok := terminalSet[s]; ok {
201+
terminalSet[s] = true
202+
} else {
203+
return false
204+
}
205+
}
206+
207+
for _, found := range terminalSet {
208+
if !found {
209+
return false
210+
}
211+
}
212+
213+
return true
214+
}

pkg/parser/validate/workflow_test.go

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,3 +234,279 @@ workflows:
234234

235235
CheckYamlErrors(t, testCases)
236236
}
237+
238+
func TestTerminalJobStatusesHint(t *testing.T) {
239+
testCases := []struct {
240+
name string
241+
yamlContent string
242+
expectHint bool
243+
expectedNewText string
244+
expectedRange protocol.Range
245+
}{
246+
{
247+
name: "All terminal statuses present - should show hint",
248+
yamlContent: `version: 2.1
249+
250+
jobs:
251+
job1:
252+
docker:
253+
- image: cimg/base:stable
254+
steps:
255+
- run: echo "test"
256+
job2:
257+
docker:
258+
- image: cimg/base:stable
259+
steps:
260+
- run: echo "test"
261+
262+
workflows:
263+
test-workflow:
264+
jobs:
265+
- job1
266+
- job2:
267+
requires:
268+
- job1: [success, failed, canceled, not_run]`,
269+
expectHint: true,
270+
expectedNewText: "terminal",
271+
expectedRange: protocol.Range{
272+
Start: protocol.Position{Line: 20, Character: 20},
273+
End: protocol.Position{Line: 20, Character: 56},
274+
},
275+
},
276+
{
277+
name: "All terminal statuses as YAML list - should show hint",
278+
yamlContent: `version: 2.1
279+
280+
jobs:
281+
job1:
282+
docker:
283+
- image: cimg/base:stable
284+
steps:
285+
- run: echo "test"
286+
job2:
287+
docker:
288+
- image: cimg/base:stable
289+
steps:
290+
- run: echo "test"
291+
292+
workflows:
293+
test-workflow:
294+
jobs:
295+
- job1
296+
- job2:
297+
requires:
298+
- job1:
299+
- success
300+
- failed
301+
- canceled
302+
- not_run`,
303+
expectHint: true,
304+
expectedNewText: " terminal",
305+
expectedRange: protocol.Range{
306+
Start: protocol.Position{Line: 20, Character: 19},
307+
End: protocol.Position{Line: 24, Character: 23},
308+
},
309+
},
310+
{
311+
name: "All terminal statuses in different order - should show hint",
312+
yamlContent: `version: 2.1
313+
314+
jobs:
315+
job1:
316+
docker:
317+
- image: cimg/base:stable
318+
steps:
319+
- run: echo "test"
320+
job2:
321+
docker:
322+
- image: cimg/base:stable
323+
steps:
324+
- run: echo "test"
325+
326+
workflows:
327+
test-workflow:
328+
jobs:
329+
- job1
330+
- job2:
331+
requires:
332+
- job1: [not_run, canceled, failed, success]`,
333+
expectHint: true,
334+
expectedNewText: "terminal",
335+
expectedRange: protocol.Range{
336+
Start: protocol.Position{Line: 20, Character: 20},
337+
End: protocol.Position{Line: 20, Character: 56},
338+
},
339+
},
340+
{
341+
name: "Only some terminal statuses - no hint",
342+
yamlContent: `version: 2.1
343+
344+
jobs:
345+
job1:
346+
docker:
347+
- image: cimg/base:stable
348+
steps:
349+
- run: echo "test"
350+
job2:
351+
docker:
352+
- image: cimg/base:stable
353+
steps:
354+
- run: echo "test"
355+
356+
workflows:
357+
test-workflow:
358+
jobs:
359+
- job1
360+
- job2:
361+
requires:
362+
- job1: [success, failed]`,
363+
expectHint: false,
364+
},
365+
{
366+
name: "Mix of terminal and non-terminal statuses - no hint",
367+
yamlContent: `version: 2.1
368+
369+
jobs:
370+
job1:
371+
docker:
372+
- image: cimg/base:stable
373+
steps:
374+
- run: echo "test"
375+
job2:
376+
docker:
377+
- image: cimg/base:stable
378+
steps:
379+
- run: echo "test"
380+
381+
workflows:
382+
test-workflow:
383+
jobs:
384+
- job1
385+
- job2:
386+
requires:
387+
- job1: [success, failed, canceled, not_run, unknown]`,
388+
expectHint: false,
389+
},
390+
{
391+
name: "All terminal statuses with anchor - should show hint",
392+
yamlContent: `version: 2.1
393+
394+
jobs:
395+
job1:
396+
docker:
397+
- image: cimg/base:stable
398+
steps:
399+
- run: echo "test"
400+
job2:
401+
docker:
402+
- image: cimg/base:stable
403+
steps:
404+
- run: echo "test"
405+
406+
workflows:
407+
test-workflow:
408+
jobs:
409+
- job1
410+
- job2:
411+
requires:
412+
- job1: &terminal-statuses [success, failed, canceled, not_run]`,
413+
expectHint: true,
414+
expectedNewText: "terminal",
415+
expectedRange: protocol.Range{
416+
Start: protocol.Position{Line: 20, Character: 39},
417+
End: protocol.Position{Line: 20, Character: 75},
418+
},
419+
},
420+
}
421+
422+
for _, tt := range testCases {
423+
t.Run(tt.name, func(t *testing.T) {
424+
val := CreateValidateFromYAML(tt.yamlContent)
425+
val.Validate()
426+
427+
diags := *val.Diagnostics
428+
429+
// Filter for only Hint severity diagnostics
430+
hintDiags := []protocol.Diagnostic{}
431+
for _, d := range diags {
432+
if d.Severity == protocol.DiagnosticSeverityHint {
433+
hintDiags = append(hintDiags, d)
434+
}
435+
}
436+
437+
if !tt.expectHint {
438+
if len(hintDiags) != 0 {
439+
t.Errorf("Expected no hint diagnostics, got %d", len(hintDiags))
440+
}
441+
return
442+
}
443+
444+
if len(hintDiags) != 1 {
445+
t.Fatalf("Expected 1 hint diagnostic, got %d", len(hintDiags))
446+
}
447+
448+
diag := hintDiags[0]
449+
if diag.Severity != protocol.DiagnosticSeverityHint {
450+
t.Errorf("Expected Hint severity, got %v", diag.Severity)
451+
}
452+
453+
if diag.Data == nil {
454+
t.Fatal("Expected diagnostic to have code actions")
455+
}
456+
457+
codeActions, ok := diag.Data.([]protocol.CodeAction)
458+
if !ok {
459+
t.Fatalf("Expected Data to be []protocol.CodeAction, got %T", diag.Data)
460+
}
461+
462+
if len(codeActions) == 0 {
463+
t.Fatal("Expected at least one code action")
464+
}
465+
466+
// Find the terminal simplification code action
467+
var terminalAction *protocol.CodeAction
468+
for i := range codeActions {
469+
if codeActions[i].Title == "Simplify these statuses to 'terminal'" {
470+
terminalAction = &codeActions[i]
471+
break
472+
}
473+
}
474+
475+
if terminalAction == nil {
476+
t.Fatal("Expected 'Simplify these statuses to 'terminal'' code action")
477+
}
478+
479+
if terminalAction.Kind != "quickfix" {
480+
t.Errorf("Expected kind 'quickfix', got %s", terminalAction.Kind)
481+
}
482+
483+
if !terminalAction.IsPreferred {
484+
t.Error("Expected IsPreferred to be true")
485+
}
486+
487+
if terminalAction.Edit == nil {
488+
t.Fatal("Expected Edit to be non-nil")
489+
}
490+
491+
changes := terminalAction.Edit.Changes
492+
if len(changes) != 1 {
493+
t.Fatalf("Expected 1 change, got %d", len(changes))
494+
}
495+
496+
for _, edits := range changes {
497+
if len(edits) != 1 {
498+
t.Fatalf("Expected 1 text edit, got %d", len(edits))
499+
}
500+
501+
edit := edits[0]
502+
if edit.NewText != tt.expectedNewText {
503+
t.Errorf("Expected NewText %q, got %q", tt.expectedNewText, edit.NewText)
504+
}
505+
506+
if edit.Range != tt.expectedRange {
507+
t.Errorf("Expected Range %v, got %v", tt.expectedRange, edit.Range)
508+
}
509+
}
510+
})
511+
}
512+
}

0 commit comments

Comments
 (0)