Skip to content
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

#438 Upgrade to support jdk17 #477

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
6 changes: 6 additions & 0 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ jobs:
with:
languages: java

- name: Setup Java 17
uses: actions/setup-java@v1
with:
java-version: 17
Comment on lines +30 to +33
Copy link
Member

Choose a reason for hiding this comment

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

question: Why is this needed for CodeQL now?


- name: Autobuild
uses: github/codeql-action/autobuild@v2


Copy link
Member

Choose a reason for hiding this comment

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

nitpick: one empty line is enough 😛

Suggested change

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
12 changes: 6 additions & 6 deletions .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:

strategy:
matrix:
java-version: ['11', '17']
java-version: ['17']

steps:
- uses: actions/checkout@v3
Expand All @@ -37,7 +37,7 @@ jobs:

strategy:
matrix:
java-version: ['11', '17']
java-version: ['17']

steps:
- uses: actions/checkout@v3
Expand All @@ -57,10 +57,10 @@ jobs:

steps:
- uses: actions/checkout@v3
- name: Set up JDK 11
- name: Set up JDK 17
uses: actions/setup-java@v1
with:
java-version: 11
java-version: 17
- name: Checkstyle
run: mvn verify -DskipTests=true -Dmaven.javadoc.skip=true -B
env:
Expand All @@ -73,10 +73,10 @@ jobs:

steps:
- uses: actions/checkout@v3
- name: Set up JDK 11
- name: Set up JDK 17
uses: actions/setup-java@v1
with:
java-version: 11
java-version: 17
- name: Generate JavaDoc
run: mvn verify -DskipTests=true -Dcheckstyle.skip=true -B
env:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public void testExample(Class<? extends SchemaGenerationExampleInterface> exampl
private static String loadResource(String resourcePath) throws IOException {
StringBuilder stringBuilder = new StringBuilder();
try (InputStream inputStream = Objects.requireNonNull(ExampleTest.class.getResourceAsStream(resourcePath));
Scanner scanner = new Scanner(inputStream, StandardCharsets.UTF_8.name())) {
Scanner scanner = new Scanner(inputStream, StandardCharsets.UTF_8)) {
while (scanner.hasNext()) {
stringBuilder.append(scanner.nextLine()).append('\n');
}
Expand Down
1 change: 1 addition & 0 deletions jsonschema-generator-bom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<packaging>pom</packaging>

<properties>
<maven.compiler.release>17</maven.compiler.release>
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Irrelevant compiler setting for a BOM.

Suggested change
<maven.compiler.release>17</maven.compiler.release>

<signAndStage.skip>true</signAndStage.skip>
</properties>

Expand Down
9 changes: 4 additions & 5 deletions jsonschema-generator-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<file.encoding>UTF-8</file.encoding>
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
Comment on lines +137 to +138
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Changing these settings and then not using them feels somewhat pointless.
Fair enough to switch to the release, but then let's please introduce a property for that again to be consistent.

Suggested change
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
<maven.compiler.release>17</maven.compiler.release>

<!-- maven plugins -->
<maven.plugin.version.checkstyle>3.3.1</maven.plugin.version.checkstyle>
<maven.plugin.version.checkstyle>3.5.0</maven.plugin.version.checkstyle>
<version.checkstyle>10.14.1</version.checkstyle>
<maven.plugin.version.compiler>3.10.1</maven.plugin.version.compiler>
<maven.plugin.version.enforcer>3.2.1</maven.plugin.version.enforcer>
Expand Down Expand Up @@ -277,9 +277,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>${maven.plugin.version.compiler}</version>
<configuration>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
<showDeprecation>true</showDeprecation>
<release>17</release>
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Let's please continue using the respective property here.

Suggested change
<release>17</release>
<release>${maven.compiler.release}</release>

</configuration>
</plugin>
<plugin>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ private String deriveFieldName() {
.map(prefix -> methodName.substring(prefix.length()))
.filter(name -> !name.isEmpty())
.findFirst();
if (!possibleFieldName.isPresent()) {
if (possibleFieldName.isEmpty()) {
return methodName + "()";
}
String methodNameWithoutPrefix = possibleFieldName.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ public TypeContext(AnnotationConfiguration annotationConfig) {
*/
public TypeContext(AnnotationConfiguration annotationConfig, SchemaGeneratorConfig generatorConfig) {
this(annotationConfig, generatorConfig.shouldDeriveFieldsFromArgumentFreeMethods());
if (annotationConfig instanceof AnnotationConfiguration.StdConfiguration) {
if (annotationConfig instanceof AnnotationConfiguration.StdConfiguration configuration) {
generatorConfig.getAnnotationInclusionOverrides()
.forEach(((AnnotationConfiguration.StdConfiguration) annotationConfig)::setInclusion);
.forEach(configuration::setInclusion);
}
}

Expand Down Expand Up @@ -340,8 +340,8 @@ public <A extends Annotation> A getTypeParameterAnnotation(Class<A> annotationCl
* @since 4.30.0
*/
public Stream<Annotation> getTypeParameterAnnotations(AnnotatedType annotatedContainerType, Integer containerItemIndex) {
if (annotatedContainerType instanceof AnnotatedParameterizedType) {
AnnotatedType[] typeArguments = ((AnnotatedParameterizedType) annotatedContainerType).getAnnotatedActualTypeArguments();
if (annotatedContainerType instanceof AnnotatedParameterizedType type) {
AnnotatedType[] typeArguments = type.getAnnotatedActualTypeArguments();
int itemIndex = containerItemIndex == null ? 0 : containerItemIndex;
if (typeArguments.length > itemIndex) {
return Stream.of(typeArguments[itemIndex].getAnnotations());
Expand Down Expand Up @@ -491,10 +491,10 @@ public String getMethodPropertyArgumentTypeDescription(ResolvedType type) {
public <R> R performActionOnMember(MemberScope<?, ?> member, Function<FieldScope, R> fieldAction,
Function<MethodScope, R> methodAction) {
R result;
if (member instanceof FieldScope) {
result = fieldAction.apply((FieldScope) member);
} else if (member instanceof MethodScope) {
result = methodAction.apply((MethodScope) member);
if (member instanceof FieldScope scope) {
result = fieldAction.apply(scope);
} else if (member instanceof MethodScope scope) {
result = methodAction.apply(scope);
} else {
throw new IllegalStateException("Unsupported member scope of type: " + member.getClass());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,21 +379,21 @@ public AttributeCollector setEnum(ObjectNode node, Collection<?> enumValues, Sch

private void addRawPropertyValue(ObjectNode node, String propertyName, Object value) {
// need to specifically add simple/primitive values by type
if (value instanceof String) {
if (value instanceof String string) {
// explicit inclusion as string results in wrapping quote symbols
node.put(propertyName, (String) value);
} else if (value instanceof BigDecimal) {
node.put(propertyName, (BigDecimal) value);
} else if (value instanceof BigInteger) {
node.put(propertyName, (BigInteger) value);
} else if (value instanceof Boolean) {
node.put(propertyName, (Boolean) value);
} else if (value instanceof Double) {
node.put(propertyName, (Double) value);
} else if (value instanceof Float) {
node.put(propertyName, (Float) value);
} else if (value instanceof Integer) {
node.put(propertyName, (Integer) value);
node.put(propertyName, string);
} else if (value instanceof BigDecimal decimal) {
node.put(propertyName, decimal);
} else if (value instanceof BigInteger integer) {
node.put(propertyName, integer);
} else if (value instanceof Boolean boolean1) {
node.put(propertyName, boolean1);
} else if (value instanceof Double double1) {
node.put(propertyName, double1);
} else if (value instanceof Float float1) {
node.put(propertyName, float1);
} else if (value instanceof Integer integer) {
node.put(propertyName, integer);
} else {
// everything else is simply forwarded as-is to the JSON Schema, it's up to the configurator to ensure the value's correctness
node.putPOJO(propertyName, value);
Expand All @@ -402,21 +402,21 @@ private void addRawPropertyValue(ObjectNode node, String propertyName, Object va

private void addRawArrayItem(ArrayNode node, Object value) {
// need to specifically add simple/primitive values by type
if (value instanceof String) {
if (value instanceof String string) {
// explicit inclusion as string results in wrapping quote symbols
node.add((String) value);
} else if (value instanceof BigDecimal) {
node.add((BigDecimal) value);
} else if (value instanceof BigInteger) {
node.add((BigInteger) value);
} else if (value instanceof Boolean) {
node.add((Boolean) value);
} else if (value instanceof Double) {
node.add((Double) value);
} else if (value instanceof Float) {
node.add((Float) value);
} else if (value instanceof Integer) {
node.add((Integer) value);
node.add(string);
} else if (value instanceof BigDecimal decimal) {
node.add(decimal);
} else if (value instanceof BigInteger integer) {
node.add(integer);
} else if (value instanceof Boolean boolean1) {
node.add(boolean1);
} else if (value instanceof Double double1) {
node.add(double1);
} else if (value instanceof Float float1) {
node.add(float1);
Comment on lines +412 to +417
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Let's please avoid ambiguous naming wherever possible, including not assigning numbered variable name, even for such short-lived ones.

Suggested change
} else if (value instanceof Boolean boolean1) {
node.add(boolean1);
} else if (value instanceof Double double1) {
node.add(double1);
} else if (value instanceof Float float1) {
node.add(float1);
} else if (value instanceof Boolean booleanValue) {
node.add(booleanValue);
} else if (value instanceof Double doubleValue) {
node.add(doubleValue);
} else if (value instanceof Float floatValue) {
node.add(floatValue);

} else if (value instanceof Integer integer) {
node.add(integer);
} else {
// everything else is simply forwarded as-is to the JSON Schema, it's up to the configurator to ensure the value's correctness
node.addPOJO(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ private Set<String> getTagNamesSupporting(SchemaKeyword.TagContent contentType)
private void finaliseSchemaParts(List<ObjectNode> schemaNodes, Consumer<ObjectNode> performCleanUpOnSingleSchemaNode) {
List<ObjectNode> nextNodesToCheck = new ArrayList<>(schemaNodes);
Consumer<JsonNode> addNodeToCheck = node -> {
if (node instanceof ObjectNode) {
nextNodesToCheck.add((ObjectNode) node);
if (node instanceof ObjectNode objectNode) {
nextNodesToCheck.add(objectNode);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,11 @@ private void generateArrayDefinition(GenericTypeDetails typeDetails, ObjectNode
}

private JsonNode populateItemMemberSchema(TypeScope targetScope) {
if (targetScope instanceof FieldScope && !((FieldScope) targetScope).isFakeContainerItemScope()) {
return this.populateFieldSchema(((FieldScope) targetScope).asFakeContainerItemScope());
if (targetScope instanceof FieldScope scope && !scope.isFakeContainerItemScope()) {
return this.populateFieldSchema(scope.asFakeContainerItemScope());
Comment on lines +438 to +439
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: A less ambiguous variable would be nice.

Suggested change
if (targetScope instanceof FieldScope scope && !scope.isFakeContainerItemScope()) {
return this.populateFieldSchema(scope.asFakeContainerItemScope());
if (targetScope instanceof FieldScope fieldScope && !fieldScope.isFakeContainerItemScope()) {
return this.populateFieldSchema(fieldScope.asFakeContainerItemScope());

}
if (targetScope instanceof MethodScope && !((MethodScope) targetScope).isFakeContainerItemScope()) {
return this.populateMethodSchema(((MethodScope) targetScope).asFakeContainerItemScope());
if (targetScope instanceof MethodScope scope && !scope.isFakeContainerItemScope()) {
return this.populateMethodSchema(scope.asFakeContainerItemScope());
Comment on lines +441 to +442
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: Similarly here.

Suggested change
if (targetScope instanceof MethodScope scope && !scope.isFakeContainerItemScope()) {
return this.populateMethodSchema(scope.asFakeContainerItemScope());
if (targetScope instanceof MethodScope methodScope && !methodScope.isFakeContainerItemScope()) {
return this.populateMethodSchema(methodScope.asFakeContainerItemScope());

}
ObjectNode arrayItemDefinition = this.generatorConfig.createObjectNode();
this.traverseGenericType(targetScope.getContainerItemType(), arrayItemDefinition, false);
Expand Down Expand Up @@ -704,9 +704,7 @@ public ObjectNode makeNullable(ObjectNode node) {
private void extendTypeDeclarationToIncludeNull(ObjectNode node) {
JsonNode fixedJsonSchemaType = node.get(this.getKeyword(SchemaKeyword.TAG_TYPE));
final String nullTypeName = this.getKeyword(SchemaKeyword.TAG_TYPE_NULL);
if (fixedJsonSchemaType instanceof ArrayNode) {
// there are already multiple "type" values
ArrayNode arrayOfTypes = (ArrayNode) fixedJsonSchemaType;
if (fixedJsonSchemaType instanceof ArrayNode arrayOfTypes) {
// one of the existing "type" values could be null
for (JsonNode arrayEntry : arrayOfTypes) {
if (nullTypeName.equals(arrayEntry.textValue())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,11 @@ public SchemaDefinitionNamingStrategy getDefinitionNamingStrategy() {
public <M extends MemberScope<?, ?>> CustomPropertyDefinition getCustomDefinition(M scope, SchemaGenerationContext context,
CustomPropertyDefinitionProvider<M> ignoredDefinitionProvider) {
CustomPropertyDefinition result;
if (scope instanceof FieldScope) {
result = this.getCustomDefinition(this.fieldConfigPart, (FieldScope) scope, context,
if (scope instanceof FieldScope fieldScope) {
result = this.getCustomDefinition(this.fieldConfigPart, fieldScope, context,
(CustomPropertyDefinitionProvider<FieldScope>) ignoredDefinitionProvider);
} else if (scope instanceof MethodScope) {
result = this.getCustomDefinition(this.methodConfigPart, (MethodScope) scope, context,
} else if (scope instanceof MethodScope methodScope) {
result = this.getCustomDefinition(this.methodConfigPart, methodScope, context,
(CustomPropertyDefinitionProvider<MethodScope>) ignoredDefinitionProvider);
} else {
throw new IllegalArgumentException("Unexpected member scope: " + (scope == null ? null : scope.getClass().getName()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,25 @@ public class SchemaGeneratorAllOfCleanUpTest {
static Stream<Arguments> parametersForTestAllOfCleanUp() {
String differentValueInMainSchema = "{ \"type\":\"object\", \"title\":\"main schema\", \"allOf\":[{ \"title\":\"different title\" }, {}] }";
String differentValueInAllOfPart = "{ \"type\":\"object\", \"allOf\":[{ \"title\":\"title X\" }, { \"title\":\"title Y\" }] }";
String equalIfTagInMainSchema = "{ \"type\":\"object\", \"if\":{ \"const\": 1 }, \"then\":{}, "
+ "\"allOf\":[{ \"if\":{ \"const\": 1 }, \"then\":{}, \"else\": { \"title\": \"otherwise...\" } }, {}] }";
String equalIfTagInAllOfPart = "{ \"type\":\"object\", \"allOf\":[{ \"if\":{ \"const\": 1 }, \"then\":{} }, "
+ "{ \"if\":{ \"const\": 1 }, \"then\":{}, \"else\": { \"title\": \"otherwise...\" } }] }";
String equalIfTagInMainSchema = """
{ "type":"object", "if":{ "const": 1 }, "then":{}, \
"allOf":[{ "if":{ "const": 1 }, "then":{}, "else": { "title": "otherwise..." } }, {}] }\
""";
String equalIfTagInAllOfPart = """
{ "type":"object", "allOf":[{ "if":{ "const": 1 }, "then":{} }, \
{ "if":{ "const": 1 }, "then":{}, "else": { "title": "otherwise..." } }] }\
""";
List<Arguments> testCases = EnumSet.allOf(SchemaVersion.class).stream()
.flatMap(schemaVersion -> Stream.of(
Arguments.of(schemaVersion, differentValueInMainSchema, differentValueInMainSchema),
Arguments.of(schemaVersion, differentValueInAllOfPart, differentValueInAllOfPart),
Arguments.of(schemaVersion, equalIfTagInMainSchema, equalIfTagInMainSchema),
Arguments.of(schemaVersion, equalIfTagInAllOfPart, equalIfTagInAllOfPart),
Arguments.of(schemaVersion,
"{ \"type\": \"object\", \"title\":\"same in all three\", "
+ "\"allOf\": [{ \"title\":\"same in all three\" }, { \"title\":\"same in all three\" }] }",
"""
{ "type": "object", "title":"same in all three", \
"allOf": [{ "title":"same in all three" }, { "title":"same in all three" }] }\
""",
"{ \"type\": \"object\", \"title\":\"same in all three\" }"),
Arguments.of(schemaVersion,
"{ \"type\": \"object\", \"allOf\": [{ \"title\":\"from allOf[0]\" }, { \"description\":\"from allOf[1]\" }] }",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,13 @@ public void testCreateStandardDefinitionReferenceForField_withCustomPropertyDefi
public void testCreateStandardDefinition() {
ResolvedType type = this.contextImpl.getTypeContext().resolve(TestClass.class);
ObjectNode result = this.contextImpl.createStandardDefinition(type, null);
Assertions.assertEquals("{\"type\":\"object\",\"properties\":{"
+ "\"booleanField\":{\"allOf\":[{},{\"title\":\"Field Title\"}]},"
+ "\"isBooleanField()\":{\"allOf\":[{},{\"title\":\"Method Title\"}]}},"
+ "\"dependentRequired\":{\"booleanField\":[\"isBooleanField()\"],\"isBooleanField()\":[\"booleanField\"]}," +
"\"description\":\"Type Description\"}",
Assertions.assertEquals("""
{"type":"object","properties":{\
"booleanField":{"allOf":[{},{"title":"Field Title"}]},\
"isBooleanField()":{"allOf":[{},{"title":"Method Title"}]}},\
"dependentRequired":{"booleanField":["isBooleanField()"],"isBooleanField()":["booleanField"]},\
"description":"Type Description"}\
""",
result.toString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,10 @@ private void addStandardModule(GeneratorModule module, SchemaGeneratorConfigBuil
configBuilder.with(new Swagger2Module());
break;
default:
throw new MojoExecutionException("Error: Module does not have a name in "
+ "['Jackson', 'JakartaValidation', 'JavaxValidation', 'Swagger15', 'Swagger2'] or does not have a custom classname.");
throw new MojoExecutionException("""
Error: Module does not have a name in \
['Jackson', 'JakartaValidation', 'JavaxValidation', 'Swagger15', 'Swagger2'] or does not have a custom classname.\
""");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ public void testPackageName(String scenario) throws Exception {
@Test
public void testFileNamePattern() throws Exception {
File testCaseLocation = new File("src/test/resources/reference-test-cases");
File generationLocation = new File("target/generated-test-sources/SchemaFileName/schemas/"+
"com/github/victools/jsonschema/plugin/maven/testpackage");
File generationLocation = new File("""
target/generated-test-sources/SchemaFileName/schemas/\
com/github/victools/jsonschema/plugin/maven/testpackage\
""");

// Execute the pom
executePom(new File("src/test/resources/reference-test-cases/SchemaFileName-pom.xml"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ protected int getPropertyIndex(MemberScope<?, ?> property) {
List<String> sortedProperties = this.propertyOrderPerDeclaringType
.computeIfAbsent(topMostHierarchyType.getErasedType(), this::getAnnotatedPropertyOrder);
String fieldName;
if (property instanceof MethodScope) {
fieldName = Optional.<MemberScope>ofNullable(((MethodScope) property).findGetterField())
if (property instanceof MethodScope scope) {
fieldName = Optional.<MemberScope>ofNullable(scope.findGetterField())
// since 4.33.1: fall-back on method's property name if no getter can be found
.orElse(property)
.getSchemaPropertyName();
Expand Down
Loading
Loading