Config reporting with line and error position

Signed-off-by: Waldemar Quevedo <wally@synadia.com>

conf/lex.go

@@ -101,12 +101,19 @@ type lexer struct {
 	// Used for processing escapable substrings in double-quoted and raw strings
 	stringParts   []string
 	stringStateFn stateFn
+
+	// lstart is the start position of the current line.
+	lstart int
+
+	// ilstart is the start position of the line from the current item.
+	ilstart int
 }
 
 type item struct {
 	typ  itemType
 	val  string
 	line int
+	pos  int
 }
 
 func (lx *lexer) nextItem() item {
@@ -147,8 +154,13 @@ func (lx *lexer) pop() stateFn {
 }
 
 func (lx *lexer) emit(typ itemType) {
-	lx.items <- item{typ, strings.Join(lx.stringParts, "") + lx.input[lx.start:lx.pos], lx.line}
+	val := strings.Join(lx.stringParts, "") + lx.input[lx.start:lx.pos]
+
+	// Position of item in line where it started.
+	pos := lx.pos - lx.ilstart - len(val)
+	lx.items <- item{typ, val, lx.line, pos}
 	lx.start = lx.pos
+	lx.ilstart = lx.lstart
 }
 
 func (lx *lexer) emitString() {
@@ -159,8 +171,11 @@ func (lx *lexer) emitString() {
 	} else {
 		finalString = lx.input[lx.start:lx.pos]
 	}
-	lx.items <- item{itemString, finalString, lx.line}
+	// Position of string in line where it started.
+	pos := lx.pos - lx.ilstart - len(finalString)
+	lx.items <- item{itemString, finalString, lx.line, pos}
 	lx.start = lx.pos
+	lx.ilstart = lx.lstart
 }
 
 func (lx *lexer) addCurrentStringPart(offset int) {
@@ -186,15 +201,20 @@ func (lx *lexer) next() (r rune) {
 
 	if lx.input[lx.pos] == '\n' {
 		lx.line++
+
+		// Mark start position of current line.
+		lx.lstart = lx.pos
 	}
 	r, lx.width = utf8.DecodeRuneInString(lx.input[lx.pos:])
 	lx.pos += lx.width
+
 	return r
 }
 
 // ignore skips over the pending input before this point.
 func (lx *lexer) ignore() {
 	lx.start = lx.pos
+	lx.ilstart = lx.lstart
 }
 
 // backup steps back one rune. Can be called only once per call of next.
@@ -221,10 +241,14 @@ func (lx *lexer) errorf(format string, values ...interface{}) stateFn {
 			values[i] = escapeSpecial(v)
 		}
 	}
+
+	// Position of error in current line.
+	pos := lx.pos - lx.lstart
 	lx.items <- item{
 		itemError,
 		fmt.Sprintf(format, values...),
 		lx.line,
+		pos,
 	}
 	return nil
 }
