Add more Build/Rule/Pool params validation.

This change adds two new kinds of checks to params validation.  First, all
BuildParams must have one or more outputs.  Second, any Pool or Rule referenced
must be visible within the Blueprint scope of the caller (e.g. if it's defined
in another Go package then an Import call must have been made).  If either of
these conditions are violated it will result in a panic.

Change-Id: Ibacb42513882d914c94eade23ef17269db5e8730
This commit is contained in:
Jamie Gennis 2014-06-13 18:25:54 -07:00
parent 71bd58a966
commit 48aed8cee0
7 changed files with 157 additions and 60 deletions

View file

@ -756,7 +756,7 @@ func (c *Context) generateSingletonBuildActions(config interface{},
// any Ninja globals and has not called Import() then we won't have an
// entry for it in the pkgs map. If that's the case then the
// singleton's scope's parent should just be nil.
var singletonScope *scope
var singletonScope *basicScope
if pkg := pkgs[singletonPkgPath(info.singleton)]; pkg != nil {
singletonScope = pkg.scope
}

View file

@ -13,7 +13,7 @@ type pkg struct {
fullName string
shortName string
pkgPath string
scope *scope
scope *basicScope
}
var pkgs = map[string]*pkg{}
@ -202,13 +202,16 @@ func (v *staticVariable) fullName(pkgNames map[*pkg]string) string {
func (v *staticVariable) value(interface{}) (*ninjaString, error) {
ninjaStr, err := parseNinjaString(v.pkg_.scope, v.value_)
if err != nil {
err = fmt.Errorf("error parsing variable %s.%s value: %s",
v.pkg_.pkgPath, v.name_, err)
err = fmt.Errorf("error parsing variable %s value: %s", v, err)
panic(err)
}
return ninjaStr, nil
}
func (v *staticVariable) String() string {
return v.pkg_.pkgPath + "." + v.name_
}
type variableFunc struct {
pkg_ *pkg
name_ string
@ -302,14 +305,17 @@ func (v *variableFunc) value(config interface{}) (*ninjaString, error) {
ninjaStr, err := parseNinjaString(v.pkg_.scope, value)
if err != nil {
err = fmt.Errorf("error parsing variable %s.%s value: %s",
v.pkg_.pkgPath, v.name_, err)
err = fmt.Errorf("error parsing variable %s value: %s", v, err)
panic(err)
}
return ninjaStr, nil
}
func (v *variableFunc) String() string {
return v.pkg_.pkgPath + "." + v.name_
}
func validateVariableMethod(name string, methodValue reflect.Value) {
methodType := methodValue.Type()
if methodType.Kind() != reflect.Func {
@ -355,6 +361,10 @@ func (v *argVariable) value(config interface{}) (*ninjaString, error) {
return nil, errVariableIsArg
}
func (v *argVariable) String() string {
return "<arg>:" + v.name_
}
type staticPool struct {
pkg_ *pkg
name_ string
@ -402,12 +412,15 @@ func (p *staticPool) fullName(pkgNames map[*pkg]string) string {
func (p *staticPool) def(config interface{}) (*poolDef, error) {
def, err := parsePoolParams(p.pkg_.scope, &p.params)
if err != nil {
panic(fmt.Errorf("error parsing PoolParams for %s.%s: %s",
p.pkg_.pkgPath, p.name_, err))
panic(fmt.Errorf("error parsing PoolParams for %s: %s", p, err))
}
return def, nil
}
func (p *staticPool) String() string {
return p.pkg_.pkgPath + "." + p.name_
}
type poolFunc struct {
pkg_ *pkg
name_ string
@ -460,18 +473,21 @@ func (p *poolFunc) def(config interface{}) (*poolDef, error) {
}
def, err := parsePoolParams(p.pkg_.scope, &params)
if err != nil {
panic(fmt.Errorf("error parsing PoolParams for %s.%s: %s",
p.pkg_.pkgPath, p.name_, err))
panic(fmt.Errorf("error parsing PoolParams for %s: %s", p, err))
}
return def, nil
}
func (p *poolFunc) String() string {
return p.pkg_.pkgPath + "." + p.name_
}
type staticRule struct {
pkg_ *pkg
name_ string
params RuleParams
argNames map[string]bool
scope_ *scope
scope_ *basicScope
}
// StaticRule returns a Rule whose value does not depend on any configuration
@ -510,7 +526,7 @@ func StaticRule(name string, params RuleParams, argNames ...string) Rule {
argNamesSet[argName] = true
}
ruleScope := (*scope)(nil) // This will get created lazily
ruleScope := (*basicScope)(nil) // This will get created lazily
r := &staticRule{pkg, name, params, argNamesSet, ruleScope}
err = pkg.scope.AddRule(r)
@ -536,16 +552,15 @@ func (r *staticRule) fullName(pkgNames map[*pkg]string) string {
func (r *staticRule) def(interface{}) (*ruleDef, error) {
def, err := parseRuleParams(r.scope(), &r.params)
if err != nil {
panic(fmt.Errorf("error parsing RuleParams for %s.%s: %s",
r.pkg_.pkgPath, r.name_, err))
panic(fmt.Errorf("error parsing RuleParams for %s: %s", r, err))
}
return def, nil
}
func (r *staticRule) scope() *scope {
// We lazily create the scope so that all the global variables get declared
// before the args are created. Otherwise we could incorrectly shadow a
// global variable with an arg variable.
func (r *staticRule) scope() *basicScope {
// We lazily create the scope so that all the package-scoped variables get
// declared before the args are created. Otherwise we could incorrectly
// shadow a package-scoped variable with an arg variable.
if r.scope_ == nil {
r.scope_ = makeRuleScope(r.pkg_.scope, r.argNames)
}
@ -556,12 +571,16 @@ func (r *staticRule) isArg(argName string) bool {
return r.argNames[argName]
}
func (r *staticRule) String() string {
return r.pkg_.pkgPath + "." + r.name_
}
type ruleFunc struct {
pkg_ *pkg
name_ string
paramsFunc func(interface{}) (RuleParams, error)
argNames map[string]bool
scope_ *scope
scope_ *basicScope
}
// RuleFunc returns a Rule whose value is determined by a function that takes a
@ -603,7 +622,7 @@ func RuleFunc(name string, f func(interface{}) (RuleParams, error),
argNamesSet[argName] = true
}
ruleScope := (*scope)(nil) // This will get created lazily
ruleScope := (*basicScope)(nil) // This will get created lazily
r := &ruleFunc{pkg, name, f, argNamesSet, ruleScope}
err = pkg.scope.AddRule(r)
@ -633,13 +652,12 @@ func (r *ruleFunc) def(config interface{}) (*ruleDef, error) {
}
def, err := parseRuleParams(r.scope(), &params)
if err != nil {
panic(fmt.Errorf("error parsing RuleParams for %s.%s: %s",
r.pkg_.pkgPath, r.name_, err))
panic(fmt.Errorf("error parsing RuleParams for %s: %s", r, err))
}
return def, nil
}
func (r *ruleFunc) scope() *scope {
func (r *ruleFunc) scope() *basicScope {
// We lazily create the scope so that all the global variables get declared
// before the args are created. Otherwise we could incorrectly shadow a
// global variable with an arg variable.
@ -653,9 +671,13 @@ func (r *ruleFunc) isArg(argName string) bool {
return r.argNames[argName]
}
func (r *ruleFunc) String() string {
return r.pkg_.pkgPath + "." + r.name_
}
type builtinRule struct {
name_ string
scope_ *scope
scope_ *basicScope
}
func (r *builtinRule) pkg() *pkg {
@ -674,7 +696,7 @@ func (r *builtinRule) def(config interface{}) (*ruleDef, error) {
return nil, errRuleIsBuiltin
}
func (r *builtinRule) scope() *scope {
func (r *builtinRule) scope() *basicScope {
if r.scope_ == nil {
r.scope_ = makeRuleScope(nil, nil)
}
@ -685,6 +707,10 @@ func (r *builtinRule) isArg(argName string) bool {
return false
}
func (r *builtinRule) String() string {
return "<builtin>:" + r.name_
}
// A ModuleType represents a type of module that can be defined in a Blueprints
// file. In order for it to be used when interpreting Blueprints files, a
// ModuleType must first be registered with a Context object via the

View file

@ -84,8 +84,6 @@ func (m *moduleContext) Variable(name, value string) {
func (m *moduleContext) Rule(name string, params RuleParams,
argNames ...string) Rule {
// TODO: Verify that params.Pool is accessible in this module's scope.
r, err := m.scope.AddLocalRule(name, &params, argNames...)
if err != nil {
panic(err)
@ -97,8 +95,6 @@ func (m *moduleContext) Rule(name string, params RuleParams,
}
func (m *moduleContext) Build(params BuildParams) {
// TODO: Verify that params.Rule is accessible in this module's scope.
def, err := parseBuildParams(m.scope, &params)
if err != nil {
panic(err)

View file

@ -1,6 +1,7 @@
package blueprint
import (
"errors"
"fmt"
"strconv"
"strings"
@ -72,7 +73,7 @@ type poolDef struct {
Depth int
}
func parsePoolParams(scope variableLookup, params *PoolParams) (*poolDef,
func parsePoolParams(scope scope, params *PoolParams) (*poolDef,
error) {
def := &poolDef{
@ -107,7 +108,7 @@ type ruleDef struct {
Variables map[string]*ninjaString
}
func parseRuleParams(scope variableLookup, params *RuleParams) (*ruleDef,
func parseRuleParams(scope scope, params *RuleParams) (*ruleDef,
error) {
r := &ruleDef{
@ -121,6 +122,10 @@ func parseRuleParams(scope variableLookup, params *RuleParams) (*ruleDef,
"specified")
}
if r.Pool != nil && !scope.IsPoolVisible(r.Pool) {
return nil, fmt.Errorf("Pool %s is not visible in this scope", r.Pool)
}
value, err := parseNinjaString(scope, params.Command)
if err != nil {
return nil, fmt.Errorf("error parsing Command param: %s", err)
@ -217,7 +222,7 @@ type buildDef struct {
Args map[Variable]*ninjaString
}
func parseBuildParams(scope variableLookup, params *BuildParams) (*buildDef,
func parseBuildParams(scope scope, params *BuildParams) (*buildDef,
error) {
rule := params.Rule
@ -226,6 +231,14 @@ func parseBuildParams(scope variableLookup, params *BuildParams) (*buildDef,
Rule: rule,
}
if !scope.IsRuleVisible(rule) {
return nil, fmt.Errorf("Rule %s is not visible in this scope", rule)
}
if len(params.Outputs) == 0 {
return nil, errors.New("Outputs param has no elements")
}
var err error
b.Outputs, err = parseNinjaStrings(scope, params.Outputs)
if err != nil {

View file

@ -24,8 +24,10 @@ type ninjaString struct {
variables []Variable
}
type variableLookup interface {
type scope interface {
LookupVariable(name string) (Variable, error)
IsRuleVisible(rule Rule) bool
IsPoolVisible(pool Pool) bool
}
func simpleNinjaString(str string) *ninjaString {
@ -37,7 +39,7 @@ func simpleNinjaString(str string) *ninjaString {
// parseNinjaString parses an unescaped ninja string (i.e. all $<something>
// occurrences are expected to be variables or $$) and returns a list of the
// variable names that the string references.
func parseNinjaString(scope variableLookup, str string) (*ninjaString, error) {
func parseNinjaString(scope scope, str string) (*ninjaString, error) {
type stateFunc func(int, rune) (stateFunc, error)
var (
stringState stateFunc
@ -194,7 +196,7 @@ func parseNinjaString(scope variableLookup, str string) (*ninjaString, error) {
return &result, nil
}
func parseNinjaStrings(scope variableLookup, strs []string) ([]*ninjaString,
func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
error) {
if len(strs) == 0 {

View file

@ -13,8 +13,9 @@ import (
type Variable interface {
pkg() *pkg
name() string // "foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path/to/pkg.foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path.to.pkg.foo"
value(config interface{}) (*ninjaString, error)
String() string
}
// A Pool represents a Ninja pool that will be written to the output .ninja
@ -22,8 +23,9 @@ type Variable interface {
type Pool interface {
pkg() *pkg
name() string // "foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path/to/pkg.foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path.to.pkg.foo"
def(config interface{}) (*poolDef, error)
String() string
}
// A Rule represents a Ninja build rule that will be written to the output
@ -31,31 +33,32 @@ type Pool interface {
type Rule interface {
pkg() *pkg
name() string // "foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path/to/pkg.foo"
fullName(pkgNames map[*pkg]string) string // "pkg.foo" or "path.to.pkg.foo"
def(config interface{}) (*ruleDef, error)
scope() *scope
scope() *basicScope
isArg(argName string) bool
String() string
}
type scope struct {
parent *scope
type basicScope struct {
parent *basicScope
variables map[string]Variable
pools map[string]Pool
rules map[string]Rule
imports map[string]*scope
imports map[string]*basicScope
}
func newScope(parent *scope) *scope {
return &scope{
func newScope(parent *basicScope) *basicScope {
return &basicScope{
parent: parent,
variables: make(map[string]Variable),
pools: make(map[string]Pool),
rules: make(map[string]Rule),
imports: make(map[string]*scope),
imports: make(map[string]*basicScope),
}
}
func makeRuleScope(parent *scope, argNames map[string]bool) *scope {
func makeRuleScope(parent *basicScope, argNames map[string]bool) *basicScope {
scope := newScope(parent)
for argName := range argNames {
_, err := scope.LookupVariable(argName)
@ -83,7 +86,7 @@ func makeRuleScope(parent *scope, argNames map[string]bool) *scope {
return scope
}
func (s *scope) LookupVariable(name string) (Variable, error) {
func (s *basicScope) LookupVariable(name string) (Variable, error) {
dotIndex := strings.IndexRune(name, '.')
if dotIndex >= 0 {
// The variable name looks like "pkg.var"
@ -127,7 +130,52 @@ func (s *scope) LookupVariable(name string) (Variable, error) {
}
}
func (s *scope) lookupImportedScope(pkgName string) (*scope, error) {
func (s *basicScope) IsRuleVisible(rule Rule) bool {
_, isBuiltin := rule.(*builtinRule)
if isBuiltin {
return true
}
name := rule.name()
for s != nil {
if s.rules[name] == rule {
return true
}
for _, import_ := range s.imports {
if import_.rules[name] == rule {
return true
}
}
s = s.parent
}
return false
}
func (s *basicScope) IsPoolVisible(pool Pool) bool {
name := pool.name()
for s != nil {
if s.pools[name] == pool {
return true
}
for _, import_ := range s.imports {
if import_.pools[name] == pool {
return true
}
}
s = s.parent
}
return false
}
func (s *basicScope) lookupImportedScope(pkgName string) (*basicScope, error) {
for ; s != nil; s = s.parent {
importedScope, ok := s.imports[pkgName]
if ok {
@ -138,7 +186,7 @@ func (s *scope) lookupImportedScope(pkgName string) (*scope, error) {
"blueprint.Import()?)", pkgName)
}
func (s *scope) AddImport(name string, importedScope *scope) error {
func (s *basicScope) AddImport(name string, importedScope *basicScope) error {
_, present := s.imports[name]
if present {
return fmt.Errorf("import %q is already defined in this scope", name)
@ -147,7 +195,7 @@ func (s *scope) AddImport(name string, importedScope *scope) error {
return nil
}
func (s *scope) AddVariable(v Variable) error {
func (s *basicScope) AddVariable(v Variable) error {
name := v.name()
_, present := s.variables[name]
if present {
@ -157,7 +205,7 @@ func (s *scope) AddVariable(v Variable) error {
return nil
}
func (s *scope) AddPool(p Pool) error {
func (s *basicScope) AddPool(p Pool) error {
name := p.name()
_, present := s.pools[name]
if present {
@ -167,7 +215,7 @@ func (s *scope) AddPool(p Pool) error {
return nil
}
func (s *scope) AddRule(r Rule) error {
func (s *basicScope) AddRule(r Rule) error {
name := r.name()
_, present := s.rules[name]
if present {
@ -179,10 +227,10 @@ func (s *scope) AddRule(r Rule) error {
type localScope struct {
namePrefix string
scope *scope
scope *basicScope
}
func newLocalScope(parent *scope, namePrefix string) *localScope {
func newLocalScope(parent *basicScope, namePrefix string) *localScope {
return &localScope{
namePrefix: namePrefix,
scope: newScope(parent),
@ -193,6 +241,14 @@ func (s *localScope) LookupVariable(name string) (Variable, error) {
return s.scope.LookupVariable(name)
}
func (s *localScope) IsRuleVisible(rule Rule) bool {
return s.scope.IsRuleVisible(rule)
}
func (s *localScope) IsPoolVisible(pool Pool) bool {
return s.scope.IsPoolVisible(pool)
}
func (s *localScope) AddLocalVariable(name, value string) (*localVariable,
error) {
@ -287,12 +343,16 @@ func (l *localVariable) value(interface{}) (*ninjaString, error) {
return l.value_, nil
}
func (l *localVariable) String() string {
return "<local var>:" + l.namePrefix + l.name_
}
type localRule struct {
namePrefix string
name_ string
def_ *ruleDef
argNames map[string]bool
scope_ *scope
scope_ *basicScope
}
func (l *localRule) pkg() *pkg {
@ -311,10 +371,14 @@ func (l *localRule) def(interface{}) (*ruleDef, error) {
return l.def_, nil
}
func (r *localRule) scope() *scope {
func (r *localRule) scope() *basicScope {
return r.scope_
}
func (r *localRule) isArg(argName string) bool {
return r.argNames[argName]
}
func (r *localRule) String() string {
return "<local rule>:" + r.namePrefix + r.name_
}

View file

@ -92,8 +92,6 @@ func (s *singletonContext) Variable(name, value string) {
func (s *singletonContext) Rule(name string, params RuleParams,
argNames ...string) Rule {
// TODO: Verify that params.Pool is accessible in this module's scope.
r, err := s.scope.AddLocalRule(name, &params, argNames...)
if err != nil {
panic(err)
@ -105,8 +103,6 @@ func (s *singletonContext) Rule(name string, params RuleParams,
}
func (s *singletonContext) Build(params BuildParams) {
// TODO: Verify that params.Rule is accessible in this module's scope.
def, err := parseBuildParams(s.scope, &params)
if err != nil {
panic(err)