fix: Simple bug fixes.

* Fixes #145 * Renamed `intergration` to `integration` globally * Fixes #217 * Added documentation comments before `syntax`, `import`, `package`, `option`, and `reserved` keywords in grammar * The root cause is the change to docComment to make it non-skipped, so we have to add it everywhere. Skipped comments (the `//` type) can be absolutely anywhere because the grammar _skips_ them. * Note, this _might_ also fix #319 Signed-off-by: Joseph Sinclair <121976561+jsync-swirlds@users.noreply.github.com>
hashgraph · Jan 13, 2025 · 499ec7d · 499ec7d
1 parent 1ad0f26
commit 499ec7d
Show file tree

Hide file tree

Showing 25 changed files with 94 additions and 130 deletions.
diff --git a/pbj-core/gradle/plugins/src/main/kotlin/com.hedera.pbj.spotless-conventions.gradle.kts b/pbj-core/gradle/plugins/src/main/kotlin/com.hedera.pbj.spotless-conventions.gradle.kts
diff --git a/pbj-core/pbj-compiler/src/main/antlr/com/hedera/hashgraph/protoparser/grammar/Protobuf3.g4 b/pbj-core/pbj-compiler/src/main/antlr/com/hedera/hashgraph/protoparser/grammar/Protobuf3.g4
@@ -6,19 +6,17 @@
  * Original source is published under Apache License 2.0.
  *
  * Changes from the source above:
- * - rewrite to antlr
+ * - rewrite to antlr.
  * - extract some group to rule.
- *
- * @author anatawa12
+ * - Allow document comments in a few added places.
  */
 
 grammar Protobuf3;
 
 proto
   : DOC_COMMENT?
     syntax
-    (
-        importStatement
+    ( importStatement
       | packageStatement
       | optionStatement
       | topLevelDef
@@ -35,25 +33,25 @@ optionComment: OPTION_LINE_COMMENT;
 // Syntax
 
 syntax
-  : SYNTAX EQ (PROTO3_LIT_SINGLE | PROTO3_LIT_DOBULE) SEMI
+  : SYNTAX EQ (PROTO3_LIT_SINGLE | PROTO3_LIT_DOUBLE) SEMI
   ;
 
 // Import Statement
 
 importStatement
-  : IMPORT ( WEAK | PUBLIC )? strLit SEMI
+  : DOC_COMMENT? IMPORT ( WEAK | PUBLIC )? strLit SEMI
   ;
 
 // Package
 
 packageStatement
-  : PACKAGE fullIdent SEMI
+  : DOC_COMMENT? PACKAGE fullIdent SEMI
   ;
 
 // Option
 
 optionStatement
-  : OPTION optionName EQ constant SEMI
+  : DOC_COMMENT? OPTION optionName EQ constant SEMI
   ;
 
 optionName
@@ -81,6 +79,8 @@ fieldNumber
 
 // Oneof and oneof field
 
+// Note, oneOf isn't a message or field, so docComment is broken here, but the current
+// PBJ compiler requires docComment.
 oneof
   : docComment ONEOF oneofName LC ( optionStatement | oneofField | emptyStatement_ )* RC
   ;
@@ -135,7 +135,7 @@ type_
 // Reserved
 
 reserved
-  : RESERVED ( ranges | reservedFieldNames ) SEMI
+  : DOC_COMMENT? RESERVED ( ranges | reservedFieldNames ) SEMI
   ;
 
 ranges
@@ -260,7 +260,7 @@ messageType: ( DOT )? ( ident DOT )* messageName;
 enumType: ( DOT )? ( ident DOT )* enumName;
 
 intLit: INT_LIT;
-strLit: STR_LIT | PROTO3_LIT_SINGLE | PROTO3_LIT_DOBULE;
+strLit: STR_LIT | PROTO3_LIT_SINGLE | PROTO3_LIT_DOUBLE;
 boolLit: BOOL_LIT;
 floatLit: FLOAT_LIT;
 
@@ -300,7 +300,7 @@ STREAM: 'stream';
 RETURNS: 'returns';
 
 PROTO3_LIT_SINGLE: '"proto3"';
-PROTO3_LIT_DOBULE: '\'proto3\'';
+PROTO3_LIT_DOUBLE: '\'proto3\'';
 
 // symbols
 

diff --git a/pbj-integration-tests/build.gradle.kts b/pbj-integration-tests/build.gradle.kts
@@ -126,7 +126,7 @@ testing {
             classpath = sources.runtimeClasspath
             useJUnitPlatform { includeTags("FUZZ_TEST") }
             enableAssertions = false
-            systemProperties["com.hedera.pbj.intergration.test.fuzz.useRandomSeed"] = true
+            systemProperties["com.hedera.pbj.integration.test.fuzz.useRandomSeed"] = true
         }
         targets.named("test") {
             testTask {

diff --git a/...ation/jmh/ComplexEqualsHashCodeBench.java → ...ation/jmh/ComplexEqualsHashCodeBench.java b/...ation/jmh/ComplexEqualsHashCodeBench.java → ...ation/jmh/ComplexEqualsHashCodeBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import com.hedera.pbj.test.proto.pbj.Hasheval;
@@ -152,4 +152,4 @@ public void benchJavaRecordNotEquals(Blackhole blackhole) {
             blackhole.consume(hashevalJavaRecord.equals(hashevalJavaRecordDifferent));
         }
     }
-}
+}
diff --git a/...intergration/jmh/EqualsHashCodeBench.java → .../integration/jmh/EqualsHashCodeBench.java b/...intergration/jmh/EqualsHashCodeBench.java → .../integration/jmh/EqualsHashCodeBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.hedera.pbj.test.proto.pbj.TimestampTest;
 import org.openjdk.jmh.annotations.Benchmark;
@@ -102,4 +102,4 @@ public void benchJavaRecordNotEquals(Blackhole blackhole) {
             blackhole.consume(timestampStandardRecord.equals(timestampStandardRecordDifferent));
         }
     }
