Merge pull request #1825 from OffchainLabs/koanf-cont

Implement linter detection of non-matching flag and filed name

linter/koanf/koanf.go

@@ -50,6 +50,8 @@ func run(dryRun bool, pass *analysis.Pass) (interface{}, error) {
 			switch v := node.(type) {
 			case *ast.StructType:
 				res = checkStruct(pass, v)
+			case *ast.FuncDecl:
+				res = checkFlagDefs(pass, v)
 			default:
 			}
 			for _, err := range res.Errors {
@@ -69,6 +71,104 @@ func run(dryRun bool, pass *analysis.Pass) (interface{}, error) {
 	return ret, nil
 }
 
+func containsFlagSet(params []*ast.Field) bool {
+	for _, p := range params {
+		se, ok := p.Type.(*ast.StarExpr)
+		if !ok {
+			continue
+		}
+		sle, ok := se.X.(*ast.SelectorExpr)
+		if !ok {
+			continue
+		}
+		if sle.Sel.Name == "FlagSet" {
+			return true
+		}
+	}
+	return false
+}
+
+// checkFlagDefs checks flag definitions in the function.
+// Result contains list of errors where flag name doesn't match field name.
+func checkFlagDefs(pass *analysis.Pass, f *ast.FuncDecl) Result {
+	// Ignore functions that does not get flagset as parameter.
+	if !containsFlagSet(f.Type.Params.List) {
+		return Result{}
+	}
+	var res Result
+	for _, s := range f.Body.List {
+		es, ok := s.(*ast.ExprStmt)
+		if !ok {
+			continue
+		}
+		callE, ok := es.X.(*ast.CallExpr)
+		if !ok {
+			continue
+		}
+		if len(callE.Args) != 3 {
+			continue
+		}
+		sl, ok := extractStrLit(callE.Args[0])
+		if !ok {
+			continue
+		}
+		s, ok := selector(callE.Args[1])
+		if !ok {
+			continue
+		}
+		if normSL := strings.ReplaceAll(sl, "-", ""); !strings.EqualFold(normSL, s) {
+			res.Errors = append(res.Errors, koanfError{
+				Pos:     pass.Fset.Position(f.Pos()),
+				Message: fmt.Sprintf("koanf tag name: %q doesn't match the field: %q", sl, s),
+			})
+		}
+
+	}
+	return res
+}
+
+func selector(e ast.Expr) (string, bool) {
+	n, ok := e.(ast.Node)
+	if !ok {
+		return "", false
+	}
+	se, ok := n.(*ast.SelectorExpr)
+	if !ok {
+		return "", false
+	}
+	return se.Sel.Name, true
+}
+
+// Extracts literal from expression that is either:
+// - string literal or
+// - sum of variable and string literal.
+// E.g.
+// strLitFromSum(`"max-size"`) = "max-size"
+// - strLitFromSum(`prefix + ".enable"“) = ".enable".
+func extractStrLit(e ast.Expr) (string, bool) {
+	if s, ok := strLit(e); ok {
+		return s, true
+	}
+	if be, ok := e.(*ast.BinaryExpr); ok {
+		if be.Op == token.ADD {
+			if s, ok := strLit(be.Y); ok {
+				// Drop the prefix dot.
+				return s[1:], true
+			}
+		}
+	}
+	return "", false
+}
+
+func strLit(e ast.Expr) (string, bool) {
+	if s, ok := e.(*ast.BasicLit); ok {
+		if s.Kind == token.STRING {
+			return strings.Trim(s.Value, "\""), true
+		}
+	}
+	return "", false
+}
+
 func checkStruct(pass *analysis.Pass, s *ast.StructType) Result {
 	var res Result
 	for _, f := range s.Fields.List {

linter/koanf/koanf_test.go

@@ -15,8 +15,8 @@ func TestAll(t *testing.T) {
 	}
 	testdata := filepath.Join(filepath.Dir(wd), "testdata")
 	res := analysistest.Run(t, testdata, analyzerForTests, "a")
-	if cnt := countErrors(res); cnt != 1 {
-		t.Errorf("analysistest.Run() got %v errors, expected 1", cnt)
+	if cnt := countErrors(res); cnt != 3 {
+		t.Errorf("analysistest.Run() got %v errors, expected 3", cnt)
 	}
 }
 

linter/testdata/src/a/a.go

@@ -1,6 +1,11 @@
 package a
 
+import (
+	"flag"
+)
+
 type Config struct {
+	// Field name doesn't match koanf tag.
 	L2       int `koanf:"chain"`
 	LogLevel int `koanf:"log-level"`
 	LogType  int `koanf:"log-type"`
@@ -9,3 +14,25 @@ type Config struct {
 	Node     int `koanf:"node"`
 	Queue    int `koanf:"queue"`
 }
+
+type BatchPosterConfig struct {
+	Enable  bool `koanf:"enable"`
+	MaxSize int  `koanf:"max-size" reload:"hot"`
+}
+
+// Flag names don't match field names from default config.
+// Contains 2 errors.
+func BatchPosterConfigAddOptions(prefix string, f *flag.FlagSet) {
+	f.Bool(prefix+".enabled", DefaultBatchPosterConfig.Enable, "enable posting batches to l1")
+	f.Int("max-sz", DefaultBatchPosterConfig.MaxSize, "maximum batch size")
+}
+
+func ConfigAddOptions(prefix string, f *flag.FlagSet) {
+	f.Bool(prefix+".enable", DefaultBatchPosterConfig.Enable, "enable posting batches to l1")
+	f.Int("max-size", DefaultBatchPosterConfig.MaxSize, "maximum batch size")
+}
+
+var DefaultBatchPosterConfig = BatchPosterConfig{
+	Enable:  false,
+	MaxSize: 100000,
+}