Skip to content

Use Mojo Annotations instead of Javadoc tags in the Maven Plugin #1476

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

Conversation

unkish
Copy link
Collaborator

@unkish unkish commented Feb 5, 2023

Switch from Javadoc tags to Mojo Annotations to reduce amount of warnings emitted during build:

Warning:  /home/runner/work/jsonschema2pojo/jsonschema2pojo/jsonschema2pojo-maven-plugin/src/main/java/org/jsonschema2pojo/maven/Jsonschema2PojoMojo.java:65: warning - @goal is an unknown tag.
Warning:  /home/runner/work/jsonschema2pojo/jsonschema2pojo/jsonschema2pojo-maven-plugin/src/main/java/org/jsonschema2pojo/maven/Jsonschema2PojoMojo.java:65: warning - @phase is an unknown tag.
Warning:  /home/runner/work/jsonschema2pojo/jsonschema2pojo/jsonschema2pojo-maven-plugin/src/main/java/org/jsonschema2pojo/maven/Jsonschema2PojoMojo.java:65: warning - @requiresDependencyResolution is an unknown tag.
Warning:  /home/runner/work/jsonschema2pojo/jsonschema2pojo/jsonschema2pojo-maven-plugin/src/main/java/org/jsonschema2pojo/maven/Jsonschema2PojoMojo.java:65: warning - @threadSafe is an unknown tag.
[INFO] java-javadoc mojo extractor found 1 mojo descriptor.
Warning:  
Warning:  Deprecated extractor java-javadoc extracted 1 descriptor. Upgrade your Mojo definitions.
Warning:  You should use Mojo Annotations instead of Javadoc tags.
Warning:  
Warning:  

Some dependencies of Maven Plugins are expected to be in provided scope.
Please make sure that dependencies listed below declared in POM
have set '<scope>provided</scope>' as well.

The following dependencies are in wrong scope:
 * org.apache.maven:maven-plugin-api:jar:2.2.1:compile
 * org.apache.maven:maven-project:jar:2.2.1:compile
 * org.apache.maven:maven-settings:jar:2.2.1:compile
 * org.apache.maven:maven-profile:jar:2.2.1:compile
 * org.apache.maven:maven-model:jar:2.2.1:compile
 * org.apache.maven:maven-artifact-manager:jar:2.2.1:compile
 * org.apache.maven:maven-repository-metadata:jar:2.2.1:compile
 * org.apache.maven:maven-plugin-registry:jar:2.2.1:compile
 * org.apache.maven:maven-artifact:jar:2.2.1:compile

Also fixed doclint errors and removed doclint-java8-disable profile

@unkish
Copy link
Collaborator Author

unkish commented Feb 5, 2023

Ran plugin 3 times and captured Jsonschema2PojoMojo field values at Jsonschema2PojoMojo::execute

  1. plugin version 1.1.3 without configuration parameters supplied, to capture "pure" state
  2. plugin version 1.1.4-SNAPSHOT without configuration parameters supplied, to capture "pure" state
  3. plugin version 1.1.4-SNAPSHOT with all configuration parameters either flipped or random values supplied

Outcome:

  • 1 vs. 2 - no diff
  • 2 vs. 3 - values were taken as provided in configuration, diff (expires in 1 month)

