manualpathconcat: 2-operand embedded-slash literal is never detected

[github-actions]

Summary

manualpathconcat (pkg/linters/manualpathconcat/manualpathconcat.go) only detects the 3-operand chain shape X + "/" + Y, where the middle operand must be the exact string literal "/". It never detects the equally common 2-operand shape X + "/subpath", where the leading slash is embedded in a longer literal (e.g. dir + "/config.yml" or home + "/.config/foo").

Evidence

matchSlashSeparator (manualpathconcat.go:120-137) requires bin.X to itself be a *ast.BinaryExpr whose Y operand satisfies isSlashLiteral — and isSlashLiteral (line 140) requires the unquoted literal to equal exactly "/":

func isSlashLiteral(expr ast.Expr) bool {
	lit, ok := expr.(*ast.BasicLit)
	if !ok || lit.Kind != token.STRING {
		return false
	}
	val, err := strconv.Unquote(lit.Value)
	return err == nil && val == "/"
}

So the 2-operand form dir + "/subpath" is structurally excluded before any slash-detection even runs: bin.X (dir) is an *ast.Ident, not a *ast.BinaryExpr, so isBinary is false and matchSlashSeparator returns immediately. The same limitation applies to analyzeAssignStmt's += handling, which requires bin.X to be exactly "/" via isSlashLiteral(bin.X) (line 94).

The testdata file (pkg/linters/manualpathconcat/testdata/src/manualpathconcat/manualpathconcat.go) confirms this is untested territory:

• goodSuffixOnly (line 63): dir + "/" — explicitly "not flagged" (trailing separator alone, no path segment)
• goodPrefixOnly (line 68): "/" + name — explicitly "not flagged" (leading separator alone, no base segment)

Neither of these is the same as dir + "/subpath" (a base identifier plus a literal that starts with a slash and continues with real path text) — that specific, and arguably most common, real-world shape has no test case at all, positive or negative.

Why this matters

The package doc states the analyzer's purpose broadly: "flags string concatenation using a literal '/' separator to build filesystem paths." The 2-operand embedded-slash form is squarely within that stated purpose (it carries exactly the same correctness hazards the doc calls out — double slashes, no Clean-style normalization, hard-coded OS separator) but is silently unimplemented. This is the same pattern_set_too_narrow bug class previously found and fixed in errstringmatch (#40244, Contains-only), sprintferrdot (verb-set), and lenstringzero (#54717, EQL/NEQ-only) — a plausible, common real-world spelling of the target pattern that the matcher's shape requirement silently excludes.

Recommendation

Extend matchSlashSeparator (and the += counterpart) to also match the 2-operand shape: X + Y where Y is a string literal that starts with / and has additional content after it (i.e. starts with / but is not exactly /), while still excluding the goodSuffixOnly-style bare separator case. Add corresponding testdata cases (e.g. bad2OpEmbeddedSlash) and adapt the diagnostic message helper to quote the literal's leading path segment sensibly.

Effort

Small — the fix is localized to matchSlashSeparator/isSlashLiteral and a few new testdata cases; no signature/type-analysis changes needed.

---

Run: §33290967014

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• api.anthropic.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.anthropic.com"

See Network Configuration for more information.

Generated by 🤖 Sergo - Serena Go Expert · claude · agent · 357.1 AIC · ⌖ 5.55 AIC · ⊞ 6.8K · ◷

• expires on Sep 5, 2026, 8:01 PM UTC-08:00