[fix] align documentation with config visibility (STAR-93)

.github/workflows/selftest.yml

@@ -9,6 +9,11 @@ on:
   pull_request:
     branches: ["master"]
   workflow_dispatch:
+    inputs:
+      coverage-ref:
+        description: "Exact commit SHA for a one-time tools coverage baseline (empty = current commit)"
+        required: false
+        type: string
 
 permissions: read-all
 
@@ -30,9 +35,20 @@ jobs:
     runs-on: ubuntu-22.04
     permissions:
       contents: read
+      id-token: write
     steps:
       - uses: actions/checkout@v5
+        with:
+          ref: ${{ inputs.coverage-ref || github.sha }}
       - uses: actions/setup-go@v6
         with:
           go-version: "1.19.x"
-      - run: go test -race ./...
+      - run: go test -race -coverprofile=coverage-tools.txt ./...
+      - name: Upload tools coverage
+        uses: codecov/codecov-action@v5
+        with:
+          use_oidc: true
+          files: coverage-tools.txt
+          override_commit: ${{ inputs.coverage-ref }}
+          disable_search: true
+          fail_ci_if_error: true

doccov/README.md

@@ -61,3 +61,11 @@ jobs:
 
 GoDoc completeness (a doc comment on every exported symbol) is a separate concern,
 covered by `revive`'s `exported` rule in the same workflow's Analyze step.
+
+Config visibility follows the complete `base` contract: public options expose
+`get_` and `set_`, secret options only `set_`, host-only options only `get_`,
+and secret + host-only options neither. `-config` recognizes multiline Go
+factory chains and the final literal `SetSecret(true/false)` /
+`SetHostOnly(true/false)` value. Non-literal visibility flags are rejected so
+an unknown contract cannot silently pass. Config names follow the existing
+`configKeyName = "name"` convention.

doccov/main.go

@@ -162,70 +162,134 @@ func scanSurface(dir string) ([]string, error) {
 	return out, nil
 }
 
-var (
-	configKeyConst = regexp.MustCompile(`(configKey\w+)\s*=\s*"([^"]+)"`)
-	configDeclLine = regexp.MustCompile(`ConfigOption\(\s*(configKey\w+)`)
-)
+// configVisibility mirrors base's independent script getter/setter controls.
+type configVisibility struct {
+	key      string
+	secret   bool
+	hostOnly bool
+}
 
-// scanConfig returns the sorted config-accessor builtin names that `base`
-// auto-generates for a module's config options: set_<name> for every option,
-// plus get_<name> for non-secret options. It is convention-based (best-effort):
-// it reads the `configKey<X> = "<name>"` constants and the
-// gen[Secret]ConfigOption(configKey<X>, …) declarations; a declaration line
-// containing "Secret" (genSecretConfigOption or a chained .SetSecret(true))
-// marks that option secret, so it gets no get_ accessor. Returns nil if the
-// module follows neither convention.
+// scanConfig recognizes the ecosystem's configKey constants and ConfigOption
+// factory chains. Parse Go syntax so multiline calls, comments, and explicit
+// false overrides have exactly the same visibility semantics as single lines.
 func scanConfig(dir string) ([]string, error) {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
 		return nil, err
 	}
