Skip to content

Conversation

sjungling
Copy link
Contributor

Migrated all security recipes from static YamlHelper utility methods to
the YamlScalarAccessor trait pattern. This improves code organization and
follows OpenRewrite best practices for LST manipulation.

Changes:

  • Created YamlScalarAccessor trait with 6 default methods
  • Migrated 7 recipes to implement the trait
  • Deleted deprecated YamlHelper utility class
  • All tests pass

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite Oct 15, 2025
@sjungling sjungling force-pushed the refactor/gha-yml-traits branch from f0f9f53 to 0a891a8 Compare October 15, 2025 20:56
Migrated all security recipes from static YamlHelper utility methods to
the YamlScalarAccessor trait pattern. This improves code organization and
follows OpenRewrite best practices for LST manipulation.

Changes:
- Created YamlScalarAccessor trait with 6 default methods
- Migrated 7 recipes to implement the trait
- Deleted deprecated YamlHelper utility class
- All tests pass
@sjungling sjungling force-pushed the refactor/gha-yml-traits branch from 0a891a8 to 47ff257 Compare October 15, 2025 20:57
Renamed org.openrewrite.github.traits to org.openrewrite.github.util
to better reflect that YamlScalarAccessor is a utility interface/mixin
rather than an OpenRewrite trait (which should extend Trait<T>).

Changes:
- Moved package from traits/ to util/
- Updated all imports in recipe files
- Updated package-info documentation
- All tests pass
Comment on lines +23 to +25
import org.openrewrite.yaml.YamlIsoVisitor;
import org.openrewrite.yaml.tree.Yaml;

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import org.openrewrite.yaml.YamlIsoVisitor;
import org.openrewrite.yaml.tree.Yaml;
import org.openrewrite.yaml.YamlIsoVisitor;

Comment on lines +62 to +64
.collect(Collectors.toList());

assertThat(steps).hasSize(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
assertThat(steps).hasSize(2);
.toList();
assertThat(steps.getFirst().getActionName()).isEqualTo("actions/checkout");
assertThat(steps.getFirst().getActionVersion()).isEqualTo("v3");

Comment on lines +90 to +91
.collect(Collectors.toList());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
.toList();
assertThat(steps.getFirst().getActionName()).isEqualTo("actions/setup-java");

spec -> spec.afterRecipe(results -> {
ActionStep.Matcher matcher = new ActionStep.Matcher();
List<ActionStep> steps = matcher.lower(results)
.collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
.toList();

Comment on lines +137 to +141
.collect(Collectors.toList());

// Test semantic version
assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout");
assertThat(steps.get(0).getActionVersion()).isEqualTo("v3");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
// Test semantic version
assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout");
assertThat(steps.get(0).getActionVersion()).isEqualTo("v3");
.toList();
assertThat(steps.getFirst().getActionName()).isEqualTo("actions/checkout");
assertThat(steps.getFirst().getActionVersion()).isEqualTo("v3");
assertThat(steps.getFirst().getActionOwner()).isEqualTo("actions");
assertThat(steps.getFirst().isVersionPinned()).isFalse();

Comment on lines +195 to +198
.collect(Collectors.toList());

assertThat(steps).hasSize(1);
assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
assertThat(steps).hasSize(1);
assertThat(steps.get(0).getActionName()).isEqualTo("actions/checkout");
.toList();
assertThat(steps.getFirst().getActionName()).isEqualTo("actions/checkout");
assertThat(steps.getFirst().getActionVersion()).isNull();
assertThat(steps.getFirst().isVersionPinned()).isFalse();

Comment on lines +221 to +222
.collect(Collectors.toList());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
.toList();
assertThat(steps.getFirst().getActionName()).isEqualTo("actions/checkout");

spec -> spec.afterRecipe(results -> {
ActionStep.Matcher matcher = new ActionStep.Matcher();
List<ActionStep> steps = matcher.lower(results)
.collect(Collectors.toList());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
.toList();

Comment on lines +279 to +282
.collect(Collectors.toList());

// Both should match - local actions are still action steps
assertThat(steps).hasSize(2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
// Both should match - local actions are still action steps
assertThat(steps).hasSize(2);
.toList();
assertThat(steps.getFirst().getActionRef()).isEqualTo("./local-action");
assertThat(steps.getFirst().getActionName()).isEqualTo("./local-action");
assertThat(steps.getFirst().getActionOwner()).isNull(); // No owner for local actions

Comment on lines +305 to +306
.collect(Collectors.toList());

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.collect(Collectors.toList());
.toList();
assertThat(steps.getFirst().getActionRef()).isEqualTo("docker://alpine:3.8");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant