Merge pull request #139 from docker/tighten-compose-completion-calculations

rcjsuen · web-flow · commit d8843d76b843 · 2025-05-02T14:57:26.000-04:00
Consider array schema and indentation before returning items
diff --git a/internal/compose/completion.go b/internal/compose/completion.go
@@ -26,6 +26,21 @@ func prefix(line string, character int) string {
 	return sb.String()
 }
 
+func array(line string, character int) bool {
+	isArray := false
+	for i := range character {
+		if unicode.IsSpace(rune(line[i])) {
+			continue
+		} else if line[i] == '-' {
+			isArray = true
+		} else {
+			isArray = false
+			break
+		}
+	}
+	return isArray
+}
+
 func Completion(ctx context.Context, params *protocol.CompletionParams, doc document.ComposeDocument) (*protocol.CompletionList, error) {
 	if params.Position.Character == 0 {
 		items := []protocol.CompletionItem{}
@@ -69,7 +84,11 @@ func Completion(ctx context.Context, params *protocol.CompletionParams, doc docu
 	}
 
 	items := []protocol.CompletionItem{}
-	nodeProps := nodeProperties(path, line, character)
+	isArray := array(lines[lspLine], character-1)
+	nodeProps, arrayAttributes := nodeProperties(path, line, character)
+	if isArray != arrayAttributes {
+		return nil, nil
+	}
 	if schema, ok := nodeProps.(*jsonschema.Schema); ok {
 		if schema.Enum != nil {
 			for _, value := range schema.Enum.Values {
diff --git a/internal/compose/completion_test.go b/internal/compose/completion_test.go
@@ -1832,6 +1832,29 @@ networks:
 			character: 2,
 			list:      nil,
 		},
+		{
+			name: "configs should not suggest attributes as-is",
+			content: `
+services:
+  test:
+    configs:
+      `,
+			line:      4,
+			character: 6,
+			list:      nil,
+		},
+		{
+			name: "array items should still consider indentation when calculating completion items",
+			content: `
+services:
+  test:
+    configs:
+      - mode: 0
+      `,
+			line:      5,
+			character: 6,
+			list:      nil,
+		},
 	}
 
 	composeFileURI := fmt.Sprintf("file:///%v", strings.TrimPrefix(filepath.ToSlash(filepath.Join(os.TempDir(), "compose.yaml")), "/"))
@@ -1846,11 +1869,7 @@ networks:
 				},
 			}, doc)
 			require.NoError(t, err)
-			if tc.list == nil {
-				require.Nil(t, list)
-			} else {
-				require.Equal(t, tc.list, list)
-			}
+			require.Equal(t, tc.list, list)
 		})
 	}
 }
diff --git a/internal/compose/schema.go b/internal/compose/schema.go
@@ -35,44 +35,44 @@ func schemaProperties() map[string]*jsonschema.Schema {
 	return composeSchema.Properties
 }
 
-func nodeProperties(nodes []*ast.MappingValueNode, line, column int) any {
+func nodeProperties(nodes []*ast.MappingValueNode, line, column int) (any, bool) {
 	if composeSchema != nil && slices.Contains(composeSchema.Types.ToStrings(), "object") && composeSchema.Properties != nil {
 		if prop, ok := composeSchema.Properties[nodes[0].Key.GetToken().Value]; ok {
 			for regexp, property := range prop.PatternProperties {
 				if regexp.MatchString(nodes[1].Key.GetToken().Value) {
 					if property.Ref != nil {
-						return recurseNodeProperties(nodes, line, column, 2, property.Ref.Properties)
+						return recurseNodeProperties(nodes, line, column, 2, property.Ref.Properties, false)
 					}
 				}
 			}
 		}
 	}
-	return nil
+	return nil, false
 }
 
-func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffset int, properties map[string]*jsonschema.Schema) any {
+func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffset int, properties map[string]*jsonschema.Schema, arrayAttributes bool) (any, bool) {
 	if len(nodes) == nodeOffset {
-		return properties
+		return properties, arrayAttributes
 	}
-	if len(nodes) >= nodeOffset+2 && nodes[nodeOffset].Key.GetToken().Position.Column <= column && column < nodes[nodeOffset+1].Key.GetToken().Position.Column {
-		return properties
+	if len(nodes) >= nodeOffset+2 && nodes[nodeOffset].Key.GetToken().Position.Column == column {
+		return properties, false
 	}
 	if column == nodes[nodeOffset].Key.GetToken().Position.Column {
-		return properties
+		return properties, false
 	}
 
 	value := nodes[nodeOffset].Key.GetToken().Value
 	if prop, ok := properties[value]; ok {
 		if prop.Ref != nil {
 			if len(prop.Ref.Properties) > 0 {
-				return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Ref.Properties)
+				return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Ref.Properties, false)
 			}
 			for regexp, property := range prop.Ref.PatternProperties {
 				nextValue := nodes[nodeOffset+1].Key.GetToken().Value
 				if regexp.MatchString(nextValue) {
 					for _, nested := range property.OneOf {
 						if slices.Contains(nested.Types.ToStrings(), "object") {
-							return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties)
+							return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties, false)
 						}
 					}
 				}
@@ -81,7 +81,7 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs
 				for _, nested := range schema.OneOf {
 					if nested.Types != nil && slices.Contains(nested.Types.ToStrings(), "object") {
 						if len(nested.Properties) > 0 {
-							return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties)
+							return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties, true)
 						}
 					}
 				}
@@ -91,19 +91,19 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs
 		for _, schema := range prop.OneOf {
 			if schema.Types != nil && slices.Contains(schema.Types.ToStrings(), "object") {
 				if len(schema.Properties) > 0 {
-					return recurseNodeProperties(nodes, line, column, nodeOffset+1, schema.Properties)
+					return recurseNodeProperties(nodes, line, column, nodeOffset+1, schema.Properties, false)
 				}
 
 				for regexp, property := range schema.PatternProperties {
 					if len(nodes) == nodeOffset+1 {
-						return nil
+						return nil, false
 					}
 
 					nextValue := nodes[nodeOffset+1].Key.GetToken().Value
 					if regexp.MatchString(nextValue) {
 						for _, nested := range property.OneOf {
 							if slices.Contains(nested.Types.ToStrings(), "object") {
-								return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties)
+								return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties, false)
 							}
 						}
 					}
@@ -115,19 +115,19 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs
 			for _, nested := range schema.OneOf {
 				if nested.Types != nil && slices.Contains(nested.Types.ToStrings(), "object") {
 					if len(nested.Properties) > 0 {
-						return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties)
+						return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties, true)
 					}
 				}
 			}
 		}
 
 		if nodes[nodeOffset].Key.GetToken().Position.Column < column {
 			if nodes[nodeOffset].Key.GetToken().Position.Line == line {
-				return prop
+				return prop, false
 			}
-			return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Properties)
+			return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Properties, false)
 		}
-		return prop.Properties
+		return prop.Properties, false
 	}
-	return properties
+	return properties, false
 }

Original file line number	Diff line number	Diff line change
`@@ -35,44 +35,44 @@ func schemaProperties() map[string]*jsonschema.Schema {`
`35`	`35`	`return composeSchema.Properties`
`36`	`36`	`}`
`37`	`37`
`38`		`-func nodeProperties(nodes []*ast.MappingValueNode, line, column int) any {`
	`38`	`+func nodeProperties(nodes []*ast.MappingValueNode, line, column int) (any, bool) {`
