forked from apache/avro
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
AVRO-3239: IDL parsing silently ignores dangling documentation commen…
…ts (apache#1377) * AVRO-3239: Add warnings for ignored doc comments * AVRO-3239: Add mock Maven Log implementation * AVRO-3239: Log parser warnings in Maven IDL mojo * AVRO-3239: Log parser warnings in the IDL tools * AVRO-3239: Document new (public) parser method * AVRO-3239: Undo merge error and fix build * AVRO-3239: Extract helper class for doc comments The helper class allows better testing of and documentation for handling doc comments. The .avpr changes all dedented comments. * AVRO-3239: Add missing license * AVRO-3239: Fix documentation comment * AVRO-3239: Fix tests * AVRO-3239: Apply review comments
- Loading branch information
Showing
25 changed files
with
795 additions
and
37 deletions.
There are no files selected for viewing
127 changes: 127 additions & 0 deletions
127
lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/DocCommentHelper.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you 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 | ||
| * | ||
| * https://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 org.apache.avro.compiler.idl; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * Utility class with {@code ThreadLocal} fields that allow the generated | ||
| * classes {@link Idl} and {@link IdlTokenManager} to exchange documentation | ||
| * comments without forcing explicit parsing of documentation comments. | ||
| * | ||
| * The reason this works is that all calls to this class happen within a call to | ||
| * the method {@link Idl#CompilationUnit()} (either directly or indirectly). | ||
| */ | ||
| public class DocCommentHelper { | ||
| /** | ||
| * Pattern to match the common whitespace indents in a multi-line String. | ||
| * Doesn't match a single-line String, fully matches any multi-line String. | ||
| * | ||
| * To use: match on a {@link String#trim() trimmed} String, and then replace all | ||
| * newlines followed by the group "indent" with a newline. | ||
| */ | ||
| private static final Pattern WS_INDENT = Pattern.compile("(?U).*\\R(?<indent>\\h*).*(?:\\R\\k<indent>.*)*"); | ||
| /** | ||
| * Pattern to match the whitespace indents plus common stars (1 or 2) in a | ||
| * multi-line String. If a String fully matches, replace all occurrences of a | ||
| * newline followed by whitespace and then the group "stars" with a newline. | ||
| * | ||
| * Note: partial matches are invalid. | ||
| */ | ||
| private static final Pattern STAR_INDENT = Pattern.compile("(?U)(?<stars>\\*{1,2}).*(?:\\R\\h*\\k<stars>.*)*"); | ||
|
|
||
| private static final ThreadLocal<DocComment> DOC = new ThreadLocal<>(); | ||
| private static final ThreadLocal<List<String>> WARNINGS = ThreadLocal.withInitial(ArrayList::new); | ||
|
|
||
| /** | ||
| * Return all warnings that were encountered while parsing, once. Subsequent | ||
| * calls before parsing again will return an empty list. | ||
| */ | ||
| static List<String> getAndClearWarnings() { | ||
| List<String> warnings = WARNINGS.get(); | ||
| WARNINGS.remove(); | ||
| return warnings; | ||
| } | ||
|
|
||
| static void setDoc(Token token) { | ||
| DocComment newDocComment = new DocComment(token); | ||
| DocComment oldDocComment = DOC.get(); | ||
| if (oldDocComment != null) { | ||
| WARNINGS.get() | ||
| .add(String.format( | ||
| "Found documentation comment at line %d, column %d. Ignoring previous one at line %d, column %d: \"%s\"\n" | ||
| + "Did you mean to use a multiline comment ( /* ... */ ) instead?", | ||
| newDocComment.line, newDocComment.column, oldDocComment.line, oldDocComment.column, oldDocComment.text)); | ||
| } | ||
| DOC.set(newDocComment); | ||
| } | ||
|
|
||
| static void clearDoc() { | ||
| DocComment oldDocComment = DOC.get(); | ||
| if (oldDocComment != null) { | ||
| WARNINGS.get() | ||
| .add(String.format( | ||
| "Ignoring out-of-place documentation comment at line %d, column %d: \"%s\"\n" | ||
| + "Did you mean to use a multiline comment ( /* ... */ ) instead?", | ||
| oldDocComment.line, oldDocComment.column, oldDocComment.text)); | ||
| } | ||
| DOC.remove(); | ||
| } | ||
|
|
||
| static String getDoc() { | ||
| DocComment docComment = DOC.get(); | ||
| DOC.remove(); | ||
| return docComment == null ? null : docComment.text; | ||
| } | ||
|
|
||
| /* Package private to facilitate testing */ | ||
| static String stripIndents(String doc) { | ||
| Matcher starMatcher = STAR_INDENT.matcher(doc); | ||
| if (starMatcher.matches()) { | ||
| return doc.replaceAll("(?U)(?:^|(\\R)\\h*)\\Q" + starMatcher.group("stars") + "\\E\\h?", "$1"); | ||
| } | ||
|
|
||
| Matcher whitespaceMatcher = WS_INDENT.matcher(doc); | ||
| if (whitespaceMatcher.matches()) { | ||
| return doc.replaceAll("(?U)(\\R)" + whitespaceMatcher.group("indent"), "$1"); | ||
| } | ||
|
|
||
| return doc; | ||
| } | ||
|
|
||
| private static class DocComment { | ||
| private final String text; | ||
| private final int line; | ||
| private final int column; | ||
|
|
||
| DocComment(Token token) { | ||
| // The token is everything after the initial '/**', including all | ||
| // whitespace and the ending '*/' | ||
| int tokenLength = token.image.length(); | ||
| this.text = stripIndents(token.image.substring(0, tokenLength - 2).trim()); | ||
| this.line = token.beginLine; | ||
| // The preceding token was "/**", and the current token includes | ||
| // everything since (also all whitespace). Thus, we can safely subtract 3 | ||
| // from the token column to get the start of the doc comment. | ||
| this.column = token.beginColumn - 3; | ||
| } | ||
| } | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you 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 | ||
| * | ||
| * https://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. | ||
| */ | ||
| @namespace("testing") | ||
| protocol Comments { | ||
| /** Documented Enum */ | ||
| enum /** Dangling Enum1 */ DocumentedEnum /** Dangling Enum2 */ { | ||
| /** Dangling Enum3 */ A, | ||
| /** Dangling Enum4 */ B, | ||
| /** Dangling Enum5 */ C | ||
| /** Dangling Enum6 */} | ||
| /** Dangling Enum7 */= | ||
| /** Dangling Enum8 */ A | ||
| /** Dangling Enum9 */; | ||
|
|
||
| // The "Dangling Enum9" doc comment above will be attributed to this enum. | ||
| enum NotUndocumentedEnum {D,E} | ||
|
|
||
| /** Dangling Fixed1 */ fixed | ||
| /** Dangling Fixed2 */ DocumentedFixed | ||
| /** Dangling Fixed3 */( | ||
| /** Dangling Fixed4 */ 16 | ||
| /** Dangling Fixed5 */) | ||
| /** Documented Fixed Type */; | ||
|
|
||
| fixed UndocumentedFixed(16); | ||
|
|
||
| /** Dangling Error1 */ error | ||
| /** Documented Error */ DocumentedError | ||
| /** Dangling Field1 */{ | ||
| /** Dangling Field2 */string | ||
| /** Dangling Field3 */reason | ||
| /** Documented Field */; | ||
| /** Dangling Error2 */} | ||
|
|
||
| // The "Dangling Error2" doc comment above will be attributed to this record. | ||
| record NotUndocumentedRecord { | ||
| string description; | ||
| } | ||
|
|
||
| /** Documented Method */ void | ||
| /** Dangling Param1 */ documentedMethod | ||
| /** Dangling Param2 */( | ||
| /** Dangling Param3 */ string | ||
| /** Dangling Param4 */ message | ||
| /** Documented Parameter */) | ||
| /** Dangling Method1 */ throws | ||
| /** Dangling Method2 */ DocumentedError | ||
| /** Dangling Method3 */; | ||
|
|
||
| // The "Dangling Method3" doc comment above will be attributed to this method. | ||
| void notUndocumentedMethod(string message); | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| { | ||
| "protocol" : "Comments", | ||
| "namespace" : "testing", | ||
| "types" : [ { | ||
| "type" : "enum", | ||
| "name" : "DocumentedEnum", | ||
| "doc" : "Documented Enum", | ||
| "symbols" : [ "A", "B", "C" ], | ||
| "default" : "A" | ||
| }, { | ||
| "type" : "enum", | ||
| "name" : "NotUndocumentedEnum", | ||
| "doc" : "Dangling Enum9", | ||
| "symbols" : [ "D", "E" ] | ||
| }, { | ||
| "type" : "fixed", | ||
| "name" : "DocumentedFixed", | ||
| "doc" : "Documented Fixed Type", | ||
| "size" : 16 | ||
| }, { | ||
| "type" : "fixed", | ||
| "name" : "UndocumentedFixed", | ||
| "size" : 16 | ||
| }, { | ||
| "type" : "error", | ||
| "name" : "DocumentedError", | ||
| "doc" : "Documented Error", | ||
| "fields" : [ { | ||
| "name" : "reason", | ||
| "type" : "string", | ||
| "doc" : "Documented Field" | ||
| } ] | ||
| }, { | ||
| "type" : "record", | ||
| "name" : "NotUndocumentedRecord", | ||
| "doc" : "Dangling Error2", | ||
| "fields" : [ { | ||
| "name" : "description", | ||
| "type" : "string" | ||
| } ] | ||
| } ], | ||
| "messages" : { | ||
| "documentedMethod" : { | ||
| "doc" : "Documented Method", | ||
| "request" : [ { | ||
| "name" : "message", | ||
| "type" : "string", | ||
| "doc" : "Documented Parameter" | ||
| } ], | ||
| "response" : "null", | ||
| "errors" : [ "DocumentedError" ] | ||
| }, | ||
| "notUndocumentedMethod" : { | ||
| "doc" : "Dangling Method3", | ||
| "request" : [ { | ||
| "name" : "message", | ||
| "type" : "string" | ||
| } ], | ||
| "response" : "null" | ||
| } | ||
| } | ||
| } |
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
Oops, something went wrong.