-}
+}
diff --git a/...edera/pbj/intergration/jmh/HashBench.java → ...hedera/pbj/integration/jmh/HashBench.java b/...edera/pbj/intergration/jmh/HashBench.java → ...hedera/pbj/integration/jmh/HashBench.java
@@ -1,6 +1,6 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
-import com.hedera.pbj.intergration.test.TestHashFunctions;
+import com.hedera.pbj.integration.test.TestHashFunctions;
 import com.hedera.pbj.runtime.io.buffer.Bytes;
 import com.hedera.pbj.test.proto.pbj.Hasheval;
 import com.hedera.pbj.test.proto.pbj.Suit;
@@ -53,4 +53,4 @@ public void hashBenchFieldWise(Blackhole blackhole) throws IOException {
             TestHashFunctions.hash2(hasheval);
         }
     }
-}
+}
diff --git a/...edera/pbj/intergration/jmh/JsonBench.java → ...hedera/pbj/integration/jmh/JsonBench.java b/...edera/pbj/intergration/jmh/JsonBench.java → ...hedera/pbj/integration/jmh/JsonBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.google.protobuf.GeneratedMessage;
 import com.google.protobuf.GeneratedMessageV3;

diff --git a/...intergration/jmh/ProtobufObjectBench.java → .../integration/jmh/ProtobufObjectBench.java b/...intergration/jmh/ProtobufObjectBench.java → .../integration/jmh/ProtobufObjectBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.google.protobuf.CodedOutputStream;
 import com.google.protobuf.GeneratedMessage;

diff --git a/...bj/intergration/jmh/SampleBlockBench.java → ...pbj/integration/jmh/SampleBlockBench.java b/...bj/intergration/jmh/SampleBlockBench.java → ...pbj/integration/jmh/SampleBlockBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.google.protobuf.CodedOutputStream;
 import com.hedera.hapi.block.stream.BlockItem;
@@ -149,14 +149,14 @@ public void writePbjByteBuffer(Blackhole blackhole) throws IOException {
 		com.hedera.hapi.block.stream.Block.PROTOBUF.write(TEST_BLOCK, outDataBuffer);
 		blackhole.consume(outDataBuffer);
 	}
-	
+
 	@Benchmark
 	public void writePbjByteDirect(Blackhole blackhole) throws IOException {
 		outDataBufferDirect.reset();
 		com.hedera.hapi.block.stream.Block.PROTOBUF.write(TEST_BLOCK, outDataBufferDirect);
 		blackhole.consume(outDataBufferDirect);
 	}
-	
+
 	@Benchmark
 	public void writePbjOutputStream(Blackhole blackhole) throws IOException {
 		bout.reset();

diff --git a/...era/pbj/intergration/jmh/VarIntBench.java → ...dera/pbj/integration/jmh/VarIntBench.java b/...era/pbj/intergration/jmh/VarIntBench.java → ...dera/pbj/integration/jmh/VarIntBench.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.jmh;
+package com.hedera.pbj.integration.jmh;
 
 import com.google.protobuf.CodedInputStream;
 import com.google.protobuf.CodedOutputStream;

diff --git a/...ntegration-tests/src/test/java/com/hedera/pbj/integration/test/CompareToNegativeTest.java b/...ntegration-tests/src/test/java/com/hedera/pbj/integration/test/CompareToNegativeTest.java
@@ -0,0 +1,46 @@
+package com.hedera.pbj.integration.test;
+
+import static com.hedera.pbj.compiler.PbjCompiler.compileFilesIn;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.File;
+import java.net.URL;
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+class CompareToNegativeTest {
+
+    @TempDir
+    private static File outputDir;
+
+    @Test
+    void testNonComparableSubObj() {
+        IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () ->
+                getCompileFilesIn("non_compilable_comparable_sub_obj.proto"));
+        assertEquals("Field NonComparableSubObj.subObject specified in `pbj.comparable` option must implement `Comparable` interface but it doesn't.",
+                exception.getMessage());
+    }
+
+    @Test
+    void testRepeatedField() {
+        IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () ->
+                getCompileFilesIn("non_compilable_comparable_repeated.proto"));
+        assertEquals("Field `int32List` specified in `pbj.comparable` option is repeated. Repeated fields are not supported by this option.",
+                exception.getMessage());
+    }
+
+    @Test
+    void testNonComparableOneOfField() {
+        IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () ->
+                getCompileFilesIn("non_compilable_comparable_oneOf.proto"));
+        assertEquals("Field NonComparableSubObj.subObject specified in `pbj.comparable` option must implement `Comparable` interface but it doesn't.",
+                exception.getMessage());
+    }
+
+    private static void getCompileFilesIn(String fileName) throws Exception {
+        URL fileUrl = CompareToNegativeTest.class.getClassLoader().getResource(fileName);
+        compileFilesIn(List.of(new File(fileUrl.toURI())), outputDir, outputDir);
+    }
+}
diff --git a/.../pbj/intergration/test/CompareToTest.java → ...a/pbj/integration/test/CompareToTest.java b/.../pbj/intergration/test/CompareToTest.java → ...a/pbj/integration/test/CompareToTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import static java.util.Collections.shuffle;
 import static java.util.Collections.sort;

diff --git a/...st/ExtendedUtf8MessageWithStringTest.java → ...st/ExtendedUtf8MessageWithStringTest.java b/...st/ExtendedUtf8MessageWithStringTest.java → ...st/ExtendedUtf8MessageWithStringTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.google.protobuf.CodedOutputStream;
 import com.hedera.pbj.runtime.io.buffer.BufferedData;

diff --git a/.../intergration/test/FieldsNonNullTest.java → ...j/integration/test/FieldsNonNullTest.java b/.../intergration/test/FieldsNonNullTest.java → ...j/integration/test/FieldsNonNullTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.hapi.node.base.FeeSchedule;
 import com.hedera.hapi.node.base.TransactionFeeSchedule;

diff --git a/...pbj/intergration/test/HashEqualsTest.java → .../pbj/integration/test/HashEqualsTest.java b/...pbj/intergration/test/HashEqualsTest.java → .../pbj/integration/test/HashEqualsTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -89,4 +89,4 @@ void differentObjectsWithNoDefaulHashCode4() {
 
         assertNotEquals(tst.hashCode(), tst2.hashCode());
     }
-}
+}
diff --git a/.../pbj/intergration/test/JsonCodecTest.java → ...a/pbj/integration/test/JsonCodecTest.java b/.../pbj/intergration/test/JsonCodecTest.java → ...a/pbj/integration/test/JsonCodecTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.google.protobuf.ByteString;
 import com.google.protobuf.util.JsonFormat;

diff --git a/...tergration/test/MalformedMessageTest.java → ...ntegration/test/MalformedMessageTest.java b/...tergration/test/MalformedMessageTest.java → ...ntegration/test/MalformedMessageTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import static org.junit.jupiter.api.Assertions.assertThrows;
 

diff --git a/...a/pbj/intergration/test/MaxDepthTest.java → ...ra/pbj/integration/test/MaxDepthTest.java b/...a/pbj/intergration/test/MaxDepthTest.java → ...ra/pbj/integration/test/MaxDepthTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.pbj.runtime.ParseException;
 import com.hedera.pbj.runtime.io.buffer.BufferedData;

diff --git a/...ra/pbj/intergration/test/MaxSizeTest.java → ...era/pbj/integration/test/MaxSizeTest.java b/...ra/pbj/intergration/test/MaxSizeTest.java → ...era/pbj/integration/test/MaxSizeTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.pbj.runtime.ParseException;
 import com.hedera.pbj.runtime.io.buffer.BufferedData;

diff --git a/...tergration/test/ParserNeverWrapsTest.java → ...ntegration/test/ParserNeverWrapsTest.java b/...tergration/test/ParserNeverWrapsTest.java → ...ntegration/test/ParserNeverWrapsTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.pbj.runtime.Codec;
 import com.hedera.pbj.runtime.io.WritableSequentialData;

diff --git a/...pbj/intergration/test/SampleFuzzTest.java → .../pbj/integration/test/SampleFuzzTest.java b/...pbj/intergration/test/SampleFuzzTest.java → .../pbj/integration/test/SampleFuzzTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.hapi.node.base.tests.AccountIDTest;
 import com.hedera.hapi.node.base.tests.ContractIDTest;
@@ -199,7 +199,7 @@ void fuzzTest() {
 
     private Random buildRandom() {
         final boolean useRandomSeed
-                = Boolean.valueOf(System.getProperty("com.hedera.pbj.intergration.test.fuzz.useRandomSeed"));
+                = Boolean.valueOf(System.getProperty("com.hedera.pbj.integration.test.fuzz.useRandomSeed"));
         final long seed = useRandomSeed ? new Random().nextLong() : FIXED_RANDOM_SEED;
 
         System.out.println("Fuzz tests are configured to use a "

diff --git a/.../intergration/test/TestHashFunctions.java → ...j/integration/test/TestHashFunctions.java b/.../intergration/test/TestHashFunctions.java → ...j/integration/test/TestHashFunctions.java
@@ -1,20 +1,4 @@
-/*
- * Copyright (C) 2025 Hedera Hashgraph, LLC
- *
- * 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.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.hedera.pbj.test.proto.pbj.Hasheval;
 import com.hedera.pbj.test.proto.pbj.TimestampTest;
@@ -27,8 +11,8 @@
 public final class TestHashFunctions {
     public static int hash1(Hasheval hashEval) {
         try {
-            byte[] hash = MessageDigest.getInstance("SHA-256")
-                    .digest(Hasheval.PROTOBUF.toBytes(hashEval).toByteArray());
+            byte[] hash = MessageDigest.getInstance("SHA-256").digest(
+                    Hasheval.PROTOBUF.toBytes(hashEval).toByteArray());
             int res = hash[0] << 24 | hash[1] << 16 | hash[2] << 8 | hash[3];
             return processForBetterDistribution(res);
         } catch (NoSuchAlgorithmException e) {
@@ -84,19 +68,18 @@ public static int hash2(Hasheval hashEval) {
         }
         if (hashEval.subObject() != Hasheval.DEFAULT.subObject()) {
             TimestampTest sub = hashEval.subObject();
-            if (sub.nanos() != TimestampTest.DEFAULT.nanos()) {
+            if (sub.nanos() != sub.DEFAULT.nanos()) {
                 result = 31 * result + Integer.hashCode(sub.nanos());
             }
-            if (sub.seconds() != TimestampTest.DEFAULT.seconds()) {
+            if (sub.seconds() != sub.DEFAULT.seconds()) {
                 result = 31 * result + Long.hashCode(sub.seconds());
             }
         }
         if (hashEval.text() != Hasheval.DEFAULT.text()) {
             result = 31 * result + hashEval.text().hashCode();
         }
         if (hashEval.bytesField() != Hasheval.DEFAULT.bytesField()) {
-            result = 31 * result
-                    + (hashEval.bytesField() == null ? 0 : hashEval.bytesField().hashCode());
+            result = 31 * result + (hashEval.bytesField() == null ? 0 : hashEval.bytesField().hashCode());
         }
 
         return processForBetterDistribution(result);

diff --git a/.../intergration/test/TimestampTestTest.java → ...j/integration/test/TimestampTestTest.java b/.../intergration/test/TimestampTestTest.java → ...j/integration/test/TimestampTestTest.java
@@ -1,4 +1,4 @@
-package com.hedera.pbj.intergration.test;
+package com.hedera.pbj.integration.test;
 
 import com.google.protobuf.CodedOutputStream;
 import com.hedera.pbj.runtime.io.buffer.BufferedData;
@@ -101,4 +101,4 @@ public static Stream<NoToStringWrapper<TimestampTest>> createModelTestArguments(
 		return ARGUMENTS.stream().map(NoToStringWrapper::new);
 	}
 
-}
+}