Skip to content

Commit fe7cea3

Browse files
authored
Merge pull request #202 from docker/bake-inheritance-args-check
Check inheritance structure for finding unrecognized ARGs
2 parents cf78c41 + 34e2fc2 commit fe7cea3

File tree

6 files changed

+342
-34
lines changed

6 files changed

+342
-34
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ All notable changes to the Docker Language Server will be documented in this fil
2929
- Bake
3030
- textDocument/publishDiagnostics
3131
- stop flagging `BUILDKIT_SYNTAX` as an unrecognized `ARG` ([#187](https://github.com/docker/docker-language-server/issues/187))
32+
- use inheritance to determine if an `ARG` is truly unused ([#198](https://github.com/docker/docker-language-server/issues/198))
3233

3334
## [0.7.0] - 2025-05-09
3435

internal/bake/hcl/diagnosticsCollector.go

Lines changed: 66 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/docker/docker-language-server/internal/types"
1616
"github.com/hashicorp/hcl/v2"
1717
"github.com/hashicorp/hcl/v2/hclsyntax"
18+
"github.com/moby/buildkit/frontend/dockerfile/parser"
1819
"github.com/moby/buildkit/solver/errdefs"
1920
)
2021

@@ -112,13 +113,23 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder
112113
}
113114
}
114115

115-
body, ok := doc.(document.BakeHCLDocument).File().Body.(*hclsyntax.Body)
116+
bakeDoc := doc.(document.BakeHCLDocument)
117+
body, ok := bakeDoc.File().Body.(*hclsyntax.Body)
116118
if !ok {
117119
return diagnostics
118120
}
119121

122+
targetDockerfiles := map[string]string{}
123+
dockerfileContent := map[string][]*parser.Node{}
124+
for _, b := range body.Blocks {
125+
if b.Type == "target" && len(b.Labels) == 1 {
126+
dockerfilePath, _ := bakeDoc.DockerfileForTarget(b)
127+
targetDockerfiles[b.Labels[0]] = dockerfilePath
128+
}
129+
}
130+
120131
for _, block := range body.Blocks {
121-
if block.Type == "target" {
132+
if block.Type == "target" && len(block.Labels) == 1 {
122133
if _, ok := block.Body.Attributes["dockerfile-inline"]; ok {
123134
if attribute, ok := block.Body.Attributes["dockerfile"]; ok {
124135
diagnostics = append(diagnostics, protocol.Diagnostic{
@@ -206,15 +217,24 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder
206217
}
207218
}
208219

209-
dockerfilePath, err := doc.(document.BakeHCLDocument).DockerfileForTarget(block)
220+
dockerfilePath, err := bakeDoc.DockerfileForTarget(block)
210221
if dockerfilePath == "" || err != nil {
211222
continue
212223
}
213224

214225
if attribute, ok := block.Body.Attributes["target"]; ok {
215226
if expr, ok := attribute.Expr.(*hclsyntax.TemplateExpr); ok && len(expr.Parts) == 1 {
216227
if literalValueExpr, ok := expr.Parts[0].(*hclsyntax.LiteralValueExpr); ok {
217-
diagnostic := c.checkTargetTarget(dockerfilePath, expr, literalValueExpr, source)
228+
dockerfile := targetDockerfiles[block.Labels[0]]
229+
if dockerfile == "" {
230+
dockerfileContent[""] = nil
231+
}
232+
nodes, ok := dockerfileContent[dockerfile]
233+
if !ok {
234+
_, nodes = document.OpenDockerfile(context.Background(), c.docs, dockerfilePath)
235+
dockerfileContent[block.Labels[0]] = nodes
236+
}
237+
diagnostic := c.checkTargetTarget(nodes, expr, literalValueExpr, source)
218238
if diagnostic != nil {
219239
diagnostics = append(diagnostics, *diagnostic)
220240
}
@@ -224,7 +244,31 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder
224244

225245
if attribute, ok := block.Body.Attributes["args"]; ok {
226246
if expr, ok := attribute.Expr.(*hclsyntax.ObjectConsExpr); ok {
227-
argsDiagnostics := c.checkTargetArgs(dockerfilePath, input, expr, source)
247+
args := make(map[string]struct{})
248+
for _, b := range body.Blocks {
249+
if b.Type == "target" && len(b.Labels) == 1 && b.Labels[0] != block.Labels[0] {
250+
parents, _ := bakeDoc.ParentTargets(b.Labels[0])
251+
if slices.Contains(parents, block.Labels[0]) {
252+
dockerfile := targetDockerfiles[b.Labels[0]]
253+
if dockerfile == "" {
254+
dockerfileContent[""] = nil
255+
}
256+
nodes, ok := dockerfileContent[dockerfile]
257+
if !ok {
258+
_, nodes = document.OpenDockerfile(context.Background(), c.docs, dockerfile)
259+
dockerfileContent[dockerfile] = nodes
260+
}
261+
c.collectARGs(nodes, args)
262+
}
263+
}
264+
}
265+
266+
nodes, ok := dockerfileContent[dockerfilePath]
267+
if !ok {
268+
_, nodes = document.OpenDockerfile(context.Background(), c.docs, dockerfilePath)
269+
dockerfileContent[dockerfilePath] = nodes
270+
}
271+
argsDiagnostics := c.checkTargetArgs(nodes, input, expr, source, args)
228272
diagnostics = append(diagnostics, argsDiagnostics...)
229273
}
230274
}
@@ -233,10 +277,7 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder
233277
return diagnostics
234278
}
235279

236-
// checkTargetArgs examines the args attribute of a target block.
237-
func (c *BakeHCLDiagnosticsCollector) checkTargetArgs(dockerfilePath string, input []byte, expr *hclsyntax.ObjectConsExpr, source string) []protocol.Diagnostic {
238-
_, nodes := document.OpenDockerfile(context.Background(), c.docs, dockerfilePath)
239-
args := []string{}
280+
func (c *BakeHCLDiagnosticsCollector) collectARGs(nodes []*parser.Node, args map[string]struct{}) {
240281
for _, child := range nodes {
241282
if strings.EqualFold(child.Value, "ARG") {
242283
child = child.Next
@@ -246,12 +287,16 @@ func (c *BakeHCLDiagnosticsCollector) checkTargetArgs(dockerfilePath string, inp
246287
if idx != -1 {
247288
value = value[:idx]
248289
}
249-
args = append(args, value)
290+
args[value] = struct{}{}
250291
child = child.Next
251292
}
252293
}
253294
}
295+
}
254296

297+
// checkTargetArgs examines the args attribute of a target block.
298+
func (c *BakeHCLDiagnosticsCollector) checkTargetArgs(nodes []*parser.Node, input []byte, expr *hclsyntax.ObjectConsExpr, source string, args map[string]struct{}) []protocol.Diagnostic {
299+
c.collectARGs(nodes, args)
255300
diagnostics := []protocol.Diagnostic{}
256301
for _, item := range expr.Items {
257302
start := item.KeyExpr.Range().Start.Byte
@@ -264,27 +309,23 @@ func (c *BakeHCLDiagnosticsCollector) checkTargetArgs(dockerfilePath string, inp
264309
if slices.Contains(builtinArgs, arg) {
265310
continue
266311
}
267-
268-
diagnostic := checkStringLiteral(
269-
source,
270-
arg,
271-
fmt.Sprintf("'%v' not defined as an ARG in your Dockerfile", arg),
272-
args,
273-
item.KeyExpr.Range(),
274-
)
275-
276-
if diagnostic != nil {
312+
if _, ok := args[arg]; !ok {
313+
diagnostic := createDiagnostic(
314+
source,
315+
fmt.Sprintf("'%v' not defined as an ARG in your Dockerfile", arg),
316+
item.KeyExpr.Range(),
317+
)
277318
diagnostics = append(diagnostics, *diagnostic)
278319
}
320+
279321
}
280322
return diagnostics
281323
}
282324

283-
func (c *BakeHCLDiagnosticsCollector) checkTargetTarget(dockerfilePath string, expr *hclsyntax.TemplateExpr, literalValueExpr *hclsyntax.LiteralValueExpr, source string) *protocol.Diagnostic {
325+
func (c *BakeHCLDiagnosticsCollector) checkTargetTarget(nodes []*parser.Node, expr *hclsyntax.TemplateExpr, literalValueExpr *hclsyntax.LiteralValueExpr, source string) *protocol.Diagnostic {
284326
value, _ := literalValueExpr.Value(&hcl.EvalContext{})
285327
target := value.AsString()
286328

287-
_, nodes := document.OpenDockerfile(context.Background(), c.docs, dockerfilePath)
288329
found := false
289330
for _, child := range nodes {
290331
if strings.EqualFold(child.Value, "FROM") {
@@ -330,7 +371,10 @@ func checkStringLiteral(diagnosticSource, attributeValue, message string, expect
330371
if slices.Contains(expectedValues, attributeValue) {
331372
return nil
332373
}
374+
return createDiagnostic(diagnosticSource, message, attributeRange)
375+
}
333376

377+
func createDiagnostic(diagnosticSource, message string, attributeRange hcl.Range) *protocol.Diagnostic {
334378
return &protocol.Diagnostic{
335379
Message: message,
336380
Source: types.CreateStringPointer(diagnosticSource),

internal/bake/hcl/diagnosticsCollector_test.go

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,21 @@ func TestCollectDiagnostics(t *testing.T) {
125125
},
126126
},
127127
},
128+
{
129+
name: "args resolution to a dockerfile that points to a variable",
130+
content: "variable var { }\ntarget \"t1\" {\n dockerfile = var\nargs = {\n missing = \"value\"\n }\n}",
131+
diagnostics: []protocol.Diagnostic{},
132+
},
133+
{
134+
name: "args resolution when inherting a parent that points to a var",
135+
content: "variable var { }\ntarget \"t1\" {\n inherits = [var]\nargs = {\n missing = \"value\"\n }\n}",
136+
diagnostics: []protocol.Diagnostic{},
137+
},
138+
{
139+
name: "args resolution when inherting a parent that points to a ${var}",
140+
content: "variable var { }\ntarget \"t1\" {\n inherits = [\"${var}\"]\nargs = {\n missing = \"value\"\n }\n}",
141+
diagnostics: []protocol.Diagnostic{},
142+
},
128143
{
129144
name: "args references built-in args",
130145
content: "target \"t1\" {\n args = {\n HTTP_PROXY = \"\"\n HTTPS_PROXY = \"\"\n FTP_PROXY = \"\"\n NO_PROXY = \"\"\n ALL_PROXY = \"\"\n BUILDKIT_SYNTAX = \"\"\n }\n}",
@@ -212,13 +227,67 @@ target "lint2" {
212227
t.Run(tc.name, func(t *testing.T) {
213228
manager := document.NewDocumentManager()
214229
bytes := []byte(tc.content)
215-
err := os.WriteFile(bakeFilePath, bytes, 0644)
216-
require.NoError(t, err)
217-
t.Cleanup(func() {
218-
err := os.Remove(bakeFilePath)
219-
require.NoError(t, err)
220-
})
230+
collector := &BakeHCLDiagnosticsCollector{docs: manager, scout: scout.NewService()}
231+
doc := document.NewBakeHCLDocument(bakeFileURI, 1, bytes)
232+
diagnostics := collector.CollectDiagnostics("docker-language-server", "", doc, "")
233+
require.Equal(t, tc.diagnostics, diagnostics)
234+
})
235+
}
236+
}
237+
238+
func TestCollectDiagnostics_InterFileDependency(t *testing.T) {
239+
testCases := []struct {
240+
name string
241+
content string
242+
diagnostics []protocol.Diagnostic
243+
}{
244+
{
245+
name: "child target's Dockerfile defines the ARG",
246+
content: `
247+
target parent {
248+
args = {
249+
other = "value2"
250+
}
251+
}
221252
253+
target foo {
254+
dockerfile = "Dockerfile2"
255+
inherits = ["parent"]
256+
}`,
257+
diagnostics: []protocol.Diagnostic{},
258+
},
259+
{
260+
name: "child target's Dockerfile defines the ARG but not another child",
261+
content: `
262+
target parent {
263+
args = {
264+
other = "value2"
265+
}
266+
}
267+
268+
target foo {
269+
dockerfile = "Dockerfile2"
270+
inherits = ["parent"]
271+
}
272+
273+
target foo2 {
274+
inherits = ["parent"]
275+
}`,
276+
diagnostics: []protocol.Diagnostic{},
277+
},
278+
}
279+
280+
wd, err := os.Getwd()
281+
require.NoError(t, err)
282+
projectRoot := filepath.Dir(filepath.Dir(filepath.Dir(wd)))
283+
diagnosticsTestFolderPath := filepath.Join(projectRoot, "testdata", "diagnostics")
284+
bakeFilePath := filepath.Join(diagnosticsTestFolderPath, "docker-bake.hcl")
285+
bakeFileURI := uri.URI(fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(bakeFilePath), "/")))
286+
287+
for _, tc := range testCases {
288+
t.Run(tc.name, func(t *testing.T) {
289+
manager := document.NewDocumentManager()
290+
bytes := []byte(tc.content)
222291
collector := &BakeHCLDiagnosticsCollector{docs: manager, scout: scout.NewService()}
223292
doc := document.NewBakeHCLDocument(bakeFileURI, 1, bytes)
224293
diagnostics := collector.CollectDiagnostics("docker-language-server", "", doc, "")

0 commit comments

Comments
 (0)