From 1c1df7f9532bb431ca2fca16d7c8b9c8fd059acf Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 12 Sep 2026 17:18:56 +1000 Subject: [PATCH] strip source quote-aware and resolve dynamic class references - strip # comments with quote-state tracking so a hash in a string no longer truncates the line - skip heredoc bodies in every @(EOT) form so documentation text is not indexed as a class reference - take every literal name out of a selector, following a bare variable through the assignments in its own file - record a class reference that stays unresolved against its file, with any literal prefix, instead of dropping it - read a class name case insensitively in a title or type reference - tag each edge as a declaration or an ordering reference --- internal/impact/index.go | 293 +++------- internal/impact/index_test.go | 19 +- internal/impact/scan.go | 536 ++++++++++++++++++ internal/impact/scan_test.go | 249 ++++++++ .../repo/site/profiles/manifests/base.pp | 1 + .../repo/site/profiles/manifests/dynamic.pp | 3 + 6 files changed, 873 insertions(+), 228 deletions(-) create mode 100644 internal/impact/scan.go create mode 100644 internal/impact/scan_test.go create mode 100644 internal/impact/testdata/repo/site/profiles/manifests/dynamic.pp diff --git a/internal/impact/index.go b/internal/impact/index.go index 80ed921..a00df35 100644 --- a/internal/impact/index.go +++ b/internal/impact/index.go @@ -17,7 +17,6 @@ import ( var manifestRoots = []string{"site", "modules"} var ( - commentRe = regexp.MustCompile(`(?m)#.*$`) // include/contain/require are functions taking a list of names; the // argument list is parsed rather than matched so every name is seen. declFuncRe = regexp.MustCompile(`(?:^|[^\w:$.])(?:include|contain|require)`) @@ -26,8 +25,11 @@ var ( classParamRe = regexp.MustCompile(`\bclass\s*\{`) classRefRe = regexp.MustCompile(`\bClass\s*\[`) classNameRe = regexp.MustCompile(`^(?:::)?[a-z][a-z0-9_]*(?:::[a-z0-9_]+)*$`) - typeNameRe = regexp.MustCompile(`^(?:::)?[A-Z][a-zA-Z0-9_]*(?:::[A-Z][a-zA-Z0-9_]*)*$`) - defineUseRe = regexp.MustCompile(`(?m)(?:^|[^\w:$.])(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)\s*\{`) + // a class name is case insensitive in a type reference or a resource + // title, so Class['Mysql::server'] and include Mysql::Server both name + // mysql::server. + anyCaseNameRe = regexp.MustCompile(`^(?:::)?[A-Za-z][A-Za-z0-9_]*(?:::[A-Za-z0-9_]+)*$`) + defineUseRe = regexp.MustCompile(`(?m)(?:^|[^\w:$.])(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)\s*\{`) // $mod::params::thing reads a class variable, which is a dependency on the // class that declares it. scopeVarRe = regexp.MustCompile(`\$(?:::)?([a-z][a-z0-9_]*(?:::[a-z0-9_]+)+)::[a-z_][a-z0-9_]*`) @@ -45,8 +47,8 @@ var ( type Index struct { Root string - // includedBy maps a class name to the classes that declare it. - includedBy map[string]map[string]bool + // includedBy maps a class name to the classes that reference it, and how. + includedBy map[string]map[string]refKind // moduleClasses maps a module name to the classes it defines. moduleClasses map[string][]string // assetOwners maps "/" to the classes referencing it. @@ -57,18 +59,34 @@ type Index struct { hieraExclude map[string][]string // known holds every class this repo defines. known map[string]bool + // unresolved maps a manifest file to the class references in it that no + // static resolution could name. + unresolved map[string][]UnresolvedRef + // unresolvedIncluders records, for each unresolved reference, the classes + // whose manifest holds it, so a class the reference might have named still + // reaches them. + unresolvedIncluders []UnresolvedIncluder +} + +// UnresolvedIncluder is an unresolved class reference together with the classes +// declared by the manifest holding it. Any class the reference could have named +// is treated as reaching those classes, because a dropped edge loses hosts. +type UnresolvedIncluder struct { + Ref UnresolvedRef + Classes []string } // BuildIndex walks the control repo at root and indexes it. func BuildIndex(root string) (*Index, error) { idx := &Index{ Root: root, - includedBy: map[string]map[string]bool{}, + includedBy: map[string]map[string]refKind{}, moduleClasses: map[string][]string{}, assetOwners: map[string][]string{}, hieraInclude: map[string][]string{}, hieraExclude: map[string][]string{}, known: map[string]bool{}, + unresolved: map[string][]UnresolvedRef{}, } for _, base := range manifestRoots { @@ -131,21 +149,35 @@ func (i *Index) addManifest(rel, full string) error { if err != nil { return fmt.Errorf("reading %s: %w", rel, err) } - body := commentRe.ReplaceAllString(string(data), "") + body := stripSource(string(data)) self := map[string]bool{} for _, name := range declared { self[name] = true } - for _, ref := range references(body) { + refs, unresolved := scanReferences(body) + for ref, kind := range refs { if self[ref] { continue } if i.includedBy[ref] == nil { - i.includedBy[ref] = map[string]bool{} + i.includedBy[ref] = map[string]refKind{} } for _, name := range declared { - i.includedBy[ref][name] = true + if old, ok := i.includedBy[ref][name]; !ok || kind > old { + i.includedBy[ref][name] = kind + } + } + } + + for idx := range unresolved { + unresolved[idx].File = rel + } + if len(unresolved) > 0 && len(declared) > 0 { + i.unresolved[rel] = unresolved + for _, ref := range unresolved { + i.unresolvedIncluders = append(i.unresolvedIncluders, + UnresolvedIncluder{Ref: ref, Classes: declared}) } } @@ -155,220 +187,6 @@ func (i *Index) addManifest(rel, full string) error { return nil } -// references extracts every class or defined-type name a manifest body names, -// across all the declaration forms this repo uses. -func references(body string) []string { - seen := map[string]bool{} - add := func(name string) { - if name != "" && name != "class" && name != "define" { - seen[name] = true - } - } - - // include/contain/require: every name in the argument list. - for _, loc := range declFuncRe.FindAllStringIndex(body, -1) { - args, ok := callArguments(body, loc[1]) - if !ok { - continue - } - for _, name := range classNames(args) { - add(name) - } - } - // class { ['a', 'b']: } declares one resource per title. - for _, loc := range classParamRe.FindAllStringIndex(body, -1) { - for _, name := range classNames(resourceTitles(body, loc[1])) { - add(name) - } - } - // Class['a', 'b'] is an ordering reference to each named class. - for _, loc := range classRefRe.FindAllStringIndex(body, -1) { - inner, _, ok := balanced(body, loc[1]-1, '[', ']') - if !ok { - continue - } - for _, name := range classNames(inner) { - add(name) - } - } - for _, re := range []*regexp.Regexp{inheritsRe, defineUseRe, scopeVarRe, createResRe} { - for _, m := range re.FindAllStringSubmatch(body, -1) { - add(strings.TrimPrefix(m[1], "::")) - } - } - // Resource references (Profiles::Foo::Bar['x']) name a defined type in - // capitalised form. - for _, m := range resourceRefRe.FindAllStringSubmatch(body, -1) { - add(strings.ToLower(m[1])) - } - - out := make([]string, 0, len(seen)) - for name := range seen { - out = append(out, name) - } - sort.Strings(out) - return out -} - -// callArguments returns the argument text of the function call whose name ends -// at i, in either the parenthesised or the bare statement form. It reports -// false when the keyword is not a call at all, such as a require => attribute. -func callArguments(s string, i int) (string, bool) { - if i >= len(s) { - return "", false - } - if s[i] != '(' && s[i] != ' ' && s[i] != '\t' { - return "", false - } - for i < len(s) && (s[i] == ' ' || s[i] == '\t') { - i++ - } - if i >= len(s) { - return "", false - } - if s[i] == '(' { - inner, _, ok := balanced(s, i, '(', ')') - return inner, ok - } - if s[i] == '=' || s[i] == ',' || s[i] == ';' || s[i] == '\n' { - return "", false - } - depth := 0 - for j := i; j < len(s); j++ { - switch c := s[j]; { - case c == '\'' || c == '"': - j = skipString(s, j) - 1 - case c == '(' || c == '[' || c == '{': - depth++ - case c == ')' || c == ']' || c == '}': - if depth == 0 { - return s[i:j], true - } - depth-- - case c == ';' && depth == 0: - return s[i:j], true - case c == '\n' && depth == 0 && !continued(s[i:j]): - return s[i:j], true - } - } - return s[i:], true -} - -// classNames pulls every class name out of a comma-separated argument list, -// descending into array literals and unwrapping quotes. -func classNames(args string) []string { - var out []string - for _, arg := range splitArgs(args) { - arg = strings.TrimSpace(arg) - if arg == "" { - continue - } - if arg[0] == '[' { - if inner, _, ok := balanced(arg, 0, '[', ']'); ok { - out = append(out, classNames(inner)...) - } - continue - } - if len(arg) > 1 && (arg[0] == '\'' || arg[0] == '"') && arg[len(arg)-1] == arg[0] { - arg = arg[1 : len(arg)-1] - } - switch { - case classNameRe.MatchString(arg): - out = append(out, strings.TrimPrefix(arg, "::")) - case typeNameRe.MatchString(arg): - // Class[Apache::Ssl] names the class in type form. - out = append(out, strings.ToLower(strings.TrimPrefix(arg, "::"))) - } - } - return out -} - -// splitArgs splits an argument list on its top-level commas. -func splitArgs(s string) []string { - var out []string - depth, start := 0, 0 - for j := 0; j < len(s); j++ { - switch c := s[j]; c { - case '\'', '"': - j = skipString(s, j) - 1 - case '(', '[', '{': - depth++ - case ')', ']', '}': - depth-- - case ',': - if depth == 0 { - out = append(out, s[start:j]) - start = j + 1 - } - } - } - return append(out, s[start:]) -} - -// resourceTitles returns the title expression of a resource body whose opening -// brace ends at i, i.e. everything up to the colon that closes the title. -func resourceTitles(s string, i int) string { - depth := 0 - for j := i; j < len(s); j++ { - switch c := s[j]; c { - case '\'', '"': - j = skipString(s, j) - 1 - case '(', '[', '{': - depth++ - case ')', ']', '}': - if depth == 0 { - return "" - } - depth-- - case ':': - if depth == 0 { - return s[i:j] - } - } - } - return "" -} - -// balanced returns the text between the delimiter at i and its match. -func balanced(s string, i int, opener, closer byte) (string, int, bool) { - depth := 0 - for j := i; j < len(s); j++ { - switch c := s[j]; c { - case '\'', '"': - j = skipString(s, j) - 1 - case opener: - depth++ - case closer: - depth-- - if depth == 0 { - return s[i+1 : j], j + 1, true - } - } - } - return "", len(s), false -} - -// skipString returns the index just past the quoted string starting at i. -func skipString(s string, i int) int { - quote := s[i] - for j := i + 1; j < len(s); j++ { - switch s[j] { - case '\\': - j++ - case quote: - return j + 1 - } - } - return len(s) -} - -// continued reports whether an argument list so far ends on a comma, meaning -// the list carries on over the newline. -func continued(s string) bool { - s = strings.TrimRight(s, " \t\r") - return strings.HasSuffix(s, ",") -} - func assetReferences(body string) []string { var out []string for _, m := range assetRe.FindAllStringSubmatch(body, -1) { @@ -431,7 +249,7 @@ func stringList(v any) []string { return out } -// IncludedBy returns the classes that declare the given class, sorted. +// IncludedBy returns the classes that reference the given class, sorted. func (i *Index) IncludedBy(class string) []string { names := make([]string, 0, len(i.includedBy[class])) for name := range i.includedBy[class] { @@ -441,6 +259,31 @@ func (i *Index) IncludedBy(class string) []string { return names } +// EdgeNote describes an edge that is not a declaration, so an explanation can +// say why the class is reached. It is empty for a plain declaration. +func (i *Index) EdgeNote(class, parent string) string { + if i.includedBy[class][parent] == refDeclare { + return "" + } + return " (ordering or data reference, not a declaration)" +} + +// UnresolvedIncluders returns the unresolved references that could have named +// the given class, matched on whatever literal prefix was recoverable. +func (i *Index) UnresolvedIncluders(class string) []UnresolvedIncluder { + var out []UnresolvedIncluder + for _, u := range i.unresolvedIncluders { + if strings.HasPrefix(class, u.Ref.Prefix) { + out = append(out, u) + } + } + return out +} + +// UnresolvedIn returns the class references in a manifest file that no static +// resolution could name. +func (i *Index) UnresolvedIn(file string) []UnresolvedRef { return i.unresolved[file] } + // ModuleClasses returns every class the module defines, sorted. func (i *Index) ModuleClasses(module string) []string { return i.moduleClasses[module] } diff --git a/internal/impact/index_test.go b/internal/impact/index_test.go index bf680c6..fb693b6 100644 --- a/internal/impact/index_test.go +++ b/internal/impact/index_test.go @@ -2,13 +2,26 @@ package impact import ( "reflect" + "sort" "testing" ) -// refs runs a manifest body through the same comment stripping addManifest -// does, then extracts its references. +// refs runs a manifest body through the same stripping addManifest does, then +// extracts the class names it resolves. func refs(body string) []string { - return references(commentRe.ReplaceAllString(body, "")) + names, _ := scanReferences(stripSource(body)) + out := make([]string, 0, len(names)) + for name := range names { + out = append(out, name) + } + sort.Strings(out) + return out +} + +// unresolvedRefs returns the references the scan could not reduce to names. +func unresolvedRefs(body string) []UnresolvedRef { + _, out := scanReferences(stripSource(body)) + return out } func TestReferencesReadsWholeArgumentList(t *testing.T) { diff --git a/internal/impact/scan.go b/internal/impact/scan.go new file mode 100644 index 0000000..f1076be --- /dev/null +++ b/internal/impact/scan.go @@ -0,0 +1,536 @@ +package impact + +import ( + "regexp" + "strings" +) + +// refKind separates a construct that applies a class from one that only names +// it for ordering or to read a value out of it. +type refKind int + +const ( + refOrdering refKind = iota + refDeclare +) + +// UnresolvedRef is a class reference the parser saw but could not reduce to +// literal names. Dropping one silently loses hosts, so it is recorded against +// its file and reported instead. +type UnresolvedRef struct { + File string `json:"file"` + Form string `json:"form"` + Expr string `json:"expr"` + Prefix string `json:"prefix,omitempty"` +} + +// reserved are bare words an argument list or selector arm can hold that are +// values rather than class names. +var reserved = map[string]bool{ + "undef": true, "true": true, "false": true, "default": true, + "class": true, "define": true, +} + +// maxVarDepth caps how far a bare variable is followed through same-file +// assignments before the reference is called unresolved. +const maxVarDepth = 4 + +// assignRe finds `$name = ` while skipping `==`, `=~` and `=>`. +var assignRe = regexp.MustCompile(`(?:^|[^\w$.])\$([a-z_][a-zA-Z0-9_]*)\s*=[^=~>]`) + +type refScan struct { + refs map[string]refKind + unresolved []UnresolvedRef + vars map[string]string + depth int +} + +// scanReferences extracts every class or defined-type name a manifest body +// names, plus every class reference it could not resolve to literal names. +func scanReferences(body string) (map[string]refKind, []UnresolvedRef) { + s := &refScan{refs: map[string]refKind{}, vars: assignments(body)} + + // include/contain/require: every name in the argument list. + for _, loc := range declFuncRe.FindAllStringIndex(body, -1) { + args, ok := callArguments(body, loc[1]) + if !ok { + continue + } + s.args(callName(body[loc[0]:loc[1]]), refDeclare, args) + } + // class { ['a', 'b']: } declares one resource per title. + for _, loc := range classParamRe.FindAllStringIndex(body, -1) { + s.args("class {}", refDeclare, resourceTitles(body, loc[1])) + } + // Class['a', 'b'] names classes that are declared elsewhere; it orders + // against them rather than applying them. + for _, loc := range classRefRe.FindAllStringIndex(body, -1) { + inner, _, ok := balanced(body, loc[1]-1, '[', ']') + if !ok { + continue + } + s.args("Class[]", refOrdering, inner) + } + for _, re := range []*regexp.Regexp{inheritsRe, defineUseRe, createResRe} { + for _, m := range re.FindAllStringSubmatch(body, -1) { + s.add(m[1], refDeclare) + } + } + // A $mod::params::thing read and a Mod::Type['x'] reference both depend on + // another class without declaring it. + for _, m := range scopeVarRe.FindAllStringSubmatch(body, -1) { + s.add(m[1], refOrdering) + } + for _, m := range resourceRefRe.FindAllStringSubmatch(body, -1) { + s.add(strings.ToLower(m[1]), refOrdering) + } + + return s.refs, dedupeUnresolved(s.unresolved) +} + +// callName trims the separator declFuncRe matched ahead of the function name. +func callName(match string) string { + return strings.TrimLeftFunc(match, func(r rune) bool { return r < 'a' || r > 'z' }) +} + +func (s *refScan) add(name string, kind refKind) { + name = strings.TrimPrefix(name, "::") + if name == "" || reserved[name] { + return + } + if old, ok := s.refs[name]; !ok || kind > old { + s.refs[name] = kind + } +} + +func (s *refScan) args(form string, kind refKind, args string) { + for _, arg := range splitArgs(args) { + s.arg(form, kind, arg) + } +} + +// arg reduces one argument expression to class names, descending into arrays, +// parentheses and selector arms. Anything it cannot reduce becomes an +// unresolved reference rather than nothing. +func (s *refScan) arg(form string, kind refKind, arg string) { + arg = strings.TrimSpace(arg) + if arg == "" { + return + } + switch arg[0] { + case '[': + s.nested(form, kind, arg, '[', ']') + case '(': + s.nested(form, kind, arg, '(', ')') + default: + if arms, ok := selectorArms(arg); ok { + s.selector(form, kind, arg, arms) + return + } + s.literal(form, kind, arg) + } +} + +func (s *refScan) nested(form string, kind refKind, arg string, opener, closer byte) { + inner, _, ok := balanced(arg, 0, opener, closer) + if !ok { + s.unresolvedAdd(form, arg, "") + return + } + s.args(form, kind, inner) +} + +// selector takes every literal name out of every arm, which is strictly better +// than widening: a selector over role names lists all the classes it can pick. +func (s *refScan) selector(form string, kind refKind, arg, arms string) { + for _, arm := range splitArgs(arms) { + _, value, ok := cutRocket(arm) + if !ok { + if strings.TrimSpace(arm) != "" { + s.unresolvedAdd(form, arg, "") + } + continue + } + s.arg(form, kind, value) + } +} + +func (s *refScan) literal(form string, kind refKind, arg string) { + lit, quoted := unquote(arg) + switch { + case classNameRe.MatchString(lit): + s.add(lit, kind) + case anyCaseNameRe.MatchString(lit): + // Class[Apache::Ssl] names the class in type form. + s.add(strings.ToLower(lit), kind) + case quoted: + // "profile::role_${role}" is a name only the compiler can finish. + s.unresolvedAdd(form, arg, literalPrefix(lit)) + case strings.HasPrefix(arg, "$"): + s.variable(form, kind, arg) + default: + s.unresolvedAdd(form, arg, "") + } +} + +// variable follows a bare $name through the assignments in the same file, and +// gives up loudly when that leads nowhere. +func (s *refScan) variable(form string, kind refKind, arg string) { + expr, ok := s.vars[strings.TrimPrefix(strings.TrimPrefix(arg, "$"), "::")] + if !ok || s.depth >= maxVarDepth { + s.unresolvedAdd(form, arg, "") + return + } + before := len(s.refs) + len(s.unresolved) + s.depth++ + s.arg(form, kind, expr) + s.depth-- + if len(s.refs)+len(s.unresolved) == before { + s.unresolvedAdd(form, arg, "") + } +} + +func (s *refScan) unresolvedAdd(form, expr, prefix string) { + s.unresolved = append(s.unresolved, UnresolvedRef{Form: form, Expr: collapse(expr), Prefix: prefix}) +} + +func dedupeUnresolved(in []UnresolvedRef) []UnresolvedRef { + if len(in) == 0 { + return nil + } + seen := map[UnresolvedRef]bool{} + out := in[:0] + for _, u := range in { + if seen[u] { + continue + } + seen[u] = true + out = append(out, u) + } + return out +} + +// literalPrefix returns the part of an interpolated name that is fixed text, +// so the report can say what family of classes was missed. +func literalPrefix(s string) string { + i := strings.Index(s, "$") + if i <= 0 { + return "" + } + return s[:i] +} + +// collapse folds whitespace and clips an expression to a length that reads in +// a report. +func collapse(s string) string { + out := strings.Join(strings.Fields(s), " ") + if len(out) > 80 { + out = out[:80] + "..." + } + return out +} + +func unquote(s string) (string, bool) { + if len(s) > 1 && (s[0] == '\'' || s[0] == '"') && s[len(s)-1] == s[0] { + return s[1 : len(s)-1], true + } + return s, false +} + +// selectorArms returns the arm text of a ` ? { ... }` selector. +func selectorArms(s string) (string, bool) { + depth := 0 + for j := 0; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + depth-- + case '?': + if depth != 0 { + continue + } + k := j + 1 + for k < len(s) && isSpace(s[k]) { + k++ + } + if k < len(s) && s[k] == '{' { + inner, _, ok := balanced(s, k, '{', '}') + return inner, ok + } + } + } + return "", false +} + +// cutRocket splits a selector arm on its top-level `=>`. +func cutRocket(s string) (string, string, bool) { + depth := 0 + for j := 0; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + depth-- + case '=': + if depth == 0 && j+1 < len(s) && s[j+1] == '>' { + return s[:j], s[j+2:], true + } + } + } + return "", "", false +} + +// assignments maps a variable assigned in this file to the expression it was +// assigned, so `$p = $x ? {...}; include $p` still yields names. +func assignments(body string) map[string]string { + out := map[string]string{} + for _, loc := range assignRe.FindAllStringSubmatchIndex(body, -1) { + name := body[loc[2]:loc[3]] + if _, seen := out[name]; seen { + continue + } + out[name] = statementExpr(body, loc[1]-1) + } + return out +} + +// statementExpr reads one expression starting at i, ending at the top-level +// semicolon, newline or closing delimiter that terminates the statement. +func statementExpr(s string, i int) string { + for i < len(s) && (s[i] == ' ' || s[i] == '\t') { + i++ + } + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; { + case c == '\'' || c == '"': + j = skipString(s, j) - 1 + case c == '(' || c == '[' || c == '{': + depth++ + case c == ')' || c == ']' || c == '}': + if depth == 0 { + return s[i:j] + } + depth-- + case c == ';' && depth == 0: + return s[i:j] + case c == '\n' && depth == 0 && !continued(s[i:j]): + return s[i:j] + } + } + return s[i:] +} + +// stripSource blanks out # comments and heredoc bodies. Both are text the +// class scan must not read: a `#` inside a string is not a comment, and an +// `include` in heredoc documentation is not an edge. Blanking rather than +// deleting keeps the line structure the statement scan relies on. +func stripSource(src string) string { + out := []byte(src) + blank := func(from, to int) { + for k := from; k < to && k < len(out); k++ { + if out[k] != '\n' { + out[k] = ' ' + } + } + } + + var pending []string + for i := 0; i < len(src); { + switch c := src[i]; { + case c == '\'' || c == '"': + i = skipString(src, i) + case c == '#': + j := i + for j < len(src) && src[j] != '\n' { + j++ + } + blank(i, j) + i = j + case c == '@' && i+1 < len(src) && src[i+1] == '(': + tag, next, ok := heredocTag(src, i+1) + if !ok { + i++ + continue + } + pending = append(pending, tag) + i = next + case c == '\n': + i++ + for _, tag := range pending { + i = blankHeredoc(src, blank, i, tag) + } + pending = pending[:0] + default: + i++ + } + } + return string(out) +} + +// heredocTag reads the tag out of @(EOT), @("EOT"), @(EOT/L) and @(EOT:json/nt). +func heredocTag(s string, i int) (string, int, bool) { + j := i + 1 + for j < len(s) && s[j] != ')' && s[j] != '\n' { + j++ + } + if j >= len(s) || s[j] != ')' { + return "", i + 1, false + } + spec := s[i+1 : j] + if k := strings.IndexAny(spec, ":/"); k >= 0 { + spec = spec[:k] + } + spec = strings.Trim(strings.TrimSpace(spec), `"'`) + if spec == "" { + return "", j + 1, false + } + return spec, j + 1, true +} + +// blankHeredoc blanks the lines from i up to and including the line that ends +// the heredoc, and returns the index just past it. +func blankHeredoc(s string, blank func(from, to int), i int, tag string) int { + for i < len(s) { + end := strings.IndexByte(s[i:], '\n') + if end < 0 { + blank(i, len(s)) + return len(s) + } + line := s[i : i+end] + blank(i, i+end) + i += end + 1 + if isHeredocEnd(line, tag) { + return i + } + } + return i +} + +// isHeredocEnd matches the terminator line, with its optional `|` indentation +// marker and `-` trailing-newline marker. +func isHeredocEnd(line, tag string) bool { + t := strings.TrimSpace(line) + t = strings.TrimSpace(strings.TrimPrefix(t, "|")) + t = strings.TrimSpace(strings.TrimPrefix(t, "-")) + return t == tag +} + +func isSpace(c byte) bool { return c == ' ' || c == '\t' || c == '\n' || c == '\r' } + +// callArguments returns the argument text of the function call whose name ends +// at i, in either the parenthesised or the bare statement form. It reports +// false when the keyword is not a call at all, such as a require => attribute. +func callArguments(s string, i int) (string, bool) { + if i >= len(s) { + return "", false + } + if s[i] != '(' && s[i] != ' ' && s[i] != '\t' { + return "", false + } + for i < len(s) && (s[i] == ' ' || s[i] == '\t') { + i++ + } + if i >= len(s) { + return "", false + } + if s[i] == '(' { + inner, _, ok := balanced(s, i, '(', ')') + return inner, ok + } + if s[i] == '=' || s[i] == ',' || s[i] == ';' || s[i] == '\n' { + return "", false + } + return statementExpr(s, i), true +} + +// splitArgs splits an argument list on its top-level commas. +func splitArgs(s string) []string { + var out []string + depth, start := 0, 0 + for j := 0; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + depth-- + case ',': + if depth == 0 { + out = append(out, s[start:j]) + start = j + 1 + } + } + } + return append(out, s[start:]) +} + +// resourceTitles returns the title expression of a resource body whose opening +// brace ends at i, i.e. everything up to the colon that closes the title. +func resourceTitles(s string, i int) string { + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case '(', '[', '{': + depth++ + case ')', ']', '}': + if depth == 0 { + return "" + } + depth-- + case ':': + if depth == 0 { + return s[i:j] + } + } + } + return "" +} + +// balanced returns the text between the delimiter at i and its match. +func balanced(s string, i int, opener, closer byte) (string, int, bool) { + depth := 0 + for j := i; j < len(s); j++ { + switch c := s[j]; c { + case '\'', '"': + j = skipString(s, j) - 1 + case opener: + depth++ + case closer: + depth-- + if depth == 0 { + return s[i+1 : j], j + 1, true + } + } + } + return "", len(s), false +} + +// skipString returns the index just past the quoted string starting at i. +func skipString(s string, i int) int { + quote := s[i] + for j := i + 1; j < len(s); j++ { + switch s[j] { + case '\\': + j++ + case quote: + return j + 1 + } + } + return len(s) +} + +// continued reports whether an argument list so far ends on a comma, meaning +// the list carries on over the newline. +func continued(s string) bool { + s = strings.TrimRight(s, " \t\r") + return strings.HasSuffix(s, ",") +} diff --git a/internal/impact/scan_test.go b/internal/impact/scan_test.go new file mode 100644 index 0000000..5152917 --- /dev/null +++ b/internal/impact/scan_test.go @@ -0,0 +1,249 @@ +package impact + +import ( + "reflect" + "testing" +) + +func TestScanSelectorArgumentKeepsEveryBranch(t *testing.T) { + tests := []struct { + name string + body string + want []string + }{ + { + name: "selector as the argument", + body: "class p {\n include $facts['role'] ? { 'web' => 'profile::web', default => 'profile::base' }\n}\n", + want: []string{"profile::base", "profile::web"}, + }, + { + name: "selector reached through a variable", + body: "class p {\n $profile = $facts['role'] ? { 'web' => 'profile::web', default => 'profile::base' }\n" + + " include $profile\n}\n", + want: []string{"profile::base", "profile::web"}, + }, + { + name: "selector inside a parenthesised call", + body: "class p {\n include(($x ? { default => 'profile::sel' }))\n}\n", + want: []string{"profile::sel"}, + }, + { + name: "selector arm holding an array", + body: "class p {\n include $x ? { 'a' => ['profile::a', 'profile::b'], default => 'profile::c' }\n}\n", + want: []string{"profile::a", "profile::b", "profile::c"}, + }, + { + name: "undef arm contributes nothing", + body: "class p {\n include $x ? { 'a' => 'profile::a', default => undef }\n}\n", + want: []string{"profile::a"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := refs(tc.body); !reflect.DeepEqual(got, tc.want) { + t.Errorf("references = %v, want %v", got, tc.want) + } + if got := unresolvedRefs(tc.body); len(got) != 0 { + t.Errorf("a fully resolved selector reported %v", got) + } + }) + } +} + +func TestScanHashInsideAStringIsNotAComment(t *testing.T) { + tests := []struct { + name string + body string + want []string + }{ + { + name: "double quoted", + body: "class p {\n $x = \"port #1\" ; include profile::afterhash\n}\n", + want: []string{"profile::afterhash"}, + }, + { + name: "single quoted", + body: "class p {\n $x = 'colour #fff' ; include profile::aftercolour\n}\n", + want: []string{"profile::aftercolour"}, + }, + { + name: "url fragment", + body: "class p {\n $u = 'https://h/p#frag'\n include profile::afterurl\n}\n", + want: []string{"profile::afterurl"}, + }, + { + name: "multi line string", + body: "class p {\n $s = \"#!/bin/sh\nexit 0\"\n include profile::afterscript\n}\n", + want: []string{"profile::afterscript"}, + }, + { + name: "a real comment is still stripped", + body: "class p {\n include profile::live # include profile::dead\n}\n", + want: []string{"profile::live"}, + }, + { + name: "a hash in a comment after a string", + body: "class p {\n $x = 'a' # ticket #42 include profile::dead\n include profile::live\n}\n", + want: []string{"profile::live"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := refs(tc.body); !reflect.DeepEqual(got, tc.want) { + t.Errorf("references = %v, want %v", got, tc.want) + } + }) + } +} + +func TestScanSkipsHeredocBodies(t *testing.T) { + body := "class p {\n" + + " $a = @(EOT)\n include profile::plain\n | EOT\n" + + " $b = @(\"EOT\")\n include profile::quoted\n |-EOT\n" + + " $c = @(EOT/L)\n include profile::escapes\n EOT\n" + + " $d = @(EOT:json/nt)\n include profile::syntax\n - EOT\n" + + " include profile::real\n}\n" + if got, want := refs(body), []string{"profile::real"}; !reflect.DeepEqual(got, want) { + t.Errorf("references = %v, want %v", got, want) + } + if got := unresolvedRefs(body); len(got) != 0 { + t.Errorf("heredoc scan reported %v", got) + } +} + +func TestScanHeredocBodyDoesNotSwallowFollowingCode(t *testing.T) { + // An unbalanced brace and quote in the body must not leak into the scan. + body := "class p {\n $a = @(EOF)\n { \"unclosed ' ]]] \n EOF\n include profile::after\n}\n" + if got, want := refs(body), []string{"profile::after"}; !reflect.DeepEqual(got, want) { + t.Errorf("references = %v, want %v", got, want) + } +} + +func TestScanReportsInterpolatedClassName(t *testing.T) { + body := "class p {\n include \"profile::role_${role}\"\n}\n" + if got := refs(body); len(got) != 0 { + t.Errorf("an interpolated name yielded %v, want nothing resolved", got) + } + got := unresolvedRefs(body) + want := []UnresolvedRef{{Form: "include", Expr: `"profile::role_${role}"`, Prefix: "profile::role_"}} + if !reflect.DeepEqual(got, want) { + t.Errorf("unresolved = %#v, want %#v", got, want) + } +} + +func TestScanReportsUnresolvableArguments(t *testing.T) { + tests := []struct { + name string + body string + want UnresolvedRef + }{ + { + name: "bare variable", + body: "class p {\n include $unknown\n}\n", + want: UnresolvedRef{Form: "include", Expr: "$unknown"}, + }, + { + name: "variable assigned from a function", + body: "class p {\n $c = lookup('profile')\n include $c\n}\n", + want: UnresolvedRef{Form: "include", Expr: "lookup('profile')"}, + }, + { + name: "function call argument", + body: "class p {\n contain lookup('profile')\n}\n", + want: UnresolvedRef{Form: "contain", Expr: "lookup('profile')"}, + }, + { + name: "class resource with a computed title", + body: "class p {\n class { $title: }\n}\n", + want: UnresolvedRef{Form: "class {}", Expr: "$title"}, + }, + { + name: "interpolated class reference", + body: "class p {\n Class[\"profile::${x}\"] -> Service['s']\n}\n", + want: UnresolvedRef{Form: "Class[]", Expr: `"profile::${x}"`, Prefix: "profile::"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := unresolvedRefs(tc.body) + if len(got) != 1 || got[0] != tc.want { + t.Errorf("unresolved = %#v, want [%#v]", got, tc.want) + } + }) + } +} + +func TestScanCaseInsensitiveClassTitle(t *testing.T) { + // Class names are case insensitive in a resource title, so a mixed-case + // one must resolve rather than widen. + body := "class p {\n service { 's': require => Class['Mysql::server'] }\n}\n" + if got, want := refs(body), []string{"mysql::server"}; !reflect.DeepEqual(got, want) { + t.Errorf("references = %v, want %v", got, want) + } + if got := unresolvedRefs(body); len(got) != 0 { + t.Errorf("unresolved = %v", got) + } +} + +func TestScanSurvivesMalformedInput(t *testing.T) { + bodies := []string{ + "", + "class p {", + "class p {\n include [foo, bar\n", + "class p {\n include 'foo\n", + "class p {\n include \"foo::${\n", + "class p {\n include (\n", + "class p {\n Class[\n", + "class p {\n class { : }\n}\n", + "class p {\n include $x ? {\n", + "class p {\n include $x ? { 'a' => }\n}\n", + "class p {\n $a = @(EOT)\n no terminator here\n", + "@(", + "@()\n", + "#", + "$", + "))))]]]}}}", + "include " + "a::b, " + "include include include", + "class p {\n include [[a::b, [c::d, e::f]], g::h]\n}\n", + } + for _, body := range bodies { + names, unresolved := scanReferences(stripSource(body)) + _, _ = names, unresolved + } + + // The nested-array case must still resolve every name. + got := refs("class p {\n include [[a::b, [c::d, e::f]], g::h]\n}\n") + want := []string{"a::b", "c::d", "e::f", "g::h"} + if !reflect.DeepEqual(got, want) { + t.Errorf("nested arrays = %v, want %v", got, want) + } +} + +func TestStripSourcePreservesLineCount(t *testing.T) { + // The statement scan reads newlines, so stripping must not move them. + body := "class p {\n $a = @(EOT)\n body\n EOT\n include foo # bar\n}\n" + stripped := stripSource(body) + if len(stripped) != len(body) { + t.Fatalf("stripped length %d, want %d", len(stripped), len(body)) + } + for i := range body { + if body[i] == '\n' && stripped[i] != '\n' { + t.Fatalf("newline at %d was stripped", i) + } + } +} + +func FuzzScanReferences(f *testing.F) { + f.Add("class p {\n include foo::bar\n}\n") + f.Add("class p {\n include $x ? { 'a' => 'b::c', default => undef }\n}\n") + f.Add("class p {\n $a = @(EOT)\n include x::y\n EOT\n}\n") + f.Add("class p {\n include \"a::${b}\" # '\n}\n") + f.Add("))))]]]}}}@(#'\"") + f.Fuzz(func(t *testing.T, body string) { + stripped := stripSource(body) + if len(stripped) != len(body) { + t.Fatalf("stripSource changed length: %d -> %d", len(body), len(stripped)) + } + scanReferences(stripped) + }) +} diff --git a/internal/impact/testdata/repo/site/profiles/manifests/base.pp b/internal/impact/testdata/repo/site/profiles/manifests/base.pp index 56da530..72ba24c 100644 --- a/internal/impact/testdata/repo/site/profiles/manifests/base.pp +++ b/internal/impact/testdata/repo/site/profiles/manifests/base.pp @@ -5,5 +5,6 @@ class profiles::base { class { 'profiles::viaclass': } include profiles::cycle::a include profiles::listed::a, profiles::listed::b + include profiles::dynamic profiles::defined::thing { 'x': } } diff --git a/internal/impact/testdata/repo/site/profiles/manifests/dynamic.pp b/internal/impact/testdata/repo/site/profiles/manifests/dynamic.pp new file mode 100644 index 0000000..5822a39 --- /dev/null +++ b/internal/impact/testdata/repo/site/profiles/manifests/dynamic.pp @@ -0,0 +1,3 @@ +class profiles::dynamic { + include "profiles::dyn_${facts['role']}" +}