chore: strip extra slashes when there is no package defined in the protofile#134
Conversation
Signed-off-by: Stuart Taylor <stuart@kadence.co>
WalkthroughThe recent updates in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- protoc_plugins/protoc-gen-php-grpc/php/template.go (2 hunks)
- protoc_plugins/protoc_test.go (2 hunks)
- protoc_plugins/testdata/NoPackage/Test/CustomImport/ServiceInterface.php (1 hunks)
Additional comments not posted (4)
protoc_plugins/testdata/NoPackage/Test/CustomImport/ServiceInterface.php (1)
9-22: InterfaceServiceInterfacecorrectly extendsGRPC\ServiceInterfaceand defines theSimpleMethod. Ensure that the\Messagetype is correctly defined elsewhere in the project as it's used here as both input and output type.protoc_plugins/protoc-gen-php-grpc/php/template.go (2)
49-58: The template correctly applies thestrip_slashesfunction to both input and output types in the method signature. This should effectively remove unnecessary backslashes, aligning with the PR's objectives.
94-96: Thestrip_slashesfunction implementation usesstrings.ReplaceAllto replace double backslashes with a single one, which is a straightforward and effective way to clean up the type names.protoc_plugins/protoc_test.go (1)
Line range hint
169-190: The updatedTest_NoPackagefunction now includes a file comparison step, which is crucial for verifying that the generated files are correct. This is a good practice, especially when changes to the generation logic are made, as it ensures the output remains as expected.
|
Hey @satdeveloping 👋 Thanks for the PR 👍 |
rustatian
changed the title
package defined in the protofile
|
Current behaviour of generation without a package: <?php
# Generated by the protocol buffer compiler (roadrunner-server/grpc). DO NOT EDIT!
# source: NoPackage/nopackage.proto
namespace Test\CustomImport;
use Spiral\RoadRunner\GRPC;
interface ServiceInterface extends GRPC\ServiceInterface
{
// GRPC specific service name.
public const NAME = "<nil>.Service";
/**
* @param GRPC\ContextInterface $ctx
* @param \\Message $in
* @return \\Message
*
* @throws GRPC\Exception\InvokeException
*/
public function SimpleMethod(GRPC\ContextInterface $ctx, \\Message $in): \\Message;
}if I tried to implement the interface, I'd get an error that: Method 'MyService::SimpleMethod()' is not compatible with method 'GRPC\ServiceInterface::SimpleMethod()'. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- protoc_plugins/protoc-gen-php-grpc/php/template.go (2 hunks)
- protoc_plugins/protoc_test.go (7 hunks)
- protoc_plugins/testdata/NoPackage/Test/CustomImport/ServiceInterface.php (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- protoc_plugins/protoc-gen-php-grpc/php/template.go
- protoc_plugins/protoc_test.go
- protoc_plugins/testdata/NoPackage/Test/CustomImport/ServiceInterface.php
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protoc_plugins/protoc-gen-php-grpc/php/template.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- protoc_plugins/protoc-gen-php-grpc/php/template.go
rustatian
left a comment
rustatian
left a comment
There was a problem hiding this comment.
LGTM, thanks @satdeveloping 👍
2 participants
Reason for This PR
Interfaces generated with no package defined in the .proto added an extra
\to message names.Description of Changes
Strip extra slashes via a template function
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the MIT license.
PR Checklist
[Author TODO: Meet these criteria.][Reviewer TODO: Verify that these criteria are met. Request changes if not]git commit -s).CHANGELOG.md.Summary by CodeRabbit
Refactor
Tests
Documentation