Skip to content

Commit 2bcb296

Browse files
stevebeattiestevebeattie
authored
lint: add linter for unused var-transform variables (#1777)
Merge pull request #1777 from stevebeattie/lint-add_unused_var-transform_linter
2 parents 8b125ac + 0996670 commit 2bcb296

15 files changed

+649
-0
lines changed

‎pkg/lint/rules.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,43 @@ var AllRules = func(l *Linter) Rules { //nolint:gocyclo
539539
return err
540540
},
541541
},
542+
{
543+
Name: "unused-var-transform",
544+
Description: "var-transforms that define variables which are never referenced",
545+
Severity: SeverityWarning,
546+
LintFunc: func(cfg config.Configuration) error {
547+
// We need to check the raw YAML because ParseConfiguration already applies
548+
// variable substitutions, so the substituted values would be in the config.
549+
var unusedVars []string
550+
for _, vt := range cfg.VarTransforms {
551+
varRef := fmt.Sprintf("${{vars.%s}}", vt.To)
552+
553+
// First check if this variable is used as input to another var-transform
554+
usedInVarTransform := false
555+
for _, other := range cfg.VarTransforms {
556+
if other.From == varRef {
557+
usedInVarTransform = true
558+
break
559+
}
560+
}
561+
562+
// If used in another var-transform or found in the raw YAML, it's not unused
563+
if usedInVarTransform || isVariableReferencedInRawYAML(cfg.Root(), varRef) {
564+
continue
565+
}
566+
567+
unusedVars = append(unusedVars, vt.To)
568+
}
569+
570+
if len(unusedVars) > 0 {
571+
if len(unusedVars) == 1 {
572+
return fmt.Errorf("var-transform creates unused variable %q", unusedVars[0])
573+
}
574+
return fmt.Errorf("var-transform creates unused variables %q", unusedVars)
575+
}
576+
return nil
577+
},
578+
},
542579

543580
// If the git repository URL and update identifiers do not match then we
544581
// will fail to auto-update the package automagically.
@@ -663,3 +700,67 @@ func extractURI(p config.Pipeline) (string, error) {
663700
// Most pipelines won't have a URI.
664701
return "", nil
665702
}
703+
704+
// isVariableReferencedInRawYAML checks if a variable reference (e.g., "${{vars.foo}}")
705+
// appears anywhere in the raw YAML configuration node tree, excluding the var-transforms
706+
// section itself (where variables are defined, not used).
707+
// This is necessary because ParseConfiguration applies variable substitutions,
708+
// so we need to check the original YAML to see if the variable was actually used.
709+
func isVariableReferencedInRawYAML(root *yaml.Node, varRef string) bool {
710+
if root == nil {
711+
return false
712+
}
713+
714+
// Recursively walk the YAML node tree
715+
var walkNode func(*yaml.Node, int) bool
716+
walkNode = func(node *yaml.Node, depth int) bool {
717+
if node == nil {
718+
return false
719+
}
720+
721+
// For mapping nodes, check keys for "var-transforms"
722+
if node.Kind == yaml.MappingNode {
723+
for i := 0; i < len(node.Content); i += 2 {
724+
key := node.Content[i]
725+
value := node.Content[i+1]
726+
727+
// Skip the entire var-transforms section
728+
if key.Kind == yaml.ScalarNode && key.Value == "var-transforms" {
729+
continue
730+
}
731+
732+
// Recursively check other sections
733+
if walkNode(value, depth+1) {
734+
return true
735+
}
736+
}
737+
return false
738+
}
739+
740+
// For sequence nodes, check each element
741+
if node.Kind == yaml.SequenceNode {
742+
for _, child := range node.Content {
743+
if walkNode(child, depth+1) {
744+
return true
745+
}
746+
}
747+
return false
748+
}
749+
750+
// Check if this node's value contains the variable reference
751+
if node.Kind == yaml.ScalarNode && strings.Contains(node.Value, varRef) {
752+
return true
753+
}
754+
755+
// For other node types, check children
756+
for _, child := range node.Content {
757+
if walkNode(child, depth+1) {
758+
return true
759+
}
760+
}
761+
762+
return false
763+
}
764+
765+
return walkNode(root, 0)
766+
}