@@ -1131,7 +1155,7 @@ func (itype itemType) String() string {
 }
 
 func (item item) String() string {
-	return fmt.Sprintf("(%s, '%s', %d)", item.typ.String(), item.val, item.line)
+	return fmt.Sprintf("(%s, '%s', %d, %d)", item.typ.String(), item.val, item.line, item.pos)
 }
 
 func escapeSpecial(c rune) string {

conf/parse.go

@@ -49,6 +49,9 @@ type parser struct {
 	// Keys stack
 	keys []string
 
+	// Keys stack as items
+	ikeys []item
+
 	// The config file path, empty by default.
 	fp string
 
@@ -118,12 +121,17 @@ func (t *token) SourceFile() string {
 	return t.sourceFile
 }
 
+func (t *token) Position() int {
+	return t.item.pos
+}
+
 func parse(data, fp string, pedantic bool) (p *parser, err error) {
 	p = &parser{
 		mapping:  make(map[string]interface{}),
 		lx:       lex(data),
 		ctxs:     make([]interface{}, 0, 4),
 		keys:     make([]string, 0, 4),
+		ikeys:    make([]item, 0, 4),
 		fp:       filepath.Dir(fp),
 		pedantic: pedantic,
 	}
@@ -176,6 +184,20 @@ func (p *parser) popKey() string {
 	return last
 }
 
+func (p *parser) pushItemKey(key item) {
+	p.ikeys = append(p.ikeys, key)
+}
+
+func (p *parser) popItemKey() item {
+	if len(p.ikeys) == 0 {
+		panic("BUG in parser, item keys stack empty")
+	}
+	li := len(p.ikeys) - 1
+	last := p.ikeys[li]
+	p.ikeys = p.ikeys[0:li]
+	return last
+}
+
 func (p *parser) processItem(it item, fp string) error {
 	setValue := func(it item, v interface{}) {
 		if p.pedantic {
@@ -189,7 +211,14 @@ func (p *parser) processItem(it item, fp string) error {
 	case itemError:
 		return fmt.Errorf("Parse error on line %d: '%s'", it.line, it.val)
 	case itemKey:
+		// Keep track of the keys as items and strings,
+		// we do this in order to be able to still support
+		// includes without many breaking changes.
 		p.pushKey(it.val)
+
+		if p.pedantic {
+			p.pushItemKey(it)
+		}
 	case itemMapStart:
 		newCtx := make(map[string]interface{})
 		p.pushContext(newCtx)
@@ -297,6 +326,13 @@ func (p *parser) processItem(it item, fp string) error {
 		}
 		for k, v := range m {
 			p.pushKey(k)
+
+			if p.pedantic {
+				switch tk := v.(type) {
+				case *token:
+					p.pushItemKey(tk.item)
+				}
+			}
 			p.setValue(v)
 		}
 	}
@@ -356,7 +392,21 @@ func (p *parser) setValue(val interface{}) {
 	// Map processing
 	if ctx, ok := p.ctx.(map[string]interface{}); ok {
 		key := p.popKey()
-		// FIXME(dlc), make sure to error if redefining same key?
-		ctx[key] = val
+
+		if p.pedantic {
+			it := p.popItemKey()
+
+			// Change the position to the beginning of the key
+			// since more useful when reporting errors.
+			switch v := val.(type) {
+			case *token:
+				v.item.pos = it.pos
+				v.item.line = it.line
+				ctx[key] = v
+			}
+		} else {
+			// FIXME(dlc), make sure to error if redefining same key?
+			ctx[key] = val
+		}
 	}
 }

server/config_check_test.go

@@ -37,6 +37,9 @@ func TestConfigCheck(t *testing.T) {
 		// errorLine is the location of the error.
 		errorLine int
 
+		// errorPos is the position of the error.
+		errorPos int
+
 		// warning errors also include a reason optionally
 		reason string
 	}{
@@ -48,20 +51,20 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "monitor"`),
 			errorLine:   2,
+			errorPos:    17,
 		},
 		{
 			name: "when default permissions are used at top level",
 			config: `
-		"default_permissions" {
-		  publish = ["_SANDBOX.>"]
-		  subscribe = ["_SANDBOX.>"]
-		}
-   		`,
+                "default_permissions" {
+                  publish = ["_SANDBOX.>"]
+                  subscribe = ["_SANDBOX.>"]
+                }
+                `,
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "default_permissions"`),
-
-			// NOTE: line number is '5' because it is where the map definition ends.
-			errorLine: 5,
+			errorLine:   2,
+			errorPos:    18,
 		},
 		{
 			name: "when authorization config is empty",
@@ -82,15 +85,16 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "foo"`),
 			errorLine:   3,
+			errorPos:    5,
 		},
 		{
 			name: "when authorization config has unknown fields",
 			config: `
-                port = 4222
+		port = 4222
 
 		authorization = {
-                  user = "hello"
-                  foo = "bar"
+		  user = "hello"
+		  foo = "bar"
 		  password = "world"
 		}
 
@@ -98,23 +102,25 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "foo"`),
 			errorLine:   6,
+			errorPos:    5,
 		},
 		{
 			name: "when user authorization config has unknown fields",
 			config: `
 		authorization = {
 		  users = [
-		    { 
-                      user = "foo"
-                      pass = "bar"
-                      token = "quux"
-                    }
+		    {
+		      user = "foo"
+		      pass = "bar"
+		      token = "quux"
+		    }
 		  ]
 		}
 		`,
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "token"`),
 			errorLine:   7,
+			errorPos:    9,
 		},
 		{
 			name: "when user authorization permissions config has unknown fields",
@@ -130,6 +136,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`Unknown field inboxes parsing permissions`),
 			pedanticErr: errors.New(`unknown field "inboxes"`),
 			errorLine:   5,
+			errorPos:    7,
 		},
 		{
 			name: "when user authorization permissions config has unknown fields within allow or deny",
@@ -137,10 +144,10 @@ func TestConfigCheck(t *testing.T) {
 		authorization {
 		  permissions {
 		    subscribe = {
-                      allow = ["hello", "world"]
-                      deny = ["foo", "bar"]
-                      denied = "_INBOX.>"
-                    }
+		      allow = ["hello", "world"]
+		      deny = ["foo", "bar"]
+		      denied = "_INBOX.>"
+		    }
 		    publish = {}
 		  }
 		}
@@ -148,6 +155,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`Unknown field name "denied" parsing subject permissions, only 'allow' or 'deny' are permitted`),
 			pedanticErr: errors.New(`unknown field "denied"`),
 			errorLine:   7,
+			errorPos:    9,
 		},
 		{
 			name: "when user authorization permissions config has unknown fields within allow or deny",
@@ -155,10 +163,10 @@ func TestConfigCheck(t *testing.T) {
 		authorization {
 		  permissions {
 		    publish = {
-                      allow = ["hello", "world"]
-                      deny = ["foo", "bar"]
-                      allowed = "_INBOX.>"
-                    }
+		      allow = ["hello", "world"]
+		      deny = ["foo", "bar"]
+		      allowed = "_INBOX.>"
+		    }
 		    subscribe = {}
 		  }
 		}
@@ -166,13 +174,14 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`Unknown field name "allowed" parsing subject permissions, only 'allow' or 'deny' are permitted`),
 			pedanticErr: errors.New(`unknown field "allowed"`),
 			errorLine:   7,
+			errorPos:    9,
 		},
 		{
 			name: "when user authorization permissions config has unknown fields using arrays",
 			config: `
-                authorization {
+		authorization {
 
-		 default_permissions { 
+		 default_permissions {
 		   subscribe = ["a"]
 		   publish = ["b"]
 		   inboxes = ["c"]
@@ -184,18 +193,19 @@ func TestConfigCheck(t *testing.T) {
 		     pass = "bar"
 		   }
 		  ]
-	        }                       
+		}
 		`,
 			defaultErr:  errors.New(`Unknown field inboxes parsing permissions`),
 			pedanticErr: errors.New(`unknown field "inboxes"`),
 			errorLine:   7,
+			errorPos:    6,
 		},
 		{
 			name: "when user authorization permissions config has unknown fields using strings",
 			config: `
-                authorization {
+		authorization {
 
-		 default_permissions { 
+		 default_permissions {
 		   subscribe = "a"
 		   requests = "b"
 		   publish = "c"
@@ -207,11 +217,12 @@ func TestConfigCheck(t *testing.T) {
 		     pass = "bar"
 		   }
 		  ]
-	        }                       
+		}
 		`,
 			defaultErr:  errors.New(`Unknown field requests parsing permissions`),
 			pedanticErr: errors.New(`unknown field "requests"`),
 			errorLine:   6,
+			errorPos:    6,
 		},
 		{
 			name: "when user authorization permissions config is empty",
@@ -244,6 +255,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`Unknown field inboxes parsing permissions`),
 			pedanticErr: errors.New(`unknown field "inboxes"`),
 			errorLine:   6,
+			errorPos:    11,
 		},
 		{
 			name: "when clustering config is empty",
@@ -258,19 +270,19 @@ func TestConfigCheck(t *testing.T) {
 		{
 			name: "when unknown option is in clustering config",
 			config: `
-                # NATS Server Configuration
-                port = 4222
+		# NATS Server Configuration
+		port = 4222
 
 		cluster = {
 
-                  port = 6222
+		  port = 6222
 
 		  foo = "bar"
 
-                  authorization {
-                    user = "hello"
-                    pass = "world"
-                  }
+		  authorization {
+		    user = "hello"
+		    pass = "world"
+		  }
 
 		}
 		`,
@@ -278,6 +290,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "foo"`),
 			errorLine:   9,
+			errorPos:    5,
 		},
 		{
 			name: "when unknown option is in clustering authorization config",
@@ -292,6 +305,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "foo"`),
 			errorLine:   4,
+			errorPos:    7,
 		},
 		{
 			name: "when unknown option is in clustering authorization permissions config",
@@ -309,6 +323,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`Unknown field hello parsing permissions`),
 			pedanticErr: errors.New(`unknown field "hello"`),
 			errorLine:   7,
+			errorPos:    9,
 		},
 		{
 			name: "when unknown option is in tls config",
@@ -320,6 +335,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`error parsing tls config, unknown field ["hello"]`),
 			pedanticErr: errors.New(`unknown field "hello"`),
 			errorLine:   3,
+			errorPos:    5,
 		},
 		{
 			name: "when unknown option is in cluster tls config",
@@ -334,6 +350,7 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  errors.New(`error parsing tls config, unknown field ["foo"]`),
 			pedanticErr: errors.New(`unknown field "foo"`),
 			errorLine:   4,
+			errorPos:    7,
 		},
 		{
 			name: "when using cipher suites in the TLS config",
@@ -343,12 +360,13 @@ func TestConfigCheck(t *testing.T) {
 			"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256",
 			"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"
 		    ]
-                    preferences = []
+		    preferences = []
 		}
 		`,
 			defaultErr:  errors.New(`error parsing tls config, unknown field ["preferences"]`),
 			pedanticErr: errors.New(`unknown field "preferences"`),
 			errorLine:   7,
+			errorPos:    7,
 		},
 		{
 			name: "when using curve preferences in the TLS config",
@@ -359,27 +377,29 @@ func TestConfigCheck(t *testing.T) {
 			"CurveP384",
 			"CurveP521"
 		    ]
-                    suites = []
+		    suites = []
 		}
 		`,
 			defaultErr:  errors.New(`error parsing tls config, unknown field ["suites"]`),
 			pedanticErr: errors.New(`unknown field "suites"`),
 			errorLine:   8,
+			errorPos:    7,
 		},
 		{
 			name: "when unknown option is in cluster config with defined routes",
 			config: `
 		cluster {
-                  port = 6222
-                  routes = [
-                    nats://127.0.0.1:6222
-                  ]
-                  peers = []
+		  port = 6222
+		  routes = [
+		    nats://127.0.0.1:6222
+		  ]
+		  peers = []
 		}
 		`,
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "peers"`),
 			errorLine:   7,
+			errorPos:    5,
 		},
 		{
 			name: "when used as variable in authorization block it should not be considered as unknown field",
@@ -425,15 +445,16 @@ func TestConfigCheck(t *testing.T) {
 			defaultErr:  nil,
 			pedanticErr: errors.New(`unknown field "unused"`),
 			errorLine:   27,
+			errorPos:    5,
 		},
 		{
 			name: "when used as variable in top level config it should not be considered as unknown field",
 			config: `
-                monitoring_port = 8222
+		monitoring_port = 8222
 
-                http_port = $monitoring_port
+		http_port = $monitoring_port
 
-                port = 4222
+		port = 4222
 		`,
 			defaultErr:  nil,
 			pedanticErr: nil,
@@ -441,32 +462,33 @@ func TestConfigCheck(t *testing.T) {
 		{
 			name: "when used as variable in cluster config it should not be considered as unknown field",
 			config: `
-                cluster {
-                  clustering_port = 6222
-                  port = $clustering_port
-                }
- 		`,
+		cluster {
+		  clustering_port = 6222
+		  port = $clustering_port
+		}
+		`,
 			defaultErr:  nil,
 			pedanticErr: nil,
 		},
 		{
 			name: "when setting permissions within cluster authorization block",
 			config: `
-                cluster {
-                  authorization {
-                    permissions = {
-                      publish = { allow = ["foo", "bar"] }
-                    }
-                  }
+		cluster {
+		  authorization {
+		    permissions = {
+		      publish = { allow = ["foo", "bar"] }
+		    }
+		  }
 
-                  permissions = {
-                    publish = { deny = ["foo", "bar"] }
-                  }
-                }
- 		`,
+		  permissions = {
+		    publish = { deny = ["foo", "bar"] }
+		  }
+		}
+		`,
 			defaultErr:  nil,
 			pedanticErr: errors.New(`invalid use of field "authorization"`),
-			errorLine:   7,
+			errorLine:   3,
+			errorPos:    5,
 			reason:      `setting "permissions" within cluster authorization block is deprecated`,
 		},
 	}
@@ -498,7 +520,7 @@ func TestConfigCheck(t *testing.T) {
 				err := checkConfig(conf, true)
 				expectedErr := test.pedanticErr
 				if err != nil && expectedErr != nil {
-					msg := fmt.Sprintf("%s in %s:%d", expectedErr.Error(), conf, test.errorLine)
+					msg := fmt.Sprintf("%s in %s:%d:%d", expectedErr.Error(), conf, test.errorLine, test.errorPos)
 					if test.reason != "" {
 						msg += ": " + test.reason
 					}
@@ -528,7 +550,7 @@ func TestConfigCheckIncludes(t *testing.T) {
 	}
 	err := opts.ProcessConfigFile("./configs/include_conf_check_a.conf")
 	if err != nil {
-		t.Errorf("Unexpected error processing include files with configuration check enabled: %s", err)
+		t.Errorf("Unexpected error processing include files with configuration check enabled: %v", err)
 	}
 
 	opts = &Options{
@@ -536,9 +558,9 @@ func TestConfigCheckIncludes(t *testing.T) {
 	}
 	err = opts.ProcessConfigFile("./configs/include_bad_conf_check_a.conf")
 	if err == nil {
-		t.Errorf("Expected error processing include files with configuration check enabled: %s", err)
+		t.Errorf("Expected error processing include files with configuration check enabled: %v", err)
 	}
-	expectedErr := errors.New(`unknown field "monitoring_port" in configs/include_bad_conf_check_b.conf:2`)
+	expectedErr := errors.New(`unknown field "monitoring_port" in configs/include_bad_conf_check_b.conf:10:19`)
 	if err != nil && expectedErr != nil && err.Error() != expectedErr.Error() {
 		t.Errorf("Expected %q, got %q", expectedErr.Error(), err.Error())
 	}

server/configs/include_bad_conf_check_b.conf

@@ -1,4 +1,12 @@
 
-monitoring_port = 8222
+
+
+
+
+
+
+
+
+                  monitoring_port = 8222
 
 include "include_conf_check_c.conf"

server/configs/include_conf_check_b.conf

@@ -1,3 +1,4 @@
+
 monitoring_port = 8222
 
 include "include_conf_check_c.conf"

server/opts.go

@@ -196,6 +196,7 @@ type token interface {
 	Line() int
 	IsUsedVariable() bool
 	SourceFile() string
+	Position() int
 }
 
 type unknownConfigFieldErr struct {
@@ -207,7 +208,7 @@ type unknownConfigFieldErr struct {
 func (e *unknownConfigFieldErr) Error() string {
 	msg := fmt.Sprintf("unknown field %q", e.field)
 	if e.token != nil {
-		return msg + fmt.Sprintf(" in %s:%d", e.configFile, e.token.Line())
+		return msg + fmt.Sprintf(" in %s:%d:%d", e.configFile, e.token.Line(), e.token.Position())
 	}
 	return msg
 }
@@ -222,7 +223,7 @@ type configWarningErr struct {
 func (e *configWarningErr) Error() string {
 	msg := fmt.Sprintf("invalid use of field %q", e.field)
 	if e.token != nil {
-		msg += fmt.Sprintf(" in %s:%d", e.configFile, e.token.Line())
+		msg += fmt.Sprintf(" in %s:%d:%d", e.configFile, e.token.Line(), e.token.Position())
 	}
 	msg += ": " + e.reason
 	return msg