Skip to content

Commit ff6cd3c

Browse files
monopolemonopole
committed
Report unused variables.
1 parent b7e8042 commit ff6cd3c

File tree

6 files changed

+167
-36
lines changed

6 files changed

+167
-36
lines changed

‎pkg/expansion/expand.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,14 @@ func syntaxWrap(input string) string {
3636
// implements the expansion semantics defined in the expansion spec; it
3737
// returns the input string wrapped in the expansion syntax if no mapping
3838
// for the input is found.
39-
func MappingFuncFor(context ...map[string]string) func(string) string {
39+
func MappingFuncFor(
40+
counts map[string]int,
41+
context ...map[string]string) func(string) string {
4042
return func(input string) string {
4143
for _, vars := range context {
4244
val, ok := vars[input]
4345
if ok {
46+
counts[input]++
4447
return val
4548
}
4649
}

‎pkg/expansion/expand_test.go

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,19 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package expansion
17+
package expansion_test
1818

1919
import (
2020
"testing"
21+
22+
. "sigs.k8s.io/kustomize/pkg/expansion"
2123
)
2224

25+
type expected struct {
26+
count int
27+
edited string
28+
}
29+
2330
func TestMapReference(t *testing.T) {
2431
type env struct {
2532
Name string
@@ -46,21 +53,23 @@ func TestMapReference(t *testing.T) {
4653
"BLU": "$(ZOO)-2",
4754
}
4855

49-
mapping := MappingFuncFor(declaredEnv)
56+
counts := make(map[string]int)
57+
mapping := MappingFuncFor(counts, declaredEnv)
5058

5159
for _, env := range envs {
5260
declaredEnv[env.Name] = Expand(env.Value, mapping)
5361
}
5462

55-
expectedEnv := map[string]string{
56-
"FOO": "bar",
57-
"ZOO": "bar-1",
58-
"BLU": "bar-1-2",
63+
expectedEnv := map[string]expected{
64+
"FOO": {count: 1, edited: "bar"},
65+
"ZOO": {count: 1, edited: "bar-1"},
66+
"BLU": {count: 0, edited: "bar-1-2"},
5967
}
6068

6169
for k, v := range expectedEnv {
62-
if e, a := v, declaredEnv[k]; e != a {
63-
t.Errorf("Expected %v, got %v", e, a)
70+
if e, a := v, declaredEnv[k]; e.edited != a || e.count != counts[k] {
71+
t.Errorf("Expected %v count=%d, got %v count=%d",
72+
e.edited, e.count, a, counts[k])
6473
} else {
6574
delete(declaredEnv, k)
6675
}
@@ -79,9 +88,7 @@ func TestMapping(t *testing.T) {
7988
"VAR_REF": "$(VAR_A)",
8089
"VAR_EMPTY": "",
8190
}
82-
mapping := MappingFuncFor(context)
83-
84-
doExpansionTest(t, mapping)
91+
doExpansionTest(t, context)
8592
}
8693

8794
func TestMappingDual(t *testing.T) {
@@ -94,51 +101,64 @@ func TestMappingDual(t *testing.T) {
94101
"VAR_C": "C",
95102
"VAR_REF": "$(VAR_A)",
96103
}
97-
mapping := MappingFuncFor(context, context2)
98104

99-
doExpansionTest(t, mapping)
105+
doExpansionTest(t, context, context2)
100106
}
101107

102-
func doExpansionTest(t *testing.T, mapping func(string) string) {
108+
func doExpansionTest(t *testing.T, context ...map[string]string) {
103109
cases := []struct {
104110
name string
105111
input string
106112
expected string
113+
counts map[string]int
107114
}{
108115
{
109116
name: "whole string",
110117
input: "$(VAR_A)",
111118
expected: "A",
119+
counts: map[string]int{"VAR_A": 1},
112120
},
113121
{
114122
name: "repeat",
115123
input: "$(VAR_A)-$(VAR_A)",
116124
expected: "A-A",
125+
counts: map[string]int{"VAR_A": 2},
126+
},
127+
{
128+
name: "multiple repeats",
129+
input: "$(VAR_A)-$(VAR_B)-$(VAR_B)-$(VAR_B)-$(VAR_A)",
130+
expected: "A-B-B-B-A",
131+
counts: map[string]int{"VAR_A": 2, "VAR_B": 3},
117132
},
118133
{
119134
name: "beginning",
120135
input: "$(VAR_A)-1",
121136
expected: "A-1",
137+
counts: map[string]int{"VAR_A": 1},
122138
},
123139
{
124140
name: "middle",
125141
input: "___$(VAR_B)___",
126142
expected: "___B___",
143+
counts: map[string]int{"VAR_B": 1},
127144
},
128145
{
129146
name: "end",
130147
input: "___$(VAR_C)",
131148
expected: "___C",
149+
counts: map[string]int{"VAR_C": 1},
132150
},
133151
{
134152
name: "compound",
135153
input: "$(VAR_A)_$(VAR_B)_$(VAR_C)",
136154
expected: "A_B_C",
155+
counts: map[string]int{"VAR_A": 1, "VAR_B": 1, "VAR_C": 1},
137156
},
138157
{
139158
name: "escape & expand",
140159
input: "$$(VAR_B)_$(VAR_A)",
141160
expected: "$(VAR_B)_A",
161+
counts: map[string]int{"VAR_A": 1},
142162
},
143163
{
144164
name: "compound escape",
@@ -154,16 +174,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
154174
name: "backslash escape ignored",
155175
input: "foo\\$(VAR_C)bar",
156176
expected: "foo\\Cbar",
177+
counts: map[string]int{"VAR_C": 1},
157178
},
158179
{
159180
name: "backslash escape ignored",
160181
input: "foo\\\\$(VAR_C)bar",
161182
expected: "foo\\\\Cbar",
183+
counts: map[string]int{"VAR_C": 1},
162184
},
163185
{
164186
name: "lots of backslashes",
165187
input: "foo\\\\\\\\$(VAR_A)bar",
166188
expected: "foo\\\\\\\\Abar",
189+
counts: map[string]int{"VAR_A": 1},
167190
},
168191
{
169192
name: "nested var references",
@@ -179,16 +202,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
179202
name: "value is a reference",
180203
input: "$(VAR_REF)",
181204
expected: "$(VAR_A)",
205+
counts: map[string]int{"VAR_REF": 1},
182206
},
183207
{
184208
name: "value is a reference x 2",
185209
input: "%%$(VAR_REF)--$(VAR_REF)%%",
186210
expected: "%%$(VAR_A)--$(VAR_A)%%",
211+
counts: map[string]int{"VAR_REF": 2},
187212
},
188213
{
189214
name: "empty var",
190215
input: "foo$(VAR_EMPTY)bar",
191216
expected: "foobar",
217+
counts: map[string]int{"VAR_EMPTY": 1},
192218
},
193219
{
194220
name: "unterminated expression",
@@ -234,6 +260,7 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
234260
name: "multiple (odd) operators, var defined",
235261
input: "$$$$$$$(VAR_A)",
236262
expected: "$$$A",
263+
counts: map[string]int{"VAR_A": 1},
237264
},
238265
{
239266
name: "missing open expression",
@@ -249,16 +276,19 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
249276
name: "trailing incomplete expression not consumed",
250277
input: "$(VAR_B)_______$(A",
251278
expected: "B_______$(A",
279+
counts: map[string]int{"VAR_B": 1},
252280
},
253281
{
254282
name: "trailing incomplete expression, no content, is not consumed",
255283
input: "$(VAR_C)_______$(",
256284
expected: "C_______$(",
285+
counts: map[string]int{"VAR_C": 1},
257286
},
258287
{
259288
name: "operator at end of input string is preserved",
260289
input: "$(VAR_A)foobarzab$",
261290
expected: "Afoobarzab$",
291+
counts: map[string]int{"VAR_A": 1},
262292
},
263293
{
264294
name: "shell escaped incomplete expr",
@@ -293,9 +323,31 @@ func doExpansionTest(t *testing.T, mapping func(string) string) {
293323
}
294324

295325
for _, tc := range cases {
326+
counts := make(map[string]int)
327+
mapping := MappingFuncFor(counts, context...)
296328
expanded := Expand(tc.input, mapping)
297329
if e, a := tc.expected, expanded; e != a {
298330
t.Errorf("%v: expected %q, got %q", tc.name, e, a)
299331
}
332+
if len(counts) != len(tc.counts) {
333+
t.Errorf("%v: len(counts)=%d != len(tc.counts)=%d",
334+
tc.name, len(counts), len(tc.counts))
335+
}
336+
if len(tc.counts) > 0 {
337+
for k, expectedCount := range tc.counts {
338+
c, ok := counts[k]
339+
if ok {
340+
if c != expectedCount {
341+
t.Errorf(
342+
"%v: k=%s, expected count %d, got %d",
343+
tc.name, k, expectedCount, c)
344+
}
345+
} else {
346+
t.Errorf(
347+
"%v: k=%s, expected count %d, got zero",
348+
tc.name, k, expectedCount)
349+
}
350+
}
351+
}
300352
}
301353
}

‎pkg/target/resaccumulator.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ package target
1818

1919
import (
2020
"fmt"
21+
"log"
22+
"strings"
23+
2124
"sigs.k8s.io/kustomize/pkg/resid"
2225
"sigs.k8s.io/kustomize/pkg/resmap"
2326
"sigs.k8s.io/kustomize/pkg/transformers"
@@ -135,8 +138,18 @@ func (ra *ResAccumulator) ResolveVars() error {
135138
if err != nil {
136139
return err
137140
}
138-
return ra.Transform(transformers.NewRefVarTransformer(
139-
replacementMap, ra.tConfig.VarReference))
141+
if len(replacementMap) == 0 {
142+
return nil
143+
}
144+
t := transformers.NewRefVarTransformer(
145+
replacementMap, ra.tConfig.VarReference)
146+
err = ra.Transform(t)
147+
if len(t.UnusedVars()) > 0 {
148+
log.Printf(
149+
"well-defined vars that were never replaced: %s\n",
150+
strings.Join(t.UnusedVars(), ","))
151+
}
152+
return err
140153
}
141154

142155
func (ra *ResAccumulator) FixBackReferences() (err error) {

‎pkg/target/resaccumulator_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ limitations under the License.
1717
package target_test
1818

1919
import (
20+
"bytes"
21+
"log"
22+
"os"
2023
"strings"
2124
"testing"
2225

@@ -120,6 +123,50 @@ func TestResolveVarsHappy(t *testing.T) {
120123
}
121124
}
122125

126+
func TestResolveVarsOneUnused(t *testing.T) {
127+
ra, _, err := makeResAccumulator()
128+
if err != nil {
129+
t.Fatalf("unexpected err: %v", err)
130+
}
131+
err = ra.MergeVars([]types.Var{
132+
{
133+
Name: "SERVICE_ONE",
134+
ObjRef: types.Target{
135+
Gvk: gvk.Gvk{Version: "v1", Kind: "Service"},
136+
Name: "backendOne"},
137+
},
138+
{
139+
Name: "SERVICE_UNUSED",
140+
ObjRef: types.Target{
141+
Gvk: gvk.Gvk{Version: "v1", Kind: "Service"},
142+
Name: "backendTwo"},
143+
},
144+
})
145+
if err != nil {
146+
t.Fatalf("unexpected err: %v", err)
147+
}
148+
var buf bytes.Buffer
149+
log.SetOutput(&buf)
150+
defer func() {
151+
log.SetOutput(os.Stderr)
152+
}()
153+
err = ra.ResolveVars()
154+
if err != nil {
155+
t.Fatalf("unexpected err: %v", err)
156+
}
157+
expectLog(t, buf, "well-defined vars that were never replaced: SERVICE_UNUSED")
158+
c := getCommand(find("deploy1", ra.ResMap()))
159+
if c != "myserver --somebackendService backendOne --yetAnother $(SERVICE_TWO)" {
160+
t.Fatalf("unexpected command: %s", c)
161+
}
162+
}
163+
164+
func expectLog(t *testing.T, log bytes.Buffer, expect string) {
165+
if !strings.Contains(log.String(), expect) {
166+
t.Fatalf("expected log containing '%s', got '%s'", expect, log.String())
167+
}
168+
}
169+
123170
func TestResolveVarsVarNeedsDisambiguation(t *testing.T) {
124171
ra, rf, err := makeResAccumulator()
125172
if err != nil {

0 commit comments

Comments
 (0)