Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

Escape Java keywords#40

Merged
andreamlin merged 31 commits into
googleapis:masterfrom
andreamlin:escape
Mar 18, 2019
Merged

Escape Java keywords#40
andreamlin merged 31 commits into
googleapis:masterfrom
andreamlin:escape

Conversation

@andreamlin

Copy link
Copy Markdown
Contributor

Using a glorifed hashmap implementation that is basically a pared-down version of gapic-generator's SymbolTable.

Comment thread plugin/utils/gapic_utils.py Outdated
def read_from_gapic_yaml(yaml_file):
with open(yaml_file) as f:
gapic_yaml = yaml.load(f)
gapic_yaml = yaml.full_load(f)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to get around the PyYAML load() deprecation: https://github.com/yaml/pyyaml/wiki/PyYAML-yaml.load(input)-Deprecation



java_reserved_symbols = {
"abstract",

@andreamlin andreamlin Mar 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This set is ripped from gapic-generator's JavaNameFormatter


private static final PathTemplate PATH_TEMPLATE =
PathTemplate.createWithoutUrlEncoding("shelves/{shelf_id}/books/{book_id}");
PathTemplate.createWithoutUrlEncoding("shelves/{shelf_id}/books/{return}");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file, all forms of book_id are replaced with the corresponding form for return.

'upper': casing_utils.lower_underscore_to_upper_camel(
f['parameter_name']),
'lower': f['parameter'],
'parameter_name_in_map':

@andreamlin andreamlin Mar 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed in the fieldMap, and the value used in the fieldMap used to just be lower, which used to mean casing_utils.lower_underscore_to_lower_camel(lit), but since lower has changed, we have to make a new variable that refers back to the old meaning.

@andreamlin

Copy link
Copy Markdown
Contributor Author

PTAL

Comment thread plugin/utils/symbol_table.py Outdated

# Resolve collisions with one or more underscores.
while (desired_name + "_") in self.symbol_table:
self.symbol_table.add(desired_name + "_")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

desired_name is not changed in this loop. Won't it be an infinite loop in that case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OMG

import copy


class SymbolTable(object):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a test for this class.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for sure

@andreamlin

Copy link
Copy Markdown
Contributor Author

Fixed a bug and verified fix with a test.

PTAL

@michaelbausor michaelbausor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one change requested

Comment thread plugin/utils/symbol_table.py Outdated

# Resolve collisions with one or more underscores.
while (desired_name + "_") in self.symbol_table:
self.symbol_table.add(desired_name + "_")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to add this here? Isn't it already in the symbol table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, removed

@andreamlin
andreamlin merged commit e152eed into googleapis:master Mar 18, 2019
@andreamlin
andreamlin deleted the escape branch March 18, 2019 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants