Skip to content

java: Use Integers in addition to Longs #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions codegen/generators/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,25 +105,23 @@ def property_type_from_ref(ref)
class_name(ref)
end

def property_type_from_type(parent_type_name, property_name, property, type:)
def property_type_from_type(parent_type_name, property_name, property)
type = property['type']
return array_type_for(type_for(parent_type_name, nil, property['items'])) if type == 'array'
return property_type_from_enum(enum_name(parent_type_name, property_name, property['enum'])) if property['enum']

unless language_translations_for_data_types.key?(type)
property = select_language_translations_for_data_types(type, property)
unless property
raise "No type mapping for JSONSchema type #{type}. Schema:\n#{JSON.pretty_generate(property)}"
end

if property['enum']
property_type_from_enum(enum_name(parent_type_name, property_name, property['enum']))
else
language_translations_for_data_types.fetch(type)
end
property
end

def type_for(parent_type_name, property_name, property)
if property['$ref']
property_type_from_ref(property['$ref'])
elsif property['type']
property_type_from_type(parent_type_name, property_name, property, type: property['type'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is the only location for this? If so this seems good. I think a lot of the initial work I did for codegen was converting into a generic structure - I didn't go through and tidy up. So good catch if so

property_type_from_type(parent_type_name, property_name, property)
else
# Inline schema (not supported)
raise "Property #{property_name} did not define 'type' or '$ref'"
Expand Down
4 changes: 2 additions & 2 deletions codegen/generators/cpp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ def format_description(raw_description, indent_string: '')

private

def language_translations_for_data_types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review here but for all generic implementors. Is the only usage of this in property_type_from_type? If so then I figure the change here is ok, but it does more heavy lifting upfront

def select_language_translations_for_data_types(type, property)
{
'integer' => 'std::size_t',
'string' => 'std::string',
'boolean' => 'bool'
}
}[type]
end
end
end
4 changes: 2 additions & 2 deletions codegen/generators/dotnet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ def format_description(raw_description, indent_string: '')

private

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'long',
'string' => 'string',
'boolean' => 'bool'
}
}[type]
end
end
end
4 changes: 2 additions & 2 deletions codegen/generators/go.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ def property_type_from_ref(ref)

private

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'int64',
'string' => 'string',
'boolean' => 'bool'
}
}[type]
end
end
end
20 changes: 14 additions & 6 deletions codegen/generators/java.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,20 @@ def format_description(raw_description, indent_string: '')

private

def language_translations_for_data_types
{
'integer' => 'Long',
'string' => 'String',
'boolean' => 'Boolean'
}
def select_language_translations_for_data_types(type, property)
if type == 'integer'
if property['maximum'] and property['maximum'] <= 2147483647
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if property['maximum'] and property['maximum'] <= 2147483647
if property['maximum']&.between?(1, 2147483647)

Copy link
Contributor Author

@mpkorstanje mpkorstanje Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Java, for the choice between integer and long only the upper bound matters. They're both signed.

'Integer'
else
'Long'
end
elsif type == 'string'
'String'
elsif type == 'boolean'
'Boolean'
else
nil
end
end
end
end
4 changes: 2 additions & 2 deletions codegen/generators/markdown.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ def property_type_from_ref(ref)

private

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'integer',
'string' => 'string',
'boolean' => 'boolean'
}
}[type]
end
end
end
4 changes: 2 additions & 2 deletions codegen/generators/perl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ def default_value_for_string(property_name, enum)
end
end

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'number',
'string' => 'string',
'boolean' => 'boolean'
}
}[type]
end
end
end
8 changes: 4 additions & 4 deletions codegen/generators/php.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ def nullable?(property_name, schema)
end

def scalar?(property)
property.key?('type') && language_translations_for_data_types.key?(property['type'])
property.key?('type') && select_language_translations_for_data_types(property['type'], property)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return false or a non-boolean value which I haven't checked downstream if this matters, but it could

end

def scalar_type_for(property)
language_translations_for_data_types[property['type']]
select_language_translations_for_data_types(property['type'], property)
end

private
Expand All @@ -68,12 +68,12 @@ def default_value(class_name, property_name, property, schema)
super(class_name, property_name, property)
end

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is never consumed here we can use _property to be memory efficient

Suggested change
def select_language_translations_for_data_types(type, property)
def select_language_translations_for_data_types(type, _property)

{
'string' => 'string',
'integer' => 'int',
'boolean' => 'bool'
}
}[type]
end

def non_nullable_non_scalar_constructor(parent_type, property, property_name, schema, source)
Expand Down
14 changes: 7 additions & 7 deletions codegen/generators/python.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ def format_description(raw_description, indent_string: ' ')
%("""\n#{lines.join("\n")}\n#{indent_string}""")
end

def select_language_translations_for_data_types(type, property)
language_translations_for_data_types[type]
end

private

def language_translations_for_data_types
{
'integer' => 'int',
Expand All @@ -71,9 +77,7 @@ def language_translations_for_data_types
'array' => 'list'
}
end

private


def default_value(parent_type_name, property_name, property)
if property['type'] == 'string'
default_value_for_string(parent_type_name, property_name, property)
Expand Down Expand Up @@ -109,10 +113,6 @@ def enum_name(parent_type_name, property_name, enum)
end
end

def property_type_from_ref(ref)
class_name(ref)
end

def class_name(ref)
return ref if language_translations_for_data_types.values.include?(ref)

Expand Down
4 changes: 2 additions & 2 deletions codegen/generators/ruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ def default_value_for_string(parent_type_name, property_name, property)
end
end

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'number',
'string' => 'string',
'boolean' => 'boolean'
}
}[type]
end

def line_as_comment(line)
Expand Down
4 changes: 2 additions & 2 deletions codegen/generators/typescript.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ def array_type_for(type_name)

private

def language_translations_for_data_types
def select_language_translations_for_data_types(type, property)
{
'integer' => 'number',
'string' => 'string',
'boolean' => 'boolean'
}
}[type]
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
@SuppressWarnings("unused")
public final class Duration {
private final Long seconds;
private final Long nanos;
private final Integer nanos;

public Duration(
Long seconds,
Long nanos
Integer nanos
) {
this.seconds = requireNonNull(seconds, "Duration.seconds cannot be null");
this.nanos = requireNonNull(nanos, "Duration.nanos cannot be null");
Expand All @@ -37,7 +37,7 @@ public Long getSeconds() {
* that count forward in time. Must be from 0 to 999,999,999
* inclusive.
*/
public Long getNanos() {
public Integer getNanos() {
return nanos;
}

Expand Down
6 changes: 3 additions & 3 deletions java/src/generated/java/io/cucumber/messages/types/Group.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
@SuppressWarnings("unused")
public final class Group {
private final java.util.List<Group> children;
private final Long start;
private final Integer start;
private final String value;

public Group(
java.util.List<Group> children,
Long start,
Integer start,
String value
) {
this.children = unmodifiableList(new ArrayList<>(requireNonNull(children, "Group.children cannot be null")));
Expand All @@ -31,7 +31,7 @@ public java.util.List<Group> getChildren() {
return children;
}

public Optional<Long> getStart() {
public Optional<Integer> getStart() {
return Optional.ofNullable(start);
}

Expand Down
12 changes: 6 additions & 6 deletions java/src/generated/java/io/cucumber/messages/types/Location.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@
// Generated code
@SuppressWarnings("unused")
public final class Location {
private final Long line;
private final Long column;
private final Integer line;
private final Integer column;

public Location(
Long line,
Long column
Integer line,
Integer column
) {
this.line = requireNonNull(line, "Location.line cannot be null");
this.column = column;
}

public Long getLine() {
public Integer getLine() {
return line;
}

public Optional<Long> getColumn() {
public Optional<Integer> getColumn() {
return Optional.ofNullable(column);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
@SuppressWarnings("unused")
public final class Timestamp {
private final Long seconds;
private final Long nanos;
private final Integer nanos;

public Timestamp(
Long seconds,
Long nanos
Integer nanos
) {
this.seconds = requireNonNull(seconds, "Timestamp.seconds cannot be null");
this.nanos = requireNonNull(nanos, "Timestamp.nanos cannot be null");
Expand All @@ -39,7 +39,7 @@ public Long getSeconds() {
* that count forward in time. Must be from 0 to 999,999,999
* inclusive.
*/
public Long getNanos() {
public Integer getNanos() {
return nanos;
}

Expand Down
5 changes: 3 additions & 2 deletions java/src/main/java/io/cucumber/messages/Convertor.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.io.PrintWriter;
import java.io.StringWriter;
import java.time.Instant;

import static java.util.Objects.requireNonNull;

Expand All @@ -30,12 +31,12 @@ private static String extractStackTrace(Throwable throwable) {

public static Timestamp toMessage(java.time.Instant instant) {
requireNonNull(instant, "instant may not be null");
return new Timestamp(instant.getEpochSecond(), (long) instant.getNano());
return new Timestamp(instant.getEpochSecond(), instant.getNano());
}

public static Duration toMessage(java.time.Duration duration) {
requireNonNull(duration, "duration may not be null");
return new Duration(duration.getSeconds(), (long) duration.getNano());
return new Duration(duration.getSeconds(), duration.getNano());
}

public static java.time.Instant toInstant(Timestamp timestamp) {
Expand Down
2 changes: 1 addition & 1 deletion java/src/test/java/io/cucumber/messages/JacksonTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void serialize_enums_using_value() throws JsonProcessingException {

@Test
void can_deserialize_envelope() throws JsonProcessingException {
Envelope source = Envelope.of(new TestRunStarted(new Timestamp(3L, 14L), UUID.randomUUID().toString()));
Envelope source = Envelope.of(new TestRunStarted(new Timestamp(3L, 14), UUID.randomUUID().toString()));
String json = OBJECT_MAPPER.writeValueAsString(source);
assertEquals(source, OBJECT_MAPPER.readValue(json, Envelope.class));
}
Expand Down
4 changes: 3 additions & 1 deletion jsonschema/src/Duration.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
},
"nanos": {
"description": "Non-negative fractions of a second at nanosecond resolution. Negative\nsecond values with fractions must still have non-negative nanos values\nthat count forward in time. Must be from 0 to 999,999,999\ninclusive.",
"type": "integer"
"type": "integer",
"minimum": 0,
"maximum": 999999999
}
},
"type": "object"
Expand Down
8 changes: 6 additions & 2 deletions jsonschema/src/Location.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@
"description": "Points to a line and a column in a text file",
"properties": {
"line": {
"type": "integer"
"type": "integer",
"minimum": 1,
"maximum": 2147483647
},
"column": {
"type": "integer"
"type": "integer",
"minimum": 1,
"maximum": 2147483647
}
},
"required": [
Expand Down
4 changes: 3 additions & 1 deletion jsonschema/src/TestCase.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
"type": "array"
},
"start": {
"type": "integer"
"type": "integer",
"minimum": 0,
"maximum": 2147483647
},
"value": {
"type": "string"
Expand Down
Loading
Loading