From bcf335b7dffcd0f4a7ec820d1919f611d1d0f24c Mon Sep 17 00:00:00 2001 From: Song Wang Date: Mon, 7 Nov 2016 15:21:23 -0800 Subject: [PATCH] Fix datastore grpc doc class generation for python and ruby (#716) * Fix datastore grpc doc class generation for python and ruby * Fix PROTO_LINK_PATTERN * Add unit tests for ProtoLink matching and Ruby/Python/NodeJS CommentFixer --- .../google/api/codegen/CommentPatterns.java | 3 +- .../api/codegen/nodejs/JSDocCommentFixer.java | 6 +- .../codegen/py/PythonSphinxCommentFixer.java | 12 +- .../api/codegen/ruby/RDocCommentFixer.java | 17 ++- .../api/codegen/ProtoDocumentLinkTest.java | 111 ++++++++++++++++++ 5 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 src/test/java/com/google/api/codegen/ProtoDocumentLinkTest.java diff --git a/src/main/java/com/google/api/codegen/CommentPatterns.java b/src/main/java/com/google/api/codegen/CommentPatterns.java index 0d625d9e07..49db3c38e1 100644 --- a/src/main/java/com/google/api/codegen/CommentPatterns.java +++ b/src/main/java/com/google/api/codegen/CommentPatterns.java @@ -26,6 +26,7 @@ public final class CommentPatterns { Pattern.compile("\\[([^\\]]+)\\]\\((\\p{Alpha}+:[^\\)]+)\\)"); public static final Pattern CLOUD_LINK_PATTERN = Pattern.compile("\\[([^\\]]+)\\]\\(((?!\\p{Alpha}+:)[^\\)]+)\\)"); - public static final Pattern PROTO_LINK_PATTERN = Pattern.compile("\\[([^\\]]+)\\]\\[[^\\]]*\\]"); + public static final Pattern PROTO_LINK_PATTERN = + Pattern.compile("\\[([^\\]]+)\\]\\[([A-Za-z_][A-Za-z_.0-9]*)?\\]"); public static final Pattern HEADLINE_PATTERN = Pattern.compile("^#+", Pattern.MULTILINE); } diff --git a/src/main/java/com/google/api/codegen/nodejs/JSDocCommentFixer.java b/src/main/java/com/google/api/codegen/nodejs/JSDocCommentFixer.java index a9c12b03a2..a6f5626036 100644 --- a/src/main/java/com/google/api/codegen/nodejs/JSDocCommentFixer.java +++ b/src/main/java/com/google/api/codegen/nodejs/JSDocCommentFixer.java @@ -35,7 +35,8 @@ private static String jsdocifyProtoMarkdownLinks(String comment) { return comment; } do { - m.appendReplacement(sb, String.format("{@link %s}", m.group(1))); + // proto display name may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement(sb, Matcher.quoteReplacement(String.format("{@link %s}", m.group(1)))); } while (m.find()); m.appendTail(sb); return sb.toString(); @@ -50,7 +51,8 @@ private static String jsdocifyCloudMarkdownLinks(String comment) { } do { String url = "https://cloud.google.com" + m.group(2); - m.appendReplacement(sb, String.format("[%s](%s)", m.group(1), url)); + // cloud markdown links may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement(sb, Matcher.quoteReplacement(String.format("[%s](%s)", m.group(1), url))); } while (m.find()); m.appendTail(sb); return sb.toString(); diff --git a/src/main/java/com/google/api/codegen/py/PythonSphinxCommentFixer.java b/src/main/java/com/google/api/codegen/py/PythonSphinxCommentFixer.java index 5f77a1040a..52278fde3e 100644 --- a/src/main/java/com/google/api/codegen/py/PythonSphinxCommentFixer.java +++ b/src/main/java/com/google/api/codegen/py/PythonSphinxCommentFixer.java @@ -37,7 +37,8 @@ private static String sphinxifyProtoMarkdownLinks(String comment) { return comment; } do { - m.appendReplacement(sb, String.format("``%s``", m.group(1))); + // proto display name may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement(sb, Matcher.quoteReplacement(String.format("``%s``", m.group(1)))); } while (m.find()); m.appendTail(sb); return sb.toString(); @@ -51,7 +52,9 @@ private static String sphinxifyAbsoluteMarkdownLinks(String comment) { return comment; } do { - m.appendReplacement(sb, String.format("`%s <%s>`_", m.group(1), m.group(2))); + // absolute markdown links may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement( + sb, Matcher.quoteReplacement(String.format("`%s <%s>`_", m.group(1), m.group(2)))); } while (m.find()); m.appendTail(sb); return sb.toString(); @@ -65,8 +68,11 @@ private static String sphinxifyCloudMarkdownLinks(String comment) { return comment; } do { + // cloud markdown links may contain '$' which needs to be escaped using Matcher.quoteReplacement m.appendReplacement( - sb, String.format("`%s `_", m.group(1), m.group(2))); + sb, + Matcher.quoteReplacement( + String.format("`%s `_", m.group(1), m.group(2)))); } while (m.find()); m.appendTail(sb); return sb.toString(); diff --git a/src/main/java/com/google/api/codegen/ruby/RDocCommentFixer.java b/src/main/java/com/google/api/codegen/ruby/RDocCommentFixer.java index 6128b4f5db..28de46f42d 100644 --- a/src/main/java/com/google/api/codegen/ruby/RDocCommentFixer.java +++ b/src/main/java/com/google/api/codegen/ruby/RDocCommentFixer.java @@ -21,7 +21,7 @@ /** Utility class for formatting source comments to follow RDoc style. */ public class RDocCommentFixer { - /** Returns a Sphinx-formatted comment string. */ + /** Returns a RDoc-formatted comment string. */ public static String rdocify(String comment) { comment = CommentPatterns.BACK_QUOTE_PATTERN.matcher(comment).replaceAll("+"); comment = rdocifyProtoMarkdownLinks(comment); @@ -61,13 +61,15 @@ private static String rdocifyProtoMarkdownLinks(String comment) { return comment; } do { - m.appendReplacement(sb, String.format("%s", protoToRubyDoc(m.group(1)))); + // proto display name may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement( + sb, Matcher.quoteReplacement(String.format("%s", protoToRubyDoc(m.group(1))))); } while (m.find()); m.appendTail(sb); return sb.toString(); } - /** Returns a string with all cloud markdown links formatted to Sphinx style. */ + /** Returns a string with all cloud markdown links formatted to RDoc style. */ private static String rdocifyCloudMarkdownLinks(String comment) { StringBuffer sb = new StringBuffer(); Matcher m = CommentPatterns.CLOUD_LINK_PATTERN.matcher(comment); @@ -76,13 +78,14 @@ private static String rdocifyCloudMarkdownLinks(String comment) { } do { String url = "https://cloud.google.com" + m.group(2); - m.appendReplacement(sb, String.format("{%s}[%s]", m.group(1), url)); + // cloud markdown links may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement(sb, Matcher.quoteReplacement(String.format("{%s}[%s]", m.group(1), url))); } while (m.find()); m.appendTail(sb); return sb.toString(); } - /** Returns a string with all cloud markdown links formatted to Sphinx style. */ + /** Returns a string with all absolute markdown links formatted to RDoc style. */ private static String rdocifyAbsoluteMarkdownLinks(String comment) { StringBuffer sb = new StringBuffer(); Matcher m = CommentPatterns.ABSOLUTE_LINK_PATTERN.matcher(comment); @@ -90,7 +93,9 @@ private static String rdocifyAbsoluteMarkdownLinks(String comment) { return comment; } do { - m.appendReplacement(sb, String.format("{%s}[%s]", m.group(1), m.group(2))); + // absolute markdown links may contain '$' which needs to be escaped using Matcher.quoteReplacement + m.appendReplacement( + sb, Matcher.quoteReplacement(String.format("{%s}[%s]", m.group(1), m.group(2)))); } while (m.find()); m.appendTail(sb); return sb.toString(); diff --git a/src/test/java/com/google/api/codegen/ProtoDocumentLinkTest.java b/src/test/java/com/google/api/codegen/ProtoDocumentLinkTest.java new file mode 100644 index 0000000000..a3ba914398 --- /dev/null +++ b/src/test/java/com/google/api/codegen/ProtoDocumentLinkTest.java @@ -0,0 +1,111 @@ +/* Copyright 2016 Google Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.api.codegen; + +import com.google.api.codegen.nodejs.JSDocCommentFixer; +import com.google.api.codegen.py.PythonSphinxCommentFixer; +import com.google.api.codegen.ruby.RDocCommentFixer; +import com.google.common.truth.Truth; +import java.util.regex.Matcher; +import org.junit.Test; + +public class ProtoDocumentLinkTest { + + /* ProtoLink in comments is defined as [display name][fully qualified name] */ + @Test + public void testMatchProtoLink() { + Matcher m; + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[Shelf][google.example.library.v1.Shelf]"); + Truth.assertThat(m.find()).isTrue(); + // Fully qualified name can be empty + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[Shelf][]"); + Truth.assertThat(m.find()).isTrue(); + // Display name may contain special character '$' + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[$Shelf][]"); + Truth.assertThat(m.find()).isTrue(); + // Display name may NOT be empty + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[][abc]"); + Truth.assertThat(m.find()).isFalse(); + // Fully qualified name may NOT contain special character '$' + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[A-Za-z_$][A-Za-z_$0-9]"); + Truth.assertThat(m.find()).isFalse(); + // Fully qualified name may NOT be numbers only + m = CommentPatterns.PROTO_LINK_PATTERN.matcher("[Shelf][123]"); + Truth.assertThat(m.find()).isFalse(); + } + + @Test + public void testRDocCommentFixer() { + Truth.assertThat(RDocCommentFixer.rdocify("[Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("Shelf"); + Truth.assertThat(RDocCommentFixer.rdocify("[$Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("$Shelf"); + + // Cloud link may contain special character '$' + Truth.assertThat(RDocCommentFixer.rdocify("[cloud docs!](/library/example/link)")) + .isEqualTo("{cloud docs!}[https://cloud.google.com/library/example/link]"); + Truth.assertThat(RDocCommentFixer.rdocify("[cloud docs!](/library/example/link$)")) + .isEqualTo("{cloud docs!}[https://cloud.google.com/library/example/link$]"); + + // Absolute link may contain special character '$' + Truth.assertThat(RDocCommentFixer.rdocify("[not a cloud link](http://www.google.com)")) + .isEqualTo("{not a cloud link}[http://www.google.com]"); + Truth.assertThat(RDocCommentFixer.rdocify("[not a cloud link](http://www.google.com$)")) + .isEqualTo("{not a cloud link}[http://www.google.com$]"); + } + + @Test + public void testPythonSphinxCommentFixer() { + Truth.assertThat(PythonSphinxCommentFixer.sphinxify("[Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("``Shelf``"); + Truth.assertThat( + PythonSphinxCommentFixer.sphinxify("[$Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("``$Shelf``"); + + // Cloud link may contain special character '$' + Truth.assertThat(PythonSphinxCommentFixer.sphinxify("[cloud docs!](/library/example/link)")) + .isEqualTo("`cloud docs! `_"); + Truth.assertThat(PythonSphinxCommentFixer.sphinxify("[cloud docs!](/library/example/link$)")) + .isEqualTo("`cloud docs! `_"); + + // Absolute link may contain special character '$' + Truth.assertThat( + PythonSphinxCommentFixer.sphinxify("[not a cloud link](http://www.google.com)")) + .isEqualTo("`not a cloud link `_"); + Truth.assertThat( + PythonSphinxCommentFixer.sphinxify("[not a cloud link](http://www.google.com$)")) + .isEqualTo("`not a cloud link `_"); + } + + @Test + public void testJSDocCommentFixer() { + Truth.assertThat(JSDocCommentFixer.jsdocify("[Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("{@link Shelf}"); + Truth.assertThat(JSDocCommentFixer.jsdocify("[$Shelf][google.example.library.v1.Shelf]")) + .isEqualTo("{@link $Shelf}"); + + // Cloud link may contain special character '$' + Truth.assertThat(JSDocCommentFixer.jsdocify("[cloud docs!](/library/example/link)")) + .isEqualTo("[cloud docs!](https://cloud.google.com/library/example/link)"); + Truth.assertThat(JSDocCommentFixer.jsdocify("[cloud docs!](/library/example/link$)")) + .isEqualTo("[cloud docs!](https://cloud.google.com/library/example/link$)"); + + // Absolute link may contain special character '$' + Truth.assertThat(JSDocCommentFixer.jsdocify("[not a cloud link](http://www.google.com)")) + .isEqualTo("[not a cloud link](http://www.google.com)"); + Truth.assertThat(JSDocCommentFixer.jsdocify("[not a cloud link](http://www.google.com$)")) + .isEqualTo("[not a cloud link](http://www.google.com$)"); + } +}