`39`	`39`	`if composeSchema != nil && slices.Contains(composeSchema.Types.ToStrings(), "object") && composeSchema.Properties != nil {`
`40`	`40`	`if prop, ok := composeSchema.Properties[nodes[0].Key.GetToken().Value]; ok {`
`41`	`41`	`for regexp, property := range prop.PatternProperties {`
`42`	`42`	`if regexp.MatchString(nodes[1].Key.GetToken().Value) {`
`43`	`43`	`if property.Ref != nil {`
`44`		`- return recurseNodeProperties(nodes, line, column, 2, property.Ref.Properties)`
	`44`	`+ return recurseNodeProperties(nodes, line, column, 2, property.Ref.Properties, false)`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`	`}`
`50`		`- return nil`
	`50`	`+ return nil, false`
`51`	`51`	`}`
`52`	`52`
`53`		`-func recurseNodeProperties(nodes []ast.MappingValueNode, line, column, nodeOffset int, properties map[string]jsonschema.Schema) any {`
	`53`	`+func recurseNodeProperties(nodes []ast.MappingValueNode, line, column, nodeOffset int, properties map[string]jsonschema.Schema, arrayAttributes bool) (any, bool) {`
`54`	`54`	`if len(nodes) == nodeOffset {`
`55`		`- return properties`
	`55`	`+ return properties, arrayAttributes`
`56`	`56`	`}`
`57`		`- if len(nodes) >= nodeOffset+2 && nodes[nodeOffset].Key.GetToken().Position.Column <= column && column < nodes[nodeOffset+1].Key.GetToken().Position.Column {`
`58`		`- return properties`
	`57`	`+ if len(nodes) >= nodeOffset+2 && nodes[nodeOffset].Key.GetToken().Position.Column == column {`
	`58`	`+ return properties, false`
`59`	`59`	`}`
`60`	`60`	`if column == nodes[nodeOffset].Key.GetToken().Position.Column {`
`61`		`- return properties`
	`61`	`+ return properties, false`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`value := nodes[nodeOffset].Key.GetToken().Value`
`65`	`65`	`if prop, ok := properties[value]; ok {`
`66`	`66`	`if prop.Ref != nil {`
`67`	`67`	`if len(prop.Ref.Properties) > 0 {`
`68`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Ref.Properties)`
	`68`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Ref.Properties, false)`
`69`	`69`	`}`
`70`	`70`	`for regexp, property := range prop.Ref.PatternProperties {`
`71`	`71`	`nextValue := nodes[nodeOffset+1].Key.GetToken().Value`
`72`	`72`	`if regexp.MatchString(nextValue) {`
`73`	`73`	`for _, nested := range property.OneOf {`
`74`	`74`	`if slices.Contains(nested.Types.ToStrings(), "object") {`
`75`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties)`
	`75`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties, false)`
`76`	`76`	`}`
`77`	`77`	`}`
`78`	`78`	`}`
`@@ -81,7 +81,7 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs`
`81`	`81`	`for _, nested := range schema.OneOf {`
`82`	`82`	`if nested.Types != nil && slices.Contains(nested.Types.ToStrings(), "object") {`
`83`	`83`	`if len(nested.Properties) > 0 {`
`84`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties)`
	`84`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties, true)`
`85`	`85`	`}`
`86`	`86`	`}`
`87`	`87`	`}`
`@@ -91,19 +91,19 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs`
`91`	`91`	`for _, schema := range prop.OneOf {`
`92`	`92`	`if schema.Types != nil && slices.Contains(schema.Types.ToStrings(), "object") {`
`93`	`93`	`if len(schema.Properties) > 0 {`
`94`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+1, schema.Properties)`
	`94`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+1, schema.Properties, false)`
`95`	`95`	`}`
`96`	`96`
`97`	`97`	`for regexp, property := range schema.PatternProperties {`
`98`	`98`	`if len(nodes) == nodeOffset+1 {`
`99`		`- return nil`
	`99`	`+ return nil, false`
`100`	`100`	`}`
`101`	`101`
`102`	`102`	`nextValue := nodes[nodeOffset+1].Key.GetToken().Value`
`103`	`103`	`if regexp.MatchString(nextValue) {`
`104`	`104`	`for _, nested := range property.OneOf {`
`105`	`105`	`if slices.Contains(nested.Types.ToStrings(), "object") {`
`106`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties)`
	`106`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+2, nested.Properties, false)`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`	`}`
`@@ -115,19 +115,19 @@ func recurseNodeProperties(nodes []*ast.MappingValueNode, line, column, nodeOffs`
`115`	`115`	`for _, nested := range schema.OneOf {`
`116`	`116`	`if nested.Types != nil && slices.Contains(nested.Types.ToStrings(), "object") {`
`117`	`117`	`if len(nested.Properties) > 0 {`
`118`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties)`
	`118`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+1, nested.Properties, true)`
`119`	`119`	`}`
`120`	`120`	`}`
`121`	`121`	`}`
`122`	`122`	`}`
`123`	`123`
`124`	`124`	`if nodes[nodeOffset].Key.GetToken().Position.Column < column {`
`125`	`125`	`if nodes[nodeOffset].Key.GetToken().Position.Line == line {`
`126`		`- return prop`
	`126`	`+ return prop, false`
`127`	`127`	`}`
`128`		`- return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Properties)`
	`128`	`+ return recurseNodeProperties(nodes, line, column, nodeOffset+1, prop.Properties, false)`
`129`	`129`	`}`
`130`		`- return prop.Properties`
	`130`	`+ return prop.Properties, false`
`131`	`131`	`}`
`132`		`- return properties`
	`132`	`+ return properties, false`
`133`	`133`	`}`