fix: Fix missing builtins in REPL

The REPL's use of the interpreter was causing it to not get the builtins
defined in builtin scripts. For example the `top` function was missing.

This change fixes the issues by ensuring the builtins are only evaluated
once and that there is only one way to get the query Interpreter that is
guaranteed to have the proper builtins.

query/compile.go

@@ -68,10 +68,8 @@ func Eval(itrp *interpreter.Interpreter, q string) error {
 		return err
 	}
 
-	_, decls := builtIns(itrp)
-
 	// Convert AST program to a semantic program
-	semProg, err := semantic.New(astProg, decls)
+	semProg, err := semantic.New(astProg, builtinDeclarations.Copy())
 	if err != nil {
 		return err
 	}
@@ -85,18 +83,13 @@ func Eval(itrp *interpreter.Interpreter, q string) error {
 // NewInterpreter returns an interpreter instance with
 // pre-constructed options and global scopes.
 func NewInterpreter() *interpreter.Interpreter {
+	// Make a copy of the builtin options since they can be modified
 	options := make(map[string]values.Value, len(builtinOptions))
-	globals := make(map[string]values.Value, len(builtinScope))
-
-	for k, v := range builtinScope {
-		globals[k] = v
-	}
-
 	for k, v := range builtinOptions {
 		options[k] = v
 	}
 
-	return interpreter.NewInterpreter(options, globals)
+	return interpreter.NewInterpreter(options, builtinValues)
 }
 
 func nowFunc(now time.Time) values.Function {
@@ -151,17 +144,12 @@ func ToSpec(itrp *interpreter.Interpreter, vals ...values.Value) *Spec {
 
 type CreateOperationSpec func(args Arguments, a *Administration) (OperationSpec, error)
 
-var builtinScope = make(map[string]values.Value)
-
-// TODO(Josh): Default option values should be registered similarly to built-in
-// functions. Default options should be registered in their own files
-// (or in a single file) using the RegisterBuiltInOption function which will
-// place the resolved option value in the following map.
+var builtinValues = make(map[string]values.Value)
 var builtinOptions = make(map[string]values.Value)
 var builtinDeclarations = make(semantic.DeclarationScope)
 
 // list of builtin scripts
-var builtins = make(map[string]string)
+var builtinScripts = make(map[string]string)
 var finalized bool
 
 // RegisterBuiltIn adds any variable declarations in the script to the builtin scope.
@@ -169,7 +157,7 @@ func RegisterBuiltIn(name, script string) {
 	if finalized {
 		panic(errors.New("already finalized, cannot register builtin"))
 	}
-	builtins[name] = script
+	builtinScripts[name] = script
 }
 
 // RegisterFunction adds a new builtin top level function.
@@ -200,11 +188,11 @@ func RegisterBuiltInValue(name string, v values.Value) {
 	if finalized {
 		panic(errors.New("already finalized, cannot register builtin"))
 	}
-	if _, ok := builtinScope[name]; ok {
+	if _, ok := builtinValues[name]; ok {
 		panic(fmt.Errorf("duplicate registration for builtin %q", name))
 	}
 	builtinDeclarations[name] = semantic.NewExternalVariableDeclaration(name, v.Type())
-	builtinScope[name] = v
+	builtinValues[name] = v
 }
 
 // RegisterBuiltInOption adds the value to the builtin scope.
@@ -225,9 +213,30 @@ func FinalizeBuiltIns() {
 		panic("already finalized")
 	}
 	finalized = true
-	// Call BuiltIns to validate all built-in values are valid.
-	// A panic will occur if any value is invalid.
-	_, _ = BuiltIns()
+
+	err := evalBuiltInScripts()
+	if err != nil {
+		panic(err)
+	}
+}
+
+func evalBuiltInScripts() error {
+	itrp := interpreter.NewMutableInterpreter(builtinOptions, builtinValues)
+	for name, script := range builtinScripts {
+		astProg, err := parser.NewAST(script)
+		if err != nil {
+			return errors.Wrapf(err, "failed to parse builtin %q", name)
+		}
+		semProg, err := semantic.New(astProg, builtinDeclarations)
+		if err != nil {
+			return errors.Wrapf(err, "failed to create semantic graph for builtin %q", name)
+		}
+
+		if err := itrp.Eval(semProg); err != nil {
+			return errors.Wrapf(err, "failed to evaluate builtin %q", name)
+		}
+	}
+	return nil
 }
 
 var TableObjectType = semantic.NewObjectType(map[string]semantic.Type{
@@ -441,27 +450,7 @@ func BuiltIns() (map[string]values.Value, semantic.DeclarationScope) {
 	if !finalized {
 		panic("builtins not finalized")
 	}
-	return builtIns(NewInterpreter())
-}
-
-func builtIns(itrp *interpreter.Interpreter) (map[string]values.Value, semantic.DeclarationScope) {
-	decls := builtinDeclarations.Copy()
-
-	for name, script := range builtins {
-		astProg, err := parser.NewAST(script)
-		if err != nil {
-			panic(errors.Wrapf(err, "failed to parse builtin %q", name))
-		}
-		semProg, err := semantic.New(astProg, decls)
-		if err != nil {
-			panic(errors.Wrapf(err, "failed to create semantic graph for builtin %q", name))
-		}
-
-		if err := itrp.Eval(semProg); err != nil {
-			panic(errors.Wrapf(err, "failed to evaluate builtin %q", name))
-		}
-	}
-	return itrp.GlobalScope().Values(), decls
+	return builtinValues, builtinDeclarations
 }
 
 type Administration struct {

query/functions/testdata/top.flux

@@ -0,0 +1,3 @@
+from(db: "test")
+    |> range(start:2018-05-22T19:53:24.421470485Z)
+    |> top(n:2)

query/functions/testdata/top.in.csv

@@ -0,0 +1,16 @@
+#datatype,string,long,dateTime:RFC3339,dateTime:RFC3339,dateTime:RFC3339,string,string
+#group,false,false,false,false,false,false,true
+#default,_result,,,,,,
+,result,table,_start,_stop,_time,_value,_field
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:54:16Z,20,used_percent
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:56Z,55,used_percent
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:54:06Z,20,used_percent
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:26Z,35,used_percent
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:46Z,70,used_percent
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:36Z,15,used_percent
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:54:16Z,20000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:56Z,55000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:54:06Z,20000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:26Z,35000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:46Z,70000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:36Z,15000,used

query/functions/testdata/top.out.csv

@@ -0,0 +1,8 @@
+#datatype,string,long,dateTime:RFC3339,dateTime:RFC3339,dateTime:RFC3339,string,string
+#group,false,false,false,false,false,false,true
+#default,_result,,,,,,
+,result,table,_start,_stop,_time,_value,_field
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:46Z,70000,used
+,,0,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:56Z,55000,used
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:46Z,70,used_percent
+,,1,2018-05-22T19:53:24.421470485Z,2018-05-22T19:54:24.421470485Z,2018-05-22T19:53:56Z,55,used_percent

query/interpreter/interpreter.go

@@ -17,13 +17,27 @@ type Interpreter struct {
 	globals *Scope
 }
 
-// NewInterpreter instantiates a new Flux Interpreter
+// NewInterpreter instantiates a new Flux Interpreter whose builtin values are not mutable.
+// Options are always mutable.
 func NewInterpreter(options, builtins map[string]values.Value) *Interpreter {
 	optionScope := NewScopeWithValues(options)
 	globalScope := optionScope.NestWithValues(builtins)
-	interpreter := new(Interpreter)
-	interpreter.options = optionScope
-	interpreter.globals = globalScope
+	interpreter := &Interpreter{
+		options: optionScope,
+		globals: globalScope.Nest(),
+	}
+	return interpreter
+}
+
+// NewMutableInterpreter instantiates a new Flux Interpreter whose builtin values are mutable.
+// Options are always mutable.
+func NewMutableInterpreter(options, builtins map[string]values.Value) *Interpreter {
+	optionScope := NewScopeWithValues(options)
+	globalScope := optionScope.NestWithValues(builtins)
+	interpreter := &Interpreter{
+		options: optionScope,
+		globals: globalScope,
+	}
 	return interpreter
 }
 
@@ -385,6 +399,7 @@ func (itrp *Interpreter) doArguments(args *semantic.ObjectExpression, scope *Sco
 		if _, ok := obj.Get(p.Key.Name); ok {
 			return nil, fmt.Errorf("duplicate keyword parameter specified: %q", p.Key.Name)
 		}
+
 		obj.Set(p.Key.Name, value)
 	}
 	return obj, nil
@@ -402,13 +417,12 @@ func NewScope() *Scope {
 		values: make(map[string]values.Value),
 	}
 }
+
+// NewScopeWithValues creates a new scope with the initial set of values.
+// The vals map will be mutated.
 func NewScopeWithValues(vals map[string]values.Value) *Scope {
-	cp := make(map[string]values.Value, len(vals))
-	for k, v := range vals {
-		cp[k] = v
-	}
 	return &Scope{
-		values: cp,
+		values: vals,
 	}
 }
 

query/semantic/types.go

@@ -201,7 +201,11 @@ func (t *objectType) Kind() Kind {
 	return Object
 }
 func (t *objectType) PropertyType(name string) Type {
-	return t.properties[name]
+	typ, ok := t.properties[name]
+	if ok {
+		return typ
+	}
+	return Invalid
 }
 func (t *objectType) Properties() map[string]Type {
 	return t.properties