refactor(spec): add selectable legacy and structural editors

[Thien Trung Vuong (trungams)]

Summary

This PR puts the existing line-oriented editor and the new structural editor behind the same public Spec API. The legacy implementation remains intact, while the structural implementation uses the parser and tree API introduced in the previous PR.

Editor selection happens once when a spec is opened. There are no mode checks scattered through individual operations and no automatic fallback from one editor to the other. This PR also adds a curated set of real-world spec fixtures to exercise the structural implementation. Production continues to use the legacy editor until the rollout configuration is introduced at the top of the stack.

Motivation

Issue #214 calls for structural awareness when editing sections and conditionals. At the same time, azldev is stabilizing and we want an escape hatch during adoption. Keeping both implementations behind one facade gives projects a controlled migration path without changing the public editing API.

Changes

• Add a private editor interface and constructor.
• Keep Spec as the unchanged public facade.
• Preserve the legacy visitor and line-oriented implementation.
• Add the structural implementation for section, tag, search, patch, and changelog operations.
• Preserve current-main release and provenance behavior.
• Keep structural visitors fully structural; macro bodies remain opaque to tag traversal.
• Preserve sequential search/replace overlays that temporarily pass through an unbalanced conditional state.
• Add curated real-world fixtures and deterministic parser/edit stress coverage.

Validation

• mage build
• mage unit

The cumulative stack was mechanically rebased onto eb9fb3f and revalidated. The complete rollout was also exercised against the full Azure Linux structural render corpus and the default-legacy E2E suite.

Known limitations

Macro preservation during section and subpackage removal is added by the next PR. The structural editor does not evaluate conditional expressions, so content after a wrapper's %endif cannot always be attributed to a section declared inside that wrapper. There is intentionally no automatic fallback between editors.

[Copilot]

🟡 Changes recommended

Unresolved moderate findings affect release handling, structural edits, tag matching, and legacy mutation coverage.

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

Pull request overview

This PR unifies legacy and structural RPM spec editing behind the existing Spec API while preserving legacy behavior by default.

Changes:

• Adds selectable editor implementations and facade delegation.
• Implements structural section, tag, search, patch, and changelog operations.
• Propagates editor selection through source preparation, overlays, release, and provenance handling.
• Adds fixtures and regression tests for macro- and conditional-heavy specs.

File summaries

File	Summary / review note
internal/rpm/spec/tree_fixture_internal_test.go	Structural fixture stress tests.
internal/rpm/spec/testdata/specs/subpackage-define-unreferenced.spec	Fixture for unreferenced subpackage definitions.
internal/rpm/spec/testdata/specs/straddling-wrapper.spec	Fixture for sections straddling wrappers.
internal/rpm/spec/testdata/specs/script-section-tag-shaped.spec	Fixture for script and tag-shaped sections.
internal/rpm/spec/testdata/specs/nested-wrappers.spec	Fixture for nested conditional wrappers.
internal/rpm/spec/testdata/specs/multi-package-mixed.spec	Fixture for mixed multi-package content.
internal/rpm/spec/testdata/specs/macro-with-parameters.spec	Fixture for parameterized macros.
internal/rpm/spec/testdata/specs/macro-continuation.spec	Fixture for macro continuations.
internal/rpm/spec/testdata/specs/macro-conditional.spec	Fixture for macro conditionals.
internal/rpm/spec/testdata/specs/if-with-continuation.spec	Fixture for conditional continuations.
internal/rpm/spec/testdata/specs/elif-with-sections.spec	Fixture for sections within elif branches.
internal/rpm/spec/testdata/specs/elif-chain.spec	Fixture for elif chains.
internal/rpm/spec/testdata/specs/comment-only-conditional.spec	Fixture for comment-only conditionals.
internal/rpm/spec/testdata_test.go	Structural fixture edit tests.
internal/rpm/spec/structural_tree_api.go	Tree accessors and mutations; section-name matching needs to remain case-insensitive (moderate, 2 votes).
internal/rpm/spec/structural_tree_api_internal_test.go	Structural tree API tests.
internal/rpm/spec/structural_spec.go	Structural spec representation.
internal/rpm/spec/structural_edit.go	Structural editing operations; replacement-driven continuation changes can leave classification stale (moderate, 1 vote).
internal/rpm/spec/spec_test.go	Spec behavior tests.
internal/rpm/spec/legacy_spec.go	Preserved legacy spec implementation.
internal/rpm/spec/legacy_edit.go	Preserved legacy editing operations.
internal/rpm/spec/editor.go	Public facade and editor selection; exported facade methods need documentation (nit, 1 vote).
internal/rpm/spec/edit_test.go	Editor behavior tests; retain default-mode mutation coverage (moderate, 1 vote).
internal/app/azldev/core/sources/upstream_provenance.go	Editor propagation for provenance handling.
internal/app/azldev/core/sources/upstream_provenance_internal_test.go	Provenance parser tests.
internal/app/azldev/core/sources/sourceprep.go	Spec editor preparer option; option documentation is misplaced (nit, 2 votes).
internal/app/azldev/core/sources/release.go	Release tag editor integration; preserve empty-value handling (moderate, 2 votes).
internal/app/azldev/core/sources/release_test.go	Release parsing tests.
internal/app/azldev/core/sources/release_internal_test.go	Release bump tests.
internal/app/azldev/core/sources/overlays.go	Overlay editor propagation.
internal/app/azldev/core/sources/overlays_test.go	Sequential overlay tests.

Review details

Suppressed comments (3)

internal/rpm/spec/edit_test.go:283

• These cases now explicitly open EditorStructural, so the edit tables that previously exercised the default editor no longer verify the preserved legacy implementation. Production still defaults to legacy, and the remaining default-mode tests cover visitors/read-only behavior rather than these mutations; retain default-mode coverage or run the mutation tables against both editor modes.

			specFile, err := spec.OpenSpec(strings.NewReader(test.input), spec.WithEditor(spec.EditorStructural))

internal/rpm/spec/editor.go:116

• The new public Spec facade methods (starting with ReplaceLine/RemoveLine) have no name-prefixed documentation; the comments on the old legacy implementations no longer document these declarations. Because the repository enables revive's exported rule (.golangci.yml:147-152), this file will fail the configured lint and the public API loses its method contracts. Add concise doc comments to each exported facade method.

func (s *Spec) ReplaceLine(lineNumber int, replacement string) {
	s.editor.ReplaceLine(lineNumber, replacement)
}
func (s *Spec) RemoveLine(lineNumber int)  { s.editor.RemoveLine(lineNumber) }
func (s *Spec) RemoveLines(start, end int) { s.editor.RemoveLines(start, end) }
func (s *Spec) InsertLinesAt(lines []string, lineNumber int) {
	s.editor.InsertLinesAt(lines, lineNumber)
}

internal/rpm/spec/structural_edit.go:604

• headerAt and the macro/conditional state are derived from the original lines, even though replacements are written back to lines before the next iteration. If a replacement adds or removes the trailing \\ on a %define/%global line, subsequent %package or %if lines change between macro text and structural content, but this walk still uses the old classification; section-scoped replacements can then target the wrong section or be skipped. Recompute classification from the updated lines (or explicitly reject replacements that change continuation structure).

	headers := findSectionHeaderLines(lines)
	headerAt := make(map[int]bool, len(headers))

• Files reviewed: 31/31 changed files
• Comments generated: 3
• Review effort level: Lite

---

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