From 82f949e7fe3dd5763dc69d8157895ff171a5b300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanis=C5=82aw=20Barzowski?= Date: Fri, 26 Jul 2019 16:28:34 +0200 Subject: [PATCH] Extract AST processing to separate packages Making it independent from the jsonnet package breaks the circular dependency during stdast generation. --- BUILD.bazel | 5 +-- README.md | 5 +++ ast/clone.go | 1 + builtins.go | 7 +--- c-bindings/BUILD.bazel | 1 + cmd/dumpstdlibast/BUILD.bazel | 4 +-- cmd/dumpstdlibast/dumpstdlibast.go | 6 ++-- imports.go | 2 +- {dump => internal/dump}/BUILD.bazel | 2 +- {dump => internal/dump}/README.md | 0 {dump => internal/dump}/dump.go | 0 {dump => internal/dump}/dump_test.go | 0 {dump => internal/dump}/pointermap.go | 0 {dump => internal/dump}/utils.go | 0 internal/transformations/BUILD.bazel | 26 +++++++++++++++ .../transformations/desugarer.go | 33 ++++++++++--------- .../transformations/desugarer_test.go | 2 +- .../transformations/static_analyzer.go | 7 ++-- .../transformations/static_analyzer_test.go | 6 ++-- internal/transformations/transformations.go | 31 +++++++++++++++++ main_test.go | 4 +-- parser/parser.go | 9 +++++ value.go | 4 +-- vm.go | 30 ++--------------- 24 files changed, 116 insertions(+), 69 deletions(-) rename {dump => internal/dump}/BUILD.bazel (84%) rename {dump => internal/dump}/README.md (100%) rename {dump => internal/dump}/dump.go (100%) rename {dump => internal/dump}/dump_test.go (100%) rename {dump => internal/dump}/pointermap.go (100%) rename {dump => internal/dump}/utils.go (100%) create mode 100644 internal/transformations/BUILD.bazel rename desugarer.go => internal/transformations/desugarer.go (98%) rename desugarer_test.go => internal/transformations/desugarer_test.go (96%) rename static_analyzer.go => internal/transformations/static_analyzer.go (94%) rename static_analyzer_test.go => internal/transformations/static_analyzer_test.go (96%) create mode 100644 internal/transformations/transformations.go diff --git a/BUILD.bazel b/BUILD.bazel index 2d1493b..fba3863 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -13,13 +13,11 @@ go_library( name = "go_default_library", srcs = [ "builtins.go", - "desugarer.go", "doc.go", "error_formatter.go", "imports.go", "interpreter.go", "runtime_error.go", - "static_analyzer.go", "thunks.go", "value.go", "vm.go", @@ -28,6 +26,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//ast:go_default_library", + "//internal/transformations:go_default_library", "//parser:go_default_library", ], ) @@ -35,11 +34,9 @@ go_library( go_test( name = "go_default_test", srcs = [ - "desugarer_test.go", "interpreter_test.go", "jsonnet_test.go", "main_test.go", - "static_analyzer_test.go", ], data = glob(["testdata/**"]), embed = [":go_default_library"], diff --git a/README.md b/README.md index 9fc9cab..5bee80c 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,11 @@ Windows | _bazel build --platforms=@io_bazel_rules_go//go/toolchain:wind For additional target platform names, see the per-Go release definitions [here](https://github.com/bazelbuild/rules_go/blob/master/go/private/sdk_list.bzl#L21-L31) in the _rules_go_ Bazel package. +Additionally if files were moved around, you may need to run the following command to update Bazel files: +``` +bazel run //:gazelle +``` + ## Build instructions (go 1.8 - 1.10) ```bash diff --git a/ast/clone.go b/ast/clone.go index 84ca861..6453ec3 100644 --- a/ast/clone.go +++ b/ast/clone.go @@ -298,6 +298,7 @@ func clone(astPtr *Node) { cloneNodeBase(*astPtr) } +// Clone creates an independent copy of an AST func Clone(astPtr Node) Node { clone(&astPtr) return astPtr diff --git a/builtins.go b/builtins.go index dfe9307..0bd0ad5 100644 --- a/builtins.go +++ b/builtins.go @@ -1027,15 +1027,10 @@ func (b *ternaryBuiltin) Name() ast.Identifier { return b.name } -var desugaredBop = map[ast.BinaryOp]ast.Identifier{ - ast.BopPercent: "mod", - ast.BopIn: "objectHasAll", -} - var bopBuiltins = []*binaryBuiltin{ + // Note that % and `in` are desugared instead of being handled here ast.BopMult: &binaryBuiltin{name: "operator*", function: builtinMult, parameters: ast.Identifiers{"x", "y"}}, ast.BopDiv: &binaryBuiltin{name: "operator/", function: builtinDiv, parameters: ast.Identifiers{"x", "y"}}, - // ast.BopPercent: , ast.BopPlus: &binaryBuiltin{name: "operator+", function: builtinPlus, parameters: ast.Identifiers{"x", "y"}}, ast.BopMinus: &binaryBuiltin{name: "operator-", function: builtinMinus, parameters: ast.Identifiers{"x", "y"}}, diff --git a/c-bindings/BUILD.bazel b/c-bindings/BUILD.bazel index 9edd4bd..2ef23fd 100644 --- a/c-bindings/BUILD.bazel +++ b/c-bindings/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "internal.h", "json.cpp", "json.h", + "libgojsonnet.h", "libjsonnet.cpp", ], cdeps = [ diff --git a/cmd/dumpstdlibast/BUILD.bazel b/cmd/dumpstdlibast/BUILD.bazel index e727f85..41e569e 100644 --- a/cmd/dumpstdlibast/BUILD.bazel +++ b/cmd/dumpstdlibast/BUILD.bazel @@ -6,8 +6,8 @@ go_library( importpath = "github.com/google/go-jsonnet/cmd/dumpstdlibast", visibility = ["//visibility:private"], deps = [ - "//:go_default_library", - "//dump:go_default_library", + "//internal/dump:go_default_library", + "//internal/transformations:go_default_library", ], ) diff --git a/cmd/dumpstdlibast/dumpstdlibast.go b/cmd/dumpstdlibast/dumpstdlibast.go index ca2c0a8..387ef04 100644 --- a/cmd/dumpstdlibast/dumpstdlibast.go +++ b/cmd/dumpstdlibast/dumpstdlibast.go @@ -19,8 +19,8 @@ import ( "os" "path/filepath" - "github.com/google/go-jsonnet" - "github.com/google/go-jsonnet/dump" + "github.com/google/go-jsonnet/internal/dump" + "github.com/google/go-jsonnet/internal/transformations" ) func main() { @@ -33,7 +33,7 @@ func main() { panic(err) } - node, err := jsonnet.SnippetToAST("", string(buf)) + node, err := transformations.SnippetToAST("", string(buf)) if err != nil { panic(err) } diff --git a/imports.go b/imports.go index 58e2037..a8b8400 100644 --- a/imports.go +++ b/imports.go @@ -107,7 +107,7 @@ func (cache *importCache) importString(importedFrom, importedPath string, i *int } func codeToPV(i *interpreter, filename string, code string) *cachedThunk { - node, err := snippetToAST(filename, code) + node, err := SnippetToAST(filename, code) if err != nil { // TODO(sbarzowski) we should wrap (static) error here // within a RuntimeError. Because whether we get this error or not diff --git a/dump/BUILD.bazel b/internal/dump/BUILD.bazel similarity index 84% rename from dump/BUILD.bazel rename to internal/dump/BUILD.bazel index 7c0bd9d..d915d05 100644 --- a/dump/BUILD.bazel +++ b/internal/dump/BUILD.bazel @@ -7,7 +7,7 @@ go_library( "pointermap.go", "utils.go", ], - importpath = "github.com/google/go-jsonnet/dump", + importpath = "github.com/google/go-jsonnet/internal/dump", visibility = ["//visibility:public"], ) diff --git a/dump/README.md b/internal/dump/README.md similarity index 100% rename from dump/README.md rename to internal/dump/README.md diff --git a/dump/dump.go b/internal/dump/dump.go similarity index 100% rename from dump/dump.go rename to internal/dump/dump.go diff --git a/dump/dump_test.go b/internal/dump/dump_test.go similarity index 100% rename from dump/dump_test.go rename to internal/dump/dump_test.go diff --git a/dump/pointermap.go b/internal/dump/pointermap.go similarity index 100% rename from dump/pointermap.go rename to internal/dump/pointermap.go diff --git a/dump/utils.go b/internal/dump/utils.go similarity index 100% rename from dump/utils.go rename to internal/dump/utils.go diff --git a/internal/transformations/BUILD.bazel b/internal/transformations/BUILD.bazel new file mode 100644 index 0000000..f76a494 --- /dev/null +++ b/internal/transformations/BUILD.bazel @@ -0,0 +1,26 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "desugarer.go", + "static_analyzer.go", + "transformations.go", + ], + importpath = "github.com/google/go-jsonnet/internal/transformations", + visibility = ["//:__subpackages__"], + deps = [ + "//ast:go_default_library", + "//parser:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = [ + "desugarer_test.go", + "static_analyzer_test.go", + ], + embed = [":go_default_library"], + deps = ["//ast:go_default_library"], +) diff --git a/desugarer.go b/internal/transformations/desugarer.go similarity index 98% rename from desugarer.go rename to internal/transformations/desugarer.go index f7952b6..ea71db1 100644 --- a/desugarer.go +++ b/internal/transformations/desugarer.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package jsonnet +package transformations import ( "bytes" @@ -27,6 +27,11 @@ import ( "github.com/google/go-jsonnet/parser" ) +var desugaredBop = map[ast.BinaryOp]ast.Identifier{ + ast.BopPercent: "mod", + ast.BopIn: "objectHasAll", +} + func makeStr(s string) *ast.LiteralString { return &ast.LiteralString{ NodeBase: ast.NodeBase{}, @@ -232,8 +237,6 @@ func wrapInArray(inside ast.Node) ast.Node { return &ast.Array{Elements: ast.Nodes{inside}} } - - func desugarArrayComp(comp *ast.ArrayComp, objLevel int) (ast.Node, error) { err := desugar(&comp.Body, objLevel) if err != nil { @@ -301,17 +304,6 @@ func desugarLocalBinds(binds ast.LocalBinds, objLevel int) (err error) { return nil } -// Desugar Jsonnet expressions to reduce the number of constructs the rest of the implementation -// needs to understand. -// -// Note that despite the name, desugar() is not idempotent. String literals have their escape -// codes translated to low-level characters during desugaring. -// -// Desugaring should happen immediately after parsing, i.e. before static analysis and execution. -// Temporary variables introduced here should be prefixed with $ to ensure they do not clash with -// variables used in user code. -// TODO(sbarzowski) Actually we may want to do some static analysis before desugaring, e.g. -// warning user about dangerous use of constructs that we desugar. func desugar(astPtr *ast.Node, objLevel int) (err error) { node := *astPtr @@ -579,7 +571,18 @@ func desugar(astPtr *ast.Node, objLevel int) (err error) { return nil } -func desugarFile(ast *ast.Node) error { +// Desugar Jsonnet expressions to reduce the number of constructs the rest of the implementation +// needs to understand. +// +// Note that despite the name, desugar() is not idempotent. String literals have their escape +// codes translated to low-level characters during desugaring. +// +// Desugaring should happen immediately after parsing, i.e. before static analysis and execution. +// Temporary variables introduced here should be prefixed with $ to ensure they do not clash with +// variables used in user code. +// TODO(sbarzowski) Actually we may want to do some static analysis before desugaring, e.g. +// warning user about dangerous use of constructs that we desugar. +func Desugar(ast *ast.Node) error { err := desugar(ast, 0) if err != nil { return err diff --git a/desugarer_test.go b/internal/transformations/desugarer_test.go similarity index 96% rename from desugarer_test.go rename to internal/transformations/desugarer_test.go index deffeea..ed3a7a3 100644 --- a/desugarer_test.go +++ b/internal/transformations/desugarer_test.go @@ -14,4 +14,4 @@ See the License for the specific language governing permissions and limitations under the License. */ -package jsonnet +package transformations diff --git a/static_analyzer.go b/internal/transformations/static_analyzer.go similarity index 94% rename from static_analyzer.go rename to internal/transformations/static_analyzer.go index d0e8b27..7061aa9 100644 --- a/static_analyzer.go +++ b/internal/transformations/static_analyzer.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package jsonnet +package transformations import ( "fmt" @@ -164,6 +164,9 @@ func analyzeVisit(a ast.Node, inObject bool, vars ast.IdentifierSet) error { return s.err } -func analyze(node ast.Node) error { +// Analyze checks variable references (these could be checked statically in Jsonnet). +// It enriches ast with additional information about free variables in every node, +// so it is necessary to always run it before executing AST. +func Analyze(node ast.Node) error { return analyzeVisit(node, false, ast.NewIdentifierSet("std")) } diff --git a/static_analyzer_test.go b/internal/transformations/static_analyzer_test.go similarity index 96% rename from static_analyzer_test.go rename to internal/transformations/static_analyzer_test.go index fe58762..3859672 100644 --- a/static_analyzer_test.go +++ b/internal/transformations/static_analyzer_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package jsonnet +package transformations import ( "testing" @@ -28,7 +28,7 @@ import ( func TestSimpleNull(t *testing.T) { ast := &ast.LiteralNull{} - err := analyze(ast) + err := Analyze(ast) if err != nil { t.Errorf("Unexpected error: %+v", err) } @@ -60,7 +60,7 @@ func TestSimpleLocal(t *testing.T) { Body: &ast.Var{Id: "x"}, } - err := analyze(node) + err := Analyze(node) if err != nil { t.Errorf("Unexpected error: %+v", err) } diff --git a/internal/transformations/transformations.go b/internal/transformations/transformations.go new file mode 100644 index 0000000..206e3ef --- /dev/null +++ b/internal/transformations/transformations.go @@ -0,0 +1,31 @@ +package transformations + +import ( + "github.com/google/go-jsonnet/ast" + "github.com/google/go-jsonnet/parser" +) + +func snippetToRawAST(filename string, snippet string) (ast.Node, error) { + tokens, err := parser.Lex(filename, snippet) + if err != nil { + return nil, err + } + return parser.Parse(tokens) +} + +// SnippetToAST converts Jsonnet code snippet to desugared and analyzed AST +func SnippetToAST(filename string, snippet string) (ast.Node, error) { + node, err := snippetToRawAST(filename, snippet) + if err != nil { + return nil, err + } + err = Desugar(&node) + if err != nil { + return nil, err + } + err = Analyze(node) + if err != nil { + return nil, err + } + return node, nil +} diff --git a/main_test.go b/main_test.go index bbc7280..4e38d88 100644 --- a/main_test.go +++ b/main_test.go @@ -137,7 +137,7 @@ func runInternalJsonnet(i jsonnetInput) jsonnetResult { vm.NativeFunction(jsonToString) vm.NativeFunction(nativeError) - rawAST, err := snippetToRawAST(i.name, string(i.input)) + rawAST, err := parser.SnippetToRawAST(i.name, string(i.input)) if err != nil { return jsonnetResult{ output: errFormatter.Format(err) + "\n", @@ -146,7 +146,7 @@ func runInternalJsonnet(i jsonnetInput) jsonnetResult { } testChildren(rawAST) - desugaredAST, err := snippetToAST(i.name, string(i.input)) + desugaredAST, err := SnippetToAST(i.name, string(i.input)) if err != nil { return jsonnetResult{ output: errFormatter.Format(err) + "\n", diff --git a/parser/parser.go b/parser/parser.go index 65384be..2b33031 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -1191,3 +1191,12 @@ func Parse(t Tokens) (ast.Node, error) { return expr, nil } + +// SnippetToRawAST converts Jsonnet code snippet to AST (without any transformations) +func SnippetToRawAST(filename string, snippet string) (ast.Node, error) { + tokens, err := Lex(filename, snippet) + if err != nil { + return nil, err + } + return Parse(tokens) +} diff --git a/value.go b/value.go index 92ddac2..20836ce 100644 --- a/value.go +++ b/value.go @@ -428,8 +428,8 @@ func (*valueObject) getType() *valueType { return objectType } -func (o *valueObject) index(i *interpreter, trace TraceElement, field string) (value, error) { - return objectIndex(i, trace, objectBinding(o), field) +func (obj *valueObject) index(i *interpreter, trace TraceElement, field string) (value, error) { + return objectIndex(i, trace, objectBinding(obj), field) } func (obj *valueObject) assertionsChecked() bool { diff --git a/vm.go b/vm.go index 02390b8..fc433e4 100644 --- a/vm.go +++ b/vm.go @@ -22,7 +22,7 @@ import ( "runtime/debug" "github.com/google/go-jsonnet/ast" - "github.com/google/go-jsonnet/parser" + "github.com/google/go-jsonnet/internal/transformations" ) // Note: There are no garbage collection params because we're using the native @@ -137,7 +137,7 @@ func (vm *VM) evaluateSnippet(filename string, snippet string, kind evalKind) (o err = fmt.Errorf("(CRASH) %v\n%s", r, debug.Stack()) } }() - node, err := snippetToAST(filename, snippet) + node, err := SnippetToAST(filename, snippet) if err != nil { return "", err } @@ -199,33 +199,9 @@ func (vm *VM) EvaluateSnippetMulti(filename string, snippet string) (files map[s return } -func snippetToRawAST(filename string, snippet string) (ast.Node, error) { - tokens, err := parser.Lex(filename, snippet) - if err != nil { - return nil, err - } - return parser.Parse(tokens) -} - -func snippetToAST(filename string, snippet string) (ast.Node, error) { - node, err := snippetToRawAST(filename, snippet) - if err != nil { - return nil, err - } - err = desugarFile(&node) - if err != nil { - return nil, err - } - err = analyze(node) - if err != nil { - return nil, err - } - return node, nil -} - // SnippetToAST parses a snippet and returns the resulting AST. func SnippetToAST(filename string, snippet string) (ast.Node, error) { - return snippetToAST(filename, snippet) + return transformations.SnippetToAST(filename, snippet) } // Version returns the Jsonnet version number.