Skip to content

Commit

Permalink
Fix generation of negative enum fields (#258)
Browse files Browse the repository at this point in the history
Fixes an issue where enums with negative field
numbers were compiled to the field's conjugate
in toProtoEnumMay pattern matches.

This commit also adds tests of negative enum codes
based on interoperability with generated Python.
  • Loading branch information
riz0id authored Nov 7, 2024
1 parent 5fd8580 commit 8d2d17e
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/Proto3/Suite/DotProto/Generate/Syntax.hs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ intE :: Integral a => a -> HsExp
intE x = noLocA $ HsOverLit synDef $ mkHsIntegral $ IL
{ il_text = NoSourceText
, il_neg = x < 0
, il_value = abs (toInteger x)
, il_value = toInteger x
}

intP :: Integral a => a -> HsPat
Expand All @@ -968,7 +968,7 @@ intP x = noLocA $ NPat synDef overlit Nothing NoExtField
overlit = L synDef $ mkHsIntegral $ IL
{ il_text = NoSourceText
, il_neg = x < 0
, il_value = abs (toInteger x)
, il_value = toInteger x
}

floatE :: forall f . RealFloat f => f -> HsExp
Expand Down
20 changes: 20 additions & 0 deletions test-files/test_proto_negative_enum.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@

syntax="proto3";

package TestProtoNegativeEnum;

enum NegativeEnum {
NEGATIVE_ENUM_0 = 0;

NEGATIVE_ENUM_NEGATIVE_1 = -1;

NEGATIVE_ENUM_1 = 1;

NEGATIVE_ENUM_NEGATIVE_128 = -128;

NEGATIVE_ENUM_128 = 128;
}

message WithNegativeEnum {
NegativeEnum v = 123;
}
13 changes: 12 additions & 1 deletion tests/SimpleDecodeDotProto.hs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import qualified TestProtoImport
import qualified TestProtoOneof
import qualified TestProtoOneofImport
import qualified TestProtoWrappers
import qualified TestProtoNegativeEnum

data Format = Binary | Jsonpb
deriving (Bounded, Enum, Eq, Read, Show)
Expand Down Expand Up @@ -59,6 +60,7 @@ tests, testCase1, testCase2, testCaseSignedInts, testCase3, testCase4,
testCase_DoubleValue, testCase_FloatValue, testCase_Int64Value,
testCase_UInt64Value, testCase_Int32Value, testCase_UInt32Value,
testCase_BoolValue, testCase_StringValue, testCase_BytesValue,
testCase_NegativeEnum,
allTestsDone :: (?format :: Format) => TestTree
tests = testGroup
("Decode protobuf messages from Python: format " ++ show ?format)
Expand All @@ -71,6 +73,7 @@ tests = testGroup
, testCase_DoubleValue, testCase_FloatValue, testCase_Int64Value
, testCase_UInt64Value, testCase_Int32Value, testCase_UInt32Value
, testCase_BoolValue, testCase_StringValue, testCase_BytesValue
, testCase_NegativeEnum
, allTestsDone -- this should always run last
]

Expand Down Expand Up @@ -398,7 +401,7 @@ testCase_DoubleValue = testCaseInFormat "DoubleValue" $ do
let w = TestProtoWrappers.TestDoubleValue
expect (w Nothing)
expect (w (Just 3.5))

testCase_FloatValue = testCaseInFormat "FloatValue" $ do
let w = TestProtoWrappers.TestFloatValue
expect (w Nothing)
Expand Down Expand Up @@ -450,6 +453,14 @@ testCase_BytesValue = testCaseInFormat "BytesValue" $ do
expect (w (Just ""))
expect (w (Just "012"))

testCase_NegativeEnum = testCaseInFormat "NegativeEnum" $ do
let w = TestProtoNegativeEnum.WithNegativeEnum . Enumerated . Right
expect (w TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_0)
expect (w TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_NEGATIVE_1)
expect (w TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_1)
expect (w TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_NEGATIVE_128)
expect (w TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_128)


allTestsDone = testCaseInFormat "Receive end of test suite sentinel message" $
do MultipleFields{..} <- readProto
Expand Down
13 changes: 13 additions & 0 deletions tests/SimpleEncodeDotProto.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import qualified TestProtoImport
import qualified TestProtoOneof
import qualified TestProtoOneofImport
import qualified TestProtoWrappers
import qualified TestProtoNegativeEnum
import Text.Read (readEither)
import System.Environment (getArgs, getProgName)
import System.Exit (die)
Expand Down Expand Up @@ -274,6 +275,15 @@ testCase_BytesValue = do
emit (Just "")
emit (Just "012")

testCase_NegativeEnum :: (?format :: Format) => IO ()
testCase_NegativeEnum = do
let emit = outputMessage . TestProtoNegativeEnum.WithNegativeEnum . Enumerated . Right
emit TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_0
emit TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_NEGATIVE_1
emit TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_1
emit TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_NEGATIVE_128
emit TestProtoNegativeEnum.NegativeEnumNEGATIVE_ENUM_128


main :: IO ()
main = do
Expand Down Expand Up @@ -323,4 +333,7 @@ main = do
testCase_StringValue
testCase_BytesValue

-- Negative enum codes
testCase_NegativeEnum

outputMessage (MultipleFields 0 0 0 0 "All tests complete" False)
2 changes: 2 additions & 0 deletions tests/TestCodeGen.hs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ compileTestDotProtos logger recStyle decodedStringType = do
protoFiles =
[ "test_proto.proto"
, "test_proto_import.proto"
, "test_proto_negative_enum.proto"
, "test_proto_oneof.proto"
, "test_proto_oneof_import.proto"
{- These tests have been temporarily removed to pass CI.
Expand All @@ -263,6 +264,7 @@ compileTestDotProtos logger recStyle decodedStringType = do
-}
, "test_proto_nested_message.proto"
, "test_proto_wrappers.proto"
, "test_proto_negative_enum.proto"
]

forM_ protoFiles $ \protoFile -> do
Expand Down
28 changes: 22 additions & 6 deletions tests/check_simple_dot_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
import io
import sys
# Import protoc generated {de,}serializers (generated from test_proto{,_import}.proto)
from google.protobuf import json_format
from test_proto_pb2 import *
from test_proto_import_pb2 import WithNesting as ImportedWithNesting
from test_proto_oneof_pb2 import Something, WithImported, DUMMY0, DUMMY1
from test_proto_oneof_import_pb2 import WithOneof
from test_proto_wrappers_pb2 import *
from google.protobuf import json_format
from test_proto_pb2 import *
from test_proto_import_pb2 import WithNesting as ImportedWithNesting
from test_proto_negative_enum_pb2 import *
from test_proto_oneof_pb2 import Something, WithImported, DUMMY0, DUMMY1
from test_proto_oneof_import_pb2 import WithOneof
from test_proto_wrappers_pb2 import *

# Python 3.7 or newer requires this
sys.stdin.reconfigure(encoding='iso-8859-1', newline='')
Expand Down Expand Up @@ -495,6 +496,21 @@ def read_proto(cls, skip_if_json=False):
assert case_BytesValue_C.HasField('wrapper')
assert case_BytesValue_C.wrapper.value == b"012"

case_NegativeEnum_A = read_proto(WithNegativeEnum)
assert case_NegativeEnum_A.v == NEGATIVE_ENUM_0

case_NegativeEnum_B = read_proto(WithNegativeEnum)
assert case_NegativeEnum_B.v == NEGATIVE_ENUM_NEGATIVE_1

case_NegativeEnum_C = read_proto(WithNegativeEnum)
assert case_NegativeEnum_C.v == NEGATIVE_ENUM_1

case_NegativeEnum_D = read_proto(WithNegativeEnum)
assert case_NegativeEnum_D.v == NEGATIVE_ENUM_NEGATIVE_128

case_NegativeEnum_E = read_proto(WithNegativeEnum)
assert case_NegativeEnum_E.v == NEGATIVE_ENUM_128

# Wait for the special 'done' messsage
done_msg = read_proto(MultipleFields)
assert done_msg.multiFieldString == "All tests complete"
Expand Down
1 change: 1 addition & 0 deletions tests/decode.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ghc \
$hsTmpDir/TestProtoOneof.hs \
$hsTmpDir/TestProtoOneofImport.hs \
$hsTmpDir/TestProtoWrappers.hs \
$hsTmpDir/TestProtoNegativeEnum.hs \
tests/Test/Dhall/Orphan.hs \
tests/SimpleDecodeDotProto.hs \
>/dev/null
Expand Down
1 change: 1 addition & 0 deletions tests/encode.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ ghc \
$hsTmpDir/TestProtoOneof.hs \
$hsTmpDir/TestProtoOneofImport.hs \
$hsTmpDir/TestProtoWrappers.hs \
$hsTmpDir/TestProtoNegativeEnum.hs \
tests/Test/Dhall/Orphan.hs \
tests/SimpleEncodeDotProto.hs \
>/dev/null
Expand Down
22 changes: 15 additions & 7 deletions tests/send_simple_dot_proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
import sys
import os
# Import protoc generated {de,}serializers (generated from test_proto{,_import}.proto)
from google.protobuf import json_format
from google.protobuf.wrappers_pb2 import *
from test_proto_pb2 import *
import test_proto_import_pb2 as test_proto_import
import test_proto_oneof_pb2 as test_proto_oneof
import test_proto_oneof_import_pb2 as test_proto_oneof_import
import test_proto_wrappers_pb2 as test_proto_wrappers
from google.protobuf import json_format
from google.protobuf.wrappers_pb2 import *
from test_proto_pb2 import *
import test_proto_import_pb2 as test_proto_import
import test_proto_negative_enum_pb2 as test_proto_negative_enum
import test_proto_oneof_pb2 as test_proto_oneof
import test_proto_oneof_import_pb2 as test_proto_oneof_import
import test_proto_wrappers_pb2 as test_proto_wrappers

# Python 3.7 or newer requires this
sys.stdout.reconfigure(encoding='iso-8859-1')
Expand Down Expand Up @@ -285,5 +286,12 @@ def write_proto(msg):
write_proto(test_proto_wrappers.TestBytesValue(wrapper=BytesValue(value=b"")))
write_proto(test_proto_wrappers.TestBytesValue(wrapper=BytesValue(value=b"012")))

# Test NegativeEnum
write_proto(test_proto_negative_enum.WithNegativeEnum(v=test_proto_negative_enum.NEGATIVE_ENUM_0))
write_proto(test_proto_negative_enum.WithNegativeEnum(v=test_proto_negative_enum.NEGATIVE_ENUM_NEGATIVE_1))
write_proto(test_proto_negative_enum.WithNegativeEnum(v=test_proto_negative_enum.NEGATIVE_ENUM_1))
write_proto(test_proto_negative_enum.WithNegativeEnum(v=test_proto_negative_enum.NEGATIVE_ENUM_NEGATIVE_128))
write_proto(test_proto_negative_enum.WithNegativeEnum(v=test_proto_negative_enum.NEGATIVE_ENUM_128))

# Send the special 'done' message
write_proto(MultipleFields(multiFieldString = "All tests complete"))

0 comments on commit 8d2d17e

Please sign in to comment.