Enforce :rules-engine has no public API outside @InternalRulesEngineAPI#3480
Open
ajpallares wants to merge 16 commits into
Open
Enforce :rules-engine has no public API outside @InternalRulesEngineAPI#3480ajpallares wants to merge 16 commits into
ajpallares wants to merge 16 commits into
Conversation
…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>
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 Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## pallares/rules-engine-skeleton #3480 +/- ##
===============================================================
Coverage 79.89% 79.89%
===============================================================
Files 369 369
Lines 14871 14871
Branches 2048 2048
===============================================================
Hits 11881 11881
Misses 2157 2157
Partials 833 833 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# 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
ajpallares
added a commit
that referenced
this pull request
May 15, 2026
`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>
This was referenced May 15, 2026
tonidero
reviewed
May 20, 2026
Contributor
tonidero
left a comment
tonidero
left a comment
There was a problem hiding this comment.
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...
ajpallares
added a commit
that referenced
this pull request
May 21, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves SDK-4340
Checklist
purchases-iosand hybridsMotivation
A baseline-diff API check (regenerate
api.txt,git diff --exit-code) is silenceable: a contributor can add a non-internalpublicdeclaration 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@InternalRulesEngineAPIannotation itself. Runs in the existingmetalavajob. 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 funis added.Stacked on #3478.
Note
Low Risk
The provided diff contains no actual code changes (only a
+++ /dev/nullmarker), so there is no functional impact or risk to runtime behavior.Overview
No functional changes are present in the supplied diff; it only shows a
+++ /dev/nullmarker without any added/removed content, so there is nothing substantive for reviewers to validate.Reviewed by Cursor Bugbot for commit 94d6c29. Bugbot is set up for automated code reviews on this repo. Configure here.