BREAKING: Tree interface functions now return errors

Have the calls that can fail return the error interface, instead just a bool indicating
success.

This removes any previously deprecated functions, such as `(*Tree).Set` (going forward,
use `(*Tree).SetType`), or `IsConfigAlreadyLoaded` (use `errors.Is`/`As` from the Go
standard library instead).

Also some minor tweaks for readability and adhering to conventions.

.gitignore

@@ -0,0 +1,26 @@
+### Go template
+# If you prefer the allow list template instead of the deny list, see community template:
+# https://github.com/github/gitignore/blob/main/community/Golang/Go.AllowList.gitignore
+#
+# Binaries for programs and plugins
+*.exe
+*.exe~
+*.dll
+*.so
+*.dylib
+
+# Test binary, built with `go test -c`
+*.test
+
+# Output of the go coverage tool, specifically when used with LiteIDE
+*.out
+
+# Dependency directories (remove the comment below to include it)
+# vendor/
+
+# Go workspace file
+go.work
+
+# Ignore IDE files:
+/.idea
+/.vscode

convenience.go

@@ -22,7 +22,7 @@ func Revert(configs ...string) {
 }
 
 // GetSections delegates to the default tree. See Tree for details.
-func GetSections(config, secType string) ([]string, bool) {
+func GetSections(config, secType string) ([]string, error) {
 	return defaultTree.GetSections(config, secType)
 }
 
@@ -41,14 +41,9 @@ func GetBool(config, section, option string) (bool, bool) {
 	return defaultTree.GetBool(config, section, option)
 }
 
-// Set delegates to the default tree. See Tree for details.
-func Set(config, section, option string, values ...string) bool {
-	return defaultTree.Set(config, section, option, values...)
-}
-
 // Del delegates to the default tree. See Tree for details.
-func Del(config, section, option string) {
-	defaultTree.Del(config, section, option)
+func Del(config, section, option string) error {
+	return defaultTree.Del(config, section, option)
 }
 
 // AddSection delegates to the default tree. See Tree for details.
@@ -57,6 +52,6 @@ func AddSection(config, section, typ string) error {
 }
 
 // DelSection delegates to the default tree. See Tree for details.
-func DelSection(config, section string) {
-	defaultTree.DelSection(config, section)
+func DelSection(config, section string) error {
+	return defaultTree.DelSection(config, section)
 }

convenience_test.go

@@ -30,9 +30,9 @@ func (m *mockTree) Revert(configs ...string) {
 	m.Called(configs)
 }
 
-func (m *mockTree) GetSections(config string, secType string) ([]string, bool) {
+func (m *mockTree) GetSections(config string, secType string) ([]string, error) {
 	args := m.Called(config, secType)
-	return []string{args.String(0)}, args.Bool(1)
+	return []string{args.String(0)}, args.Error(1)
 }
 
 func (m *mockTree) Get(config, section, option string) ([]string, bool) {
@@ -50,27 +50,24 @@ func (m *mockTree) GetBool(config, section, option string) (bool, bool) {
 	return args.Bool(0), args.Bool(1)
 }
 
-func (m *mockTree) Set(config, section, option string, values ...string) bool {
-	args := m.Called(config, section, option, values)
-	return args.Bool(0)
-}
-
-func (m *mockTree) SetType(config, section, option string, typ OptionType, values ...string) bool {
+func (m *mockTree) SetType(config, section, option string, typ OptionType, values ...string) error {
 	args := m.Called(config, section, option, typ, values)
-	return args.Bool(0)
+	return args.Error(0)
 }
 
-func (m *mockTree) Del(config, section, option string) {
+func (m *mockTree) Del(config, section, option string) error {
 	m.Called(config, section, option)
+	return nil
 }
 
 func (m *mockTree) AddSection(config, section, typ string) error {
 	args := m.Called(config, section, typ)
 	return args.Error(0)
 }
 
-func (m *mockTree) DelSection(config, section string) {
+func (m *mockTree) DelSection(config, section string) error {
 	m.Called(config, section)
+	return nil
 }
 
 func TestMain(m *testing.M) {
@@ -84,7 +81,8 @@ func TestConvenienceLoadConfig(t *testing.T) {
 	m.On("LoadConfig", "foo", true).Return(nil)
 	m.On("LoadConfig", "bar", false).Return(io.ErrUnexpectedEOF)
 	assert.NoError(LoadConfig("foo", true))
-	assert.Error(LoadConfig("bar", false))
+	err := LoadConfig("bar", false)
+	assert.Error(err, io.ErrUnexpectedEOF)
 	m.AssertExpectations(t)
 }
 
@@ -106,9 +104,9 @@ func TestConvenienceRevert(t *testing.T) {
 func TestConvenienceGetSections(t *testing.T) {
 	assert := assert.New(t)
 	m := defaultTree.(*mockTree)
-	m.On("GetSections", "foo", "bar").Return("sec1", true)
-	list, ok := GetSections("foo", "bar")
-	assert.True(ok)
+	m.On("GetSections", "foo", "bar").Return("sec1", nil)
+	list, err := GetSections("foo", "bar")
+	assert.NoError(err)
 	assert.EqualValues([]string{"sec1"}, list)
 	m.AssertExpectations(t)
 }
@@ -143,36 +141,31 @@ func TestConvenienceGetBool(t *testing.T) {
 	m.AssertExpectations(t)
 }
 
-func TestConvenienceSet(t *testing.T) {
-	assert := assert.New(t)
-	m := defaultTree.(*mockTree)
-	m.On("Set", "foo", "bar", "valid", []string{"42"}).Return(true)
-	m.On("Set", "foo", "bar", "invalid", []string(nil)).Return(false)
-	assert.False(Set("foo", "bar", "invalid"))
-	assert.True(Set("foo", "bar", "valid", "42"))
-	m.AssertExpectations(t)
-}
-
 func TestConvenienceDel(t *testing.T) {
 	m := defaultTree.(*mockTree)
 	m.On("Del", "foo", "bar", "opt").Return()
-	Del("foo", "bar", "opt")
+	err := Del("foo", "bar", "opt")
+	assert.NoError(t, err)
 	m.AssertExpectations(t)
 }
 
 func TestConvenienceAddSection(t *testing.T) {
 	assert := assert.New(t)
 	m := defaultTree.(*mockTree)
+	addSectionErr := errors.New("invalid")
 	m.On("AddSection", "foo", "bar", "system").Return(nil)
-	m.On("AddSection", "foo", "bar", "interface").Return(errors.New("invalid")) //nolint:goerr113
-	assert.Error(AddSection("foo", "bar", "interface"))
+	m.On("AddSection", "foo", "bar", "interface").Return(addSectionErr) //nolint:goerr113
+	err := AddSection("foo", "bar", "interface")
+	assert.Error(err)
+	assert.EqualError(err, addSectionErr.Error())
 	assert.NoError(AddSection("foo", "bar", "system"))
 	m.AssertExpectations(t)
 }
 
 func TestConvenienceDelSection(t *testing.T) {
 	m := defaultTree.(*mockTree)
 	m.On("DelSection", "foo", "bar").Return()
-	DelSection("foo", "bar")
+	err := DelSection("foo", "bar")
+	assert.NoError(t, err)
 	m.AssertExpectations(t)
 }

doc.go

@@ -3,16 +3,17 @@ Package uci implements a binding to OpenWrt's UCI (Unified Configuration
 Interface) files in pure Go.
 
 The typical use case is reading and modifying UCI config options:
+
 	import "github.com/digineo/go-uci"
 
 	uci.Get("network", "lan", "ifname") //=> []string{"eth0.1"}, true
-	uci.Set("network", "lan", "ipaddr", "192.168.7.1")
+	uci.SetType("network", "lan", uci.TypeOption, "ipaddr", "192.168.7.1")
 	uci.Commit() // or uci.Revert()
 
 For more details head over to the OpenWrt wiki, or dive into UCI's C
 source code:
- - https://openwrt.org/docs/guide-user/base-system/uci
- - https://git.openwrt.org/?p=project/uci.git;a=summary
+  - https://openwrt.org/docs/guide-user/base-system/uci
+  - https://git.openwrt.org/?p=project/uci.git;a=summary
 
 The lexer is heavily inspired by Rob Pike's 2011 GTUG Sydney talk
 "Lexical Scanning in Go" (https://talks.golang.org/2011/lex.slide,

errors.go

@@ -1,6 +1,8 @@
 package uci
 
-import "fmt"
+import (
+	"fmt"
+)
 
 // ErrConfigAlreadyLoaded is returned by LoadConfig, if the given config
 // name is already present.
@@ -21,17 +23,6 @@ func (err ErrUnknownOptionType) Error() string {
 	return fmt.Sprintf("Unknown Option type %s", err.Type)
 }
 
-// IsConfigAlreadyLoaded reports, whether err is of type ErrConfigAlredyLoaded.
-//
-// Deprecated: use errors.Is or errors.As.
-func IsConfigAlreadyLoaded(err error) bool {
-	if err == nil {
-		return false
-	}
-	_, is := err.(*ErrConfigAlreadyLoaded) //nolint:errorlint
-	return is
-}
-
 // ErrSectionTypeMismatch is returned by AddSection if the section-to-add
 // already exists with a different type.
 type ErrSectionTypeMismatch struct {
@@ -45,30 +36,23 @@ func (err ErrSectionTypeMismatch) Error() string {
 		err.Config, err.Section, err.ExistingType, err.NewType)
 }
 
-// IsSectionTypeMismatch reports, whether err is of type ErrSectionTypeMismatch.
-//
-// Deprecated: use errors.Is or errors.As.
-func IsSectionTypeMismatch(err error) bool {
-	if err == nil {
-		return false
-	}
-	_, is := err.(*ErrSectionTypeMismatch) //nolint:errorlint
-	return is
+type ParseError struct {
+	errstr string
+	token  token
 }
 
-type ParseError string
-
 func (err ParseError) Error() string {
-	return fmt.Sprintf("parse error: %s", string(err))
+	if err.token.typ != scanToken(0) { // check if we got a valid token, or if it is a generic parse error
+		return fmt.Sprintf("parse error: %s, token: %s", err.errstr, err.token.String())
+	}
+	return fmt.Sprintf("parse error: %s", err.errstr)
 }
 
-// IsParseError reports, whether err is of type ParseError.
-//
-// Deprecated: use errors.Is or errors.As.
-func IsParseError(err error) bool {
-	if err == nil {
-		return false
-	}
-	_, is := err.(*ParseError) //nolint:errorlint
-	return is
+// ErrSectionNotFound is returned by Get
+type ErrSectionNotFound struct {
+	Section string
+}
+
+func (err ErrSectionNotFound) Error() string {
+	return fmt.Sprintf("section %s not found", err.Section)
 }

items.go

@@ -124,7 +124,7 @@ type scanFn func(*scanner) scanFn
 type scanToken int
 
 const (
-	tokError scanToken = iota
+	tokError scanToken = iota + 1
 	tokEOF
 
 	tokPackage // item-seq: (package, string)
@@ -147,7 +147,10 @@ func (t scanToken) String() string {
 		return "option"
 	case tokList:
 		return "list"
+	case scanToken(0):
+		return "[not initialized]"
 	}
+
 	return fmt.Sprintf("%%scanToken(%d)", int(t))
 }
 

items_test.go

@@ -30,13 +30,14 @@ func TestItemTypeString(t *testing.T) {
 func TestScanTokenString(t *testing.T) {
 	assert := assert.New(t)
 	names := []string{
+		"[not initialized]",
 		"error",
 		"eof",
 		"package",
 		"config",
 		"option",
 		"list",
-		"%scanToken(6)",
+		"%scanToken(7)",
 	}
 
 	for i, expected := range names {

parser.go

@@ -233,12 +233,12 @@ func parse(name, input string) (cfg *config, err error) {
 	scan(name, input).each(func(tok token) bool {
 		switch tok.typ { //nolint:exhaustive
 		case tokError:
-			perr := ParseError(tok.items[0].val)
+			perr := ParseError{errstr: "token error", token: tok}
 			err = &perr
 			return false
 
 		case tokPackage:
-			err = ParseError("UCI imports/exports are not yet supported")
+			err = ParseError{errstr: "UCI imports/exports are not yet supported"}
 			return false
 
 		case tokSection:

testdata/writeconfig.emptyfile

@@ -0,0 +1 @@
+

testdata/writeconfig.emptysection

@@ -0,0 +1,3 @@
+
+config foo 'bar'
+

testdata/writeconfig.luci

@@ -0,0 +1,26 @@
+
+config core 'main'
+	option lang 'auto'
+	option mediaurlbase '/luci-static/bootstrap'
+	option resourcebase '/luci-static/resources'
+
+config extern 'flash_keep'
+	option uci '/etc/config/'
+	option dropbear '/etc/dropbear/'
+	option openvpn '/etc/openvpn/'
+	option passwd '/etc/passwd'
+	option opkg '/etc/opkg.conf'
+	option firewall '/etc/firewall.user'
+	option uploads '/lib/uci/upload/'
+
+config internal 'languages'
+
+config internal 'sauth'
+	option sessionpath '/tmp/luci-sessions'
+	option sessiontime '3600'
+
+config internal 'ccache'
+	option enable '1'
+
+config internal 'themes'
+