diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eafdbe..7195e45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ All notable changes to the Docker Language Server will be documented in this fil - convert links properly if a WSL URI with a dollar sign is used ([#378](https://github.com/docker/docker-language-server/issues/378)) - textDocument/inlineCompletion - convert links properly if a WSL URI with a dollar sign is used ([#384](https://github.com/docker/docker-language-server/issues/384)) + - textDocument/publishDiagnostics + - update the URI handling so that a WSL URI with a dollar sign can be scanned for errors ([#386](https://github.com/docker/docker-language-server/issues/386)) ## [0.14.0] - 2025-07-16 diff --git a/internal/bake/hcl/diagnosticsCollector.go b/internal/bake/hcl/diagnosticsCollector.go index f26e7fb..14c010d 100644 --- a/internal/bake/hcl/diagnosticsCollector.go +++ b/internal/bake/hcl/diagnosticsCollector.go @@ -65,7 +65,12 @@ func (c *BakeHCLDiagnosticsCollector) SupportsLanguageIdentifier(languageIdentif func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder string, doc document.Document, text string) []protocol.Diagnostic { input := doc.Input() - _, err := bake.ParseFile(input, doc.URI().Filename()) + dp, err := doc.DocumentPath() + if err != nil { + return nil + } + + _, err = bake.ParseFile(input, dp.FileName) diagnostics := []protocol.Diagnostic{} if err != nil { var sourceError *errdefs.SourceError @@ -119,12 +124,14 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder return diagnostics } - targetDockerfiles := map[string]string{} dockerfileContent := map[string][]*parser.Node{} for _, b := range body.Blocks { if b.Type == "target" && len(b.Labels) == 1 { - dockerfilePath, _ := bakeDoc.DockerfileForTarget(b) - targetDockerfiles[b.Labels[0]] = dockerfilePath + if _, ok := dockerfileContent[b.Labels[0]]; !ok { + targetDockerfileURI, targetDockerfilePath, _ := bakeDoc.DockerfileDocumentPathForTarget(b) + _, nodes := document.OpenDockerfile(context.Background(), c.docs, targetDockerfileURI, targetDockerfilePath) + dockerfileContent[b.Labels[0]] = nodes + } } } @@ -217,7 +224,7 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder } } - dockerfilePath, err := bakeDoc.DockerfileForTarget(block) + _, dockerfilePath, err := bakeDoc.DockerfileDocumentPathForTarget(block) if dockerfilePath == "" || err != nil { continue } @@ -225,18 +232,11 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder if attribute, ok := block.Body.Attributes["target"]; ok { if expr, ok := attribute.Expr.(*hclsyntax.TemplateExpr); ok && len(expr.Parts) == 1 { if literalValueExpr, ok := expr.Parts[0].(*hclsyntax.LiteralValueExpr); ok { - dockerfile := targetDockerfiles[block.Labels[0]] - if dockerfile == "" { - dockerfileContent[""] = nil - } - nodes, ok := dockerfileContent[dockerfile] - if !ok { - _, nodes = document.OpenDockerfile(context.Background(), c.docs, "", dockerfilePath) - dockerfileContent[block.Labels[0]] = nodes - } - diagnostic := c.checkTargetTarget(nodes, expr, literalValueExpr, source) - if diagnostic != nil { - diagnostics = append(diagnostics, *diagnostic) + if nodes, ok := dockerfileContent[block.Labels[0]]; ok { + diagnostic := c.checkTargetTarget(nodes, expr, literalValueExpr, source) + if diagnostic != nil { + diagnostics = append(diagnostics, *diagnostic) + } } } } @@ -249,27 +249,17 @@ func (c *BakeHCLDiagnosticsCollector) CollectDiagnostics(source, workspaceFolder if b.Type == "target" && len(b.Labels) == 1 && b.Labels[0] != block.Labels[0] { parents, _ := bakeDoc.ParentTargets(b.Labels[0]) if slices.Contains(parents, block.Labels[0]) { - dockerfile := targetDockerfiles[b.Labels[0]] - if dockerfile == "" { - dockerfileContent[""] = nil - } - nodes, ok := dockerfileContent[dockerfile] - if !ok { - _, nodes = document.OpenDockerfile(context.Background(), c.docs, "", dockerfile) - dockerfileContent[dockerfile] = nodes + if nodes, ok := dockerfileContent[b.Labels[0]]; ok { + c.collectARGs(nodes, args) } - c.collectARGs(nodes, args) } } } - nodes, ok := dockerfileContent[dockerfilePath] - if !ok { - _, nodes = document.OpenDockerfile(context.Background(), c.docs, "", dockerfilePath) - dockerfileContent[dockerfilePath] = nodes + if nodes, ok := dockerfileContent[block.Labels[0]]; ok { + argsDiagnostics := c.checkTargetArgs(nodes, input, expr, source, args) + diagnostics = append(diagnostics, argsDiagnostics...) } - argsDiagnostics := c.checkTargetArgs(nodes, input, expr, source, args) - diagnostics = append(diagnostics, argsDiagnostics...) } } } diff --git a/internal/bake/hcl/diagnosticsCollector_test.go b/internal/bake/hcl/diagnosticsCollector_test.go index dac813d..3a547d0 100644 --- a/internal/bake/hcl/diagnosticsCollector_test.go +++ b/internal/bake/hcl/diagnosticsCollector_test.go @@ -1,6 +1,7 @@ package hcl import ( + "context" "fmt" "os" "path/filepath" @@ -52,7 +53,7 @@ func TestCollectDiagnostics(t *testing.T) { }, }, { - name: "target block with network attribute empty string", + name: "target block with alpine:3.17.0 is flagged with vulnerabilities", content: "target \"t1\" {\n tags = [ \"alpine:3.17.0\" ]\n}", diagnostics: []protocol.Diagnostic{ { @@ -126,10 +127,26 @@ func TestCollectDiagnostics(t *testing.T) { }, }, { - name: "args resolution to a dockerfile that points to a variable", - content: "variable var { }\ntarget \"t1\" {\n dockerfile = var\nargs = {\n missing = \"value\"\n }\n}", + name: "args resolution to a dockerfile that points to a valid variable", + content: "variable var { default = \"./backend/Dockerfile\" }\ntarget \"t1\" {\n dockerfile = var\nargs = {\n BACKEND_VAR = \"newValue\"\n }\n}", diagnostics: []protocol.Diagnostic{}, }, + { + name: "args resolution to a dockerfile that points to a valid variable", + content: "variable var { default = \"./backend/Dockerfile\" }\ntarget \"t1\" {\n dockerfile = var\nargs = {\n missing = \"newValue\"\n }\n}", + + diagnostics: []protocol.Diagnostic{ + { + Message: "'missing' not defined as an ARG in your Dockerfile", + Source: types.CreateStringPointer("docker-language-server"), + Severity: types.CreateDiagnosticSeverityPointer(protocol.DiagnosticSeverityError), + Range: protocol.Range{ + Start: protocol.Position{Line: 4, Character: 4}, + End: protocol.Position{Line: 4, Character: 11}, + }, + }, + }, + }, { name: "args resolution when inherting a parent that points to a var", content: "variable var { }\ntarget \"t1\" {\n inherits = [var]\nargs = {\n missing = \"value\"\n }\n}", @@ -215,30 +232,56 @@ target "lint2" { diagnostics: []protocol.Diagnostic{}, }, { - name: "context has a variable", + name: "context has a variable and the referenced ARG is valid", content: ` -variable "GITHUB_WORKSPACE" { - default = "." +variable "VAR" { + default = "./backend" } target "build" { - context = "${GITHUB_WORKSPACE}/folder/subfolder" + context = "${VAR}" dockerfile = "Dockerfile" args = { - VAR = "value" + BACKEND_VAR = "newValue" } }`, diagnostics: []protocol.Diagnostic{}, }, + { + name: "context has a variable and the referenced ARG is invalid", + content: ` +variable "VAR" { + default = "./backend" +} + +target "build" { + context = "${VAR}" + dockerfile = "Dockerfile" + args = { + NON_EXISTENT_VAR = "newValue" + } +}`, + diagnostics: []protocol.Diagnostic{ + { + Message: "'NON_EXISTENT_VAR' not defined as an ARG in your Dockerfile", + Source: types.CreateStringPointer("docker-language-server"), + Severity: types.CreateDiagnosticSeverityPointer(protocol.DiagnosticSeverityError), + Range: protocol.Range{ + Start: protocol.Position{Line: 9, Character: 4}, + End: protocol.Position{Line: 9, Character: 20}, + }, + }, + }, + }, { name: "context has a variable and the target is inherited", content: ` -variable "GITHUB_WORKSPACE" { +variable "VAR" { default = "." } target "common-base" { - context = "${GITHUB_WORKSPACE}/folder/subfolder" + context = "${VAR}/folder/subfolder" dockerfile = "Dockerfile" } @@ -253,12 +296,12 @@ target "build" { { name: "parent target cannot be resolved but local target is resolvable", content: ` -variable "GITHUB_WORKSPACE" { +variable "VAR" { default = "." } target "common-base" { - dockerfile = "${GITHUB_WORKSPACE}/folder/subfolder/Dockerfile" + dockerfile = "${VAR}/folder/subfolder/Dockerfile" } target "build" { @@ -336,15 +379,84 @@ target "build" { } } -func TestCollectDiagnostics_InterFileDependency(t *testing.T) { +func TestCollectDiagnostics_WSL(t *testing.T) { testCases := []struct { - name string - content string - diagnostics []protocol.Diagnostic + name string + content string + dockerfileContent string + diagnostics []protocol.Diagnostic }{ { - name: "child target's Dockerfile defines the ARG", - content: ` + name: "target found in Dockerfile", + content: "target \"t1\" {\n target = \"base\"\n}", + dockerfileContent: "FROM scratch AS base", + diagnostics: []protocol.Diagnostic{}, + }, + { + name: "target cannot be found in Dockerfile", + content: "target \"t1\" {\n target = \"nonexistent\"\n}", + dockerfileContent: "FROM scratch AS base", + diagnostics: []protocol.Diagnostic{ + { + Message: "target could not be found in your Dockerfile", + Source: types.CreateStringPointer("docker-language-server"), + Severity: types.CreateDiagnosticSeverityPointer(protocol.DiagnosticSeverityError), + Range: protocol.Range{ + Start: protocol.Position{Line: 1, Character: 11}, + End: protocol.Position{Line: 1, Character: 24}, + }, + }, + }, + }, + { + name: "args can be found in Dockerfile", + content: "target \"t1\" {\n args = {\n VAR = \"newValue\"\n }\n}", + dockerfileContent: "ARG VAR=value\nFROM scratch", + diagnostics: []protocol.Diagnostic{}, + }, + { + name: "args cannot be found in Dockerfile", + content: "target \"t1\" {\n args = {\n missing = \"newValue\"\n }\n}", + dockerfileContent: "ARG VAR=value\nFROM scratch", + diagnostics: []protocol.Diagnostic{ + { + Message: "'missing' not defined as an ARG in your Dockerfile", + Source: types.CreateStringPointer("docker-language-server"), + Severity: types.CreateDiagnosticSeverityPointer(protocol.DiagnosticSeverityError), + Range: protocol.Range{ + Start: protocol.Position{Line: 2, Character: 4}, + End: protocol.Position{Line: 2, Character: 11}, + }, + }, + }, + }, + } + + dockerfileURI := "file://wsl%24/docker-desktop/tmp/Dockerfile" + bakeFileURI := uri.URI("file://wsl%24/docker-desktop/tmp/docker-bake.hcl") + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + manager := document.NewDocumentManager() + changed, err := manager.Write(context.Background(), uri.URI(dockerfileURI), protocol.DockerfileLanguage, 1, []byte(tc.dockerfileContent)) + require.NoError(t, err) + require.True(t, changed) + collector := &BakeHCLDiagnosticsCollector{docs: manager, scout: scout.NewService()} + doc := document.NewBakeHCLDocument(bakeFileURI, 1, []byte(tc.content)) + diagnostics := collector.CollectDiagnostics("docker-language-server", "", doc, "") + require.Equal(t, tc.diagnostics, diagnostics) + }) + } +} + +var hierarchyTests = []struct { + name string + content string + diagnostics []protocol.Diagnostic +}{ + { + name: "child target's Dockerfile defines the ARG", + content: ` target parent { args = { other = "value2" @@ -355,11 +467,11 @@ target foo { dockerfile = "Dockerfile2" inherits = ["parent"] }`, - diagnostics: []protocol.Diagnostic{}, - }, - { - name: "child target's Dockerfile defines the ARG but not another child", - content: ` + diagnostics: []protocol.Diagnostic{}, + }, + { + name: "child target's Dockerfile defines the ARG but not another child", + content: ` target parent { args = { other = "value2" @@ -374,10 +486,11 @@ target foo { target foo2 { inherits = ["parent"] }`, - diagnostics: []protocol.Diagnostic{}, - }, - } + diagnostics: []protocol.Diagnostic{}, + }, +} +func TestCollectDiagnostics_Hierarchy(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) projectRoot := filepath.Dir(filepath.Dir(filepath.Dir(wd))) @@ -385,9 +498,29 @@ target foo2 { bakeFilePath := filepath.Join(diagnosticsTestFolderPath, "docker-bake.hcl") bakeFileURI := uri.URI(fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(bakeFilePath), "/"))) - for _, tc := range testCases { + for _, tc := range hierarchyTests { + t.Run(tc.name, func(t *testing.T) { + manager := document.NewDocumentManager() + bytes := []byte(tc.content) + collector := &BakeHCLDiagnosticsCollector{docs: manager, scout: scout.NewService()} + doc := document.NewBakeHCLDocument(bakeFileURI, 1, bytes) + diagnostics := collector.CollectDiagnostics("docker-language-server", "", doc, "") + require.Equal(t, tc.diagnostics, diagnostics) + }) + } +} + +func TestCollectDiagnostics_WSLHierarchy(t *testing.T) { + dockerfileContent := "ARG other=value\nFROM scratch AS build" + dockerfileURI := uri.URI("file://wsl%24/docker-desktop/tmp/Dockerfile2") + bakeFileURI := uri.URI("file://wsl%24/docker-desktop/tmp/docker-bake.hcl") + + for _, tc := range hierarchyTests { t.Run(tc.name, func(t *testing.T) { manager := document.NewDocumentManager() + changed, err := manager.Write(context.Background(), dockerfileURI, protocol.DockerfileLanguage, 1, []byte(dockerfileContent)) + require.NoError(t, err) + require.True(t, changed) bytes := []byte(tc.content) collector := &BakeHCLDiagnosticsCollector{docs: manager, scout: scout.NewService()} doc := document.NewBakeHCLDocument(bakeFileURI, 1, bytes) diff --git a/internal/pkg/document/bakeDocument.go b/internal/pkg/document/bakeDocument.go index a4692e3..85e3413 100644 --- a/internal/pkg/document/bakeDocument.go +++ b/internal/pkg/document/bakeDocument.go @@ -25,6 +25,7 @@ type BakeHCLDocument interface { Decoder() *decoder.PathDecoder File() *hcl.File DockerfileForTarget(block *hclsyntax.Block) (string, error) + DockerfileDocumentPathForTarget(block *hclsyntax.Block) (dockerfileURI string, dockerfileAbsolutePath string, err error) ParentTargets(target string) ([]string, bool) } @@ -294,3 +295,32 @@ func (d *bakeHCLDocument) DockerfileForTarget(block *hclsyntax.Block) (string, e } return "", nil } + +func (d *bakeHCLDocument) DockerfileDocumentPathForTarget(block *hclsyntax.Block) (dockerfileURI string, dockerfileAbsolutePath string, err error) { + if d.bakePrintOutput == nil || len(block.Labels) != 1 { + return "", "", errors.New("cannot parse Bake file") + } + + switch resolvableDockerfile(block) { + case Undefined: + targets, _ := d.ParentTargets(block.Labels[0]) + for _, target := range targets { + body := d.file.Body.(*hclsyntax.Body) + for _, b := range body.Blocks { + if len(b.Labels) == 1 && b.Labels[0] == target && resolvableDockerfile(b) == Unresolvable { + return "", "", nil + } + } + } + } + + path, _ := d.DocumentPath() + if target, ok := d.bakePrintOutput.Target[block.Labels[0]]; ok { + if target.DockerfileInline != nil { + return "", "", errors.New("dockerfile-inline defined") + } + uri, file := types.Concatenate(filepath.Join(path.Folder, *target.Context), *target.Dockerfile, path.WSLDollarSignHost) + return uri, file, nil + } + return "", "", fmt.Errorf("no target block named %v", block.Labels[0]) +} diff --git a/internal/pkg/document/bakeDocument_test.go b/internal/pkg/document/bakeDocument_test.go index 4298f39..b216604 100644 --- a/internal/pkg/document/bakeDocument_test.go +++ b/internal/pkg/document/bakeDocument_test.go @@ -1,9 +1,16 @@ package document import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" "testing" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/stretchr/testify/require" + "go.lsp.dev/uri" ) func TestParentTargets(t *testing.T) { @@ -108,3 +115,112 @@ target t2 { }`, }) } } + +func TestDockerfileDocumentPathForTarget(t *testing.T) { + root := os.TempDir() + tmp := filepath.Join(root, "tmp") + tmp2 := filepath.Join(tmp, "tmp2") + temporaryBakeFile := fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp2, "docker-bake.hcl")), "/")) + + testCases := []struct { + name string + documentURI string + content string + uri string + path string + err error + }{ + { + name: "empty block with no label name", + content: `target {}`, + err: errors.New("cannot parse Bake file"), + }, + { + name: "group block before target block", + content: `group g1 { targets = [ "t1" ] } + target t1 {}`, + err: errors.New("no target block named g1"), + }, + { + name: "dockerfile set", + content: `target t1 { dockerfile = "Dockerfile2" }`, + uri: fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp2, "Dockerfile2")), "/")), + path: filepath.Join(tmp2, "Dockerfile2"), + }, + { + name: "dockerfile set to variable", + content: ` + target "stage1" { + dockerfile = var + } + variable "var" { + default = "Dockerfile2" + }`, + uri: fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp2, "Dockerfile2")), "/")), + path: filepath.Join(tmp2, "Dockerfile2"), + }, + { + name: "dockerfile-inline set", + content: `target t1 { dockerfile-inline = "FROM scratch" }`, + err: errors.New("dockerfile-inline defined"), + }, + { + name: "context set to subfolder", + content: `target t1 { context = "subfolder" }`, + uri: fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp2, "subfolder", "Dockerfile")), "/")), + path: filepath.Join(tmp2, "subfolder", "Dockerfile"), + }, + { + name: "context set to ../other", + content: `target t1 { context = "../other" }`, + uri: fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp, "other", "Dockerfile")), "/")), + path: filepath.Join(tmp, "other", "Dockerfile"), + }, + { + name: "context set to subfolder and dockerfile defined", + content: `target t1 { + context = "subfolder" + dockerfile = "Dockerfile2" + }`, + uri: fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(tmp2, "subfolder", "Dockerfile2")), "/")), + path: filepath.Join(tmp2, "subfolder", "Dockerfile2"), + }, + { + name: "wsl$ with dockerfile set", + documentURI: "file://wsl%24/docker-desktop/tmp/tmp2/docker-bake.hcl", + content: `target t1 { dockerfile = "Dockerfile2" }`, + uri: "file://wsl%24/docker-desktop/tmp/tmp2/Dockerfile2", + path: "\\\\wsl$\\docker-desktop\\tmp\\tmp2\\Dockerfile2", + }, + { + name: "wsl$ with context set", + documentURI: "file://wsl%24/docker-desktop/tmp/tmp2/docker-bake.hcl", + content: `target t1 { context = "subfolder" }`, + uri: "file://wsl%24/docker-desktop/tmp/tmp2/subfolder/Dockerfile", + path: "\\\\wsl$\\docker-desktop\\tmp\\tmp2\\subfolder\\Dockerfile", + }, + { + name: "wsl$ with context set to ../other", + documentURI: "file://wsl%24/docker-desktop/tmp/tmp2/docker-bake.hcl", + content: `target t1 { context = "../other" }`, + uri: "file://wsl%24/docker-desktop/tmp/other/Dockerfile", + path: "\\\\wsl$\\docker-desktop\\tmp\\other\\Dockerfile", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + documentURI := tc.documentURI + if tc.documentURI == "" { + documentURI = temporaryBakeFile + } + doc := NewBakeHCLDocument(uri.URI(documentURI), 1, []byte(tc.content)) + body, ok := doc.File().Body.(*hclsyntax.Body) + require.True(t, ok) + uri, path, err := doc.DockerfileDocumentPathForTarget(body.Blocks[0]) + require.Equal(t, tc.err, err) + require.Equal(t, tc.uri, uri) + require.Equal(t, tc.path, path) + }) + } +}