diff --git a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/DocCommentHelper.java b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/DocCommentHelper.java index 1ccc101a0b0..5d0ec5218dd 100644 --- a/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/DocCommentHelper.java +++ b/lang/java/compiler/src/main/java/org/apache/avro/compiler/idl/DocCommentHelper.java @@ -74,6 +74,14 @@ static void setDoc(Token token) { DOC.set(newDocComment); } + /** + * Clear any documentation (and generate a warning if there was). + * + * This method should NOT be used after an optional component in a grammar + * (i.e., after a @code{[…]} or @code{…*} construct), because the optional + * grammar part may have already caused parsing a doc comment special token + * placed after the code block. + */ static void clearDoc() { DocComment oldDocComment = DOC.get(); if (oldDocComment != null) { diff --git a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj index ee467ff9902..1f931a640cd 100644 --- a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj +++ b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj @@ -1065,7 +1065,7 @@ Protocol CompilationUnit(): /* * Declaration syntax follows. */ -private Schema NamedSchemaDeclaration(Map props): +private Schema NamedSchemaDeclaration(String doc, Map props): { Schema s; String savedSpace = this.namespace; @@ -1076,9 +1076,9 @@ private Schema NamedSchemaDeclaration(Map props): this.namespace = getTextProp("namespace", props, token); } ( - s = FixedDeclaration() - | s = EnumDeclaration() - | s = RecordDeclaration() + s = FixedDeclaration(doc) + | s = EnumDeclaration(doc) + | s = RecordDeclaration(doc) ) { this.namespace = savedSpace; @@ -1125,11 +1125,12 @@ Schema UnionDefinition(): Protocol ProtocolDeclaration(): { - String name; + String doc, name; Protocol p; Map props = new LinkedHashMap<>(); } { + doc = Documentation() ( SchemaProperty(props) )* { if (props.containsKey("namespace")) @@ -1138,7 +1139,7 @@ Protocol ProtocolDeclaration(): "protocol" name = Identifier() { - p = new Protocol(name, DocCommentHelper.getDoc(), namespace); + p = new Protocol(name, doc, namespace); for (String key : props.keySet()) if ("namespace".equals(key)) { // already handled: ignore } else { // add all other properties @@ -1152,17 +1153,30 @@ Protocol ProtocolDeclaration(): } -Schema EnumDeclaration(): +String Documentation(): +{ + //noinspection ResultOfMethodCallIgnored + getToken(1); // Parse, but don't consume, at least one token; this triggers parsing special tokens like doc comments. +} +{ + // Don't parse anything, just return the doc string + { + return DocCommentHelper.getDoc(); + } +} + + +Schema EnumDeclaration(String doc): { String name; List symbols; String defaultSymbol = null; } { - "enum" { String doc = DocCommentHelper.getDoc(); } + "enum" name = Identifier() symbols = EnumBody() - [ defaultSymbol=Identifier() ] + [ defaultSymbol=Identifier() { DocCommentHelper.clearDoc(); } ] { Schema s = Schema.createEnum(name, doc, namespace, symbols, defaultSymbol); names.put(s.getFullName(), s); @@ -1175,10 +1189,11 @@ List EnumBody(): List symbols = new ArrayList<>(); } { - "{" + "{" { DocCommentHelper.clearDoc(); } [ EnumConstant(symbols) ( "," EnumConstant(symbols) )* ] "}" { + DocCommentHelper.clearDoc(); return symbols; } } @@ -1193,13 +1208,14 @@ void EnumConstant(List symbols): void ProtocolBody(Protocol p): { + String doc; Schema schema; Message message; Protocol importProtocol; Map props = new LinkedHashMap<>(); } { - "{" + "{" { DocCommentHelper.clearDoc(); } ( ((( importProtocol = ImportIdl() | importProtocol = ImportProtocol()) { @@ -1208,21 +1224,26 @@ void ProtocolBody(Protocol p): p.getMessages().putAll(importProtocol.getMessages()); }) | schema = ImportSchema() - ) + ) { + DocCommentHelper.clearDoc(); + } | + doc = Documentation() ( SchemaProperty(props) )* ( - schema = NamedSchemaDeclaration(props) + schema = NamedSchemaDeclaration(doc, props) | - message = MessageDeclaration(p, props) { + message = MessageDeclaration(p, doc, props) { p.getMessages().put(message.getName(), message); } - ) { props.clear(); } + ) { + props.clear(); + } ) * "}" - { p.setTypes(names.values()); + DocCommentHelper.clearDoc(); } } @@ -1273,7 +1294,7 @@ Schema ImportSchema() : { } } -Schema FixedDeclaration(): +Schema FixedDeclaration(String doc): { String name; Token sizeTok; @@ -1282,14 +1303,14 @@ Schema FixedDeclaration(): "fixed" name = Identifier() "(" sizeTok = ")" ";" { - Schema s = Schema.createFixed(name, DocCommentHelper.getDoc(), this.namespace, - Integer.parseInt(sizeTok.image)); + DocCommentHelper.clearDoc(); + Schema s = Schema.createFixed(name, doc, this.namespace, Integer.parseInt(sizeTok.image)); names.put(s.getFullName(), s); return s; } } -Schema RecordDeclaration(): +Schema RecordDeclaration(String doc): { String name; List fields = new ArrayList<>(); @@ -1302,14 +1323,14 @@ Schema RecordDeclaration(): ) name = Identifier() { - Schema result = Schema.createRecord( - name, DocCommentHelper.getDoc(), this.namespace, isError); + Schema result = Schema.createRecord(name, doc, this.namespace, isError); names.put(result.getFullName(), result); } - "{" + "{" { DocCommentHelper.clearDoc(); } ( FieldDeclaration(fields) )* "}" { + DocCommentHelper.clearDoc(); result.setFields(fields); return result; } @@ -1332,27 +1353,27 @@ private void SchemaProperty(Map properties): void FieldDeclaration(List fields): { + String defaultDoc; Schema type; } { + defaultDoc = Documentation() type = Type() - VariableDeclarator(type, fields) ( "," VariableDeclarator(type, fields) )* - ";" + VariableDeclarator(type, defaultDoc, fields) ( "," VariableDeclarator(type, defaultDoc, fields) )* + ";" { DocCommentHelper.clearDoc(); } } -void VariableDeclarator(Schema type, List fields): +void VariableDeclarator(Schema type, String defaultDoc, List fields): { - String name; + String doc, name; JsonNode defaultValue = null; Map props = new LinkedHashMap<>(); } { - ( SchemaProperty(props) )* - + doc = Documentation() + ( SchemaProperty(props) )* name = Identifier() - - [ defaultValue=Json() ] - + [ defaultValue=Json() ] { Field.Order order = Field.Order.ASCENDING; for (String key : props.keySet()) @@ -1361,7 +1382,7 @@ void VariableDeclarator(Schema type, List fields): boolean validate = SchemaResolver.isFullyResolvedSchema(type); Schema fieldType = fixOptionalSchema(type, defaultValue); - Field field = Accessor.createField(name, fieldType, DocCommentHelper.getDoc(), defaultValue, validate, order); + Field field = Accessor.createField(name, fieldType, doc == null ? defaultDoc : doc, defaultValue, validate, order); for (String key : props.keySet()) if ("order".equals(key)) { // already handled: ignore } else if ("aliases".equals(key)) { // aliases @@ -1371,22 +1392,13 @@ void VariableDeclarator(Schema type, List fields): Accessor.addProp(field, key, props.get(key)); } fields.add(field); + DocCommentHelper.clearDoc(); } } -String MessageDocumentation(): -{} -{ - // Don't parse anything, just return the doc string - { - return DocCommentHelper.getDoc(); - } -} - -private Message MessageDeclaration(Protocol p, Map props): +private Message MessageDeclaration(Protocol p, String msgDoc, Map props): { - String msgDoc; String name; Schema request; Schema response; @@ -1395,13 +1407,12 @@ private Message MessageDeclaration(Protocol p, Map props): errorSchemata.add(Protocol.SYSTEM_ERROR); } { - msgDoc = MessageDocumentation() - response = ResultType() - name = Identifier() + response = ResultType() name = Identifier() request = FormalParameters() [ "oneway" {oneWay = true; } | "throws" ErrorList(errorSchemata) ] ";" { + DocCommentHelper.clearDoc(); Schema errors = Schema.createUnion(errorSchemata); if (oneWay && response.getType() != Type.NULL) throw error("One-way message'"+name+"' must return void", token); @@ -1426,19 +1437,23 @@ Schema FormalParameters(): List fields = new ArrayList<>(); } { - "(" [ FormalParameter(fields) ( "," FormalParameter(fields) )* ] ")" + "(" { DocCommentHelper.clearDoc(); } + [ FormalParameter(fields) ( "," FormalParameter(fields) )* ] ")" { + DocCommentHelper.clearDoc(); return Schema.createRecord(null, null, null, false, fields); } } void FormalParameter(List fields): { + String doc; Schema type; } { + doc = Documentation() type = Type() - VariableDeclarator(type, fields) + VariableDeclarator(type, doc, fields) } Schema Type(): diff --git a/lang/java/compiler/src/test/idl/input/comments.avdl b/lang/java/compiler/src/test/idl/input/comments.avdl index 5c619965ba4..38024af415a 100644 --- a/lang/java/compiler/src/test/idl/input/comments.avdl +++ b/lang/java/compiler/src/test/idl/input/comments.avdl @@ -27,41 +27,38 @@ protocol Comments { /** Dangling Enum8 */ A /** Dangling Enum9 */; - // The "Dangling Enum9" doc comment above will be attributed to this enum. - enum NotUndocumentedEnum {D,E} + enum UndocumentedEnum {D,E} - /** Dangling Fixed1 */ fixed - /** Dangling Fixed2 */ DocumentedFixed - /** Dangling Fixed3 */( - /** Dangling Fixed4 */ 16 - /** Dangling Fixed5 */) - /** Documented Fixed Type */; + /** Documented Fixed Type */ fixed + /** Dangling Fixed1 */ DocumentedFixed + /** Dangling Fixed2 */( + /** Dangling Fixed3 */ 16 + /** Dangling Fixed4 */) + /** Dangling Fixed5 */; fixed UndocumentedFixed(16); - /** Dangling Error1 */ error - /** Documented Error */ DocumentedError + /** Documented Error */ error + /** Dangling Error1 */ DocumentedError /** Dangling Field1 */{ - /** Dangling Field2 */string - /** Dangling Field3 */reason - /** Documented Field */; + /** Default Doc Explanation Field */string + /** Documented Reason Field */reason, explanation + /** Dangling Field2 */; /** Dangling Error2 */} - // The "Dangling Error2" doc comment above will be attributed to this record. - record NotUndocumentedRecord { + record UndocumentedRecord { string description; } /** Documented Method */ void /** Dangling Param1 */ documentedMethod /** Dangling Param2 */( - /** Dangling Param3 */ string - /** Dangling Param4 */ message - /** Documented Parameter */) + string /** Documented Parameter */ message, + /** Default Documented Parameter */ string defMsg + /** Dangling Param3 */) /** Dangling Method1 */ throws /** Dangling Method2 */ DocumentedError /** Dangling Method3 */; - // The "Dangling Method3" doc comment above will be attributed to this method. - void notUndocumentedMethod(string message); + void undocumentedMethod(string message); } diff --git a/lang/java/compiler/src/test/idl/output/comments.avpr b/lang/java/compiler/src/test/idl/output/comments.avpr index 2d98962a576..9901f8eebc8 100644 --- a/lang/java/compiler/src/test/idl/output/comments.avpr +++ b/lang/java/compiler/src/test/idl/output/comments.avpr @@ -9,8 +9,7 @@ "default" : "A" }, { "type" : "enum", - "name" : "NotUndocumentedEnum", - "doc" : "Dangling Enum9", + "name" : "UndocumentedEnum", "symbols" : [ "D", "E" ] }, { "type" : "fixed", @@ -28,12 +27,15 @@ "fields" : [ { "name" : "reason", "type" : "string", - "doc" : "Documented Field" + "doc" : "Documented Reason Field" + }, { + "name" : "explanation", + "type" : "string", + "doc" : "Default Doc Explanation Field" } ] }, { "type" : "record", - "name" : "NotUndocumentedRecord", - "doc" : "Dangling Error2", + "name" : "UndocumentedRecord", "fields" : [ { "name" : "description", "type" : "string" @@ -46,12 +48,15 @@ "name" : "message", "type" : "string", "doc" : "Documented Parameter" + }, { + "name" : "defMsg", + "type" : "string", + "doc" : "Default Documented Parameter" } ], "response" : "null", "errors" : [ "DocumentedError" ] }, - "notUndocumentedMethod" : { - "doc" : "Dangling Method3", + "undocumentedMethod" : { "request" : [ { "name" : "message", "type" : "string" diff --git a/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestIdl.java b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestIdl.java index 7e609ba5c7a..37e6b2bb2c0 100644 --- a/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestIdl.java +++ b/lang/java/compiler/src/test/java/org/apache/avro/compiler/idl/TestIdl.java @@ -35,9 +35,11 @@ import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; +import static java.util.Objects.requireNonNull; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -46,12 +48,12 @@ /** * Simple test harness for Idl. This relies on an input/ and output/ directory. * Inside the input/ directory are .avdl files. Each file should have a - * corresponding .avpr file in output/. When the test is run, it generates and + * corresponding .avpr file in output/. When you run the test, it generates and * stringifies each .avdl file and compares it to the expected output, failing * if the two differ. * * To make it simpler to write these tests, you can run ant -Dtestcase=TestIdl - * -Dtest.idl.mode=write which will *replace* all expected output. + * -Dtest.idl.mode=write, which will *replace* all expected output. */ public class TestIdl { private static final File TEST_DIR = new File(System.getProperty("test.idl.dir", "src/test/idl")); @@ -71,7 +73,7 @@ public void loadTests() { assertTrue(TEST_OUTPUT_DIR.exists()); tests = new ArrayList<>(); - for (File inF : TEST_INPUT_DIR.listFiles()) { + for (File inF : requireNonNull(TEST_INPUT_DIR.listFiles())) { if (!inF.getName().endsWith(".avdl")) continue; if (inF.getName().startsWith(".")) @@ -100,7 +102,7 @@ public void runTests() throws Exception { } if (failed > 0) { - fail(String.valueOf(failed) + " tests failed"); + fail(failed + " tests failed"); } } @@ -121,44 +123,45 @@ public void testDocCommentsAndWarnings() throws Exception { final List warnings = parser.getWarningsAfterParsing(); assertEquals("Documented Enum", protocol.getType("testing.DocumentedEnum").getDoc()); - assertEquals("Dangling Enum9", protocol.getType("testing.NotUndocumentedEnum").getDoc()); // Arguably a bug + assertEquals("Documented Fixed Type", protocol.getType("testing.DocumentedFixed").getDoc()); - assertNull(protocol.getType("testing.UndocumentedFixed").getDoc()); + final Schema documentedError = protocol.getType("testing.DocumentedError"); assertEquals("Documented Error", documentedError.getDoc()); - assertEquals("Documented Field", documentedError.getField("reason").doc()); - assertEquals("Dangling Error2", protocol.getType("testing.NotUndocumentedRecord").getDoc()); // Arguably a bug + assertEquals("Documented Reason Field", documentedError.getField("reason").doc()); + assertEquals("Default Doc Explanation Field", documentedError.getField("explanation").doc()); + final Map messages = protocol.getMessages(); - assertEquals("Documented Method", messages.get("documentedMethod").getDoc()); - assertEquals("Documented Parameter", messages.get("documentedMethod").getRequest().getField("message").doc()); - assertEquals("Dangling Method3", messages.get("notUndocumentedMethod").getDoc()); // Arguably a bug + final Protocol.Message documentedMethod = messages.get("documentedMethod"); + assertEquals("Documented Method", documentedMethod.getDoc()); + assertEquals("Documented Parameter", documentedMethod.getRequest().getField("message").doc()); + assertEquals("Default Documented Parameter", documentedMethod.getRequest().getField("defMsg").doc()); - assertEquals(23, warnings.size()); - final String pattern = "Found documentation comment at line %d, column %d. Ignoring previous one at line %d, column %d: \"%s\"" + assertNull(protocol.getType("testing.UndocumentedEnum").getDoc()); + assertNull(protocol.getType("testing.UndocumentedFixed").getDoc()); + assertNull(protocol.getType("testing.UndocumentedRecord").getDoc()); + assertNull(messages.get("undocumentedMethod").getDoc()); + + final String pattern1 = "Found documentation comment at line %d, column %d. Ignoring previous one at line %d, column %d: \"%s\"" + + "\nDid you mean to use a multiline comment ( /* ... */ ) instead?"; + final String pattern2 = "Ignoring out-of-place documentation comment at line %d, column %d: \"%s\"" + "\nDid you mean to use a multiline comment ( /* ... */ ) instead?"; - assertEquals(String.format(pattern, 21, 47, 21, 10, "Dangling Enum1"), warnings.get(0)); - assertEquals(String.format(pattern, 22, 9, 21, 47, "Dangling Enum2"), warnings.get(1)); - assertEquals(String.format(pattern, 23, 9, 22, 9, "Dangling Enum3"), warnings.get(2)); - assertEquals(String.format(pattern, 24, 9, 23, 9, "Dangling Enum4"), warnings.get(3)); - assertEquals(String.format(pattern, 25, 5, 24, 9, "Dangling Enum5"), warnings.get(4)); - assertEquals(String.format(pattern, 26, 5, 25, 5, "Dangling Enum6"), warnings.get(5)); - assertEquals(String.format(pattern, 27, 5, 26, 5, "Dangling Enum7"), warnings.get(6)); - assertEquals(String.format(pattern, 28, 5, 27, 5, "Dangling Enum8"), warnings.get(7)); - assertEquals(String.format(pattern, 34, 5, 33, 5, "Dangling Fixed1"), warnings.get(8)); - assertEquals(String.format(pattern, 35, 5, 34, 5, "Dangling Fixed2"), warnings.get(9)); - assertEquals(String.format(pattern, 36, 5, 35, 5, "Dangling Fixed3"), warnings.get(10)); - assertEquals(String.format(pattern, 37, 5, 36, 5, "Dangling Fixed4"), warnings.get(11)); - assertEquals(String.format(pattern, 38, 5, 37, 5, "Dangling Fixed5"), warnings.get(12)); - assertEquals(String.format(pattern, 43, 5, 42, 5, "Dangling Error1"), warnings.get(13)); - assertEquals(String.format(pattern, 45, 5, 44, 5, "Dangling Field1"), warnings.get(14)); - assertEquals(String.format(pattern, 46, 5, 45, 5, "Dangling Field2"), warnings.get(15)); - assertEquals(String.format(pattern, 47, 5, 46, 5, "Dangling Field3"), warnings.get(16)); - assertEquals(String.format(pattern, 57, 5, 56, 5, "Dangling Param1"), warnings.get(17)); - assertEquals(String.format(pattern, 58, 9, 57, 5, "Dangling Param2"), warnings.get(18)); - assertEquals(String.format(pattern, 59, 9, 58, 9, "Dangling Param3"), warnings.get(19)); - assertEquals(String.format(pattern, 60, 5, 59, 9, "Dangling Param4"), warnings.get(20)); - assertEquals(String.format(pattern, 62, 5, 61, 5, "Dangling Method1"), warnings.get(21)); - assertEquals(String.format(pattern, 63, 5, 62, 5, "Dangling Method2"), warnings.get(22)); + assertEquals(Arrays.asList(String.format(pattern1, 21, 47, 21, 10, "Dangling Enum1"), + String.format(pattern2, 21, 47, "Dangling Enum2"), String.format(pattern1, 23, 9, 22, 9, "Dangling Enum3"), + String.format(pattern1, 24, 9, 23, 9, "Dangling Enum4"), + String.format(pattern1, 25, 5, 24, 9, "Dangling Enum5"), String.format(pattern2, 25, 5, "Dangling Enum6"), + String.format(pattern1, 27, 5, 26, 5, "Dangling Enum7"), + String.format(pattern1, 28, 5, 27, 5, "Dangling Enum8"), String.format(pattern2, 28, 5, "Dangling Enum9"), + String.format(pattern1, 34, 5, 33, 5, "Dangling Fixed1"), + String.format(pattern1, 35, 5, 34, 5, "Dangling Fixed2"), + String.format(pattern1, 36, 5, 35, 5, "Dangling Fixed3"), + String.format(pattern1, 37, 5, 36, 5, "Dangling Fixed4"), String.format(pattern2, 37, 5, "Dangling Fixed5"), + String.format(pattern1, 43, 5, 42, 5, "Dangling Error1"), String.format(pattern2, 43, 5, "Dangling Field1"), + String.format(pattern2, 46, 5, "Dangling Field2"), String.format(pattern2, 47, 5, "Dangling Error2"), + String.format(pattern1, 55, 5, 54, 5, "Dangling Param1"), String.format(pattern2, 55, 5, "Dangling Param2"), + String.format(pattern2, 58, 5, "Dangling Param3"), String.format(pattern1, 60, 5, 59, 5, "Dangling Method1"), + String.format(pattern1, 61, 5, 60, 5, "Dangling Method2"), + String.format(pattern2, 61, 5, "Dangling Method3")), warnings); } }