docs(skills): order declarations by reach

cmd/declscope/load.go

@@ -124,31 +124,6 @@ func loadIsAnalyzable(pkg *packages.Package) bool {
 	return !loadIsTestMain(pkg)
 }
 
-// loadIsTestMain identifies the synthetic executable built to run a package's
-// tests. Depending on the go command's response, its file is either the
-// generated _testmain.go or a build-cache archive; a real package is loaded
-// from .go files even when its import path happens to end in ".test".
-func loadIsTestMain(pkg *packages.Package) bool {
-	if pkg.Name != "main" || !strings.HasSuffix(pkg.PkgPath, ".test") {
-		return false
-	}
-	paths := append(slices.Clone(pkg.GoFiles), pkg.CompiledGoFiles...)
-	for _, path := range paths {
-		if filepath.Base(path) == "_testmain.go" {
-			return true
-		}
-	}
-	for _, path := range paths {
-		// A real package is loaded from Go source. With the go command's
-		// driver, the synthetic executable instead points at its build-cache
-		// archive (a path ending in -d), even when NeedSyntax is requested.
-		if filepath.Ext(path) == ".go" {
-			return false
-		}
-	}
-	return len(paths) > 0
-}
-
 // loadIsExternalTest identifies package foo_test as loaded for package foo.
 // ForTest is the build system's answer; trimming an import-path suffix would
 // also discard an ordinary package that is genuinely named foo_test.
@@ -260,3 +235,28 @@ func loadedPackageDir(pkg *packages.Package) string {
 	// and every caller drops those first.
 	return ""
 }
+
+// loadIsTestMain identifies the synthetic executable built to run a package's
+// tests. Depending on the go command's response, its file is either the
+// generated _testmain.go or a build-cache archive; a real package is loaded
+// from .go files even when its import path happens to end in ".test".
+func loadIsTestMain(pkg *packages.Package) bool {
+	if pkg.Name != "main" || !strings.HasSuffix(pkg.PkgPath, ".test") {
+		return false
+	}
+	paths := append(slices.Clone(pkg.GoFiles), pkg.CompiledGoFiles...)
+	for _, path := range paths {
+		if filepath.Base(path) == "_testmain.go" {
+			return true
+		}
+	}
+	for _, path := range paths {
+		// A real package is loaded from Go source. With the go command's
+		// driver, the synthetic executable instead points at its build-cache
+		// archive (a path ending in -d), even when NeedSyntax is requested.
+		if filepath.Ext(path) == ".go" {
+			return false
+		}
+	}
+	return len(paths) > 0
+}

internal/collect.go

@@ -142,6 +142,51 @@ func (c *collection) collectTargets(pass *analysis.Pass, opts Options) {
 	})
 }
 
+// collectRefs records every ident naming a tracked object, along with the file
+// it appears in.
+//
+// An ident can play both roles at once: an embedded field's ident defines the
+// field and uses the type, so Defs and Uses are consulted independently rather
+// than one shadowing the other. Returning after a hit in Defs would drop the
+// type use, and with it both a rename edit and a boundary diagnostic.
+//
+//declscope:shared // the pipeline's third stage, driven from analyzer.go
+func (c *collection) collectRefs(pass *analysis.Pass) {
+	in := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	// The inspector holds pass.Files in order, and c.files is a subsequence of
+	// them, so the files are visited in the order c.files lists them.
+	for fc := range in.Root().Children() {
+		fi := c.byFile[fc.Node().(*ast.File)]
+		if fi == nil {
+			continue
+		}
+		for cur := range fc.Preorder((*ast.CompositeLit)(nil), (*ast.Ident)(nil)) {
+			if lit, ok := cur.Node().(*ast.CompositeLit); ok {
+				c.collectUnkeyedFields(pass, fi, lit)
+				continue
+			}
+			ident := cur.Node().(*ast.Ident)
+			// Only a target's idents and refs are ever read, and byObj is
+			// complete, so nothing else is recorded. Most idents in a package
+			// name locals, parameters and imported objects.
+			if obj := pass.TypesInfo.Defs[ident]; c.byObj[obj] != nil {
+				c.idents[obj] = append(c.idents[obj], ident)
+			}
+			obj := origin(pass.TypesInfo.Uses[ident])
+			if obj == nil {
+				continue
+			}
+			if c.byObj[obj] != nil {
+				c.idents[obj] = append(c.idents[obj], ident)
+				c.refs[obj] = append(c.refs[obj], ref{node: ident, file: fi})
+			}
+			if tn := embeddedTypeName(obj); c.byObj[tn] != nil {
+				c.idents[tn] = append(c.idents[tn], ident)
+			}
+		}
+	}
+}
+
 func (c *collection) addFuncToCollection(pass *analysis.Pass, opts Options, fi *fileInfo, d *ast.FuncDecl) {
 	// Parsed before anything is skipped, so that a directive on a function
 	// declscope does not check, the blank one or init, is reported unused
@@ -428,48 +473,3 @@ func (c *collection) collectUnkeyedFields(pass *analysis.Pass, fi *fileInfo, lit
 		}
 	}
 }