-	keyValue := map[string]string{} // configKey ident -> option name
-	declared := map[string]bool{}   // configKey ident -> registered as an option
-	secret := map[string]bool{}     // configKey ident -> secret (set_ only)
-	for _, e := range entries {
-		name := e.Name()
-		if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+	keys := map[string]string{}
+	options := map[string]configVisibility{}
+	for _, entry := range entries {
+		name := entry.Name()
+		if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
 			continue
 		}
-		data, err := os.ReadFile(filepath.Join(dir, name))
+		file, err := parser.ParseFile(token.NewFileSet(), filepath.Join(dir, name), nil, 0)
 		if err != nil {
 			return nil, err
 		}
-		text := string(data)
-		for _, m := range configKeyConst.FindAllStringSubmatch(text, -1) {
-			keyValue[m[1]] = m[2]
-		}
-		for _, line := range strings.Split(text, "\n") {
-			m := configDeclLine.FindStringSubmatch(line)
-			if m == nil {
-				continue
+		var scanErr error
+		ast.Inspect(file, func(node ast.Node) bool {
+			if spec, ok := node.(*ast.ValueSpec); ok {
+				for i, ident := range spec.Names {
+					if i < len(spec.Values) && strings.HasPrefix(ident.Name, "configKey") {
+						if value := stringLit(spec.Values[i]); value != "" {
+							keys[ident.Name] = value
+						}
+					}
+				}
 			}
-			declared[m[1]] = true
-			if strings.Contains(line, "Secret") {
-				secret[m[1]] = true
+			expr, ok := node.(ast.Expr)
+			if !ok {
+				return true
+			}
+			option, found, err := configOption(expr)
+			if err != nil {
+				scanErr = err
+				return false
 			}
+			if found {
+				options[option.key] = option
+				return false
+			}
+			return true
+		})
+		if scanErr != nil {
+			return nil, fmt.Errorf("%s: %w", name, scanErr)
 		}
 	}
 	set := map[string]bool{}
-	for ident := range declared {
-		val, ok := keyValue[ident]
+	for key, option := range options {
+		name, ok := keys[key]
 		if !ok {
 			continue
 		}
-		set["set_"+val] = true
-		if !secret[ident] {
-			set["get_"+val] = true
+		if !option.hostOnly {
+			set["set_"+name] = true
+		}
+		if !option.secret {
+			set["get_"+name] = true
 		}
 	}
 	out := make([]string, 0, len(set))
-	for k := range set {
-		out = append(out, k)
+	for name := range set {
+		out = append(out, name)
 	}
 	sort.Strings(out)
 	return out, nil
 }
 
+func configOption(expr ast.Expr) (configVisibility, bool, error) {
+	call, ok := expr.(*ast.CallExpr)
+	if !ok {
+		return configVisibility{}, false, nil
+	}
+	if selector, ok := call.Fun.(*ast.SelectorExpr); ok {
+		option, found, err := configOption(selector.X)
+		if err != nil || found {
+			if err == nil && (selector.Sel.Name == "SetHostOnly" || selector.Sel.Name == "SetSecret") {
+				if len(call.Args) != 1 {
+					return option, true, fmt.Errorf("%s needs a literal boolean", selector.Sel.Name)
+				}
+				flag, ok := call.Args[0].(*ast.Ident)
+				if !ok || (flag.Name != "true" && flag.Name != "false") {
+					return option, true, fmt.Errorf("%s needs a literal boolean", selector.Sel.Name)
+				}
+				if selector.Sel.Name == "SetHostOnly" {
+					option.hostOnly = flag.Name == "true"
+				} else {
+					option.secret = flag.Name == "true"
+				}
+			}
+			return option, found, err
+		}
+	}
+	name := configFactoryName(call.Fun)
+	if !strings.HasSuffix(name, "ConfigOption") {
+		return configVisibility{}, false, nil
+	}
+	for _, arg := range call.Args {
+		if key, ok := arg.(*ast.Ident); ok && strings.HasPrefix(key.Name, "configKey") {
+			return configVisibility{key: key.Name, secret: strings.Contains(name, "Secret")}, true, nil
+		}
+	}
+	return configVisibility{}, false, nil
+}
+
+func configFactoryName(expr ast.Expr) string {
+	switch expr := expr.(type) {
+	case *ast.Ident:
+		return expr.Name
+	case *ast.SelectorExpr:
+		return expr.Sel.Name
+	case *ast.IndexExpr:
+		return configFactoryName(expr.X)
+	case *ast.IndexListExpr:
+		return configFactoryName(expr.X)
+	}
+	return ""
+}
+
 // stringLit extracts a string constant from a builtin's first argument. It
 // resolves a plain literal ("module.fn") and the common "ModuleName + \".fn\""
 // concatenation, returning the literal portion.

doccov/main_test.go

@@ -5,6 +5,7 @@
 package main
 
 import (
+	"fmt"
 	"go/parser"
 	"os"
 	"path/filepath"
@@ -139,3 +140,97 @@ func TestRunMissingReadme(t *testing.T) {
 		t.Fatal("a missing documentation file should fail")
 	}
 }
+
+// The accessor contract is orthogonal: host-only hides setters, secret hides
+// getters. Test the whole product, multiline chains, and later false overrides.
+func TestConfigVisibilityContract(t *testing.T) {
+	for _, secret := range []bool{false, true} {
+		for _, host := range []bool{false, true} {
+			for _, factory := range []string{"genConfigOption", "genSecretConfigOption"} {
+				dir := t.TempDir()
+				src := fmt.Sprintf("package m\nconst configKeyLimit = `limit`\nvar option = %s(\nconfigKeyLimit, \"Secret in description is not a flag\", 1).\nSetSecret(%v).\nSetHostOnly(%v)\n", factory, secret, host)
+				if err := os.WriteFile(filepath.Join(dir, "mod.go"), []byte(src), 0o600); err != nil {
+					t.Fatal(err)
+				}
+				got, err := scanConfig(dir)
+				if err != nil {
+					t.Fatal(err)
+				}
+				want := []string{}
+				if !secret {
+					want = append(want, "get_limit")
+				}
+				if !host {
+					want = append(want, "set_limit")
+				}
+				if strings.Join(got, ",") != strings.Join(want, ",") {
+					t.Fatalf("secret=%v host=%v factory=%s: %v, want %v", secret, host, factory, got, want)
+				}
+			}
+		}
+	}
+	for _, tc := range []struct{ chain, want string }{
+		{`.SetHostOnly(true).SetHostOnly(false).SetSecret(true).SetSecret(false)`, "get_limit,set_limit"},
+		{`.SetHostOnly(true)`, "get_limit"},
+	} {
+		dir := t.TempDir()
+		src := "package m\nconst configKeyLimit = \"limit\"\nvar option = genConfigOption(configKeyLimit, \"Secret\", 0)" + tc.chain
+		if err := os.WriteFile(filepath.Join(dir, "mod.go"), []byte(src), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		got, err := scanConfig(dir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Join(got, ",") != tc.want {
+			t.Fatalf("%s: %v, want %s", tc.chain, got, tc.want)
+		}
+	}
+}
+
+func TestConfigDeclarationSyntax(t *testing.T) {
+	for _, tc := range []struct{ expression, want string }{
+		{`genConfigOption[int](configKeyLimit, "limit", 0)`, "get_limit,set_limit"},
+		{`pkg.genConfigOption[int, string](configKeyLimit, "limit", 0).WithDescription("text").SetHostOnly(true)`, "get_limit"},
+		{`base.NewNamedConfigOption("module", configKeyLimit, "limit", 0).SetSecret(true)`, "set_limit"},
+		{`genSecretConfigOption(configKeyLimit, "limit", 0)`, "set_limit"},
+		{`genConfigOption(configKeyUnknown, "limit", 0)`, ""},
+		{`genConfigOption("literal without key convention", 0)`, ""},
+		{`unrelated(configKeyLimit)`, ""},
+		{`func() interface{} { return nil }()`, ""},
+		{`pkg.unrelated(configKeyLimit)`, ""},
+	} {
+		dir := t.TempDir()
+		src := "package m\nconst configKeyLimit = \"limit\"\nvar option = " + tc.expression
+		if err := os.WriteFile(filepath.Join(dir, "mod.go"), []byte(src), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		got, err := scanConfig(dir)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Join(got, ",") != tc.want {
+			t.Fatalf("%s: %v, want %s", tc.expression, got, tc.want)
+		}
+	}
+	for _, suffix := range []string{`.SetSecret()`, `.SetSecret(flag)`, `.SetHostOnly(1)`, `.SetHostOnly(true).SetSecret(flag)`} {
+		dir := t.TempDir()
+		src := "package m\nconst configKeyLimit = \"limit\"\nvar option = genConfigOption(configKeyLimit, 0)" + suffix
+		if err := os.WriteFile(filepath.Join(dir, "mod.go"), []byte(src), 0o600); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := scanConfig(dir); err == nil || !strings.Contains(err.Error(), "literal boolean") {
+			t.Fatalf("%s: %v", suffix, err)
+		}
+	}
+	if _, err := scanConfig(filepath.Join(t.TempDir(), "missing")); err == nil {
+		t.Fatal("missing directory accepted")
+	}
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "bad.go"), []byte("not go"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := scanConfig(dir); err == nil {
+		t.Fatal("malformed Go accepted")
+	}
+}