Fix some remaining issues

Author: sbarzowski

.gitignore

@@ -27,4 +27,6 @@ gojsonnet.egg-info/
 /linter/jsonnet-lint/jsonnet-lint
 /tests_path.source
 
+/jsonnet-lint
+
 /builtin-benchmark-results

cpp-jsonnet

@@ -158,19 +158,26 @@ func calcTP(node ast.Node, g *typeGraph) typePlaceholder {
 		}
 
 	case *ast.DesugaredObject:
-		// XXX: not sure what about it
 		obj := &objectDesc{
 			allFieldsKnown: true,
 			fieldContains:  make(map[string][]placeholderID),
 		}
 		for _, field := range node.Fields {
-			// XXX: what about plussuper, how does it change things here?
+			// XXX(sbarzowski): what about plussuper, how does it change things here?
 			switch fieldName := field.Name.(type) {
 			case *ast.LiteralString:
-				obj.fieldContains[fieldName.Value] = append(obj.fieldContains[fieldName.Value], g.getExprPlaceholder(field.Body))
+				if field.PlusSuper {
+					obj.fieldContains[fieldName.Value] = []placeholderID{anyType}
+				} else {
+					obj.fieldContains[fieldName.Value] = append(obj.fieldContains[fieldName.Value], g.getExprPlaceholder(field.Body))
+				}
 			default:
-				obj.allContain = append(obj.allContain, g.getExprPlaceholder(field.Body))
-				obj.allFieldsKnown = false
+				if field.PlusSuper {
+					obj.allContain = []placeholderID{anyType}
+				} else {
+					obj.allContain = append(obj.allContain, g.getExprPlaceholder(field.Body))
+					obj.allFieldsKnown = false
+				}
 			}
 		}
 		return concreteTP(TypeDesc{ObjectDesc: obj})

linter/internal/types/build_graph.go

@@ -1,12 +1,18 @@
 package types
 
 import (
+	"math"
 	"strings"
 
 	"github.com/google/go-jsonnet/ast"
 )
 