-
-// collectRefs records every ident naming a tracked object, along with the file
-// it appears in.
-//
-// An ident can play both roles at once: an embedded field's ident defines the
-// field and uses the type, so Defs and Uses are consulted independently rather
-// than one shadowing the other. Returning after a hit in Defs would drop the
-// type use, and with it both a rename edit and a boundary diagnostic.
-//
-//declscope:shared // the pipeline's third stage, driven from analyzer.go
-func (c *collection) collectRefs(pass *analysis.Pass) {
-	in := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
-	// The inspector holds pass.Files in order, and c.files is a subsequence of
-	// them, so the files are visited in the order c.files lists them.
-	for fc := range in.Root().Children() {
-		fi := c.byFile[fc.Node().(*ast.File)]
-		if fi == nil {
-			continue
-		}
-		for cur := range fc.Preorder((*ast.CompositeLit)(nil), (*ast.Ident)(nil)) {
-			if lit, ok := cur.Node().(*ast.CompositeLit); ok {
-				c.collectUnkeyedFields(pass, fi, lit)
-				continue
-			}
-			ident := cur.Node().(*ast.Ident)
-			// Only a target's idents and refs are ever read, and byObj is
-			// complete, so nothing else is recorded. Most idents in a package
-			// name locals, parameters and imported objects.
-			if obj := pass.TypesInfo.Defs[ident]; c.byObj[obj] != nil {
-				c.idents[obj] = append(c.idents[obj], ident)
-			}
-			obj := origin(pass.TypesInfo.Uses[ident])
-			if obj == nil {
-				continue
-			}
-			if c.byObj[obj] != nil {
-				c.idents[obj] = append(c.idents[obj], ident)
-				c.refs[obj] = append(c.refs[obj], ref{node: ident, file: fi})
-			}
-			if tn := embeddedTypeName(obj); c.byObj[tn] != nil {
-				c.idents[tn] = append(c.idents[tn], ident)
-			}
-		}
-	}
-}

internal/config/config.go

@@ -83,6 +83,25 @@ func (b *boolSetting) UnmarshalYAML(node *yaml.Node) error {
 	return nil
 }
 
+// File is the on-disk configuration. Every field is optional, and no setting
+// has a zero value that means anything, so a field left empty is skipped by
+// Apply: omitting a key keeps the built-in default rather than silently
+// disabling a rule.
+type File struct {
+	Defaults defaultsSection `yaml:"defaults"`
+	Rules    rulesSection    `yaml:"rules"`
+
+	// Filter narrows and subtracts. only alone keeps nothing outside it, omit
+	// alone takes files out of everything, and both together narrow first.
+	Filter filterSection `yaml:"filter"`
+
+	// Baseline is a path relative to this config file.
+	Baseline string `yaml:"baseline"`
+
+	// path is where this config was read from, used to resolve Baseline.
+	path string
+}
+
 // Each section is a named type so that go-yaml's strict-decoding error can name
 // the section a misspelled key sits in, rather than printing the anonymous
 // struct's whole type literal.
@@ -128,25 +147,6 @@ type namingSection struct {
 	Vocabulary map[string][]string `yaml:"vocabulary"`
 }
 
-// File is the on-disk configuration. Every field is optional, and no setting
-// has a zero value that means anything, so a field left empty is skipped by
-// Apply: omitting a key keeps the built-in default rather than silently
-// disabling a rule.
-type File struct {
-	Defaults defaultsSection `yaml:"defaults"`
-	Rules    rulesSection    `yaml:"rules"`
-
-	// Filter narrows and subtracts. only alone keeps nothing outside it, omit
-	// alone takes files out of everything, and both together narrow first.
-	Filter filterSection `yaml:"filter"`
-
-	// Baseline is a path relative to this config file.
-	Baseline string `yaml:"baseline"`
-
-	// path is where this config was read from, used to resolve Baseline.
-	path string
-}
-
 // Resolve produces the options for analyzing a package directory: built-in
 // defaults, with the nearest config file layered on top, and the baseline
 // loaded — the one the config names, else the nearest default-named file
@@ -280,6 +280,33 @@ func DefaultBaseline(dir, root string) (string, bool) {
 	return "", false
 }
 
