Enforce :rules-engine has no public API outside @InternalRulesEngineAPI by ajpallares · Pull Request #3480 · RevenueCat/purchases-android

ajpallares · 2026-05-14T10:30:46Z

Resolves SDK-4340

Checklist

If applicable, unit tests
If applicable, create follow-up issues for purchases-ios and hybrids

Motivation

A baseline-diff API check (regenerate api.txt, git diff --exit-code) is silenceable: a contributor can add a non-internal public declaration and just regenerate the baseline.

Description

Adds an intrinsic CI assertion that the metalava signature dump (rules-engine/build/api-dump.txt) contains nothing beyond the @InternalRulesEngineAPI annotation itself. Runs in the existing metalava job. Bypassing it requires editing the script's allow-list, not regenerating a file.

Verified locally: passes on current source; fails (with the offender printed) when a non-annotated public fun is added.

Stacked on #3478.

Note

Low Risk
Adds a CI-only API surface check for :rules-engine; no runtime or customer-facing behavior changes.

Overview
Adds a CI guard in the existing metalava job so :rules-engine cannot expose a public API beyond what is marked @InternalRulesEngineAPI.

Instead of relying only on a baseline api.txt diff (which someone can “fix” by regenerating), the job inspects the Metalava signature dump at rules-engine/build/api-dump.txt and fails if any public surface appears that is not on an explicit allow-list (the annotation itself). Bypassing the check means changing that allow-list/script, not refreshing a checked-in API file.

This is aimed at SDK-4340 and is stacked on #3478.

^{Reviewed by Cursor Bugbot for commit 86cdfe3. Bugbot is set up for automated code reviews on this repo. Configure here.}

…public API Adds scripts/check-rules-engine-internal-only.sh which regenerates rules-engine/api.txt and asserts it contains only the @InternalRulesEngineAPI annotation declaration. Unlike a baseline diff, this check is intrinsic and cannot be silenced by regenerating api.txt. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

…ine-enforce-internal-api

…merge) Co-authored-by: Cursor <cursoragent@cursor.com>

…github.com/RevenueCat/purchases-android into pallares/rules-engine-enforce-internal-api

…(do not merge)" This reverts commit b7bbd5e.

…rge)" This reverts commit 6e38341.

codecov · 2026-05-14T12:03:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.92%. Comparing base (1b1649a) to head (86cdfe3).

Additional details and impacted files

@@                       Coverage Diff                       @@
##           pallares/rules-engine-skeleton    #3480   +/-   ##
===============================================================
  Coverage                           79.92%   79.92%           
===============================================================
  Files                                 369      369           
  Lines                               14880    14880           
  Branches                             2049     2049           
===============================================================
  Hits                                11893    11893           
  Misses                               2154     2154           
  Partials                              833      833

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

# Conflicts: # scripts/api-dump.sh

…into pallares/rules-engine-enforce-internal-api

The `metalava { hiddenAnnotations.add(…) }` block lives in this branch (rather than the skeleton) because the `scripts/check-rules-engine-internal-only.sh` guardrail is the only thing that depends on it: it asserts that, once `@InternalRulesEngineAPI`-annotated declarations are hidden, the generated `api.txt` contains nothing but the annotation interface itself. Anything else is a leaked non-internal public API. Without this block, the skeleton would have nothing exercising metalava on `:rules-engine` (publishing wiring also lives in a separate draft PR), so we keep it co-located with the check that needs it. Co-authored-by: Cursor <cursoragent@cursor.com>

…into pallares/rules-engine-enforce-internal-api

`aed6e2f88` committed an `api.txt` for parity with other public-library convention-plugin modules, but the immediate follow-up #3480 (no-public-apis enforcement) replaces that baseline with an explicit annotate-or-fail check that doesn't keep a file in source. Committing the baseline here just to delete it there is churn, so: - Drop `rules-engine/api.txt`. - Set `enforceCheck = false` so the standard `check` task no longer runs `metalavaCheckCompatibility*` against a missing baseline. - Redirect `metalavaGenerateSignature*` output to `build/api-dump.txt` so local spot-checks don't drop a file at the module root. - Keep `hiddenAnnotations.add(...InternalRulesEngineAPI)` so the generated dump filters gated symbols. - Update the trailing comment to call out both follow-up PRs (publishing/distribution + #3480 enforcement). Co-authored-by: Cursor <cursoragent@cursor.com>

…into pallares/rules-engine-enforce-internal-api Co-authored-by: Cursor <cursoragent@cursor.com>

After merging the latest skeleton, the metalava block carried a comment that referred to PR #3480 as the home of the no-public-APIs check, which no longer made sense on this branch (this is PR #3480). Replace it with a short note explaining what the dump is and why `enforceCheck` is off. The skeleton's `filename.set(...)` now writes to `build/api-dump.txt` instead of `rules-engine/api.txt`, so retarget the check script at the new path. The "must match `filename.set(...)`" comment exists so a future tweak in either file flags the other. Co-authored-by: Cursor <cursoragent@cursor.com>

tonidero

I think the logic makes sense in general, but from my comment in the previous PR, I do wonder if it makes sense to just expose the API as "public" but just make it clearer devs shouldn't use it with the artifact name...

Mirrors the iOS counterpart shift on PR #6787 (commit `931f36373`): the per-declaration opt-in marker is dropped in favor of a structural guarantee. On Android the structural guarantee was already in place — `:rules-engine` is consumed by `:purchases` / `:ui:revenuecatui` as an `implementation` dependency, which keeps every symbol off the SDK's transitive compile classpath, so third-party consumers never see the declarations regardless of their visibility modifier. The annotation was therefore signaling intent rather than enforcing it. Removing it cleans up: - `InternalRulesEngineAPI.kt` (the `@RequiresOptIn` annotation) is deleted outright. - `RulesEngine` becomes a plain `public object`. Doc comment trimmed to point readers at the structural-via-`implementation` guarantee rather than at the now-gone annotation. - `RulesEngineTest` drops its `@OptIn(InternalRulesEngineAPI::class)`. - `rules-engine/build.gradle.kts` no longer needs the `metalava` block. With no annotation to hide, the convention plugin's defaults produce a committed `rules-engine/api.txt` at the module root with `enforceCheck` on — same shape as `:feature:amazon` and the rest of the published modules. This also removes the `build/api-dump.txt` spot-check redirection. - `rules-engine/api.txt` baseline is committed for the first time. Skeleton surface is exactly `RulesEngine` (the namespace object). The follow-up commit on PR #3480 will drop `scripts/check-rules-engine-internal-only.sh` (its premise — "api dump must contain only the annotation declaration" — no longer holds) and the corresponding CircleCI step. PR #3480 itself ends up empty as a result; iOS `4c32721b8` made the same call and rescoped its enforcement PR. No platform-specific Detekt equivalent of iOS's `no_leaking_rules_engine_import` SwiftLint rule is added at this time. Per-module `implementation` dependencies plus Kotlin's `internal` visibility on every non-namespace declaration cover the intended surface for now. Verified: `:rules-engine:testDefaultsDebugUnitTest`, `detektAll`, `metalavaGenerateSignatureDefaultsRelease`, and `metalavaCheckDefaultsRelease` all green. Co-authored-by: Cursor <cursoragent@cursor.com>

…is gone Follows up the skeleton-side change (commit dropping `@InternalRulesEngineAPI`). The check this PR introduced — `scripts/check-rules-engine-internal-only.sh` — asserts that the metalava api dump only contains the `InternalRulesEngineAPI` annotation declaration and treats anything else as a leak. With the annotation deleted that assertion is vacuously false (the dump now contains every public symbol of the module), so the script either fails by definition or has to be inverted into a different kind of check. iOS made the same call on `4c32721b8`: the equivalent `check_rules_engine_no_public_api` Fastlane lane was deleted rather than rescoped, because once the per-declaration markers are gone the guarantee shifts to the *consumer* side (import form on iOS, `implementation` dependency on Android). On Android the consumer-side guarantee was already structural — `:purchases` / `:ui:revenuecatui` pull `:rules-engine` in as `implementation`, so nothing in the module ever lands on the SDK's transitive compile classpath — so we don't need the iOS lint-rule equivalent either. Removed: - `scripts/check-rules-engine-internal-only.sh` (the script). - `.circleci/config.yml` step that invoked the script under the `api-tester` job. The `metalava { hiddenAnnotations = … }` block in `rules-engine/build.gradle.kts` was already dropped by the skeleton merge — no annotation to hide, default plugin config now produces a committed `rules-engine/api.txt` baseline at the module root with `enforceCheck` on, same shape as `:feature:amazon` and the rest of the published modules. Verified: `:rules-engine:testDefaultsDebugUnitTest`, `detektAll`, and `:rules-engine:metalavaCheckDefaultsRelease` all green. Co-authored-by: Cursor <cursoragent@cursor.com>

…ine-enforce-internal-api

ajpallares · 2026-05-27T17:59:57Z

Closing. We won't enforce this in Android for now

ajpallares and others added 2 commits May 14, 2026 12:29

Android: wire :rules-engine internal-only check into metalava CI job

969af75

Co-authored-by: Cursor <cursoragent@cursor.com>

ajpallares added the pr:other label May 14, 2026

ajpallares and others added 6 commits May 14, 2026 13:11

TEST: add leaked public API to verify CI guardrail (do not merge)

6e38341

Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'pallares/rules-engine-skeleton' into pallares/rules-eng…

940ed23

…ine-enforce-internal-api

TEST: regenerate api.txt to simulate baseline-bypass attempt (do not …

b7bbd5e

…merge) Co-authored-by: Cursor <cursoragent@cursor.com>

Merge branch 'pallares/rules-engine-enforce-internal-api' of https://…

e31e0fc

…github.com/RevenueCat/purchases-android into pallares/rules-engine-enforce-internal-api

Revert "TEST: regenerate api.txt to simulate baseline-bypass attempt …

c296e0c

…(do not merge)" This reverts commit b7bbd5e.

Revert "TEST: add leaked public API to verify CI guardrail (do not me…

1d543f6

…rge)" This reverts commit 6e38341.

ajpallares and others added 5 commits May 14, 2026 19:06

Merge pallares/rules-engine-skeleton (PR feedback)

aa12742

# Conflicts: # scripts/api-dump.sh

Merge pallares/rules-engine-skeleton (defer publishing wiring)

9d9e737

Merge remote-tracking branch 'origin/pallares/rules-engine-skeleton' …

f1370ba

…into pallares/rules-engine-enforce-internal-api

Merge remote-tracking branch 'origin/pallares/rules-engine-skeleton' …

0e150a0

…into pallares/rules-engine-enforce-internal-api

ajpallares and others added 2 commits May 15, 2026 20:13

Merge remote-tracking branch 'origin/pallares/rules-engine-skeleton' …

615ff58

…into pallares/rules-engine-enforce-internal-api Co-authored-by: Cursor <cursoragent@cursor.com>

ajpallares marked this pull request as ready for review May 15, 2026 18:22

ajpallares requested a review from a team as a code owner May 15, 2026 18:22

ajpallares requested a review from a team May 15, 2026 18:23

This was referenced May 15, 2026

Add :rules-engine-internal skeleton module #3478

Merged

RulesEngine: add JSON Logic predicate evaluator #3482

Merged

tonidero reviewed May 20, 2026

View reviewed changes

ajpallares and others added 2 commits May 21, 2026 18:38

Merge branch 'pallares/rules-engine-skeleton' into pallares/rules-eng…

86cdfe3

…ine-enforce-internal-api

ajpallares closed this May 27, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enforce :rules-engine has no public API outside @InternalRulesEngineAPI#3480

Enforce :rules-engine has no public API outside @InternalRulesEngineAPI#3480
ajpallares wants to merge 17 commits into
pallares/rules-engine-skeletonfrom
pallares/rules-engine-enforce-internal-api

ajpallares commented May 14, 2026 •

edited by cursor Bot

Loading

Uh oh!

codecov Bot commented May 14, 2026 •

edited

Loading

Uh oh!

tonidero left a comment

Uh oh!

ajpallares commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ajpallares commented May 14, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Motivation

Description

Uh oh!

codecov Bot commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

ajpallares commented May 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ajpallares commented May 14, 2026 •

edited by cursor Bot

Loading

codecov Bot commented May 14, 2026 •

edited

Loading