refactor(spec): add lossless structural parser and tree API

[Thien Trung Vuong (trungams)]

Summary

RPM spec files are only loosely structured, and the existing line-oriented editor has to infer section boundaries after the fact. That becomes fragile around nested conditionals, multiline macros, and sections that begin or end inside %if blocks.

This PR adds the private foundation for a structural editor. It parses a spec into a lossless tree of sections, conditionals, text, and macro definitions, and provides transactional query and mutation primitives over that tree. Parsing and serializing a valid spec preserves its contents byte for byte.

Nothing selects this parser in production yet, and the public overlay configuration is unchanged.

Motivation

Issue #214 collects several cases where line ranges are not enough to preserve section and conditional boundaries safely. A structural representation lets later changes reason about those boundaries directly without trying to evaluate RPM conditions or expand macros.

Changes

• Parse %if/%elif/%else/%endif, including nested and empty branches.
• Keep directive-shaped text inside multiline %define and %global bodies opaque.
• Preserve backslash continuations, brace-delimited macros, Lua bodies, shell ${...} expressions, comments, blank lines, and original ordering.
• Serialize valid input byte-for-byte.
• Reject malformed structures instead of panicking or silently truncating them.
• Add the private structuralSpec, specTree, and section handles under their final filenames and types.
• Add transactional parse/inspect/mutate helpers and section query primitives.

Validation

• mage build
• mage unit

The cumulative stack was mechanically rebased onto eb9fb3f and revalidated after the rebase.

Known limitations

The parser deliberately does not evaluate RPM macros or conditional expressions. Sections created dynamically through macro expansion are therefore not visible to it. Public editor selection and actual overlay integration arrive in later PRs in the stack.

[Copilot]

🔵 Needs a closer look

The PR adds a substantial new parsing subsystem with complex lexical/state handling, so it warrants final human review despite strong test coverage and only minor review findings.

Pull request overview

Introduces a private, lossless structural RPM spec parser that builds a block tree (sections, conditionals, text, macro definitions) and provides a transactional tree API for querying and performing safe structural edits while preserving byte-for-byte serialization.

Changes:

• Added a two-pass structural parser and serializer to round-trip spec input without altering whitespace/comments.
• Added internal specTree / sectionHandle APIs for section queries and transactional mutation with post-mutation validation.
• Added unit tests covering conditional nesting/branches, macro continuation opacity (including Lua/expand bodies), and removal validation invariants.

File summaries

File	Description
internal/rpm/spec/tree.go	Implements the structural parse/serialize logic and macro/conditional/section scanning helpers.
internal/rpm/spec/tree_test.go	Adds round-trip and malformed-input tests for the structural parser, plus macro edge cases.
internal/rpm/spec/tree_raw_braces_test.go	Adds regression coverage for raw-brace shell fragments inside %{expand: ...} macro bodies.
internal/rpm/spec/structural_tree_api.go	Adds internal tree query/mutation primitives (sections, append/prepend lines, remove sections).
internal/rpm/spec/structural_tree_api_internal_test.go	Adds transactional semantics tests and removal-safety validation tests for the tree API.
internal/rpm/spec/structural_spec.go	Introduces the private structuralSpec wrapper holding raw spec lines.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The change introduces a substantial new parsing subsystem whose correctness depends on many subtle edge cases, warranting careful human review despite good test coverage.

Review details

Suppressed comments (2)

internal/rpm/spec/structural_tree_api.go:232

• Same as above: this can refer to a '%elif…' header as an “%if block”. Using a neutral term like “conditional block” would make the error accurate for both %if and %elif nodes.

		if wouldEmptySectionWrapper(child, removeSet) && index+1 < len(children) {
			next := children[index+1]
			if next.Kind == conditionalBlock && !containsSectionBlocks(next) && conditionalHasTextOrMacroContent(next) {
				return fmt.Errorf("content in %%if block at %#q would be orphaned after removing the preceding section:\n%w",
					next.Header, ErrConditionalSpansSections)
			}

internal/rpm/spec/tree.go:232

• The comment says this function respects general backslash continuations, but the implementation only skips directive-shaped lines inside multi-line '%define'/'%global' bodies (and intentionally does not treat ordinary '\' continuations as structural suppression per parseTree's doc comment). This is misleading for future maintainers.

// findSectionHeaderLines returns the 0-indexed line numbers of all section headers,
// respecting line continuations (backslash-terminated lines suppress the next line).

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Antonio Salinas (Tonisal-byte)]

The parser deliberately represents the preamble as an implicit section with empty name and package. Sections("", "") can return that handle, and RemoveSections() accepts it without restriction in structural_tree_api.go:82-128.

The existing API explicitly rejects this operation in edit.go:810-815. Removing the structural preamble would discard global fields such as Name, Version, Release, and sources.

[Thien Trung Vuong (trungams)]

Thanks for the catch!

[Antonio Salinas (Tonisal-byte)]

NIT: It seems only the unit test uses this function; do we still need it? I understand if it's required for later PRs in the stack :D

[Thien Trung Vuong (trungams)]

It is used later on when we need to preserve macro definitions when removing sections of a spec file

[Copilot]

🟡 Changes recommended

Conditional branch validation and section-removal safety issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

internal/rpm/spec/structural_tree_api.go:235

• When the last section in a wrapper is removed, this safety check only examines a following conditional block. For input such as %if 1, %package one, %endif, followed by a non-empty text or macro block before the next section, removal succeeds and leaves content whose RPM section ownership is ambiguous (and can leak the removed section's tags or commands). Reject any non-empty non-section siblings in the straddling area, not only conditional siblings, or otherwise preserve them transactionally.

		if wouldEmptySectionWrapper(child, removeSet) && index+1 < len(children) {
			next := children[index+1]
			if next.Kind == conditionalBlock && !containsSectionBlocks(next) && conditionalHasTextOrMacroContent(next) {
				return fmt.Errorf("content in conditional block at %#q would be orphaned after removing the preceding section:\n%w",
					next.Header, ErrConditionalSpansSections)

internal/rpm/spec/tree.go:338

• This parser delegates opener detection to conditionalDepthChange, which recognizes only %if, %ifarch, %ifnarch, %ifos, and %ifnos. Valid RPM conditionals such as %ifmacro, %ifnmacro, %ifexist, and %ifnot are therefore treated as ordinary text; a section inside one is lifted to the surrounding tree, so later section edits can move content outside its conditional. Extend the shared conditional matcher (and branch matcher as needed) before using it to build the tree.

		if conditionalDepthChange(line) == 1 {

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite