code review

pkg/tfgen/docs.go

@@ -819,26 +819,36 @@ func (p *tfMarkdownParser) parseSchemaWithNestedSections(subsection []string) {
 	parseTopLevelSchemaIntoDocs(&p.ret, topLevelSchema, p.sink.warn)
 }
 
+type markdownLineInfo struct {
+	name, desc          string
+	isFound, isIndented bool
+}
+
 // parseArgFromMarkdownLine takes a line of Markdown and attempts to parse it for a Terraform argument and its
-// description
-func parseArgFromMarkdownLine(line string) (string, string, bool, bool) {
+// description. It returns a struct containing the name and description of the arg, whether an arg was found,
+// and whether it has indented space, denoting it as a subproperty of a previously found property.
+func parseArgFromMarkdownLine(line string) markdownLineInfo {
 	matches := argumentBulletRegexp.FindStringSubmatch(line)
-	indentedBullet := false
+	var parsed markdownLineInfo
 	if len(matches) > 4 {
 		if strings.HasPrefix(matches[0], "  ") {
-			indentedBullet = true
+			parsed.isIndented = true
 		}
-		return matches[1], matches[4], true, indentedBullet
+		parsed.name = matches[1]
+		parsed.desc = matches[4]
+		parsed.isFound = true
+
 	}
-	return "", "", false, indentedBullet
+	return parsed
 }
 
 var genericNestedRegexp = regexp.MustCompile("supports? the following:")
 
 var nestedObjectRegexps = []*regexp.Regexp{
 	// For example:
 	// s3_bucket.html.markdown: "The `website` object supports the following:"
-	// appmesh_gateway_route.html.markdown: "The `grpc_route`, `http_route` and `http2_route` objects supports the following:"
+	// appmesh_gateway_route.html.markdown:
+	//		"The `grpc_route`, `http_route` and `http2_route` objects supports the following:"
 	// ami.html.markdown: "When `virtualization_type` is "hvm" the following additional arguments apply:"
 	regexp.MustCompile("`([a-z_0-9]+)`.*following"),
 
@@ -866,12 +876,12 @@ var nestedObjectRegexps = []*regexp.Regexp{
 
 	// For example:
 	// sql_database_instance.html.markdown:
-	// "The optional `settings.ip_configuration.authorized_networks[]`` sublist supports:"
+	// 		"The optional `settings.ip_configuration.authorized_networks[]`` sublist supports:"
 	regexp.MustCompile("`([a-zA-Z_.\\[\\]]+)`.*supports:"),
 
 	// For example when blocks/subblocks/sublists are defined across more than one line
 	// sql_database_instance.html.markdown:
-	//"The optional `settings.maintenance_window` subblock for instances declares a one-hour"
+	//		"The optional `settings.maintenance_window` subblock for instances declares a one-hour"
 	regexp.MustCompile("The .* `([a-zA-Z_.\\[\\]]+)` sublist .*"),
 	regexp.MustCompile("The .* `([a-zA-Z_.\\[\\]]+)` subblock .*"),
 	regexp.MustCompile("The .* `([a-zA-Z_.\\[\\]]+)` block .*"),
@@ -905,8 +915,11 @@ func getMultipleNestedBlockNames(match string) []string {
 			blockName = strings.Replace(blockName, " ", "_", -1)
 			blockName = strings.TrimSuffix(blockName, "[]")
 			if subNest != "" {
-				// For the format ""The `grpc_route`, `http_route` and `http2_route` 's `action` object
-				//supports the following:" the result should be grpc_route.action
+				// For the format:
+				//
+				//		The `grpc_route`, `http_route` and `http2_route` 's `action` object supports the following:
+				//
+				// the result should be grpc_route.action
 				blockName = blockName + "." + subNest
 			}
 			nestedBlockNames = append(nestedBlockNames, blockName)
@@ -915,7 +928,13 @@ func getMultipleNestedBlockNames(match string) []string {
 	return nestedBlockNames
 }
 
-func splitMatchOnColon(match string) []string {
+// getNestedNameWithColon handles cases where a property is shown as a subpoperty by means of a colon in the docs.
+// For example:
+//
+//	#### logs_config: s3_logs
+//
+// should return []string{"logs_config.s3_logs"}.
+func getNestedNameWithColon(match string) []string {
 	var nestedBlockNames []string
 	parts := strings.Split(match, ":")
 
@@ -951,7 +970,7 @@ func getNestedBlockNames(line string) []string {
 			break
 		} else if len(matches) >= 2 && i == 2 {
 			// there's a colon in the subheader; split the line
-			nestedBlockNames = splitMatchOnColon(matches[1])
+			nestedBlockNames = getNestedNameWithColon(matches[1])
 			break
 		} else if len(matches) >= 2 {
 			nested = strings.ToLower(matches[1])
@@ -1017,7 +1036,12 @@ func parseArgReferenceSection(subsection []string, ret *entityDocs) {
 
 	for _, line := range subsection {
 		// We have found a new resource on this line.
-		if name, desc, matchFound, isIndented := parseArgFromMarkdownLine(line); matchFound {
+		parsedArg := parseArgFromMarkdownLine(line)
+		name := parsedArg.name
+		desc := parsedArg.desc
+		matchFound := parsedArg.isFound
+		isIndented := parsedArg.isIndented
+		if matchFound {
 			// We have found a new argument.
 			// If a bullet point is indented, we have most likely found a sub-field of the previous line.
 			// See: https://github.com/pulumi/pulumi-terraform-bridge/issues/1875

pkg/tfgen/docs_test.go

@@ -805,11 +805,11 @@ func TestParseArgFromMarkdownLine(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		name, desc, found, indented := parseArgFromMarkdownLine(test.input)
-		assert.Equal(t, test.expectedName, name)
-		assert.Equal(t, test.expectedDesc, desc)
-		assert.Equal(t, test.expectedFound, found)
-		assert.Equal(t, test.expectedIndent, indented)
+		parsedLine := parseArgFromMarkdownLine(test.input)
+		assert.Equal(t, test.expectedName, parsedLine.name)
+		assert.Equal(t, test.expectedDesc, parsedLine.desc)
+		assert.Equal(t, test.expectedFound, parsedLine.isFound)
+		assert.Equal(t, test.expectedIndent, parsedLine.isIndented)
 	}
 }
 
@@ -888,24 +888,24 @@ func TestGetNestedBlockName(t *testing.T) {
 		input    string
 		expected []string
 	}{
-		//{"", []string(nil)},
-		//{"The `website` object supports the following:", []string{"website"}},
-		//{"The `website` and `pages` objects support the following:", []string{"website", "pages"}},
-		//{"The optional `settings.location_preference` subblock supports:", []string{"settings.location_preference"}},
-		//{"The optional `settings.ip_configuration.authorized_networks[]` sublist supports:", []string{"settings.ip_configuration.authorized_networks"}},
-		//{"#### result_configuration Argument Reference", []string{"result_configuration"}},
-		//{"### advanced_security_options", []string{"advanced_security_options"}},
-		//{"### `server_side_encryption`", []string{"server_side_encryption"}},
-		//{"### Failover Routing Policy", []string{"failover_routing_policy"}},
-		//{"##### `log_configuration`", []string{"log_configuration"}},
-		//{"### data_format_conversion_configuration", []string{"data_format_conversion_configuration"}},
+		{"", []string(nil)},
+		{"The `website` object supports the following:", []string{"website"}},
+		{"The `website` and `pages` objects support the following:", []string{"website", "pages"}},
+		{"The optional `settings.location_preference` subblock supports:", []string{"settings.location_preference"}},
+		{"The optional `settings.ip_configuration.authorized_networks[]` sublist supports:", []string{"settings.ip_configuration.authorized_networks"}},
+		{"#### result_configuration Argument Reference", []string{"result_configuration"}},
+		{"### advanced_security_options", []string{"advanced_security_options"}},
+		{"### `server_side_encryption`", []string{"server_side_encryption"}},
+		{"### Failover Routing Policy", []string{"failover_routing_policy"}},
+		{"##### `log_configuration`", []string{"log_configuration"}},
+		{"### data_format_conversion_configuration", []string{"data_format_conversion_configuration"}},
 		{"#### build_batch_config: restrictions", []string{"build_batch_config.restrictions"}},
-		//{"#### logs_config: s3_logs", []string{"logs_config.s3_logs"}},
-		//{"###### S3 Input Format Config", []string{"s3_input_format_config"}},
-		//// This is a common starting line of base arguments, so should result in nil value:
-		//{"The following arguments are supported:", []string(nil)},
-		//{"* `kms_key_id` - ...", []string(nil)},
-		//{"## Import", []string(nil)},
+		{"#### logs_config: s3_logs", []string{"logs_config.s3_logs"}},
+		{"###### S3 Input Format Config", []string{"s3_input_format_config"}},
+		// This is a common starting line of base arguments, so should result in nil value:
+		{"The following arguments are supported:", []string(nil)},
+		{"* `kms_key_id` - ...", []string(nil)},
+		{"## Import", []string(nil)},
 	}
 
 	for _, tt := range tests {

pkg/tfgen/generate.go

@@ -384,7 +384,7 @@ func (g *Generator) makePropertyType(typePath paths.TypePath,
 
 	// Handle single-nested blocks next.
 	if blockType, ok := sch.Elem().(shim.Resource); ok && sch.Type() == shim.TypeMap {
-		return g.makeObjectPropertyType(typePath, docsPath(objectName), blockType, elemInfo, out, entityDocs)
+		return g.makeObjectPropertyType(typePath, blockType, elemInfo, out, entityDocs)
 	}
 
 	// IsMaxItemOne lists and sets are flattened, transforming List[T] to T. Detect if this is the case.
@@ -409,7 +409,7 @@ func (g *Generator) makePropertyType(typePath paths.TypePath,
 	case shim.Schema:
 		element = g.makePropertyType(elemPath, objectName, elem, elemInfo, out, entityDocs)
 	case shim.Resource:
-		element = g.makeObjectPropertyType(elemPath, docsPath(objectName), elem, elemInfo, out, entityDocs)
+		element = g.makeObjectPropertyType(elemPath, elem, elemInfo, out, entityDocs)
 	}
 
 	if flatten {
@@ -436,7 +436,7 @@ func getDocsFromSchemaMap(key string, schemaMap shim.SchemaMap) string {
 }
 
 func (g *Generator) makeObjectPropertyType(typePath paths.TypePath,
-	objPath docsPath, res shim.Resource, info *tfbridge.SchemaInfo,
+	res shim.Resource, info *tfbridge.SchemaInfo,
 	out bool, entityDocs entityDocs) *propertyType {
 	t := &propertyType{
 		kind: kindObject,
@@ -470,7 +470,7 @@ func (g *Generator) makeObjectPropertyType(typePath paths.TypePath,
 		fullDocsPath = strings.TrimSuffix(fullDocsPath, ".")
 	}
 
-	objPath = docsPath(fullDocsPath)
+	objPath := docsPath(fullDocsPath)
 
 	for _, key := range stableSchemas(res.Schema()) {
 		propertySchema := res.Schema()