track location correctly

libs/dyn/jsonloader/json.go

@@ -1,21 +1,90 @@
 package jsonloader
 
 import (
+	"bytes"
 	"encoding/json"
+	"fmt"
+	"io"
 
 	"github.com/databricks/cli/libs/dyn"
 )
 
 func LoadJSON(data []byte) (dyn.Value, error) {
-	var root map[string]interface{}
+	offsets := BuildLineOffsets(data)
-	err := json.Unmarshal(data, &root)
+	reader := bytes.NewReader(data)
+	decoder := json.NewDecoder(reader)
+
+	// Start decoding from the top-level value
+	value, err := decodeValue(decoder, offsets)
+	if err != nil {
+		if err == io.EOF {
+			err = fmt.Errorf("unexpected end of JSON input")
+		}
+		return dyn.InvalidValue, err
+	}
+	return value, nil
+}
+
+func decodeValue(decoder *json.Decoder, offsets []LineOffset) (dyn.Value, error) {
+	// Read the next JSON token
+	token, err := decoder.Token()
 	if err != nil {
 		return dyn.InvalidValue, err
 	}
 
-	loc := dyn.Location{
+	// Get the current byte offset
-		Line:   1,
+	offset := decoder.InputOffset()
-		Column: 1,
+	location := GetPosition(offset, offsets)
+
+	switch tok := token.(type) {
+	case json.Delim:
+		if tok == '{' {
+			// Decode JSON object
+			obj := make(map[string]dyn.Value)
+			for decoder.More() {
+				// Decode the key
+				keyToken, err := decoder.Token()
+				if err != nil {
+					return dyn.InvalidValue, err
 				}
-	return newLoader().load(&root, loc)
+				key, ok := keyToken.(string)
+				if !ok {
+					return dyn.InvalidValue, fmt.Errorf("expected string for object key")
+				}
+
+				// Decode the value recursively
+				val, err := decodeValue(decoder, offsets)
+				if err != nil {
+					return dyn.InvalidValue, err
+				}
+
+				obj[key] = val
+			}
+			// Consume the closing '}'
+			if _, err := decoder.Token(); err != nil {
+				return dyn.InvalidValue, err
+			}
+			return dyn.NewValue(obj, []dyn.Location{location}), nil
+		} else if tok == '[' {
+			// Decode JSON array
+			var arr []dyn.Value
+			for decoder.More() {
+				val, err := decodeValue(decoder, offsets)
+				if err != nil {
+					return dyn.InvalidValue, err
+				}
+				arr = append(arr, val)
+			}
+			// Consume the closing ']'
+			if _, err := decoder.Token(); err != nil {
+				return dyn.InvalidValue, err
+			}
+			return dyn.NewValue(arr, []dyn.Location{location}), nil
+		}
+	default:
+		// Primitive types: string, number, bool, or null
+		return dyn.NewValue(tok, []dyn.Location{location}), nil
+	}
+
+	return dyn.InvalidValue, fmt.Errorf("unexpected token: %v", token)
 }

libs/dyn/jsonloader/loader.go

@@ -1,99 +0,0 @@
-package jsonloader
-
-import (
-	"fmt"
-	"reflect"
-
-	"github.com/databricks/cli/libs/dyn"
-)
-
-type loader struct {
-}
-
-func newLoader() *loader {
-	return &loader{}
-}
-
-func errorf(loc dyn.Location, format string, args ...interface{}) error {
-	return fmt.Errorf("json (%s): %s", loc, fmt.Sprintf(format, args...))
-}
-
-func (d *loader) load(node any, loc dyn.Location) (dyn.Value, error) {
-	var value dyn.Value
-	var err error
-
-	if node == nil {
-		return dyn.NilValue, nil
-	}
-
-	if reflect.TypeOf(node).Kind() == reflect.Ptr {
-		return d.load(reflect.ValueOf(node).Elem().Interface(), loc)
-	}
-
-	switch reflect.TypeOf(node).Kind() {
-	case reflect.Map:
-		value, err = d.loadMapping(node.(map[string]interface{}), loc)
-	case reflect.Slice:
-		value, err = d.loadSequence(node.([]interface{}), loc)
-	case reflect.String, reflect.Bool,
-		reflect.Float64, reflect.Float32,
-		reflect.Int, reflect.Int32, reflect.Int64,
-		reflect.Uint, reflect.Uint32, reflect.Uint64:
-		value, err = d.loadScalar(node, loc)
-
-	default:
-		return dyn.InvalidValue, errorf(loc, "unknown node kind: %v", reflect.TypeOf(node).Kind())
-	}
-
-	if err != nil {
-		return dyn.InvalidValue, err
-	}
-
-	return value, nil
-}
-
-func (d *loader) loadScalar(node any, loc dyn.Location) (dyn.Value, error) {
-	switch reflect.TypeOf(node).Kind() {
-	case reflect.String:
-		return dyn.NewValue(node.(string), []dyn.Location{loc}), nil
-	case reflect.Bool:
-		return dyn.NewValue(node.(bool), []dyn.Location{loc}), nil
-	case reflect.Float64, reflect.Float32:
-		return dyn.NewValue(node.(float64), []dyn.Location{loc}), nil
-	case reflect.Int, reflect.Int32, reflect.Int64:
-		return dyn.NewValue(node.(int64), []dyn.Location{loc}), nil
-	case reflect.Uint, reflect.Uint32, reflect.Uint64:
-		return dyn.NewValue(node.(uint64), []dyn.Location{loc}), nil
-	default:
-		return dyn.InvalidValue, errorf(loc, "unknown scalar type: %v", reflect.TypeOf(node).Kind())
-	}
-}
-
-func (d *loader) loadSequence(node []interface{}, loc dyn.Location) (dyn.Value, error) {
-	dst := make([]dyn.Value, len(node))
-	for i, value := range node {
-		v, err := d.load(value, loc)
-		if err != nil {
-			return dyn.InvalidValue, err
-		}
-		dst[i] = v
-	}
-	return dyn.NewValue(dst, []dyn.Location{loc}), nil
-}
-
-func (d *loader) loadMapping(node map[string]interface{}, loc dyn.Location) (dyn.Value, error) {
-	dst := make(map[string]dyn.Value)
-	index := 0
-	for key, value := range node {
-		index += 1
-		v, err := d.load(value, dyn.Location{
-			Line:   loc.Line + index,
-			Column: loc.Column,
-		})
-		if err != nil {
-			return dyn.InvalidValue, err
-		}
-		dst[key] = v
-	}
-	return dyn.NewValue(dst, []dyn.Location{loc}), nil
-}

libs/dyn/jsonloader/locations.go

@@ -0,0 +1,44 @@
+package jsonloader
+
+import (
+	"sort"
+
+	"github.com/databricks/cli/libs/dyn"
+)
+
+type LineOffset struct {
+	Line  int
+	Start int64
+}
+
+// buildLineOffsets scans the input data and records the starting byte offset of each line.
+func BuildLineOffsets(data []byte) []LineOffset {
+	offsets := []LineOffset{{Line: 1, Start: 0}}
+	line := 1
+	for i, b := range data {
+		if b == '\n' {
+			line++
+			offsets = append(offsets, LineOffset{Line: line, Start: int64(i + 1)})
+		}
+	}
+	return offsets
+}
+
+// GetPosition maps a byte offset to its corresponding line and column numbers.
+func GetPosition(offset int64, offsets []LineOffset) dyn.Location {
+	// Binary search to find the line
+	idx := sort.Search(len(offsets), func(i int) bool {
+		return offsets[i].Start > offset
+	}) - 1
+
+	if idx < 0 {
+		idx = 0
+	}
+
+	lineOffset := offsets[idx]
+	return dyn.Location{
+		File:   "(inline)",
+		Line:   lineOffset.Line,
+		Column: int(offset-lineOffset.Start) + 1,
+	}
+}

libs/flags/json_flag_test.go

@@ -167,3 +167,45 @@ func TestJsonUnmarshalRequestMismatch(t *testing.T) {
 	require.ErrorContains(t, err, `json input error:
 - unknown field: settings`)
 }
+
+const wrontTypeJsonData = `
+{
+    "job_id": 123,
+    "new_settings": {
+        "name": "new job",
+        "email_notifications": {
+            "on_start": [],
+            "on_success": [],
+            "on_failure": []
+        },
+        "notification_settings": {
+            "no_alert_for_skipped_runs": true,
+            "no_alert_for_canceled_runs": true
+        },
+        "timeout_seconds": "wrong_type",
+        "max_concurrent_runs": {},
+        "tasks": [
+            {
+                "task_key": "new task",
+                "email_notifications": {},
+                "notification_settings": {},
+                "timeout_seconds": 0,
+                "max_retries": 0,
+                "min_retry_interval_millis": 0,
+                "retry_on_timeout": "true"
+            }
+        ]
+    }
+}
+`
+
+func TestJsonUnmarshalWrongTypeReportsCorrectLocation(t *testing.T) {
+	var body JsonFlag
+
+	var r jobs.ResetJob
+	err := body.Set(wrontTypeJsonData)
+	require.NoError(t, err)
+
+	err = body.Unmarshal(&r)
+	require.ErrorContains(t, err, `(inline):15:40: expected an int, found a string`)
+}