Add add-function.php to scaffold new bridge functions#4
Open
sonalidudhia wants to merge 1 commit into
Open
Conversation
Adding one bridge function today means hand-editing six files across
three languages (nativephp.json, Contract, Plugin, Facade, Kotlin,
Swift) plus a test — miss one and it only fails at runtime on a
device. add-function.php runs after configure.php, inside an already
configured plugin, and does all of it in one command:
php add-function.php --name=Level --description="Reads the battery level."
Everything it needs is discovered at runtime (nativephp.json,
composer.json's psr-4 map, the single Contract/Facade files, the
resources/android and resources/ios bridge files) so the script has
no baked-in coupling to any one plugin and configure.php never has to
touch it (confirmed: configure.php's placeholder map doesn't collide
with the {{ function }}/{{ function_camel }} family the scaffolder
uses, plus a one-line defensive skip added to configure.php anyway).
Validates before writing anything: PascalCase names only, rejects
duplicates (checked against nativephp.json and the contract), rejects
PHP/Kotlin/Swift reserved words. --skip-android/--skip-ios omit that
platform's file and manifest key. --dry-run previews with zero writes.
If a native bridge file or insertion anchor can't be found, that one
file is skipped with a warning and the rest still completes.
Also: relaxed tests/ManifestTest.php's hardcoded bridge_functions
count assertion, since it broke the instant a second function was
added — it now checks the Example entry specifically instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds ChangesBridge Function Scaffolding
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant FunctionScaffolder
participant Manifest
participant PHPSource
participant NativeBridge
participant GeneratedTest
Developer->>FunctionScaffolder: run add-function.php with options
FunctionScaffolder->>Manifest: append bridge function entry
FunctionScaffolder->>PHPSource: insert contract, plugin, and facade updates
FunctionScaffolder->>NativeBridge: insert Kotlin and Swift bridge classes
FunctionScaffolder->>GeneratedTest: render bridge-function test
FunctionScaffolder-->>Developer: print touched files and warnings
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@add-function.php`:
- Around line 203-211: The write loop in the scaffolding command ignores mkdir()
and file_put_contents() failures, allowing partial plugin generation. Update the
foreach block that processes $writes to validate directory creation and each
file write, abort on any error, and roll back already-created artifacts or stage
writes atomically before reporting success.
- Around line 227-239: Normalize the Swift reserved-word comparison in
validateReservedWords so mixed-case names such as Type and Protocol are
rejected. Either lowercase every self::SWIFT_RESERVED entry or perform a
case-insensitive comparison while preserving the existing skipIos behavior.
In `@docs/creating-first-plugin.md`:
- Line 6: Update the plugin creation step referencing `add-function.php` to say
that each command adds the required function artifacts, rather than replacing
the template `Example` function; mention that the existing `Example` artifacts
must be removed separately if applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fae823c1-3b64-4528-962e-9d6c1d4c22ed
📒 Files selected for processing (7)
README.mdadd-function.phpconfigure.phpdocs/bridge-functions.mddocs/creating-first-plugin.mdstubs/bridge-function-test.stubtests/ManifestTest.php
Comment on lines
+203
to
+211
| foreach ($writes as $path => $contents) { | ||
| $directory = dirname($path); | ||
|
|
||
| if (! is_dir($directory)) { | ||
| mkdir($directory, 0777, true); | ||
| } | ||
|
|
||
| file_put_contents($path, $contents); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Prevent partial scaffolding writes from leaving an inconsistent plugin.
mkdir() and file_put_contents() failures are ignored, so the command can report success after writing only some artifacts. A failure while writing the manifest, PHP files, or test can leave the repository in a mixed state. Check every operation and preferably stage/rollback the complete set of files before reporting success.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@add-function.php` around lines 203 - 211, The write loop in the scaffolding
command ignores mkdir() and file_put_contents() failures, allowing partial
plugin generation. Update the foreach block that processes $writes to validate
directory creation and each file write, abort on any error, and roll back
already-created artifacts or stage writes atomically before reporting success.
Comment on lines
+227
to
+239
| private function validateReservedWords(string $methodName, string $name, bool $skipAndroid, bool $skipIos): void | ||
| { | ||
| if (in_array(strtolower($methodName), self::PHP_RESERVED, true)) { | ||
| $this->fail("[{$name}] camelCases to the PHP reserved word [{$methodName}] and cannot be used as a method name. Choose a different name."); | ||
| } | ||
|
|
||
| if (! $skipAndroid && in_array(strtolower($name), self::KOTLIN_RESERVED, true)) { | ||
| $this->fail("[{$name}] collides with a Kotlin reserved word. Choose a different name or pass --skip-android."); | ||
| } | ||
|
|
||
| if (! $skipIos && in_array(strtolower($name), self::SWIFT_RESERVED, true)) { | ||
| $this->fail("[{$name}] collides with a Swift reserved word. Choose a different name or pass --skip-ios."); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant sections with line numbers.
git ls-files | rg '(^|/)add-function\.php$|(^|/)AddFunction\.php$|(^|/)add-function\.php$'
echo '---'
# Show the file if found via a common path search
fd -a 'add-function.php|AddFunction.php' . 2>/dev/null || true
echo '---'
# Search for the validator and reserved-word constants/usages
rg -n "validateReservedWords|PHP_RESERVED|KOTLIN_RESERVED|SWIFT_RESERVED" .Repository: MrPunyapal/nativephp-plugin-template
Length of output: 975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the constants and the validator context.
sed -n '1,90p' add-function.php
echo '---'
sed -n '110,255p' add-function.php
echo '---'
# Search for where the method name and name are derived/normalized.
rg -n "camel|methodName|strtolower\\(|ucfirst|reserved" add-function.phpRepository: MrPunyapal/nativephp-plugin-template
Length of output: 12636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
swift_reserved = [
'associatedtype', 'class', 'deinit', 'enum', 'extension', 'fileprivate',
'func', 'import', 'init', 'inout', 'internal', 'let', 'open', 'operator',
'private', 'protocol', 'public', 'rethrows', 'static', 'struct',
'subscript', 'typealias', 'var', 'self', 'Self', 'Type', 'Protocol',
]
candidates = ['Self', 'self', 'Protocol', 'protocol', 'Type', 'type', 'TYPE']
for name in candidates:
blocked = name.lower() in swift_reserved
print(f"{name:10} -> {'blocked' if blocked else 'bypasses'}")
PYRepository: MrPunyapal/nativephp-plugin-template
Length of output: 331
Normalize the Swift reserved-word check add-function.php:237
strtolower($name) still lets Type and Protocol through because self::SWIFT_RESERVED mixes lowercase and capitalized entries. Lowercase the reserved list too, or compare case-insensitively, so those names are rejected as intended.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@add-function.php` around lines 227 - 239, Normalize the Swift reserved-word
comparison in validateReservedWords so mixed-case names such as Type and
Protocol are rejected. Either lowercase every self::SWIFT_RESERVED entry or
perform a case-insensitive comparison while preserving the existing skipIos
behavior.
| 3. Review `composer.json`, `nativephp.json`, PHP namespaces, and native bridge targets. | ||
| 4. Replace the template bridge function body with platform code. | ||
| 5. Run tests and static analysis. | ||
| 4. Run `php add-function.php --name=Level --description="Reads the battery level."` for each bridge function your plugin needs, replacing the template `Example` function with your own. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not describe scaffolding as replacing the Example function.
add-function.php appends the new manifest entry, methods, native classes, and test; it does not remove the existing Example artifacts. Update this step to say “add” or document the separate cleanup required for Example.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/creating-first-plugin.md` at line 6, Update the plugin creation step
referencing `add-function.php` to say that each command adds the required
function artifacts, rather than replacing the template `Example` function;
mention that the existing `Example` artifacts must be removed separately if
applicable.
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
1 participant
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.
Problem
Adding one bridge function today means hand-editing six files across three languages —
nativephp.json, theContract,Plugin, theFacade, the Kotlin class, the Swift class — plus a test. Miss one and it doesn't fail until runtime, on a real device. Thestubs/directory has sat unused since the template shipped, reserved for "future scaffolding automation" per the README.The one-liner
Flags
--name=Level--description="..."nativephp.jsonentry.--skip-androidandroidkey.--skip-iosioskey.--dry-run--no-interactionHow it works (no baked-in coupling)
configure.phpruns once, inside the raw template, and deletes itself.add-function.phpships alongside it but runs after configure, repeatedly, inside the developer's already-configured plugin — so it can't have configure-time placeholders baked in. Instead it discovers everything at runtime: the bridge namespace fromnativephp.json, the PHP namespace fromcomposer.json'sautoload.psr-4, the Contract/Facade files by scanningsrc/Contractsandsrc/Facades, and the native bridge files by globbingresources/android/resources/ios. It reuses the existingstubs/bridge-function-android.stubandstubs/bridge-function-ios.stub(extracting the nested-class block via brace-counting and splicing it into the existing bridge file) and adds one new stub,stubs/bridge-function-test.stub, for the generated Pest test.Validates before writing anything: PascalCase-only names, duplicate detection (checked against both
nativephp.jsonand the Contract, since they can drift), and rejection of names that collide with PHP, Kotlin, or Swift reserved words. If a native bridge file or its insertion anchor can't be found, that one file is skipped with a warning and everything else still completes — nothing hard-fails on a missing platform file.Verification
php -lon every touched file; manually checked the Kotlin/Swift output for balanced braces and structure matchingExample.Level,ChargeState,TemperatureCelsius) →composer teststill green (catches insertion-anchor drift across repeated runs).--name→ fails with the duplicate message,git statusclean (verified before and after — zero writes on failure).--name=list→ rejected (not PascalCase).--name=Print→ rejected (PHP reserved word).--dry-run→ prints the plan,git statusclean.--skip-android/--skip-ios→ manifest omits the corresponding key, that platform's file untouched.resources/androidentirely and re-ran → warns, skips only that file, still finishes PHP + iOS + test + exits 0.grep -rn '{{ 'clean except the{{ function }}/{{ function_camel }}family the scaffolder itself uses, which doesn't collide with configure's replacement keys).composer test/composer lint(Pint + PHPStan level 8 + Rector dry-run +composer validate) on the generated Contract/Plugin/Facade/test code — clean as generated. (Pint's pre-existing failures onPlugin.php,Manifest.php, etc. are unrelated — confirmed identical onmainbefore this branch touches anything, sincecomposer.lockisn't committed and a newerlaravel/pintresolves fresh with stricter rules than the repo was authored against.)Also touched
tests/ManifestTest.php— relaxed the hardcodedtoHaveCount(1)assertion. It broke the instant a second function was added; it now looks up theExampleentry by name instead of asserting the whole array's size, so it still verifies what it originally verified without breaking on legitimate growth.configure.php— one defensive line soadd-function.phpis never in its placeholder-rewrite scan (belt-and-suspenders; it was already safe in practice since none of the placeholder keys collide, but this removes any doubt).README.md,docs/creating-first-plugin.md,docs/bridge-functions.md— replaced the "future automation" wording with the actual command and a flag table;docs/creating-first-plugin.mdnow leads withadd-function.phpand keeps the manual six-step walkthrough as the "what it does under the hood" explanation.For maintainer input
class {{ function }}line in the stub / last}in the target file, rather than adding// nativephp:functions-style markers. It's simpler and needed no changes to existing files, but it's more brittle if someone hand-edits the bridge files into an unusual shape. Happy to switch to markers if you'd rather have the more robust (but more invasive) contract.nativephp-plugin-maker-style tool? Kept it here since it's zero-dependency and the stubs already lived in this repo unused.add-function.php. Open to trimming or growing them.Scope was kept tight: no event scaffolding via
stubs/event.stub, no function-removal command — both listed here as explicit follow-ups rather than folded into this diff.Summary by CodeRabbit
New Features
--dry-run,--skip-android,--skip-ios, and non-interactive options.Documentation
Tests