-
Notifications
You must be signed in to change notification settings - Fork 190
Fix Java examples using Kotlin-specific syntax #1259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@ChiragAgg5k has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (16)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR makes parameter example generation language-aware across the SDK languages. The abstract getParamExample signature gained an optional $lang parameter, and most language implementations were updated to accept it. Kotlin's implementation was extended to render Java- or Kotlin-style examples (arrays, objects, permissions) via new helpers (getKotlinMapExample, getJavaMapExample, getArrayExample) and registers a javaParamExample Twig filter. Two templates (android and kotlin Java example templates) were changed to conditionally import Permission/Role and use javaParamExample for non-enum params. CI PHP setup was pinned to 8.3. A verification script verify_signatures.php was added to assert the new method signature across SDK classes. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fd111dd to
f42759e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/SDK/Language/Kotlin.php (2)
206-288: Language-awaregetParamExampleis reasonable but docblock is staleThe new
$lang = 'kotlin'parameter and Java/Kotlin branches for arrays and objects make sense and should keep existing Kotlin call sites working while enabling Java examples viajavaParamExample.However, the docblock still mentions
@param array $specwhich no longer exists in the signature and doesn’t mention the default for$lang. Cleaning this up will avoid IDE/static-analysis noise and confusion.- /** - * @param array $param - * @param array $spec - * @param string $lang Language variant: 'kotlin' (default) or 'java' - * @return string - */ - public function getParamExample(array $param, string $lang = 'kotlin'): string + /** + * @param array $param + * @param string $lang Language variant: 'kotlin' (default) or 'java' + * @return string + */ + public function getParamExample(array $param, string $lang = 'kotlin'): string
290-352: Map helpers look fine; consider shared value-formatting helper
getKotlinMapExample()andgetJavaMapExample()both format keys/values similarly and differ mainly in container syntax (mapOfvsnew HashMap.../Collections.emptyList()). The current duplication is acceptable, but you might factor out the value-formatting (string/bool/null/arraycases) into a small private helper to keep these in sync if you add more types later.Also, the simplified handling of nested arrays (
listOf()/Collections.emptyList()) is fine for docs, just something to be aware of if you ever want examples to reflect real nested content.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/SDK/Language/Kotlin.php(5 hunks)templates/android/docs/java/example.md.twig(2 hunks)templates/kotlin/docs/java/example.md.twig(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/Kotlin.php (1)
src/SDK/Language.php (2)
getParamExample(75-75)getPermissionExample(252-278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: Agent
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Python310)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: apple (client)
- GitHub Check: android (client)
🔇 Additional comments (4)
src/SDK/Language/Kotlin.php (2)
571-573:javaParamExampleTwig filter correctly routes to Java branchThe new
javaParamExamplefilter cleanly delegates togetParamExample($param, 'java')and is markedis_safe => ['html'], matching existing filter patterns. This aligns with the updated templates that now use{{ parameter | javaParamExample }}for Java examples.No issues from my side here.
354-369: Kotlin'sgetPermissionExampleoverride is correct and necessaryThe review's concern is based on a misunderstanding of the input contract. The
isPermissionString()method detects inputs matching a JSON array pattern like["read(\"role_id\")", "write(\"role_id\")"], not raw permission strings.The base
Language::getPermissionExample()expects raw permission format and callsextractPermissionParts()to parse it—this would fail or produce incorrect results with JSON-formatted input. Kotlin's override correctly handles the JSON-formatted input by simply wrapping it inlistOf()/Arrays.asList(), which is the appropriate behavior for this input type.Delegating to the parent method (as the review suggests) would actually introduce a bug, since the parent cannot correctly process JSON-formatted permission arrays. The current implementation is correct and the semantic bypass is intentional, not problematic.
Likely an incorrect or invalid review comment.
templates/android/docs/java/example.md.twig (1)
1-3: Java imports andjavaParamExampleusage are correctThe added
java.util.Arrays,Collections, andHashMapimports match the Java examples produced bygetParamExample($param, 'java'):Arrays.asList(...),Collections.emptyList(), andnew HashMap<String, Object>() {{ ... }}. ThejavaParamExamplefilter is properly inherited from the Kotlin parent class and correctly applied in the template. Conditional imports forPermission,Role, andInputFileare also correctly handled based on parameter types.templates/kotlin/docs/java/example.md.twig (1)
1-3: Java imports andjavaParamExampleusage confirmed consistent with Android templateVerification confirms the template mirrors the Android Java docs: shared
java.util.*imports (Arrays, Collections, HashMap), conditionalPermission/Roleimports based onhasPermissionParam, and non-enum params rendered viajavaParamExamplefilter. The filter properly delegates togetParamExample($param, 'java')which generates correct Java syntax for all parameter types—objects asHashMap, arrays asCollections.emptyList()orArrays.asList(), and primitives as default values.Template syntax is valid; generated Java examples will compile correctly with the provided imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Java code examples in Kotlin and Android SDK templates that were incorrectly using Kotlin-specific syntax like listOf() and mapOf() instead of proper Java equivalents like Arrays.asList() and new HashMap<>().
- Refactored
getParamExample()to support language-specific code generation with an optional$langparameter - Added separate helper methods for generating Kotlin-style and Java-style map initializations
- Updated Java example templates to use proper Java standard library imports and syntax
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/SDK/Language/Kotlin.php | Enhanced with language-aware example generation methods and new javaParamExample Twig filter |
| templates/kotlin/docs/java/example.md.twig | Updated to use javaParamExample filter and added Java standard library imports |
| templates/android/docs/java/example.md.twig | Updated to use javaParamExample filter and added Java standard library imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f42759e to
b17b151
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/SDK/Language/Kotlin.php (1)
312-316: Consider making indentation configurable.The hardcoded indentation (8 spaces for entries, 4 spaces for closing braces) might not align with all template contexts. If the generated examples appear misaligned in documentation, consider accepting an indentation parameter or using the template's context-aware formatting.
Also applies to: 344-348
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/tests.yml(1 hunks)src/SDK/Language/Kotlin.php(5 hunks)templates/android/docs/java/example.md.twig(2 hunks)templates/kotlin/docs/java/example.md.twig(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/android/docs/java/example.md.twig
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/Kotlin.php (1)
src/SDK/Language.php (2)
getParamExample(75-75)getPermissionExample(252-278)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, Go118)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: swift (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (8)
.github/workflows/tests.yml (1)
97-97: LGTM! Consistent PHP version for lint job.The lint job doesn't use a matrix strategy, so hardcoding the PHP version to '8.3' (matching the matrix definition on line 15) is appropriate and avoids potential issues with undefined variable references.
templates/kotlin/docs/java/example.md.twig (3)
1-3: LGTM! Required Java utility imports.These imports are necessary for the Java-specific code generation (Arrays.asList, Collections.emptyList, HashMap initialization) that replaces Kotlin constructs.
10-13: LGTM! Conditional permission imports.The conditional import logic correctly imports Permission and Role classes only when permission parameters are present, keeping the generated code clean.
45-45: Filter registration verified and correct.The
javaParamExamplefilter is properly registered insrc/SDK/Language/Kotlin.php(line 571) and correctly delegates togetParamExample($param, 'java'). The method handles the language parameter correctly, producing valid Java syntax:
- Objects:
new HashMap<String, Object>() {{ put("a", "b"); }}- Arrays:
Collections.emptyList()The implementation is sound and the filter produces appropriate Java examples for the template.
src/SDK/Language/Kotlin.php (4)
208-211: LGTM! Backward-compatible signature extension.The addition of an optional
$langparameter with a default value maintains backward compatibility while enabling language-specific example generation.
234-256: LGTM! Clean language-specific branching.The conditional logic correctly generates Java-style syntax (HashMap, Collections.emptyList) for Java and Kotlin-style syntax (mapOf, listOf) for Kotlin, properly delegating to language-specific map renderers for complex objects.
571-573: LGTM! Proper Twig filter registration.The
javaParamExamplefilter correctly delegates togetParamExamplewith the 'java' language parameter and is properly marked as safe for HTML output.
361-369: No issues found—method override is compatible with parent class.Verification confirms the method signature override is safe:
- Parent
Language.phpdoes not callgetPermissionExample()internally (verified: 0 calls found in source)- Other language implementations (Dart, DotNet, Ruby, Python, PHP, Swift) each have their own implementation and call with 1 parameter on
$this- The optional
$langparameter maintains backward compatibility with the parent's required signatureThe override is valid and does not introduce polymorphic incompatibility.
411ac57 to
08b4402
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/SDK/Language/Kotlin.php (2)
288-312: The previously flagged escaping and nested object issues remain unaddressed.As noted in the earlier review, this method has critical issues:
- Line 294: String values are not escaped, which will cause syntax errors if values contain quotes, backslashes, or newlines.
- Line 302: Nested objects are cast to string, producing invalid Kotlin syntax.
320-344: The previously flagged escaping and nested object issues remain unaddressed, plus Map.of() limitations.As noted in the earlier review, this method has critical issues with string escaping (line 326) and nested object handling (line 334).
Additionally,
Map.of()is limited to 10 key-value pairs. For maps with more than 10 entries, consider usingMap.ofEntries()withMap.entry()calls instead.
🧹 Nitpick comments (2)
src/SDK/Language/Kotlin.php (2)
404-436: Method signature override is backward-compatible, but note collection usage inconsistency.This method overrides the parent class
Language::getPermissionExample()with an additional optional parameter. The default value ensures backward compatibility.However, this uses
Arrays.asList()for Java (line 433), whilegetArrayExample()usesList.of(). While both are valid, standardizing on one approach would improve consistency.Arrays.asList()has broader Java version compatibility (1.2+) compared toList.of()(9+).
288-344: Consider extracting shared logic from map example methods.
getKotlinMapExample()andgetJavaMapExample()contain nearly identical logic for iterating over map entries and formatting values. Consider extracting the common value formatting logic into a shared helper method to reduce duplication and improve maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/tests.yml(1 hunks)src/SDK/Language/Kotlin.php(5 hunks)templates/android/docs/java/example.md.twig(2 hunks)templates/kotlin/docs/java/example.md.twig(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/kotlin/docs/java/example.md.twig
- .github/workflows/tests.yml
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/Kotlin.php (3)
src/SDK/Language/PHP.php (3)
getParamExample(360-410)getStaticAccessOperator(133-136)getStringQuote(138-141)src/SDK/Language.php (8)
getParamExample(75-75)getPermissionExample(252-278)getStaticAccessOperator(39-39)getStringQuote(45-45)getPermissionPrefix(220-223)extractPermissionParts(166-203)transformPermissionAction(231-234)transformPermissionRole(242-245)src/SDK/Language/Node.php (4)
getParamExample(145-173)getStaticAccessOperator(15-18)getStringQuote(20-23)getPermissionPrefix(30-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: Agent
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: swift (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (6)
src/SDK/Language/Kotlin.php (4)
208-211: LGTM: Backward-compatible signature enhancement.The addition of the optional
$langparameter with a default value maintains backward compatibility while enabling language-specific rendering.
234-240: LGTM: Appropriate language-specific fallbacks.The ternary logic correctly switches between Java and Kotlin collection constructors based on the language parameter.
244-268: LGTM: Proper delegation to language-specific handlers.The example handling correctly delegates to language-specific methods and includes appropriate fallbacks for decode failures.
638-640: LGTM: Clean integration of Java parameter rendering.The new Twig filter properly delegates to
getParamExample()with the Java language context and is correctly marked as safe for HTML output.templates/android/docs/java/example.md.twig (2)
6-9: LGTM: Clean conditional import logic.The conditional imports for Permission and Role classes ensure that these are only imported when needed, keeping the generated examples clean and minimal.
44-44: LGTM: Correct application of Java-specific parameter rendering.The switch from
paramExampletojavaParamExampleensures that Java examples use proper Java syntax instead of Kotlin constructs, which is the primary goal of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
01bef57 to
5f30d13
Compare
Java examples were incorrectly using Kotlin-specific syntax like listOf() and mapOf(). This commit fixes the issue by:
- Added language-specific parameter support to getParamExample() method in Kotlin.php
- Created getKotlinMapExample() and getJavaMapExample() helper methods for proper syntax generation
- Updated getPermissionExample() to support both Java (Arrays.asList) and Kotlin (listOf) syntax
- Added new javaParamExample Twig filter that generates Java-compatible syntax
- Updated Java example templates to use javaParamExample filter
- Fixed import ordering in Java templates to follow Java conventions (java.* imports first, then project imports)
Now Java examples correctly use:
- Arrays.asList() instead of listOf()
- new HashMap<String, Object>() {{ put() }} instead of mapOf()
- Collections.emptyList() instead of listOf()
5f30d13 to
13d2386
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/SDK/Language/Kotlin.php (3)
282-322: String keys/values ingetKotlinMapExampleare not escaped
$keyand string$valueare interpolated directly into Kotlin string literals without escaping. Any embedded quotes, backslashes, or control characters will produce invalid Kotlin code.Consider centralizing a small Kotlin string‑escaping helper (handling
",\, newlines, tabs, etc.) and using it for both keys and values before wrapping in quotes.- $formattedKey = '"' . $key . '"'; + $formattedKey = '"' . $this->escapeKotlinString($key) . '"'; if (is_string($value)) { - $formattedValue = '"' . $value . '"'; + $formattedValue = '"' . $this->escapeKotlinString($value) . '"';
324-364: String escaping and Map.of usage ingetJavaMapExample
- String
$keyand$valueare injected into Java string literals without escaping; embedded quotes or backslashes will break the generated code. This mirrors the open issue from the previous review.- As above, using
Map.of(...)assumes Java 9+; confirm compatibility with your Java targets.- $formattedKey = '"' . $key . '"'; + $formattedKey = '"' . addslashes($key) . '"'; if (is_string($value)) { - $formattedValue = '"' . $value . '"'; + $formattedValue = '"' . addslashes($value) . '"';From which Java version are Map.of and List.of available, and are they supported on the minimum Java/Android versions targeted by this SDK?
366-423: Array example helper: missing string escaping and Java 9 dependencyWithin
getArrayExample:
- Primitive string items are emitted as
"{$item}"without escaping, so problematic characters will generate invalid Java/Kotlin.- Java output uses
List.of(...)both for decoded JSON arrays and the fallback path, again assuming Java 9+.To harden this:
- Reuse the same escaping strategy as the map helpers for any string items.
- Consider
Arrays.asList(...)/Collections.emptyList()(or another Java‑8‑compatible pattern) if any generated examples must compile on Java < 9.- if (is_string($item)) { - $arrayItems[] = '"' . $item . '"'; + if (is_string($item)) { + $arrayItems[] = '"' . addslashes($item) . '"';
🧹 Nitpick comments (3)
.github/workflows/tests.yml (1)
94-99: Hardcoding PHP version in lint job can drift from build matrixPinning
php-version: '8.3'here is fine functionally, but it can get out of sync with the build job’s matrix if that ever changes. Consider referencing a single source (e.g., a reusable workflow input or matrix) to avoid version drift.src/SDK/Language/Kotlin.php (2)
244-257: Fallback for invalid/non-JSON object examples is opinionatedWhen JSON decoding fails, both languages fall back to a canned map example (
Map.of("a", "b")/mapOf("a" to "b")). That’s acceptable as a last resort, but it may surprise users if they provided a malformed example. Consider logging or otherwise surfacing that the original example couldn’t be parsed so it’s easier to debug why the output doesn’t match the input.
425-464: Permission example helper is clean; Java list factory still version-dependentThe language‑aware
getPermissionExamplenicely reuses existing permission parsing and only switches the outer collection factory (List.ofvslistOf). The only concern is again Java version support forList.of(...); if you move other Java examples to a Java‑8‑compatible list representation, this should be updated as well for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/tests.yml(1 hunks)src/SDK/Language/Kotlin.php(5 hunks)templates/android/docs/java/example.md.twig(2 hunks)templates/kotlin/docs/java/example.md.twig(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/android/docs/java/example.md.twig
- templates/kotlin/docs/java/example.md.twig
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (8.3, FlutterStable)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: swift (server)
- GitHub Check: kotlin (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (2)
src/SDK/Language/Kotlin.php (2)
264-268: Permission vs generic array handling is clear and reusableDelegating permission arrays through
getPermissionExample($example, $lang)and everything else throughgetArrayExample($example, $lang)keepsgetParamExamplefocused and readable. No functional issues here.
666-668: NewjavaParamExampleTwig filter is appropriateExposing a dedicated
javaParamExamplefilter that simply forwards togetParamExample($param, 'java')keeps the templates simple and makes the language switch explicit. Marking itis_safe => ['html']matches existing patterns for code examples.
0e824c4 to
7f63565
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/SDK/Language/Kotlin.php (3)
425-464: Permission example +javaParamExamplefilter: behavior is coherent, but shares the Java 9 list concernThe lang-aware
getPermissionExample()correctly reuses the generic permission-parsing logic and only varies the outer collection (listOfvsList.of). The newjavaParamExampleTwig filter is a straightforward and safe way to expose Java-specific examples into templates (is_safe => ['html']is consistent with other filters).The only outstanding issue here is the same Java 9 vs Java 8 concern: for the Java branch you likely want to return an
Arrays.asList(...)orCollections.emptyList()expression instead ofList.of(...), to align with the Android/Java 8 target and with whatever you choose ingetArrayExample().Also applies to: 666-668
282-364: Unescaped strings in generated Kotlin/Java examples can break code (keys, values, and array items)
getKotlinMapExample,getJavaMapExample, andgetArrayExampleall wrap raw strings in double quotes without escaping. If any key/value/array element contains",\, newlines, or other special characters, the generated Kotlin/Java snippets will be syntactically invalid.This affects:
- Map keys and string values in
getKotlinMapExampleandgetJavaMapExample.- String items in
getArrayExample(both primitives and nested structures).This was already flagged in earlier review comments; it still applies to the current implementation.
You can fix this minimally by escaping both keys and string values before concatenation, for example:
- $formattedKey = '"' . $key . '"'; + $formattedKey = '"' . addslashes((string) $key) . '"'; if (is_string($value)) { - $formattedValue = '"' . $value . '"'; + $formattedValue = '"' . addslashes($value) . '"';Apply the same pattern to:
- The string branch in
getJavaMapExample().- The string branch in
getArrayExample():- if (is_string($item)) { - $arrayItems[] = '"' . $item . '"'; + if (is_string($item)) { + $arrayItems[] = '"' . addslashes($item) . '"';If you want to be stricter, consider a small private helper like
escapeForJavaString()/escapeForKotlinString()instead of relying onaddslashes(), so you can control escaping semantics explicitly.Also applies to: 366-423
233-239: Java 9+Map.of/List.ofusage conflicts with Android/Java 8 target; switch to Java 8–compatible collectionsThe Java branches still emit
Map.of(...)andList.of(...)in:
getParamExample()– default OBJECT/ARRAY examples and non-JSON object fallback.getJavaMapExample()– full map rendering.getArrayExample()– JSON and non-JSON array examples.getPermissionExample()– permission list wrapper.Earlier review comments already noted that:
Map.of/List.ofare Java 9+ APIs.- The Android templates target Java 8 (e.g.,
sourceCompatibility JavaVersion.VERSION_1_8andminSdkVersion 21) without explicit core-library desugaring.That mismatch will cause generated Android Java examples to fail compilation on the minimum supported API.
A Java 8–compatible approach would be to favor:
- Maps:
new HashMap<String, Object>() {{ put("a", "b"); }}orCollections.singletonMap("a", "b")for simple cases.- Lists:
Collections.emptyList()for empty arrays.Arrays.asList(...)for non-empty examples (including permissions).Conceptually:
- $output .= $lang === 'java' - ? 'Map.of("a", "b")' - : 'mapOf( "a" to "b" )'; + $output .= $lang === 'java' + ? 'new HashMap<String, Object>() {{ put("a", "b"); }}' + : 'mapOf( "a" to "b" )';- $output .= $lang === 'java' ? 'List.of()' : 'listOf()'; + $output .= $lang === 'java' ? 'Collections.emptyList()' : 'listOf()';And in
getArrayExample()/getPermissionExample():- return $lang === 'java' - ? 'List.of(' . implode(', ', $arrayItems) . ')' - : 'listOf(' . implode(', ', $arrayItems) . ')'; + if ($lang === 'java') { + return empty($arrayItems) + ? 'Collections.emptyList()' + : 'Arrays.asList(' . implode(', ', $arrayItems) . ')'; + } + return 'listOf(' . implode(', ', $arrayItems) . ')';- if ($lang === 'java') { - return 'List.of(' . $permissionsString . ')'; - } + if ($lang === 'java') { + return 'Arrays.asList(' . $permissionsString . ')'; + }Please also update the docblock above
getJavaMapExample()once you move away fromMap.of().To double-check the target Java/Android configuration and catch remaining
Map.of/List.ofoccurrences:#!/bin/bash # Confirm Java/Android target and find all Map.of/List.of usage in generators/templates echo "Android/Java version hints:" rg -n 'JavaVersion\.VERSION_1_8|minSdkVersion' templates tests -C3 || true echo echo "All Map.of/List.of occurrences:" rg -n 'Map\.of\(|List\.of\(' . -C2 || trueAlso applies to: 253-255, 331-363, 408-421, 432-463
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/SDK/Language.php(1 hunks)src/SDK/Language/Kotlin.php(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/SDK/Language/Kotlin.php (1)
src/SDK/Language.php (4)
getParamExample(76-76)getPermissionExample(253-279)getStaticAccessOperator(39-39)getStringQuote(45-45)
src/SDK/Language.php (3)
src/SDK/Language/Kotlin.php (1)
getParamExample(211-280)src/SDK/Language/GraphQL.php (1)
getParamExample(133-159)src/SDK/Language/Node.php (1)
getParamExample(145-173)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Ruby30)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Python39)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, Node16)
- GitHub Check: build (8.3, Go112)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: kotlin (server)
- GitHub Check: android (client)
🔇 Additional comments (1)
src/SDK/Language/Kotlin.php (1)
206-280: Lang-awaregetParamExamplewiring is reasonable; behavior differences to be aware ofThe
$langbranching for arrays/objects and delegation into language-specific helpers (getKotlinMapExample,getJavaMapExample,getArrayExample,getPermissionExample) is cohesive and keeps the method readable. The “no-example” fallbacks for primitive types remain unchanged, which preserves existing behavior for Kotlin.Be mindful that:
- Java branches now produce structurally different examples than Kotlin; any tests that assert on exact example strings for objects/arrays will need to be updated accordingly.
- The default
$langvalue here ('kotlin') differs from the abstract base (''), which is allowed in PHP but means calls made via aLanguage-typed reference with no second argument will get different defaults depending on the concrete class. That’s acceptable here, but it’s worth being intentional about.Overall the structure looks fine; main concerns are in the helper methods below (escaping and Java version compatibility).
| /** | ||
| * @param array $param | ||
| * @param string $lang Optional language variant (for multi-language SDKs) | ||
| * @return string | ||
| */ | ||
| abstract public function getParamExample(array $param): string; | ||
| abstract public function getParamExample(array $param, string $lang = ''): string; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Language-aware getParamExample contract looks good; ensure all subclasses are updated
The new $lang parameter on getParamExample is a clean way to support language variants. Please verify that every concrete Language subclass has been updated to match this abstract signature (including the optional second argument), otherwise PHP will complain about incompatible method declarations at runtime.
🏁 Script executed:
#!/bin/bash
# Verify all Language subclasses implement the new getParamExample signature
rg -nP 'class\s+\w+\s+extends\s+Language' src/SDK -C3
echo
echo "Checking getParamExample declarations:"
rg -n 'function\s+getParamExample\s*\(' src/SDK -C2Length of output: 7940
Critical: 15 Language subclasses missing updated getParamExample signature
Only Kotlin.php has been updated to match the new abstract signature with the string $lang parameter. All other concrete implementations must be updated:
src/SDK/Language/Web.php:155src/SDK/Language/Ruby.php:294src/SDK/Language/Swift.php:414src/SDK/Language/ReactNative.php:193src/SDK/Language/REST.php:89src/SDK/Language/Python.php:363src/SDK/Language/Node.php:145src/SDK/Language/PHP.php:360src/SDK/Language/Go.php:243src/SDK/Language/Deno.php:186src/SDK/Language/DotNet.php:266src/SDK/Language/GraphQL.php:133src/SDK/Language/CLI.php:382src/SDK/Language/Dart.php:239
Each needs the string $lang = '' parameter added to match the abstract method signature, otherwise PHP will raise signature compatibility errors.
🤖 Prompt for AI Agents
In src/SDK/Language.php around lines 71-77 the abstract method
getParamExample(array $param, string $lang = '') was changed; update all
concrete implementations listed (src/SDK/Language/Web.php:155, Ruby.php:294,
Swift.php:414, ReactNative.php:193, REST.php:89, Python.php:363, Node.php:145,
PHP.php:360, Go.php:243, Deno.php:186, DotNet.php:266, GraphQL.php:133,
CLI.php:382, Dart.php:239) so their getParamExample signatures match exactly
(add the string $lang = '' parameter and type), and adjust any internal usage to
accept the new parameter (or ignore it safely) to restore signature
compatibility with the abstract class; after changes run static checks/unit
tests to ensure no further signature mismatches.
1d05b73 to
bb0f335
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/SDK/Language/Kotlin.php (5)
206-280: Language-awaregetParamExampleis useful; Java defaults still rely onMap.of/List.ofThe new
$langparameter and delegation togetJavaMapExample/getKotlinMapExample/getArrayExamplelook structurally sound and keep the default'kotlin'behavior intact.However, the Java branches still emit Java 9+ collection factories:
- Line 234–239:
TYPE_OBJECTdefaults useMap.of("a", "b").- Line 239:
TYPE_ARRAYdefaults useList.of().Given the Android/Java targets elsewhere in this repo are Java 8, these examples will not compile on typical Android toolchains unless core library desugaring is explicitly configured. Consider switching to Java‑8‑compatible constructs here (e.g.,
new HashMap<String, Object>() {{ put("a", "b"); }},Collections.emptyList(),Arrays.asList(...)) to avoid version‑specific surprises and keep Java examples consistent with the PR’s stated goals.If you want, I can propose a concrete Java‑8‑compatible replacement for the Java branches and a small script to scan other generators for lingering
Map.of/List.ofusage.
289-322:getKotlinMapExample: string keys/values need escaping to avoid invalid KotlinThe recursive map rendering is a nice improvement, but as written it can generate invalid Kotlin literals:
- Line 295: keys are concatenated as
"{$key}"with no escaping.- Line 297: string values are concatenated as
"{$value}"with no escaping.If
$key/$valuecontain",\, or control characters, the resulting Kotlin snippet will not compile. This was already raised in a previous review and still seems unfixed here.A minimal fix is to escape the PHP string content before wrapping it in quotes, for example:
- foreach ($data as $key => $value) { - $formattedKey = '"' . $key . '"'; - if (is_string($value)) { - $formattedValue = '"' . $value . '"'; + foreach ($data as $key => $value) { + $formattedKey = '"' . addslashes($key) . '"'; + if (is_string($value)) { + $formattedValue = '"' . addslashes($value) . '"';If you need stricter Kotlin‑specific escaping (e.g., for
$interpolation), we can factor this into a small helper to centralize the logic.
331-364:getJavaMapExample: same escaping issue plus Java 9Map.ofusageThis helper mirrors the Kotlin version and inherits two problems:
Unescaped strings
- Line 337: keys are emitted as
"{$key}"without escaping.- Line 339: string values are emitted as
"{$value}"without escaping.
Any quotes, backslashes, or control characters in the key/value will break the generated Java code. This matches an earlier critical comment and still needs addressing.Java 9 dependency
- Line 360–362: the map is rendered as
Map.of(...).
On Java 8 (especially Android withsourceCompatibility 1.8), this won’t compile without additional libraries/desugaring, which conflicts with the typical Android targets mentioned in prior reviews.At minimum, I’d suggest:
- foreach ($data as $key => $value) { - $formattedKey = '"' . $key . '"'; - if (is_string($value)) { - $formattedValue = '"' . $value . '"'; + foreach ($data as $key => $value) { + $formattedKey = '"' . addslashes($key) . '"'; + if (is_string($value)) { + $formattedValue = '"' . addslashes($value) . '"';and then replacing the
Map.of(...)construction with a Java‑8‑compatible expression, for example a double‑bracenew HashMap<String, Object>() {{ put(...); }}pattern or a small helper method, to align with Android targets.
373-423:getArrayExample: missing string escaping and Java 9List.ofusageThe JSON‑aware array rendering is a good improvement, but two concerns remain:
Unescaped string items
- Line 397–399: string items are emitted as
"{$item}"without escaping.
Any embedded"/\/ newlines will break Kotlin/Java source exactly as in the map helpers. This was already raised in an earlier review.Java 9
List.of
- Line 408–410 and line 420–421: Java output uses
List.of(...).
This requires Java 9+ and is inconsistent with Java 8 / Android 1.8 compatibility expectations. Other parts of the codebase (e.g., permission examples) also useList.of, so any switch toArrays.asList(...)/Collections.emptyList()should be done consistently.You can fix the escaping minimally like:
- } else { - // Primitive value - if (is_string($item)) { - $arrayItems[] = '"' . $item . '"'; + } else { + // Primitive value + if (is_string($item)) { + $arrayItems[] = '"' . addslashes($item) . '"';and then update the Java branches to emit Java‑8‑compatible
Arrays.asList(...)/Collections.emptyList()(and ensure the templates importjava.util.Arrays/java.util.Collections).
432-464:getPermissionExample: logic OK, but Java branch still usesList.ofThe permission parsing and construction look solid, and reusing
getStaticAccessOperator(),getStringQuote(), and the transformation helpers keeps things consistent.For the Java branch, though (lines 459–462), you again rely on
List.of(...), which is Java 9+ only. Given these examples feed into Android/Java docs, that can be surprising or fail outright on Java 8 unless desugaring is in place.To align with Java 8 and with the rest of the PR’s intent, consider changing the Java branch to:
- if ($lang === 'java') { - return 'List.of(' . $permissionsString . ')'; - } - return 'listOf(' . $permissionsString . ')'; + if ($lang === 'java') { + return 'Arrays.asList(' . $permissionsString . ')'; + } + return 'listOf(' . $permissionsString . ')';and ensure the Java examples import
java.util.Arrays.
🧹 Nitpick comments (1)
src/SDK/Language/Kotlin.php (1)
666-668:javaParamExampleTwig filter wiring looks correctExposing a dedicated
javaParamExamplefilter that simply delegates togetParamExample($param, 'java')keeps the templates clean and makes the language switch explicit. Marking it asis_safe => ['html']also matches its usage for code snippets in markdown/HTML docs.If you anticipate additional language variants in the future, you might eventually want a more generic
{lang}ParamExampleconvention, but for the current Kotlin/Java split this is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/SDK/Language.php(1 hunks)src/SDK/Language/CLI.php(1 hunks)src/SDK/Language/Dart.php(1 hunks)src/SDK/Language/Deno.php(1 hunks)src/SDK/Language/DotNet.php(1 hunks)src/SDK/Language/Go.php(1 hunks)src/SDK/Language/GraphQL.php(1 hunks)src/SDK/Language/Kotlin.php(5 hunks)src/SDK/Language/Node.php(1 hunks)src/SDK/Language/PHP.php(1 hunks)src/SDK/Language/Python.php(1 hunks)src/SDK/Language/REST.php(1 hunks)src/SDK/Language/ReactNative.php(1 hunks)src/SDK/Language/Ruby.php(1 hunks)src/SDK/Language/Swift.php(1 hunks)src/SDK/Language/Web.php(1 hunks)verify_signatures.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
src/SDK/Language/Swift.php (3)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/ReactNative.php (1)
getParamExample(194-222)
src/SDK/Language/Web.php (1)
src/SDK/Language.php (1)
getParamExample(76-76)
src/SDK/Language/Dart.php (2)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)
verify_signatures.php (1)
src/SDK/Language.php (1)
Language(5-280)
src/SDK/Language/ReactNative.php (6)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/Deno.php (1)
getParamExample(187-216)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/Node.php (1)
getParamExample(146-174)src/SDK/Language/Python.php (1)
getParamExample(364-392)src/SDK/Language/Web.php (1)
getParamExample(156-185)
src/SDK/Language/Python.php (3)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/ReactNative.php (1)
getParamExample(194-222)
src/SDK/Language/REST.php (8)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/Dart.php (1)
getParamExample(240-269)src/SDK/Language/Deno.php (1)
getParamExample(187-216)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/Node.php (1)
getParamExample(146-174)src/SDK/Language/Python.php (1)
getParamExample(364-392)src/SDK/Language/ReactNative.php (1)
getParamExample(194-222)src/SDK/Language/Web.php (1)
getParamExample(156-185)
src/SDK/Language/CLI.php (3)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/Node.php (1)
getParamExample(146-174)
src/SDK/Language/Go.php (3)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/ReactNative.php (1)
getParamExample(194-222)
src/SDK/Language/Ruby.php (13)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/CLI.php (1)
getParamExample(383-441)src/SDK/Language/Dart.php (1)
getParamExample(240-269)src/SDK/Language/Deno.php (1)
getParamExample(187-216)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)src/SDK/Language/Kotlin.php (1)
getParamExample(211-280)src/SDK/Language/Node.php (1)
getParamExample(146-174)src/SDK/Language/PHP.php (1)
getParamExample(361-411)src/SDK/Language/Python.php (1)
getParamExample(364-392)src/SDK/Language/REST.php (1)
getParamExample(90-136)src/SDK/Language/ReactNative.php (1)
getParamExample(194-222)src/SDK/Language/Swift.php (1)
getParamExample(415-472)src/SDK/Language/Web.php (1)
getParamExample(156-185)
src/SDK/Language/Deno.php (1)
src/SDK/Language.php (1)
getParamExample(76-76)
src/SDK/Language/Node.php (2)
src/SDK/Language.php (1)
getParamExample(76-76)src/SDK/Language/GraphQL.php (1)
getParamExample(134-160)
🪛 PHPMD (2.15.0)
src/SDK/Language/Swift.php
415-415: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/DotNet.php
267-267: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Web.php
156-156: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Dart.php
240-240: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/ReactNative.php
194-194: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Python.php
364-364: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/REST.php
90-90: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/CLI.php
383-383: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Go.php
244-244: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/PHP.php
361-361: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/GraphQL.php
134-134: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Ruby.php
295-295: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Deno.php
187-187: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
src/SDK/Language/Node.php
146-146: Avoid unused parameters such as '$lang'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, WebChromium)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, Python311)
- GitHub Check: build (8.3, AppleSwift56)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
🔇 Additional comments (16)
src/SDK/Language/CLI.php (1)
380-383: Signature correctly updated to conform to interface.The addition of the optional
$langparameter aligns with the abstract interface requirement fromsrc/SDK/Language.php. While the parameter is unused in this implementation (as flagged by PHPMD), this is expected and intentional—CLI doesn't require language-specific example rendering. The default empty string preserves backward compatibility.src/SDK/Language/Python.php (1)
361-364: Signature correctly updated to conform to interface.The optional
$langparameter aligns with the abstract method signature insrc/SDK/Language.php. The parameter is unused in this implementation because Python SDK generation doesn't require language-specific variants, which is the expected behavior.src/SDK/Language/Go.php (1)
241-244: Signature correctly updated to conform to interface.The addition of the optional
$langparameter conforms to the abstract interface requirement. Since Go SDK generation doesn't require language-specific parameter examples, the unused parameter is expected and appropriate.src/SDK/Language/GraphQL.php (1)
131-134: Signature correctly updated to conform to interface.The optional
$langparameter addition aligns with the abstract interface requirement insrc/SDK/Language.php. The parameter is unused in this implementation, which is expected since GraphQL doesn't require language-specific example rendering.src/SDK/Language/Swift.php (1)
412-415: Signature correctly updated to conform to interface.The addition of the optional
$langparameter conforms to the abstract method signature. Since Swift SDK generation doesn't require language-specific parameter examples, the unused parameter is expected and intentional.src/SDK/Language/Web.php (1)
153-156: Signature correctly updated to conform to interface.The optional
$langparameter addition aligns with the abstract interface requirement. The parameter is unused in this implementation because Web SDK generation doesn't require language-specific variants, which is the expected and appropriate behavior.src/SDK/Language/REST.php (1)
87-90: Signature correctly updated to conform to interface.The addition of the optional
$langparameter conforms to the abstract method signature insrc/SDK/Language.php. Since REST documentation doesn't require language-specific variants, the unused parameter is expected and appropriate.src/SDK/Language/Node.php (1)
143-146: Signature correctly updated to conform to interface.The optional
$langparameter addition aligns with the abstract interface requirement. The parameter is unused in this implementation because Node.js SDK generation doesn't require language-specific parameter examples, which is the expected behavior.src/SDK/Language/Dart.php (1)
237-240: Signature correctly aligned with abstract interface.The addition of the optional
$langparameter matches the updated abstract method signature inLanguage.php. While the parameter is currently unused in Dart, this maintains interface consistency across all language implementations and allows for future language-variant support if needed.verify_signatures.php (1)
26-70: Robust signature validation implementation.The reflection-based checks comprehensively validate the method signature including parameter count, names, types, default values, and return type. This ensures consistency across all language implementations.
src/SDK/Language/DotNet.php (1)
264-267: Signature correctly aligned with abstract interface.The optional
$langparameter addition maintains consistency with the abstract method signature. The unused parameter is acceptable as DotNet doesn't require language variants.src/SDK/Language/Ruby.php (1)
292-295: Signature correctly aligned with abstract interface.The optional
$langparameter maintains interface consistency. Ruby doesn't require language-specific rendering, so the unused parameter is acceptable.src/SDK/Language.php (1)
73-76: Clean interface extension for multi-language SDK support.The addition of the optional
$langparameter is well-designed, maintaining backward compatibility while enabling language-specific example rendering (particularly for Kotlin/Java differentiation mentioned in the PR objectives).src/SDK/Language/Deno.php (1)
184-187: Signature correctly aligned with abstract interface.The optional
$langparameter maintains interface consistency. Deno doesn't require language-specific rendering, so the unused parameter is acceptable.src/SDK/Language/PHP.php (1)
358-361: Signature correctly aligned with abstract interface.The optional
$langparameter maintains interface consistency. PHP doesn't require language-specific rendering, so the unused parameter is acceptable.src/SDK/Language/ReactNative.php (1)
191-194: Signature correctly aligned with abstract interface.The optional
$langparameter maintains interface consistency. ReactNative doesn't require language-specific rendering, so the unused parameter is acceptable.
bb0f335 to
9902d29
Compare
Summary
Fixes Java examples that were incorrectly using Kotlin-specific syntax like
listOf()andmapOf()instead of Java syntax.Problem
Java examples in both Android and Kotlin templates were using Kotlin syntax:
listOf()instead ofArrays.asList()mapOf("key" to "value")instead ofnew HashMap<>()Solution
Enhanced Kotlin.php language class:
getParamExample()to accept optional$langparametergetKotlinMapExample()for Kotlin-style map initializationgetJavaMapExample()for Java-style map initializationgetPermissionExample()to support both languagesjavaParamExampleTwig filterUpdated Java example templates:
javaParamExamplefilterjava.util.*)Changes
src/SDK/Language/Kotlin.php- Added language-aware example generationtemplates/kotlin/docs/java/example.md.twig- Updated to use Java syntaxtemplates/android/docs/java/example.md.twig- Updated to use Java syntaxResult
Java examples now correctly use:
Arrays.asList(...)instead oflistOf(...)new HashMap<String, Object>() {{ put("key", "value"); }}instead ofmapOf("key" to "value")Collections.emptyList()instead oflistOf()Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.