‎pkg/lint/rules_test.go‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,142 @@ func TestLinter_Rules(t *testing.T) {
617617
wantErr: false,
618618
matches: 0,
619619
},
620+
// var-transform tests
621+
{
622+
file: "var-transform-unused.yaml",
623+
minSeverity: SeverityWarning,
624+
want: EvalResult{
625+
File: "var-transform-unused",
626+
Errors: EvalRuleErrors{
627+
{
628+
Rule: Rule{
629+
Name: "unused-var-transform",
630+
Severity: SeverityWarning,
631+
},
632+
Error: fmt.Errorf("[unused-var-transform]: var-transform creates unused variable \"unused-version\" (WARNING)"),
633+
},
634+
},
635+
},
636+
wantErr: false,
637+
matches: 1,
638+
},
639+
{
640+
file: "var-transform-used.yaml",
641+
minSeverity: SeverityWarning,
642+
want: EvalResult{},
643+
wantErr: false,
644+
matches: 0,
645+
},
646+
{
647+
file: "var-transform-multiple-partial-unused.yaml",
648+
minSeverity: SeverityWarning,
649+
want: EvalResult{
650+
File: "var-transform-multiple-partial-unused",
651+
Errors: EvalRuleErrors{
652+
{
653+
Rule: Rule{
654+
Name: "unused-var-transform",
655+
Severity: SeverityWarning,
656+
},
657+
Error: fmt.Errorf("[unused-var-transform]: var-transform creates unused variables [\"unused-name\" \"another-unused\"] (WARNING)"),
658+
},
659+
},
660+
},
661+
wantErr: false,
662+
matches: 1,
663+
},
664+
{
665+
file: "var-transform-used-in-subpackage.yaml",
666+
minSeverity: SeverityWarning,
667+
want: EvalResult{},
668+
wantErr: false,
669+
matches: 0,
670+
},
671+
{
672+
file: "var-transform-used-in-with.yaml",
673+
minSeverity: SeverityWarning,
674+
want: EvalResult{},
675+
wantErr: false,
676+
matches: 0,
677+
},
678+
{
679+
file: "var-transform-used-in-test-env.yaml",
680+
minSeverity: SeverityWarning,
681+
want: EvalResult{},
682+
wantErr: false,
683+
matches: 0,
684+
},
685+
{
686+
file: "var-transform-used-in-test-packages.yaml",
687+
minSeverity: SeverityWarning,
688+
want: EvalResult{},
689+
wantErr: false,
690+
matches: 0,
691+
},
692+
{
693+
file: "var-transform-chained-second-unused.yaml",
694+
minSeverity: SeverityWarning,
695+
want: EvalResult{
696+
File: "var-transform-chained-second-unused",
697+
Errors: EvalRuleErrors{
698+
{
699+
Rule: Rule{
700+
Name: "unused-var-transform",
701+
Severity: SeverityWarning,
702+
},
703+
Error: fmt.Errorf("[unused-var-transform]: var-transform creates unused variable \"second-transform-unused\" (WARNING)"),
704+
},
705+
},
706+
},
707+
wantErr: false,
708+
matches: 1,
709+
},
710+
{
711+
file: "var-transform-chained-both-used.yaml",
712+
minSeverity: SeverityWarning,
713+
want: EvalResult{},
714+
wantErr: false,
715+
matches: 0,
716+
},
717+
{
718+
file: "var-transform-wrong-syntax-not-matched.yaml",
719+
minSeverity: SeverityWarning,
720+
want: EvalResult{
721+
File: "var-transform-wrong-syntax-not-matched",
722+
Errors: EvalRuleErrors{
723+
{
724+
Rule: Rule{
725+
Name: "unused-var-transform",
726+
Severity: SeverityWarning,
727+
},
728+
Error: fmt.Errorf("[unused-var-transform]: var-transform creates unused variable \"my-version\" (WARNING)"),
729+
},
730+
},
731+
},
732+
wantErr: false,
733+
matches: 1,
734+
},
735+
{
736+
file: "var-transform-used-in-runtime-deps.yaml",
737+
minSeverity: SeverityWarning,
738+
want: EvalResult{},
739+
wantErr: false,
740+
matches: 0,
741+
},
742+
{
743+
file: "var-transform-used-in-env-packages.yaml",
744+
minSeverity: SeverityWarning,
745+
want: EvalResult{},
746+
wantErr: false,
747+
matches: 0,
748+
},
749+
{
750+
file: "var-transform-chained-first-only-in-second.yaml",
751+
minSeverity: SeverityWarning,
752+
want: EvalResult{},
753+
wantErr: false,
754+
matches: 0,
755+
},
620756
}
621757

