Skip to content

De-duplicate declarative recipes#6560

Closed
sambsnyd wants to merge 10 commits intomainfrom
no-duplicate-declaratives
Closed

De-duplicate declarative recipes#6560
sambsnyd wants to merge 10 commits intomainfrom
no-duplicate-declaratives

Conversation

@sambsnyd
Copy link
Copy Markdown
Member

@sambsnyd sambsnyd commented Jan 17, 2026

Let's stop spending time executing recipes that shouldn't do anything anyway - because a copy of them earlier in the recipe list already did the work

When initializing DeclarativeRecipe instances, work with a copy of the
uninitialized list rather than modifying the original. This ensures that
when the same recipe instance is initialized in multiple contexts, the
modifications don't corrupt the instance state for other uses.

The fix:
1. Create a copy of the uninitialized list before iteration
2. Remove duplicates from the copy during deduplication
3. Clear the original uninitialized list at the end to mark initialization complete

This preserves recipe instance integrity across multiple initializations.
@github-project-automation github-project-automation Bot moved this to In Progress in OpenRewrite Jan 17, 2026
@sambsnyd sambsnyd changed the title no-duplicate-declaratives Deduplicate declarative recipes Jan 17, 2026
@sambsnyd sambsnyd changed the title Deduplicate declarative recipes De-duplicate declarative recipes Jan 17, 2026
Comment thread rewrite-core/src/main/java/org/openrewrite/config/DeclarativeRecipe.java Outdated
Copy link
Copy Markdown
Member

@jkschneider jkschneider left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we please wait until at least Monday. I'm not saying it's wrong but would like to carefully consider. Very much worry about this.

@jkschneider
Copy link
Copy Markdown
Member

jkschneider commented Jan 18, 2026

One of my initial concerns: it feels pretty wrong to apply this uniquely to DeclarativeRecipe. Like using buildRecipe with a language that has less verbose syntax, I don't think YAML even has much attraction. And yet the same consideration should apply in those situations. Feels like the wrong place to solve this.

@sambsnyd
Copy link
Copy Markdown
Member Author

We will do this instead: #6619

@sambsnyd sambsnyd closed this Jan 27, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Done in OpenRewrite Jan 27, 2026
@sambsnyd sambsnyd deleted the no-duplicate-declaratives branch April 1, 2026 05:21
sambsnyd added a commit that referenced this pull request Apr 24, 2026
…leton recipes. (#7468)

I had proposed something very similar to this before, here: #6560

That didn't get merged because it felt too magical / special case for DeclarativeRecipe to deduplicate as a silent implementation detail. Instead of doing this the first time we decided on creating Singleton.

But with Singleton alone I cannot find a way to guard the scanning phase without breaking ScanningRecipe.generate(). I tried several things. But a common usage for scanning recipes is to verify whether a file already exists, if I skip those then generate() recipes will create files they shouldn't because their scanner was skipped by Singleton.

So I'm resurrecting the declarative recipe duplicate loading skip. But this time I'm keying off the presence of Singleton, which explicitly opts-in a recipe to desiring this behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants