From 2a2c58ef46cc0942e74d3099a4edd570893f5d59 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 13 May 2020 09:06:17 +0100 Subject: [PATCH] Support checking syntax of generated Blueprint files Adds a CheckBlueprintSyntax(...) method to check the syntax of a Blueprint file. Changes processModuleDef and newModule from being method on *Context to being standalone functions. That ensures that CheckBlueprintSyntax(...) does not need to take a context and so there is no chance that it can change its state. --- context.go | 16 ++++----- module_ctx.go | 47 +++++++++++++++++++++++-- module_ctx_test.go | 86 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 10 deletions(-) diff --git a/context.go b/context.go index 9a3253a..3018209 100644 --- a/context.go +++ b/context.go @@ -692,7 +692,7 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string, var scopedModuleFactories map[string]ModuleFactory var addModule func(module *moduleInfo) []error - addModule = func(module *moduleInfo) ([]error) { + addModule = func(module *moduleInfo) []error { // Run any load hooks immediately before it is sent to the moduleCh and is // registered by name. This allows load hooks to set and/or modify any aspect // of the module (including names) using information that is not available when @@ -716,7 +716,7 @@ func (c *Context) ParseFileList(rootDir string, filePaths []string, for _, def := range file.Defs { switch def := def.(type) { case *parser.Module: - module, errs := c.processModuleDef(def, file.Name, scopedModuleFactories) + module, errs := processModuleDef(def, file.Name, c.moduleFactories, scopedModuleFactories, c.ignoreUnknownModuleTypes) if len(errs) == 0 && module != nil { errs = addModule(module) } @@ -1349,7 +1349,7 @@ func (c *Context) prettyPrintGroupVariants(group *moduleGroup) string { return strings.Join(variants, "\n ") } -func (c *Context) newModule(factory ModuleFactory) *moduleInfo { +func newModule(factory ModuleFactory) *moduleInfo { logicModule, properties := factory() module := &moduleInfo{ @@ -1362,15 +1362,15 @@ func (c *Context) newModule(factory ModuleFactory) *moduleInfo { return module } -func (c *Context) processModuleDef(moduleDef *parser.Module, - relBlueprintsFile string, scopedModuleFactories map[string]ModuleFactory) (*moduleInfo, []error) { +func processModuleDef(moduleDef *parser.Module, + relBlueprintsFile string, moduleFactories, scopedModuleFactories map[string]ModuleFactory, ignoreUnknownModuleTypes bool) (*moduleInfo, []error) { - factory, ok := c.moduleFactories[moduleDef.Type] + factory, ok := moduleFactories[moduleDef.Type] if !ok && scopedModuleFactories != nil { factory, ok = scopedModuleFactories[moduleDef.Type] } if !ok { - if c.ignoreUnknownModuleTypes { + if ignoreUnknownModuleTypes { return nil, nil } @@ -1382,7 +1382,7 @@ func (c *Context) processModuleDef(moduleDef *parser.Module, } } - module := c.newModule(factory) + module := newModule(factory) module.typeName = moduleDef.Type module.relBlueprintsFile = relBlueprintsFile diff --git a/module_ctx.go b/module_ctx.go index 012142e..36e05a4 100644 --- a/module_ctx.go +++ b/module_ctx.go @@ -17,9 +17,11 @@ package blueprint import ( "fmt" "path/filepath" + "strings" "sync" "text/scanner" + "github.com/google/blueprint/parser" "github.com/google/blueprint/pathtools" "github.com/google/blueprint/proptools" ) @@ -988,7 +990,7 @@ func (mctx *mutatorContext) Rename(name string) { } func (mctx *mutatorContext) CreateModule(factory ModuleFactory, props ...interface{}) Module { - module := mctx.context.newModule(factory) + module := newModule(factory) module.relBlueprintsFile = mctx.module.relBlueprintsFile module.pos = mctx.module.pos @@ -1035,7 +1037,7 @@ type LoadHookContext interface { } func (l *loadHookContext) CreateModule(factory ModuleFactory, props ...interface{}) Module { - module := l.context.newModule(factory) + module := newModule(factory) module.relBlueprintsFile = l.module.relBlueprintsFile module.pos = l.module.pos @@ -1123,3 +1125,44 @@ func runAndRemoveLoadHooks(ctx *Context, config interface{}, module *moduleInfo, return nil, nil } + +// Check the syntax of a generated blueprint file. +// +// This is intended to perform a quick sanity check for generated blueprint +// code to ensure that it is syntactically correct, where syntactically correct +// means: +// * No variable definitions. +// * Valid module types. +// * Valid property names. +// * Valid values for the property type. +// +// It does not perform any semantic checking of properties, existence of referenced +// files, or dependencies. +// +// At a low level it: +// * Parses the contents. +// * Invokes relevant factory to create Module instances. +// * Unpacks the properties into the Module. +// * Does not invoke load hooks or any mutators. +// +// The filename is only used for reporting errors. +func CheckBlueprintSyntax(moduleFactories map[string]ModuleFactory, filename string, contents string) []error { + scope := parser.NewScope(nil) + file, errs := parser.Parse(filename, strings.NewReader(contents), scope) + if len(errs) != 0 { + return errs + } + + for _, def := range file.Defs { + switch def := def.(type) { + case *parser.Module: + _, moduleErrs := processModuleDef(def, filename, moduleFactories, nil, false) + errs = append(errs, moduleErrs...) + + default: + panic(fmt.Errorf("unknown definition type: %T", def)) + } + } + + return errs +} diff --git a/module_ctx_test.go b/module_ctx_test.go index e72be8c..e98ae82 100644 --- a/module_ctx_test.go +++ b/module_ctx_test.go @@ -195,3 +195,89 @@ func TestAliases(t *testing.T) { "\n 1:a, 2:a\n 1:a, 2:b\n 1:b, 2:a\n 1:b, 2:b") }) } + +func expectedErrors(t *testing.T, errs []error, expectedMessages ...string) { + t.Helper() + if len(errs) != len(expectedMessages) { + t.Errorf("expected %d error, found: %q", len(expectedMessages), errs) + } else { + for i, expected := range expectedMessages { + err := errs[i] + if err.Error() != expected { + t.Errorf("expected error %q found %q", expected, err) + } + } + } +} + +func TestCheckBlueprintSyntax(t *testing.T) { + factories := map[string]ModuleFactory{ + "test": newModuleCtxTestModule, + } + + t.Run("valid", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test { + name: "test", +} +`) + expectedErrors(t, errs) + }) + + t.Run("syntax error", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test { + name: "test", + +`) + + expectedErrors(t, errs, `path/Blueprint:5:1: expected "}", found EOF`) + }) + + t.Run("unknown module type", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test2 { + name: "test", +} +`) + + expectedErrors(t, errs, `path/Blueprint:2:1: unrecognized module type "test2"`) + }) + + t.Run("unknown property name", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test { + nam: "test", +} +`) + + expectedErrors(t, errs, `path/Blueprint:3:5: unrecognized property "nam"`) + }) + + t.Run("invalid property type", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test { + name: false, +} +`) + + expectedErrors(t, errs, `path/Blueprint:3:8: can't assign bool value to string property "name"`) + }) + + t.Run("multiple failures", func(t *testing.T) { + errs := CheckBlueprintSyntax(factories, "path/Blueprint", ` +test { + name: false, +} + +test2 { + name: false, +} +`) + + expectedErrors(t, errs, + `path/Blueprint:3:8: can't assign bool value to string property "name"`, + `path/Blueprint:6:1: unrecognized module type "test2"`, + ) + }) +}