+// Load reads and parses a config file.
+//
+//declscope:ignore overexported // config_test parses files through it
+func Load(path string) (*File, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return nil, err
+	}
+	// The path is kept absolute because the exclude patterns and the baseline
+	// are resolved against the directory holding it. A -config given relative
+	// to the working directory would otherwise name a directory that no
+	// reported file path is ever taken from.
+	if abs, err := filepath.Abs(path); err == nil {
+		path = abs
+	}
+	f := File{path: path}
+	dec := yaml.NewDecoder(bytes.NewReader(data))
+	// Unknown keys are an error rather than a silent no-op: a typo in a rule
+	// name would otherwise leave the rule at its default with no sign of it.
+	dec.KnownFields(true)
+	// An empty document is a valid config that changes nothing.
+	if err := dec.Decode(&f); err != nil && !errors.Is(err, io.EOF) {
+		return nil, fmt.Errorf("%s: %w", path, namedKeys(err))
+	}
+	return &f, nil
+}
+
 func findUp(dir string, names []string) string {
 	for d := range dirsUp(dir) {
 		if path := firstFile(d, names); path != "" {
@@ -335,33 +362,6 @@ func sameDir(a, b string) bool {
 	return errA == nil && errB == nil && os.SameFile(sa, sb)
 }
 
-// Load reads and parses a config file.
-//
-//declscope:ignore overexported // config_test parses files through it
-func Load(path string) (*File, error) {
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	// The path is kept absolute because the exclude patterns and the baseline
-	// are resolved against the directory holding it. A -config given relative
-	// to the working directory would otherwise name a directory that no
-	// reported file path is ever taken from.
-	if abs, err := filepath.Abs(path); err == nil {
-		path = abs
-	}
-	f := File{path: path}
-	dec := yaml.NewDecoder(bytes.NewReader(data))
-	// Unknown keys are an error rather than a silent no-op: a typo in a rule
-	// name would otherwise leave the rule at its default with no sign of it.
-	dec.KnownFields(true)
-	// An empty document is a valid config that changes nothing.
-	if err := dec.Decode(&f); err != nil && !errors.Is(err, io.EOF) {
-		return nil, fmt.Errorf("%s: %w", path, namedKeys(err))
-	}
-	return &f, nil
-}
-
 // The sections a key can sit in, by the type name go-yaml puts in its
 // strict-decoding complaint.
 var sections = map[string]struct {

internal/ignore.go

@@ -32,7 +32,6 @@ type ignoreBook struct {
 // per target, it would be reported unused whenever any sibling did not need
 // it, and a wholly unused one would be reported once per sibling.
 type ignoreSite struct {
-	ig directive.Ignore
 	// decls names the declarations the directive reaches, in source order,
 	// for the report. It is empty for a file-level directive, and for one
 	// carried only by declarations declscope does not check (init, _, an
@@ -42,7 +41,6 @@ type ignoreSite struct {
 	decls []string
 	//declscope:shared // collect.go marks it when the directive is the file's
 	fileLevel bool
-	used      bool
 	// targets are the declarations the directive reaches, decls' objects. A
 	// file the build excluded may cross into one of them by name.
 	//
@@ -53,6 +51,9 @@ type ignoreSite struct {
 	//
 	//declscope:shared // collect.go records them as it parses the group
 	siblings []directive.Ignore
+
+	ig   directive.Ignore
+	used bool
 }
 
 // siteOfIgnore returns the accounting entry for ig, keyed by where it is written.
@@ -117,30 +118,6 @@ func (c *collection) ignoreWouldSilence(t *target, r rule.Rule) bool {
 	return false
 }
 
-// ignoreSilencesFile reports whether the file stands the problem's rule down
-// for everything it holds, and marks the ignore that did it used.
-//
-// The marking is what keeps an ignore written for a directive problem from
-// being reported as unused itself: it silences a report that is not attached to
-// any declaration, so the per-target accounting never sees it work.
-//
-// An ignore never silences the report written at its own position, which is
-// the report that it is unused. One that could would never be called unused,
-// and that report exists to catch it.
-func (c *collection) ignoreSilencesFile(fi *fileInfo, p directive.Problem) bool {
-	if fi == nil {
-		return false
-	}
-	hit := false
-	for _, ig := range fi.ignores {
-		if ig.Pos != p.Pos && ig.Covers(p.Rule) {
-			c.siteOfIgnore(ig).used = true
-			hit = true
-		}
-	}
-	return hit
-}
-
 // ignored reports whether any directive silences r, marking every directive
 // that does as used. All of them are marked, not just the first, so that
 // overlapping directives are not reported as unused.
@@ -318,3 +295,27 @@ func (c *collection) problemsLeftByIgnores(pass *analysis.Pass) []directive.Prob
 		return c.ignoreSilencesFile(c.fileAt(pass, p.Pos), p)
 	})
 }
+
+// ignoreSilencesFile reports whether the file stands the problem's rule down
+// for everything it holds, and marks the ignore that did it used.
+//
+// The marking is what keeps an ignore written for a directive problem from
+// being reported as unused itself: it silences a report that is not attached to
+// any declaration, so the per-target accounting never sees it work.
+//
+// An ignore never silences the report written at its own position, which is
+// the report that it is unused. One that could would never be called unused,
+// and that report exists to catch it.
+func (c *collection) ignoreSilencesFile(fi *fileInfo, p directive.Problem) bool {
+	if fi == nil {
+		return false
+	}
+	hit := false
+	for _, ig := range fi.ignores {
+		if ig.Pos != p.Pos && ig.Covers(p.Rule) {
+			c.siteOfIgnore(ig).used = true
+			hit = true
+		}
+	}
+	return hit
+}