+// Technically on 64-bit system, if someone really tries, maybe they can
+// go over that and get strange errors. At this point I don't care.
+const maxPossibleArity = math.MaxInt32
+
 type arrayDesc struct {
+	// TODO(sbarzowski) Change to "otherContain" to improve precision.
 	allContain []placeholderID
 
 	elementContains [][]placeholderID
@@ -34,6 +40,7 @@ func (a *arrayDesc) normalize() {
 }
 
 type objectDesc struct {
+	// Change to unknownContain to improve precision
 	allContain     []placeholderID
 	fieldContains  map[string][]placeholderID
 	allFieldsKnown bool
@@ -129,8 +136,7 @@ type TypeDesc struct {
 
 // Any returns whether all values are allowed (i.e. we know nothing about it).
 func (t *TypeDesc) Any() bool {
-	// XXX(sbarzowski) BUG - must check that function, object and array allow any values inside
-	return t.Bool && t.Number && t.String && t.Null && t.Function() && t.Object() && t.Array()
+	return t.Bool && t.Number && t.String && t.Null && t.AnyFunction() && t.AnyObject() && t.AnyArray()
 }
 
 // Void returns whether the type is empty (no values are possible).
@@ -143,16 +149,52 @@ func (t *TypeDesc) Function() bool {
 	return t.FunctionDesc != nil
 }
 
+func (t *TypeDesc) AnyFunction() bool {
+	if !t.Function() || t.FunctionDesc.maxArity < maxPossibleArity || t.FunctionDesc.minArity > 0 || t.FunctionDesc.params != nil {
+		return false
+	}
+	for _, elemType := range t.FunctionDesc.resultContains {
+		if elemType == anyType {
+			return true
+		}
+	}
+	return false
+}
+
 // Object returns whether the types contains an object.
 func (t *TypeDesc) Object() bool {
 	return t.ObjectDesc != nil
 }
 
+func (t *TypeDesc) AnyObject() bool {
+	if !t.Object() || t.ObjectDesc.allFieldsKnown {
+		return false
+	}
+	for _, elemType := range t.ObjectDesc.allContain {
+		if elemType == anyType {
+			return true
+		}
+	}
+	return false
+}
+
 // Array returns whether the types contains an array.
 func (t *TypeDesc) Array() bool {
 	return t.ArrayDesc != nil
 }
 
+func (t *TypeDesc) AnyArray() bool {
+	if !t.Array() {
+		return false
+	}
+	for _, elemType := range t.ArrayDesc.allContain {
+		if elemType == anyType {
+			return true
+		}
+	}
+	return false
+}
+
 func voidTypeDesc() TypeDesc {
 	return TypeDesc{}
 }

linter/internal/types/desc.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"fmt"
-	"math"
 	"sort"
 
 	"github.com/google/go-jsonnet/ast"
@@ -155,7 +154,6 @@ func (g *typeGraph) newPlaceholder() placeholderID {
 	g._placeholders = append(g._placeholders, typePlaceholder{})
 	g.elementType = append(g.elementType, nil)
 
-	// fmt.Println("placeholder:", len(g._placeholders)-1)
 	return placeholderID(len(g._placeholders) - 1)
 }
 
@@ -214,7 +212,7 @@ func (g *typeGraph) separateElementTypes() {
 		contains := make([]placeholderID, 0, 1)
 
 		// Direct indexing
-		// XXX(barzowski) handle fromBuiltin.concrete
+		// XXX(sbarzowski) handle fromBuiltin.concrete
 		if index.indexType == knownStringIndex {
 			if c.concrete.Object() {
 				contains = append(contains, c.concrete.ObjectDesc.allContain...)
@@ -659,10 +657,8 @@ func NewTypeGraph(varAt map[ast.Node]*common.Variable, importFunc ImportFunc) *t
 	}
 
 	anyFunctionDesc := &functionDesc{
-		minArity: 0,
-		// Technically on 64-bit system if someone really tries maybe they can
-		// go over that and get strange errors. At this point I don't care.
-		maxArity:       math.MaxInt32,
+		minArity:       0,
+		maxArity:       maxPossibleArity,
 		resultContains: []placeholderID{anyType},
 	}
 

linter/internal/types/graph.go

@@ -15,9 +15,6 @@ import (
 	"github.com/google/go-jsonnet/linter/internal/utils"
 )
 
-// XXX(sbarzowski) refactoring idea - put ast.Node and its path in a struct as it gets
-// passed around together constantly
-
 // ErrorWriter encapsulates a writer and an error state indicating when at least
 // one error has been written to the writer.
 type ErrorWriter struct {
@@ -33,10 +30,17 @@ func (e *ErrorWriter) writeError(vm *jsonnet.VM, err errors.StaticError) {
 	}
 }
 
+// nodeWithLocation represents a Jsonnet program with its location
+// for the importer.
+type nodeWithLocation struct {
+	node ast.Node
+	path string
+}
+
 // Lint analyses a node and reports any issues it encounters to an error writer.
-func lint(vm *jsonnet.VM, node ast.Node, currentPath string, errWriter *ErrorWriter) {
+func lint(vm *jsonnet.VM, node nodeWithLocation, errWriter *ErrorWriter) {
 	roots := make(map[string]ast.Node)
-	getImports(vm, node, roots, currentPath, errWriter)
+	getImports(vm, node, roots, errWriter)
 
 	variablesInFile := make(map[string]common.VariableInfo)
 
@@ -46,13 +50,13 @@ func lint(vm *jsonnet.VM, node ast.Node, currentPath string, errWriter *ErrorWri
 		VariableKind: common.VarStdlib,
 	}
 
-	findVariables := func(node ast.Node, currentPath string) common.VariableInfo {
+	findVariables := func(node nodeWithLocation) common.VariableInfo {
 		variableInfo := common.VariableInfo{
 			Variables: nil,
 			VarAt:     make(map[ast.Node]*common.Variable),
 		}
 		variableInfo.Variables = append(variableInfo.Variables, &std)
-		findVariables(node, &variableInfo, vScope{"std": &std})
+		findVariables(node.node, &variableInfo, vScope{"std": &std})
 		for _, v := range variableInfo.Variables {
 			for _, u := range v.Occurences {
 				variableInfo.VarAt[u] = v
@@ -61,9 +65,9 @@ func lint(vm *jsonnet.VM, node ast.Node, currentPath string, errWriter *ErrorWri
 		return variableInfo
 	}
 
-	variableInfo := findVariables(node, currentPath)
+	variableInfo := findVariables(node)
 	for importedPath, rootNode := range roots {
-		variablesInFile[importedPath] = findVariables(rootNode, importedPath)
+		variablesInFile[importedPath] = findVariables(nodeWithLocation{rootNode, importedPath})
 	}
 
 	for _, v := range variableInfo.Variables {
@@ -89,22 +93,28 @@ func lint(vm *jsonnet.VM, node ast.Node, currentPath string, errWriter *ErrorWri
 
 	g.AddRoots(roots, vars)
 
-	g.PrepareTypes(node, et, variableInfo.VarAt)
-	types.Check(node, et, &ec)
+	g.PrepareTypes(node.node, et, variableInfo.VarAt)
+
+	// TODO(sbarzowski) Useful for debugging – expose it in CLI
+	// t := et[node.node]
+	// fmt.Fprintf(os.Stderr, "%v\n", types.Describe(&t))
+
+	types.Check(node.node, et, &ec)
 
-	traversal.Traverse(node, &ec)
+	traversal.Traverse(node.node, &ec)
 
 	for _, err := range ec.Errs {
 		errWriter.writeError(vm, err)
 	}
 }
 
-func getImports(vm *jsonnet.VM, node ast.Node, roots map[string]ast.Node, currentPath string, errWriter *ErrorWriter) {
+func getImports(vm *jsonnet.VM, node nodeWithLocation, roots map[string]ast.Node, errWriter *ErrorWriter) {
 	// TODO(sbarzowski) consider providing some way to disable warnings about nonexistent imports
 	// At least for 3rd party code.
 	// Perhaps there may be some valid use cases for conditional imports where one of the imported
 	// files doesn't exist.
-	switch node := node.(type) {
+	currentPath := node.path
+	switch node := node.node.(type) {
 	case *ast.Import:
 		p := node.File.Value
 		contents, foundAt, err := vm.ImportAST(currentPath, p)
@@ -113,7 +123,7 @@ func getImports(vm *jsonnet.VM, node ast.Node, roots map[string]ast.Node, curren
 		} else {
 			if _, visited := roots[foundAt]; !visited {
 				roots[foundAt] = contents
-				getImports(vm, contents, roots, foundAt, errWriter)
+				getImports(vm, nodeWithLocation{contents, foundAt}, roots, errWriter)
 			}
 		}
 	case *ast.ImportStr:
@@ -124,7 +134,7 @@ func getImports(vm *jsonnet.VM, node ast.Node, roots map[string]ast.Node, curren
 		}
 	default:
 		for _, c := range parser.Children(node) {
-			getImports(vm, c, roots, currentPath, errWriter)
+			getImports(vm, nodeWithLocation{c, currentPath}, roots, errWriter)
 		}
 	}
 }
@@ -161,7 +171,7 @@ func (linter *Linter) Check() bool {
 		errWriter.writeError(linter.vm, err.(errors.StaticError)) // ugly but true
 		return true
 	}
-	lint(linter.vm, node, linter.filename, &errWriter)
+	lint(linter.vm, nodeWithLocation{node, linter.filename}, &errWriter)
 	return errWriter.ErrorsFound
 }
 

linter/linter.go

@@ -0,0 +1 @@
+{}["foo"]

linter/testdata/index_object_with_string.jsonnet

@@ -1,5 +1,5 @@
-testdata/index_object_with_string:1:1 Unexpected end of file
-
+testdata/index_object_with_string:1:1-10 Indexed object has no field "foo"
 
+{}["foo"]
 
 

linter/testdata/index_object_with_string.linter.golden

@@ -0,0 +1,9 @@
+[
+    2,
+    true,
+    "str",
+    null,
+    {},
+    function() null,
+    []
+][1+3].foo

linter/testdata/not_any.jsonnet

@@ -0,0 +1,9 @@
+[
+    2,
+    true,
+    "str",
+    null,
+    {},
+    function() null,
+    []
+][4].foo

linter/testdata/not_any.linter.golden

@@ -0,0 +1,3 @@
+{
+    a +:: {},
+}.a + 3