Configuration used to check values are set as supplied in configuration:

                <executions>
                    <execution>
                        <id>execution-with-error</id>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                        <phase>generate-sources</phase>
                        <configuration>
                            <outputDirectory>/a_random_non_existing_output_directory</outputDirectory>
                            <sourceDirectory>/a_random_non_existing_source_directory</sourceDirectory>
                            <sourcePaths>
                                <sourcePath>source_path_1</sourcePath>
                                <sourcePath>source_path_2</sourcePath>
                                <sourcePath>source_path_3</sourcePath>
                            </sourcePaths>
                            <targetPackage>org.target.package</targetPackage>
                            <generateBuilders>true</generateBuilders>
                            <includeTypeInfo>true</includeTypeInfo>
                            <usePrimitives>true</usePrimitives>
                            <addCompileSourceRoot>false</addCompileSourceRoot>
                            <skip>true</skip>
                            <propertyWordDelimiters>property-word-delimiters</propertyWordDelimiters>
                            <useLongIntegers>true</useLongIntegers>
                            <useBigIntegers>true</useBigIntegers>
                            <useDoubleNumbers>false</useDoubleNumbers>
                            <useBigDecimals>true</useBigDecimals>
                            <includeHashcodeAndEquals>false</includeHashcodeAndEquals>
                            <includeToString>false</includeToString>
                            <toStringExcludes>
                                <toStringExclude>to_string_exclude</toStringExclude>
                                <toStringExclude>another_to_string_exclude</toStringExclude>
                            </toStringExcludes>
                            <annotationStyle>none</annotationStyle>
                            <useTitleAsClassname>true</useTitleAsClassname>
                            <inclusionLevel>inclusion-level</inclusionLevel>
                            <customAnnotator>org.custom.Annotator</customAnnotator>
                            <customRuleFactory>org.custom.rule.Factory</customRuleFactory>
                            <includeJsr303Annotations>true</includeJsr303Annotations>
                            <includeJsr305Annotations>true</includeJsr305Annotations>
                            <useOptionalForGetters>true</useOptionalForGetters>
                            <sourceType>json</sourceType>
                            <removeOldOutput>true</removeOldOutput>
                            <outputEncoding>ASCII</outputEncoding>
                            <useJodaDates>true</useJodaDates>
                            <useJodaLocalDates>true</useJodaLocalDates>
                            <useJodaLocalTimes>true</useJodaLocalTimes>
                            <dateTimeType>java.time.LocalDateTime</dateTimeType>
                            <timeType>java.time.LocalTime</timeType>
                            <dateType>java.time.LocalDate</dateType>
                            <useCommonsLang3>true</useCommonsLang3>
                            <parcelable>true</parcelable>
                            <serializable>true</serializable>
                            <initializeCollections>false</initializeCollections>
                            <includes>
                                <include>an_include</include>
                                <include>another_include</include>
                            </includes>
                            <excludes>
                                <exclude>an_exclude</exclude>
                                <exclude>another_exclude</exclude>
                            </excludes>
                            <classNamePrefix>class_name_prefix</classNamePrefix>
                            <classNameSuffix>class_name_suffix</classNameSuffix>
                            <fileExtensions>
                                <extension>json</extension>
                                <extension>yaml</extension>
                            </fileExtensions>
                            <includeConstructors>true</includeConstructors>
                            <constructorsRequiredPropertiesOnly>true</constructorsRequiredPropertiesOnly>
                            <includeRequiredPropertiesConstructor>true</includeRequiredPropertiesConstructor>
                            <includeAllPropertiesConstructor>false</includeAllPropertiesConstructor>
                            <includeCopyConstructor>true</includeCopyConstructor>
                            <includeAdditionalProperties>false</includeAdditionalProperties>
                            <includeGetters>false</includeGetters>
                            <includeSetters>false</includeSetters>
                            <targetVersion>21</targetVersion>
                            <includeDynamicAccessors>true</includeDynamicAccessors>
                            <includeDynamicGetters>true</includeDynamicGetters>
                            <includeDynamicSetters>true</includeDynamicSetters>
                            <includeDynamicBuilders>true</includeDynamicBuilders>
                            <formatDates>true</formatDates>
                            <formatTimes>true</formatTimes>
                            <formatDateTimes>true</formatDateTimes>
                            <customDatePattern>custom_date_pattern</customDatePattern>
                            <customTimePattern>custom_time_pattern</customTimePattern>
                            <customDateTimePattern>custom_date_time_pattern</customDateTimePattern>
                            <refFragmentPathDelimiters>ref_fragment_path_delimiters</refFragmentPathDelimiters>
                            <sourceSortOrder>source_sort_order</sourceSortOrder>
                            <formatTypeMapping>
                                <first>one</first>
                                <second>two</second>
                            </formatTypeMapping>
                            <useInnerClassBuilders>true</useInnerClassBuilders>
                            <includeConstructorPropertiesAnnotation>true</includeConstructorPropertiesAnnotation>
                            <includeGeneratedAnnotation>false</includeGeneratedAnnotation>
                            <useJakartaValidation>true</useJakartaValidation>
                        </configuration>
                    </execution>

*/
public void setConstructorsRequiredPropertiesOnly(boolean constructorsRequiredPropertiesOnly) {
this.constructorsRequiredPropertiesOnly = constructorsRequiredPropertiesOnly;
}

/**
* Sets the 'constructorsIncludeRequiredPropertiesConstructor' configuration option. This property works in collaboration with the {@link
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like The 'constructorsIncludeRequiredPropertiesConstructor' configuration option ... has been copy-pasted several times.
Replaced constructorsIncludeRequiredPropertiesConstructor with actual property that the Javadoc is for (also in GenerationConfig and JsonSchema2PojoMojo)

* The 'constructorsRequiredPropertiesOnly' configuration option. This is a legacy configuration option used to turn on the {@link
* #isIncludeAllPropertiesConstructor()} and off the {@link * #isConstructorsIncludeAllPropertiesConstructor()} configuration options.
* It is specifically tied to the {@link #isIncludeConstructors()} * property, and will do nothing if that property is not enabled
* The 'constructorsRequiredPropertiesOnly' configuration option.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since given Javadoc contained invalid {@link ...} value as well as inaccurate text it had to be rewritten a bit.
Needs to be checked whether description is better now or needs changes

Comment on lines +105 to +108
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-plugin-api</artifactId>
</dependency>
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry if this I have missed something obvious, but is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a great question @joelittlejohn and a place where a decision can be made.
With proposed solution this is needed as there were changes in pom.xml:

                <groupId>org.apache.maven</groupId>
                <artifactId>maven-plugin-api</artifactId>
                <version>2.2.1</version>
                <scope>provided</scope>

specifically <scope>provided</scope> was added to avoid:

The following dependencies are in wrong scope:

  • org.apache.maven:maven-plugin-api:jar:2.2.1:compile

Which in turn made maven-plugin-api no longer available/visible to jsonschema2pojo-integration-tests.
There are several classes (AnnotationStyleIT, CustomAnnotatorIT, CodeGenerationHelper) in jsonschema2pojo-integration-tests that depend on org.apache.maven.plugin.MojoExecutionException (maven-plugin-api)


It is possible to remove provided scope from maven-plugin-api though that would make warning reappear.
What do you think, should the provided scope be removed or not ?

Copy link
Owner

Choose a reason for hiding this comment

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

Ah I see. Yes, that makes sense. I think it's good that because this dependency is now provided in the jsonschema2pojo-maven-plugin, the integration tests must declare this dependency explicitly.

This explicit dependency should have arguably always been there in the pom.xml for the integration tests. I don't like to rely on a transitive dependency if the dependency is in fact direct.

@joelittlejohn joelittlejohn merged commit 25bfc76 into joelittlejohn:master Feb 6, 2023
@joelittlejohn joelittlejohn changed the title Use Mojo Annotations instead of Javadoc tags Use Mojo Annotations instead of Javadoc tags in the Maven Plugin Feb 6, 2023
@joelittlejohn joelittlejohn added this to the 1.2.0 milestone Feb 6, 2023
@unkish unkish deleted the mojo_javadoc_tags_to_mojo_annotations branch February 7, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants