Config transformers and filters now error if no matches

hack/generator/azure-arm.yaml

@@ -32,10 +32,6 @@ typeFilters:
     group: microsoft.compute
     name: VirtualMachineScaleSetsExtensionsChildResource;VirtualMachinesExtensionsChildResource
     because: _childResource type which "points" to a resource type that doesn't exist. Need to work with schema owners on why.
-  - action: prune
-    group: microsoft.automation
-    name: AutomationAccountsRunbooksDraftChildResource
-    because: Uses OneOf in a weird way that makes detecting actual resource difficult, skipping for now.
   - action: prune
     group: microsoft.web
     version: '*2018*'
@@ -76,10 +72,6 @@ typeFilters:
     because: Some types use OneOf to model inheritance, which we don't currently support.
 exportFilters:
   # First some always-applicable exclusions:
-  - action: exclude
-    group: definitions
-    name: NumberOrExpression
-    because: this type is not really used, it's just float64. We remap this type to float64 in typetransformers so we can skip exporting it too
   - action: exclude
     group: definitions
     name: '*ResourceBase*'
@@ -184,11 +176,11 @@ typeTransformers:
   - name: "*"
     property: Tags
     ifType: 
-        map:
-          key:
-            name: string
-          value:
-            name: any
+      map:
+        key:
+          name: string
+        value:
+          name: any
     target:
       map:
         key: 

hack/generator/azure-crossplane.yaml

@@ -30,10 +30,6 @@ typeFilters:
     version: '*'
     because: Prune everything else
 exportFilters:
-  - action: exclude
-    group: definitions
-    name: NumberOrExpression
-    because: this type is not really used, it's just float64. We remap this type to float64 in typetransformers so we can skip exporting it too
   - action: exclude
     group: definitions
     name: '*ResourceBase*'
@@ -79,6 +75,7 @@ typeTransformers:
     because: NumberOrExpression is an ARM template artifact that doesn't make sense in CRDs
     target:
       name: float
+    matchRequired: false  # TODO: Remove this if/when we actually require it
   - name: "*"
     property: Tags
     ifType:
@@ -147,11 +144,6 @@ typeTransformers:
       optional: true
     remove: true
     because: It exists on ARMResource but doesn't make sense in the context of a CRD
-  - group: deploymenttemplate
-    name: ResourceLocations
-    target:
-      name: string
-    because: Modeling this as an enum doesn't work well in the context of CRDs because new regions are regularly added
 
   # Deal with readonly properties that were not properly pruned in the JSON schema
   - name: ResourceIdentity

hack/generator/pkg/codegen/pipeline_apply_export_filters.go

@@ -31,10 +31,7 @@ func filterTypes(
 
 	newDefinitions := make(astmodel.Types)
 
-	filterer, err := configuration.BuildExportFilterer(definitions)
-	if err != nil {
-		return nil, err
-	}
+	filterer := configuration.BuildExportFilterer(definitions)
 
 	for _, def := range definitions {
 		defName := def.Name()
@@ -57,5 +54,11 @@ func filterTypes(
 		}
 	}
 
+	// Ensure that the export filters had no issues
+	err := configuration.GetExportFiltersError()
+	if err != nil {
+		return nil, err
+	}
+
 	return newDefinitions, nil
 }

hack/generator/pkg/codegen/pipeline_apply_property_rewrites.go

@@ -42,6 +42,12 @@ func applyPropertyRewrites(config *config.Configuration) PipelineStage {
 				newTypes.Add(t.WithType(objectType))
 			}
 
+			// Ensure that the property transformers had no errors
+			err := config.GetPropertyTransformersError()
+			if err != nil {
+				return nil, err
+			}
+
 			return newTypes, nil
 		})
 }

hack/generator/pkg/codegen/pipeline_load_schema.go

@@ -159,6 +159,17 @@ func loadSchemaIntoTypes(
 				return nil, errors.Wrapf(err, "failed to walk JSON schema")
 			}
 
+			// Ensure that the type filters/transformers that are applied during schema graph walking
+			// are checked for errors before proceeding. These are the TypeTransformers and TypeFilters
+			err = configuration.GetTypeFiltersError()
+			if err != nil {
+				return nil, err
+			}
+			err = configuration.GetTypeTransformersError()
+			if err != nil {
+				return nil, err
+			}
+
 			return defs, nil
 		})
 }

hack/generator/pkg/config/configuration.go

@@ -84,6 +84,46 @@ func (config *Configuration) FullTypesRegistrationOutputFilePath() string {
 		config.TypeRegistrationOutputFile)
 }
 
+func (config *Configuration) GetTypeFiltersError() error {
+	for _, filter := range config.TypeFilters {
+		if !filter.MatchedRequiredTypes() {
+			return errors.Errorf("Type filter action: %q, target: %q matched no types", filter.Action, filter.String())
+		}
+	}
+
+	return nil
+}
+
+func (config *Configuration) GetTypeTransformersError() error {
+	for _, filter := range config.typeTransformers {
+		if !filter.MatchedRequiredTypes() {
+			return errors.Errorf("Type transformer target: %q matched no types", filter.String())
+		}
+	}
+
+	return nil
+}
+
+func (config *Configuration) GetPropertyTransformersError() error {
+	for _, filter := range config.propertyTransformers {
+		if !filter.MatchedRequiredTypes() {
+			return errors.Errorf("Type transformer target: %q for property %q matched no types", filter.String(), filter.Property)
+		}
+	}
+
+	return nil
+}
+
+func (config *Configuration) GetExportFiltersError() error {
+	for _, filter := range config.ExportFilters {
+		if !filter.MatchedRequiredTypes() {
+			return errors.Errorf("Export filter action: %q, target: %q matched no types", filter.Action, filter.String())
+		}
+	}
+
+	return nil
+}
+
 // NewConfiguration returns a new empty Configuration
 func NewConfiguration() *Configuration {
 	return &Configuration{}
@@ -267,7 +307,7 @@ func absDirectoryPathToURL(path string) *url.URL {
 
 type ExportFilterFunc func(astmodel.TypeName) (result ShouldExportResult, because string)
 
-func buildExportFilterFunc(f *ExportFilter, allTypes astmodel.Types) (ExportFilterFunc, error) {
+func buildExportFilterFunc(f *ExportFilter, allTypes astmodel.Types) ExportFilterFunc {
 	switch f.Action {
 	case ExportFilterExclude:
 		return func(typeName astmodel.TypeName) (ShouldExportResult, string) {
@@ -276,7 +316,7 @@ func buildExportFilterFunc(f *ExportFilter, allTypes astmodel.Types) (ExportFilt
 			}
 
 			return "", ""
-		}, nil
+		}
 
 	case ExportFilterInclude:
 		return func(typeName astmodel.TypeName) (ShouldExportResult, string) {
@@ -285,29 +325,23 @@ func buildExportFilterFunc(f *ExportFilter, allTypes astmodel.Types) (ExportFilt
 			}
 
 			return "", ""
-		}, nil
+		}
 
 	case ExportFilterIncludeTransitive:
 		applicableTypes := make(astmodel.TypeNameSet)
-		foundMatch := false
 		for tn := range allTypes {
 			if f.AppliesToType(tn) {
-				foundMatch = true
 				collectAllReferencedTypes(allTypes, tn, applicableTypes)
 			}
 		}
 
-		if !foundMatch {
-			return nil, errors.Errorf("no types matched for include-transitive filter: %v", f)
-		}
-
 		return func(typeName astmodel.TypeName) (ShouldExportResult, string) {
 			if applicableTypes.Contains(typeName) {
 				return Export, f.Because
 			}
 
 			return "", ""
-		}, nil
+		}
 
 	default:
 		panic(errors.Errorf("unknown exportfilter directive: %s", f.Action))
@@ -325,13 +359,10 @@ func collectAllReferencedTypes(allTypes astmodel.Types, root astmodel.TypeName,
 
 // BuildExportFilterer tests for whether a given type should be exported as Go code
 // Returns a result indicating whether export should occur as well as a reason for logging
-func (config *Configuration) BuildExportFilterer(allTypes astmodel.Types) (ExportFilterFunc, error) {
+func (config *Configuration) BuildExportFilterer(allTypes astmodel.Types) ExportFilterFunc {
 	var filters []ExportFilterFunc
 	for _, f := range config.ExportFilters {
-		filter, err := buildExportFilterFunc(f, allTypes)
-		if err != nil {
-			return nil, errors.Wrap(err, "building export filter func")
-		}
+		filter := buildExportFilterFunc(f, allTypes)
 		filters = append(filters, filter)
 	}
 
@@ -345,7 +376,7 @@ func (config *Configuration) BuildExportFilterer(allTypes astmodel.Types) (Expor
 
 		// By default we export all types
 		return Export, ""
-	}, nil
+	}
 }
 
 // ShouldPrune tests for whether a given type should be extracted from the JSON schema or pruned

hack/generator/pkg/config/configuration_test.go

@@ -42,8 +42,7 @@ func Test_WithSingleFilter_FiltersExpectedTypes(t *testing.T) {
 	c := config.NewConfiguration()
 	c = c.WithExportFilters(&filter)
 
-	f, err := c.BuildExportFilterer(make(astmodel.Types))
-	g.Expect(err).ToNot(HaveOccurred())
+	f := c.BuildExportFilterer(make(astmodel.Types))
 
 	g.Expect(f(person)).To(Equal(config.Export))
 	g.Expect(f(post)).To(Equal(config.Export))
@@ -68,8 +67,7 @@ func Test_WithMultipleFilters_FiltersExpectedTypes(t *testing.T) {
 	c := config.NewConfiguration()
 	c = c.WithExportFilters(&versionFilter, &nameFilter)
 
-	f, err := c.BuildExportFilterer(make(astmodel.Types))
-	g.Expect(err).ToNot(HaveOccurred())
+	f := c.BuildExportFilterer(make(astmodel.Types))
 
 	g.Expect(f(person)).To(Equal(config.Export))
 	g.Expect(f(post)).To(Equal(config.Export))
@@ -89,8 +87,7 @@ func Test_WithMultipleFilters_GivesPrecedenceToEarlierFilters(t *testing.T) {
 	c := config.NewConfiguration()
 	c = c.WithExportFilters(&alwaysExportPerson, &exclude2019)
 
-	f, err := c.BuildExportFilterer(make(astmodel.Types))
-	g.Expect(err).ToNot(HaveOccurred())
+	f := c.BuildExportFilterer(make(astmodel.Types))
 
 	g.Expect(f(person2019TypeName)).To(Equal(config.Export))
 	g.Expect(f(student2019TypeName)).To(Equal(config.Skip))
@@ -118,8 +115,7 @@ func Test_IncludeTransitive(t *testing.T) {
 	types.Add(astmodel.MakeTypeDefinition(person2019TypeName, astmodel.NewArrayType(student2019TypeName)))
 	types.Add(astmodel.MakeTypeDefinition(student2019TypeName, astmodel.StringType))
 
-	f, err := c.BuildExportFilterer(types)
-	g.Expect(err).ToNot(HaveOccurred())
+	f := c.BuildExportFilterer(types)
 
 	// Person is top-level:
 	g.Expect(f(person2019TypeName)).To(Equal(config.Export))

hack/generator/pkg/config/type_matcher.go

@@ -26,32 +26,44 @@ type TypeMatcher struct {
 	nameRegex *regexp.Regexp
 	// Because is used to articulate why the filter applied to a type (used to generate explanatory logs in debug mode)
 	Because string
+	// MatchRequired indicates if an error will be raised if this TypeMatcher doesn't match at least one type.
+	// The default is true.
+	MatchRequired *bool `yaml:"matchRequired,omitempty"`
+
+	// matchedTypes is the set of all type names matched by this matcher
+	matchedTypes astmodel.TypeNameSet
 }
 
 var _ fmt.Stringer = &TypeMatcher{}
 
 // Initialize initializes the type matcher
-func (typeMatcher *TypeMatcher) Initialize() error {
-	typeMatcher.groupRegex = createGlobbingRegex(typeMatcher.Group)
-	typeMatcher.versionRegex = createGlobbingRegex(typeMatcher.Version)
-	typeMatcher.nameRegex = createGlobbingRegex(typeMatcher.Name)
+func (t *TypeMatcher) Initialize() error {
+	t.groupRegex = createGlobbingRegex(t.Group)
+	t.versionRegex = createGlobbingRegex(t.Version)
+	t.nameRegex = createGlobbingRegex(t.Name)
+	t.matchedTypes = make(astmodel.TypeNameSet)
+	// Default MatchRequired
+	if t.MatchRequired == nil {
+		temp := true
+		t.MatchRequired = &temp
+	}
 
 	return nil
 }
 
-func (typeMatcher *TypeMatcher) groupMatches(schema string) bool {
-	return typeMatcher.matches(typeMatcher.Group, &typeMatcher.groupRegex, schema)
+func (t *TypeMatcher) groupMatches(schema string) bool {
+	return t.matches(t.Group, &t.groupRegex, schema)
 }
 
-func (typeMatcher *TypeMatcher) versionMatches(version string) bool {
-	return typeMatcher.matches(typeMatcher.Version, &typeMatcher.versionRegex, version)
+func (t *TypeMatcher) versionMatches(version string) bool {
+	return t.matches(t.Version, &t.versionRegex, version)
 }
 
-func (typeMatcher *TypeMatcher) nameMatches(name string) bool {
-	return typeMatcher.matches(typeMatcher.Name, &typeMatcher.nameRegex, name)
+func (t *TypeMatcher) nameMatches(name string) bool {
+	return t.matches(t.Name, &t.nameRegex, name)
 }
 
-func (typeMatcher *TypeMatcher) matches(glob string, regex **regexp.Regexp, name string) bool {
+func (t *TypeMatcher) matches(glob string, regex **regexp.Regexp, name string) bool {
 	if glob == "" {
 		return true
 	}
@@ -64,14 +76,19 @@ func (typeMatcher *TypeMatcher) matches(glob string, regex **regexp.Regexp, name
 }
 
 // AppliesToType indicates whether this filter should be applied to the supplied type definition
-func (typeMatcher *TypeMatcher) AppliesToType(typeName astmodel.TypeName) bool {
+func (t *TypeMatcher) AppliesToType(typeName astmodel.TypeName) bool {
 	if localRef, ok := typeName.PackageReference.AsLocalPackage(); ok {
 		group := localRef.Group()
 		version := localRef.Version()
 
-		result := typeMatcher.groupMatches(group) &&
-			typeMatcher.versionMatches(version) &&
-			typeMatcher.nameMatches(typeName.Name())
+		result := t.groupMatches(group) &&
+			t.versionMatches(version) &&
+			t.nameMatches(typeName.Name())
+
+		// Track this match so we can later report if we didn't match anything
+		if result {
+			t.matchedTypes = t.matchedTypes.Add(typeName)
+		}
 
 		return result
 	}
@@ -80,6 +97,19 @@ func (typeMatcher *TypeMatcher) AppliesToType(typeName astmodel.TypeName) bool {
 	return false
 }
 
+func (t *TypeMatcher) MatchedRequiredTypes() bool {
+	if *t.MatchRequired {
+		return t.HasMatches()
+	}
+
+	return true
+}
+
+// HasMatches returns true if this matcher has ever matched at least 1 type
+func (t *TypeMatcher) HasMatches() bool {
+	return len(t.matchedTypes) > 0
+}
+
 // String returns a description of this filter
 func (t *TypeMatcher) String() string {
 	var result strings.Builder