From 777bf6c5d51c4e8ae1c12c4b1ae1f0a005b69cbc Mon Sep 17 00:00:00 2001 From: Tyleresch <113705714+Tyleresch@users.noreply.github.com> Date: Tue, 25 Aug 2026 11:06:29 -0500 Subject: [PATCH 1/4] Add recipe to replace always conditions --- .../ReplaceAlwaysWithSuccessOrFailure.java | 117 ++++++++ .../resources/META-INF/rewrite/examples.yml | 28 ++ .../resources/META-INF/rewrite/recipes.csv | 1 + ...ReplaceAlwaysWithSuccessOrFailureTest.java | 280 ++++++++++++++++++ 4 files changed, 426 insertions(+) create mode 100644 src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java create mode 100644 src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java diff --git a/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java b/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java new file mode 100644 index 0000000..0227294 --- /dev/null +++ b/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java @@ -0,0 +1,117 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github; + +import lombok.Getter; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.yaml.JsonPathMatcher; +import org.openrewrite.yaml.YamlIsoVisitor; +import org.openrewrite.yaml.trait.BlockScalar; +import org.openrewrite.yaml.tree.Yaml; + +import java.util.Optional; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class ReplaceAlwaysWithSuccessOrFailure extends Recipe { + private static final Pattern ALWAYS_CALL = Pattern.compile("(? getVisitor() { + return Preconditions.check(new IsGitHubActionsFile(), new YamlIsoVisitor() { + private final JsonPathMatcher jobCondition = new JsonPathMatcher("$.jobs.*.if"); + private final JsonPathMatcher stepCondition = new JsonPathMatcher("$..steps[*].if"); + + @Override + public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { + Yaml.Mapping.Entry e = super.visitMappingEntry(entry, ctx); + if ((jobCondition.matches(getCursor()) || stepCondition.matches(getCursor())) && + e.getValue() instanceof Yaml.Scalar) { + Yaml.Scalar condition = (Yaml.Scalar) e.getValue(); + Optional blockScalar = new BlockScalar.Matcher().get(condition, getCursor()); + String value = blockScalar.isPresent() ? blockScalar.get().getBody() : condition.getValue(); + String updated = replaceAlways(value); + if (!value.equals(updated)) { + return e.withValue(blockScalar.isPresent() ? + blockScalar.get().withBody(updated) : condition.withValue(updated)); + } + } + return e; + } + }); + } + + private static String replaceAlways(String condition) { + Matcher matcher = ALWAYS_CALL.matcher(condition); + String expression = condition.trim(); + if (expression.startsWith("${{") && expression.endsWith("}}")) { + expression = expression.substring(3, expression.length() - 2).trim(); + } + + String replacement = ALWAYS_CALL.matcher(expression).matches() ? + REPLACEMENT : "(" + REPLACEMENT + ")"; + StringBuilder updated = null; + int lastMatchEnd = 0; + while (matcher.find()) { + if (isInsideStringLiteral(condition, matcher.start())) { + continue; + } + if (updated == null) { + updated = new StringBuilder(condition.length() + replacement.length()); + } + updated.append(condition, lastMatchEnd, matcher.start()).append(replacement); + lastMatchEnd = matcher.end(); + } + return updated == null ? condition : updated.append(condition, lastMatchEnd, condition.length()).toString(); + } + + private static boolean isInsideStringLiteral(String expression, int offset) { + boolean singleQuoted = false; + boolean doubleQuoted = false; + for (int i = 0; i < offset; i++) { + char current = expression.charAt(i); + if (singleQuoted) { + if (current == '\'' && i + 1 < offset && expression.charAt(i + 1) == '\'') { + i++; + } else if (current == '\'') { + singleQuoted = false; + } + } else if (doubleQuoted) { + if (current == '\\' && i + 1 < offset) { + i++; + } else if (current == '"') { + doubleQuoted = false; + } + } else if (current == '\'') { + singleQuoted = true; + } else if (current == '"') { + doubleQuoted = true; + } + } + return singleQuoted || doubleQuoted; + } +} diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 1cda55a..107cc44 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/src/main/resources/META-INF/rewrite/examples.yml @@ -541,6 +541,34 @@ examples: language: yaml --- type: specs.openrewrite.org/v1beta/example +recipeName: org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure +examples: +- description: '`ReplaceAlwaysWithSuccessOrFailureTest#replacesJobAndStepConditions`' + sources: + - before: | + on: push + jobs: + build: + if: always() + runs-on: ubuntu-latest + steps: + - name: Upload results + if: ${{ always() }} + run: ./upload-results.sh + after: | + on: push + jobs: + build: + if: success() || failure() + runs-on: ubuntu-latest + steps: + - name: Upload results + if: ${{ success() || failure() }} + run: ./upload-results.sh + path: .github/workflows/ci.yml + language: yaml +--- +type: specs.openrewrite.org/v1beta/example recipeName: org.openrewrite.github.ReplaceDependabotReviewersWithCodeowners examples: - description: '`ReplaceDependabotReviewersWithCodeownersTest#migrateReviewersToNewCodeownersFile`' diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 81e0f49..715f39f 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -22,6 +22,7 @@ maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.Prefe maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveAllCronTriggers,Remove all cron triggers,Removes all cron triggers from a workflow.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveUnusedWorkflowDispatchInputs,Remove unused workflow dispatch inputs,Remove workflow_dispatch inputs that are not referenced anywhere in the workflow file.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveWorkflowInputArgument,Remove workflow input argument,Remove a specific input argument from calls to a reusable workflow.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""workflowReference"",""type"":""String"",""displayName"":""Workflow reference"",""description"":""The workflow reference to match (e.g., `org/repo/.github/workflows/myWorkflow.yml`)."",""example"":""org/repo/.github/workflows/myWorkflow.yml"",""required"":true},{""name"":""version"",""type"":""String"",""displayName"":""Version"",""description"":""The version of the workflow to match (e.g., `v1.2.3`)."",""example"":""v1.2.3"",""required"":true},{""name"":""inputArgumentName"",""type"":""String"",""displayName"":""Input argument name"",""description"":""The name of the input argument to remove."",""example"":""myInputToRemove"",""required"":true}]", +maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure,Replace `always()` with `success() || failure()`,Replace `always()` in GitHub Actions job and step conditions with `success() || failure()` so that canceled workflows do not continue running or hang until they time out.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceDependabotReviewersWithCodeowners,Replace Dependabot `reviewers` with `CODEOWNERS`,"Replaces the [removed](https://github.blog/changelog/2025-04-29-dependabot-reviewers-configuration-option-being-replaced-by-code-owners/) `reviewers` option in `.github/dependabot.yml` with equivalent `CODEOWNERS` entries. Each reviewer is mapped onto the manifest files Dependabot updates for that `package-ecosystem` and `directory`, so ownership stays as narrow as the Dependabot configuration was. Update entries whose `package-ecosystem` has no known manifests are left untouched.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""codeownersPath"",""type"":""String"",""displayName"":""`CODEOWNERS` path"",""description"":""Where to write the migrated reviewers when the repository does not have a `CODEOWNERS` file yet. Defaults to `.github/CODEOWNERS`. When a `CODEOWNERS` file already exists in any of the locations GitHub recognizes, that file is appended to instead and this option is ignored."",""example"":""CODEOWNERS""}]", maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceOssrhSecretsWithSonatype,Replace OSSRH secrets with Sonatype secrets,Replace deprecated OSSRH_S01 secrets with new Sonatype secrets in GitHub Actions workflows. This is an example use of the `ReplaceSecrets` and `ReplaceSecretKeys` recipes combined used to update the Maven publishing secrets in OpenRewrite's GitHub organization.,5,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceRunners,Replace runners for a job,Replaces the runners of a given job.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""jobName"",""type"":""String"",""displayName"":""Job Name"",""description"":""The name of the job to update, use * to affect all the workflow jobs"",""example"":""build"",""required"":true},{""name"":""runners"",""type"":""List"",""displayName"":""Runners"",""description"":""The new list of runners to set"",""example"":""ubuntu-latest"",""required"":true}]", diff --git a/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java b/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java new file mode 100644 index 0000000..0bbef37 --- /dev/null +++ b/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java @@ -0,0 +1,280 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.github; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.yaml.Assertions.yaml; + +class ReplaceAlwaysWithSuccessOrFailureTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceAlwaysWithSuccessOrFailure()); + } + + @DocumentExample + @Test + void replacesJobAndStepConditions() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + if: always() + runs-on: ubuntu-latest + steps: + - name: Upload results + if: ${{ always() }} + run: ./upload-results.sh + """, + """ + on: push + jobs: + build: + if: success() || failure() + runs-on: ubuntu-latest + steps: + - name: Upload results + if: ${{ success() || failure() }} + run: ./upload-results.sh + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void replacesCompositeActionStepCondition() { + rewriteRun( + //language=yaml + yaml( + """ + name: Upload results + description: Upload test results + runs: + using: composite + steps: + - if: always() + shell: bash + run: ./upload-results.sh + """, + """ + name: Upload results + description: Upload test results + runs: + using: composite + steps: + - if: success() || failure() + shell: bash + run: ./upload-results.sh + """, + spec -> spec.path(".github/actions/upload/action.yml") + ) + ); + } + + @Test + void preservesPrecedenceInLargerExpressions() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: always() && github.ref == 'refs/heads/main' + run: ./publish.sh + - if: ${{ cancelled() || always() }} + run: ./cleanup.sh + """, + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: (success() || failure()) && github.ref == 'refs/heads/main' + run: ./publish.sh + - if: ${{ cancelled() || (success() || failure()) }} + run: ./cleanup.sh + """, + spec -> spec.path(".github/workflows/ci.yaml") + ) + ); + } + + @Test + void replacesCallsWithInternalWhitespace() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + if: ${{ always ( ) }} + runs-on: ubuntu-latest + steps: + - run: ./build.sh + """, + """ + on: push + jobs: + build: + if: ${{ success() || failure() }} + runs-on: ubuntu-latest + steps: + - run: ./build.sh + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void preservesBlockScalarStyle() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: >- + always() && + github.ref == 'refs/heads/main' + run: ./publish.sh + """, + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: >- + (success() || failure()) && + github.ref == 'refs/heads/main' + run: ./publish.sh + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void doesNotChangeOtherConditionsOrValues() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + if: notalways() + runs-on: ubuntu-latest + env: + DESCRIPTION: always() + steps: + - if: success() || failure() + run: echo always() + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void doesNotReplaceTextInsideStringLiterals() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: contains(github.event.head_commit.message, 'always()') + run: ./build.sh + - if: always() && contains(github.event.head_commit.message, 'always()') + run: ./publish.sh + """, + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: contains(github.event.head_commit.message, 'always()') + run: ./build.sh + - if: (success() || failure()) && contains(github.event.head_commit.message, 'always()') + run: ./publish.sh + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + + @Test + void doesNotChangeUnrelatedIfKeys() { + rewriteRun( + //language=yaml + yaml( + """ + name: Example + inputs: + if: + description: A regular input named if + default: always() + runs: + using: composite + steps: + - run: ./build.sh + shell: bash + """, + spec -> spec.path("action.yml") + ) + ); + } + + @Test + void doesNotChangeNonGitHubActionsYaml() { + rewriteRun( + //language=yaml + yaml( + """ + jobs: + build: + if: always() + steps: + - if: always() + run: ./build.sh + """, + spec -> spec.path("pipeline.yml") + ) + ); + } +} From da2d8048d00ea5b17549c1cdfc16f652a72c36cd Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 25 Aug 2026 19:34:20 +0200 Subject: [PATCH 2/4] Add ReplaceAlwaysWithSuccessOrFailure to GitHubActionsBestPractices --- src/main/resources/META-INF/rewrite/github.yml | 3 ++- src/main/resources/META-INF/rewrite/recipes.csv | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/github.yml b/src/main/resources/META-INF/rewrite/github.yml index d5e6e98..879f968 100644 --- a/src/main/resources/META-INF/rewrite/github.yml +++ b/src/main/resources/META-INF/rewrite/github.yml @@ -18,7 +18,7 @@ type: specs.openrewrite.org/v1beta/recipe name: org.openrewrite.github.GitHubActionsBestPractices displayName: GitHub Actions best practices -description: Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, and upgrading official actions to their latest versions. +description: Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, replacing `always()` conditions, and upgrading official actions to their latest versions. tags: - github - actions @@ -27,6 +27,7 @@ recipeList: - org.openrewrite.github.PreferBlockStyleJobDependencies - org.openrewrite.github.PreferTemurinDistributions - org.openrewrite.github.RemoveUnusedWorkflowDispatchInputs + - org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure - org.openrewrite.github.SetupJavaCaching - org.openrewrite.github.UpgradeOfficialGitHubActions --- diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 715f39f..8873993 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -11,7 +11,7 @@ maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.Depen maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.DependabotCheckForGithubActionsUpdatesWeekly,Check for github-actions updates weekly,Set dependabot to check for github-actions updates weekly.,2,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.FindGitHubActionSecretReferences,Find GitHub action secret references,Help identify and inventory your GitHub secrets that are being used in GitHub actions.,2,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,,"[{""name"":""org.openrewrite.table.TextMatches"",""displayName"":""Text matches"",""instanceName"":""Text matches"",""description"":""Lines matching simple text search."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file.""},{""name"":""match"",""type"":""String"",""displayName"":""Match"",""description"":""The text of the match.""}]}]" maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.FindMissingTimeout,Find jobs missing timeout,Find GitHub Actions jobs missing a timeout.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, -maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.GitHubActionsBestPractices,GitHub Actions best practices,"Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, and upgrading official actions to their latest versions.",7,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, +maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.GitHubActionsBestPractices,GitHub Actions best practices,"Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, replacing `always()` conditions, and upgrading official actions to their latest versions.",8,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionDefinition,Is GitHub Action definition,"Checks if the file is a GitHub Action definition (`action.yml`), such as a composite action.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionsFile,Is GitHub Actions workflow or action definition,"Checks if the file is either a GitHub Actions workflow file, or a GitHub Action definition (`action.yml`). Steps, and the `uses:` references within them, appear in both, so prefer this over `IsGitHubActionsWorkflow` as a precondition for any recipe that operates on steps. Recipes that read workflow-only keys such as `on:`, `permissions:`, `runs-on:` or `needs:` should keep the narrower `IsGitHubActionsWorkflow`.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionsWorkflow,Is GitHub Actions Workflow,Checks if the file is a GitHub Actions workflow file.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, From 8bd588951a6febd89b53d80274eacee441a569c3 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 25 Aug 2026 19:53:55 +0200 Subject: [PATCH 3/4] Simplify always() replacement and tighten the visit hot path --- .../ReplaceAlwaysWithSuccessOrFailure.java | 89 +++++++------------ .../resources/META-INF/rewrite/recipes.csv | 2 +- ...ReplaceAlwaysWithSuccessOrFailureTest.java | 32 +++++++ 3 files changed, 64 insertions(+), 59 deletions(-) diff --git a/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java b/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java index 0227294..fdbf490 100644 --- a/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java +++ b/src/main/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailure.java @@ -30,35 +30,46 @@ import java.util.regex.Pattern; public class ReplaceAlwaysWithSuccessOrFailure extends Recipe { - private static final Pattern ALWAYS_CALL = Pattern.compile("(? getVisitor() { return Preconditions.check(new IsGitHubActionsFile(), new YamlIsoVisitor() { - private final JsonPathMatcher jobCondition = new JsonPathMatcher("$.jobs.*.if"); - private final JsonPathMatcher stepCondition = new JsonPathMatcher("$..steps[*].if"); - @Override public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionContext ctx) { Yaml.Mapping.Entry e = super.visitMappingEntry(entry, ctx); - if ((jobCondition.matches(getCursor()) || stepCondition.matches(getCursor())) && - e.getValue() instanceof Yaml.Scalar) { - Yaml.Scalar condition = (Yaml.Scalar) e.getValue(); - Optional blockScalar = new BlockScalar.Matcher().get(condition, getCursor()); - String value = blockScalar.isPresent() ? blockScalar.get().getBody() : condition.getValue(); - String updated = replaceAlways(value); - if (!value.equals(updated)) { - return e.withValue(blockScalar.isPresent() ? - blockScalar.get().withBody(updated) : condition.withValue(updated)); - } + if (!"if".equals(e.getKey().getValue()) || !(e.getValue() instanceof Yaml.Scalar)) { + return e; + } + Yaml.Scalar condition = (Yaml.Scalar) e.getValue(); + if (!condition.getValue().contains("always") || + !(STEP_CONDITION.matches(getCursor()) || JOB_CONDITION.matches(getCursor()))) { + return e; + } + Optional blockScalar = BLOCK_SCALAR.get(condition, getCursor()); + String value = blockScalar.isPresent() ? blockScalar.get().getBody() : condition.getValue(); + // `''` is YAML's escape for a quote, not a closed expression string + String updated = condition.getStyle() == Yaml.Scalar.Style.SINGLE_QUOTED ? + replaceAlways(value.replace("''", "'")).replace("'", "''") : + replaceAlways(value); + if (!value.equals(updated)) { + return e.withValue(blockScalar.isPresent() ? + blockScalar.get().withBody(updated) : condition.withValue(updated)); } return e; } @@ -66,52 +77,14 @@ public Yaml.Mapping.Entry visitMappingEntry(Yaml.Mapping.Entry entry, ExecutionC } private static String replaceAlways(String condition) { + String replacement = ONLY_ALWAYS.matcher(condition).matches() ? REPLACEMENT : PARENTHESIZED_REPLACEMENT; Matcher matcher = ALWAYS_CALL.matcher(condition); - String expression = condition.trim(); - if (expression.startsWith("${{") && expression.endsWith("}}")) { - expression = expression.substring(3, expression.length() - 2).trim(); - } - - String replacement = ALWAYS_CALL.matcher(expression).matches() ? - REPLACEMENT : "(" + REPLACEMENT + ")"; - StringBuilder updated = null; - int lastMatchEnd = 0; + StringBuffer updated = new StringBuffer(); while (matcher.find()) { - if (isInsideStringLiteral(condition, matcher.start())) { - continue; - } - if (updated == null) { - updated = new StringBuilder(condition.length() + replacement.length()); - } - updated.append(condition, lastMatchEnd, matcher.start()).append(replacement); - lastMatchEnd = matcher.end(); - } - return updated == null ? condition : updated.append(condition, lastMatchEnd, condition.length()).toString(); - } - - private static boolean isInsideStringLiteral(String expression, int offset) { - boolean singleQuoted = false; - boolean doubleQuoted = false; - for (int i = 0; i < offset; i++) { - char current = expression.charAt(i); - if (singleQuoted) { - if (current == '\'' && i + 1 < offset && expression.charAt(i + 1) == '\'') { - i++; - } else if (current == '\'') { - singleQuoted = false; - } - } else if (doubleQuoted) { - if (current == '\\' && i + 1 < offset) { - i++; - } else if (current == '"') { - doubleQuoted = false; - } - } else if (current == '\'') { - singleQuoted = true; - } else if (current == '"') { - doubleQuoted = true; + if (matcher.group(1) != null) { + matcher.appendReplacement(updated, replacement); } } - return singleQuoted || doubleQuoted; + return matcher.appendTail(updated).toString(); } } diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 8873993..3ae711a 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -22,7 +22,7 @@ maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.Prefe maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveAllCronTriggers,Remove all cron triggers,Removes all cron triggers from a workflow.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveUnusedWorkflowDispatchInputs,Remove unused workflow dispatch inputs,Remove workflow_dispatch inputs that are not referenced anywhere in the workflow file.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.RemoveWorkflowInputArgument,Remove workflow input argument,Remove a specific input argument from calls to a reusable workflow.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""workflowReference"",""type"":""String"",""displayName"":""Workflow reference"",""description"":""The workflow reference to match (e.g., `org/repo/.github/workflows/myWorkflow.yml`)."",""example"":""org/repo/.github/workflows/myWorkflow.yml"",""required"":true},{""name"":""version"",""type"":""String"",""displayName"":""Version"",""description"":""The version of the workflow to match (e.g., `v1.2.3`)."",""example"":""v1.2.3"",""required"":true},{""name"":""inputArgumentName"",""type"":""String"",""displayName"":""Input argument name"",""description"":""The name of the input argument to remove."",""example"":""myInputToRemove"",""required"":true}]", -maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure,Replace `always()` with `success() || failure()`,Replace `always()` in GitHub Actions job and step conditions with `success() || failure()` so that canceled workflows do not continue running or hang until they time out.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, +maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure,Replace `always()` with `success() || failure()`,Replace `always()` in GitHub Actions job and step conditions with `success() || failure()` so that canceled workflows do not continue running or hang until they time out. Note that teardown steps deliberately using `always()` to still run on cancellation will no longer run.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceDependabotReviewersWithCodeowners,Replace Dependabot `reviewers` with `CODEOWNERS`,"Replaces the [removed](https://github.blog/changelog/2025-04-29-dependabot-reviewers-configuration-option-being-replaced-by-code-owners/) `reviewers` option in `.github/dependabot.yml` with equivalent `CODEOWNERS` entries. Each reviewer is mapped onto the manifest files Dependabot updates for that `package-ecosystem` and `directory`, so ownership stays as narrow as the Dependabot configuration was. Update entries whose `package-ecosystem` has no known manifests are left untouched.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""codeownersPath"",""type"":""String"",""displayName"":""`CODEOWNERS` path"",""description"":""Where to write the migrated reviewers when the repository does not have a `CODEOWNERS` file yet. Defaults to `.github/CODEOWNERS`. When a `CODEOWNERS` file already exists in any of the locations GitHub recognizes, that file is appended to instead and this option is ignored."",""example"":""CODEOWNERS""}]", maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceOssrhSecretsWithSonatype,Replace OSSRH secrets with Sonatype secrets,Replace deprecated OSSRH_S01 secrets with new Sonatype secrets in GitHub Actions workflows. This is an example use of the `ReplaceSecrets` and `ReplaceSecretKeys` recipes combined used to update the Maven publishing secrets in OpenRewrite's GitHub organization.,5,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.ReplaceRunners,Replace runners for a job,Replaces the runners of a given job.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,"[{""name"":""jobName"",""type"":""String"",""displayName"":""Job Name"",""description"":""The name of the job to update, use * to affect all the workflow jobs"",""example"":""build"",""required"":true},{""name"":""runners"",""type"":""List"",""displayName"":""Runners"",""description"":""The new list of runners to set"",""example"":""ubuntu-latest"",""required"":true}]", diff --git a/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java b/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java index 0bbef37..b101297 100644 --- a/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java +++ b/src/test/java/org/openrewrite/github/ReplaceAlwaysWithSuccessOrFailureTest.java @@ -238,6 +238,38 @@ void doesNotReplaceTextInsideStringLiterals() { ); } + @Test + void doesNotReplaceTextInsideQuotedScalarStringLiterals() { + rewriteRun( + //language=yaml + yaml( + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: 'always() && contains(github.event.head_commit.message, ''always()'')' + run: ./publish.sh + - if: "always() && contains(github.event.head_commit.message, 'always()')" + run: ./upload.sh + """, + """ + on: push + jobs: + build: + runs-on: ubuntu-latest + steps: + - if: '(success() || failure()) && contains(github.event.head_commit.message, ''always()'')' + run: ./publish.sh + - if: "(success() || failure()) && contains(github.event.head_commit.message, 'always()')" + run: ./upload.sh + """, + spec -> spec.path(".github/workflows/ci.yml") + ) + ); + } + @Test void doesNotChangeUnrelatedIfKeys() { rewriteRun( From e4ced88835cd1ebd782e4255f9ac1681a0d0b086 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Tue, 25 Aug 2026 19:55:29 +0200 Subject: [PATCH 4/4] Keep ReplaceAlwaysWithSuccessOrFailure out of GitHubActionsBestPractices --- src/main/resources/META-INF/rewrite/github.yml | 3 +-- src/main/resources/META-INF/rewrite/recipes.csv | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/resources/META-INF/rewrite/github.yml b/src/main/resources/META-INF/rewrite/github.yml index 879f968..d5e6e98 100644 --- a/src/main/resources/META-INF/rewrite/github.yml +++ b/src/main/resources/META-INF/rewrite/github.yml @@ -18,7 +18,7 @@ type: specs.openrewrite.org/v1beta/recipe name: org.openrewrite.github.GitHubActionsBestPractices displayName: GitHub Actions best practices -description: Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, replacing `always()` conditions, and upgrading official actions to their latest versions. +description: Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, and upgrading official actions to their latest versions. tags: - github - actions @@ -27,7 +27,6 @@ recipeList: - org.openrewrite.github.PreferBlockStyleJobDependencies - org.openrewrite.github.PreferTemurinDistributions - org.openrewrite.github.RemoveUnusedWorkflowDispatchInputs - - org.openrewrite.github.ReplaceAlwaysWithSuccessOrFailure - org.openrewrite.github.SetupJavaCaching - org.openrewrite.github.UpgradeOfficialGitHubActions --- diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 3ae711a..9f78616 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -11,7 +11,7 @@ maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.Depen maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.DependabotCheckForGithubActionsUpdatesWeekly,Check for github-actions updates weekly,Set dependabot to check for github-actions updates weekly.,2,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.FindGitHubActionSecretReferences,Find GitHub action secret references,Help identify and inventory your GitHub secrets that are being used in GitHub actions.,2,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,,"[{""name"":""org.openrewrite.table.TextMatches"",""displayName"":""Text matches"",""instanceName"":""Text matches"",""description"":""Lines matching simple text search."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file.""},{""name"":""match"",""type"":""String"",""displayName"":""Match"",""description"":""The text of the match.""}]}]" maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.FindMissingTimeout,Find jobs missing timeout,Find GitHub Actions jobs missing a timeout.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, -maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.GitHubActionsBestPractices,GitHub Actions best practices,"Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, replacing `always()` conditions, and upgrading official actions to their latest versions.",8,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, +maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.GitHubActionsBestPractices,GitHub Actions best practices,"Applies best practices to GitHub Actions workflows, including enabling dependency caching, using cached distributions, finding missing timeouts, removing unused inputs, preferring block-style job dependencies, and upgrading official actions to their latest versions.",7,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionDefinition,Is GitHub Action definition,"Checks if the file is a GitHub Action definition (`action.yml`), such as a composite action.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionsFile,Is GitHub Actions workflow or action definition,"Checks if the file is either a GitHub Actions workflow file, or a GitHub Action definition (`action.yml`). Steps, and the `uses:` references within them, appear in both, so prefer this over `IsGitHubActionsWorkflow` as a precondition for any recipe that operates on steps. Recipes that read workflow-only keys such as `on:`, `permissions:`, `runs-on:` or `needs:` should keep the narrower `IsGitHubActionsWorkflow`.",1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,, maven,org.openrewrite.recipe:rewrite-github-actions,org.openrewrite.github.IsGitHubActionsWorkflow,Is GitHub Actions Workflow,Checks if the file is a GitHub Actions workflow file.,1,,GitHub Actions,,Recipes to perform [GitHub Actions](https://docs.github.com/en/actions) hygiene and migration tasks.,,