feat: reduce possibility of name conflicts #682
Merged
Conversation
This file contains 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
alexander-fenster
requested review from
bcoe,
summer-ji-eng and
xiaozhenliu-gg5
google-cla
bot
added
the
cla: yes
| @@ -112,21 +112,6 @@ service Naming { | |||
| body: "*" | |||
| }; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
FYI: the lines removed from the test proto correspond to a feature (reserved words) that I'm not going to cover soon.
| return this.innerApiCalls.getProjectId(request, options, callback); | ||
| } | ||
| function( |
There was a problem hiding this comment.
FYI: The lines removed from the test baseline correspond to a feature (reserved words) that I'm not going to cover soon.
xiaozhenliu-gg5
approved these changes
Aug 28, 2020
gcf-merge-on-green bot
pushed a commit
that referenced
this pull request
Aug 28, 2020
🤖 I have created a release \*beep\* \*boop\* --- ## [1.1.0](https://www.github.com/googleapis/gapic-generator-typescript/compare/v1.0.8...v1.1.0) (2020-08-28) ### Features * reduce possibility of name conflicts ([#682](https://www.github.com/googleapis/gapic-generator-typescript/issues/682)) ([420cbbc](https://www.github.com/googleapis/gapic-generator-typescript/commit/420cbbcd545ac85c21cb61e23b6981a5f6f86166)) ### Bug Fixes * **deps:** update dependency prettier to ^2.1.1 ([#678](https://www.github.com/googleapis/gapic-generator-typescript/issues/678)) ([41b68e9](https://www.github.com/googleapis/gapic-generator-typescript/commit/41b68e976bf2ced9fa744d297feb3b16f4cd921b)) * allow trailing segments after version in package name ([#667](https://www.github.com/googleapis/gapic-generator-typescript/issues/667)) ([43680ad](https://www.github.com/googleapis/gapic-generator-typescript/commit/43680add38d45d07e19d9772d04a92a748db30f5)) * **deps:** update dependency prettier to ^2.1.0 ([#674](https://www.github.com/googleapis/gapic-generator-typescript/issues/674)) ([ee48ad4](https://www.github.com/googleapis/gapic-generator-typescript/commit/ee48ad45383c27991dabc26c92ef1faeadc51717)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR introduces a namer that, being a part of the template, makes sure that the generated extra method (such as
FooAsyncfor methodFoo, orcheckFooProgressfor long running methodFoo) do not conflict with existing RPCs having the same names.Implementation: a template may provide a JS file
namer.jsthat will be loaded by the generator during its runtime and passed back to the template, so it could use the logic in JS to resolve name conflicts.Let's assume the proto has a long running RPC
Foo. Previously, we would've generated an extra methodcheckFooProgress, and if the proto has an actual RPCCheckFooProgress, the two names would've got into the conflict. With this change, the namer will detect the conflict, and the generated extra method will be calledcheckFooProgress1(2, 3, etc.) The same will happen if the proto has methodsInitialize,GetProjectId, etc. (which can conflict with the methods added by the generator).To use the namer in the template, use
{{ id.get("name") }}instead of just{{ name }}.Note that only "naming" baseline (a special proto with a lot of naming conflicts) is changed. No existing API should be affected by this feature.
This feature will unblock generation of Ads TypeScript library. Cc: @aohren
Small note: I removed the part of the
naming.protothat checks for TypeScript keywords conflicts, I'll get to that part later in a separate PR.