622758
for _, tt := range tests {

‎pkg/lint/testdata/files/var-transform-chained-both-used.yaml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package:
2+
name: var-transform-chained-both-used
3+
version: 1.2.3
4+
epoch: 0
5+
description: Test chained var-transforms where both are used
6+
copyright:
7+
- license: MIT
8+
9+
environment:
10+
contents:
11+
packages:
12+
- build-base
13+
- busybox
14+
15+
var-transforms:
16+
- from: ${{package.version}}
17+
match: \.
18+
replace: _
19+
to: first-transform
20+
- from: ${{vars.first-transform}}
21+
match: _
22+
replace: '-'
23+
to: second-transform
24+
25+
pipeline:
26+
- runs: |
27+
echo "first: ${{vars.first-transform}}"
28+
echo "second: ${{vars.second-transform}}"
29+
30+
update:
31+
enabled: true
32+
github:
33+
identifier: example/repo

‎pkg/lint/testdata/files/var-transform-chained-first-only-in-second.yaml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package:
2+
name: var-transform-chained-first-only-in-second
3+
version: 1.2.3
4+
epoch: 0
5+
description: Test chained var-transforms where first is only used as input to second
6+
copyright:
7+
- license: MIT
8+
9+
environment:
10+
contents:
11+
packages:
12+
- build-base
13+
- busybox
14+
15+
var-transforms:
16+
- from: ${{package.version}}
17+
match: \.
18+
replace: _
19+
to: first-transform
20+
- from: ${{vars.first-transform}}
21+
match: _
22+
replace: '-'
23+
to: second-transform
24+
25+
pipeline:
26+
- runs: 'echo "using second: ${{vars.second-transform}}"'
27+
28+
update:
29+
enabled: true
30+
github:
31+
identifier: example/repo

‎pkg/lint/testdata/files/var-transform-chained-second-unused.yaml‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package:
2+
name: var-transform-chained-second-unused
3+
version: 1.2.3
4+
epoch: 0
5+
description: Test chained var-transforms where second is unused
6+
copyright:
7+
- license: MIT
8+
9+
environment:
10+
contents:
11+
packages:
12+
- build-base
13+
- busybox
14+
15+
var-transforms:
16+
- from: ${{package.version}}
17+
match: \.
18+
replace: _
19+
to: first-transform
20+
- from: ${{vars.first-transform}}
21+
match: _
22+
replace: '-'
23+
to: second-transform-unused
24+
25+
pipeline:
26+
- runs: 'echo "using first: ${{vars.first-transform}}"'
27+
28+
update:
29+
enabled: true
30+
github:
31+
identifier: example/repo

‎pkg/lint/testdata/files/var-transform-multiple-partial-unused.yaml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package:
2+
name: var-transform-multiple-partial-unused
3+
version: 1.2.3
4+
epoch: 0
5+
description: Test package with multiple var-transforms, some unused
6+
copyright:
7+
- license: MIT
8+
9+
environment:
10+
contents:
11+
packages:
12+
- build-base
13+
- busybox
14+
15+
var-transforms:
16+
- from: ${{package.version}}
17+
match: \.
18+
replace: _
19+
to: used-version
20+
- from: ${{package.name}}
21+
match: '-'
22+
replace: _
23+
to: unused-name
24+
- from: ${{package.version}}
25+
match: \.
26+
replace: "-"
27+
to: another-unused
28+
29+
pipeline:
30+
- runs: 'echo "using version: ${{vars.used-version}}"'
31+
32+
update:
33+
enabled: true
34+
github:
35+
identifier: example/repo

0 commit comments

Comments
 (0)