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

Doclint #883

Merged
merged 5 commits into from
May 12, 2014
Merged

Doclint #883

merged 5 commits into from
May 12, 2014

Conversation

stefanbirkner
Copy link
Contributor

Verify Javadoc with doclint. We only use the checks of the groups accessibility and reference, because all other groups report issues.

@Tibor17
Copy link
Contributor

Tibor17 commented Apr 27, 2014

@stefanbirkner
Do you know where my comment has gone?
It was about moving the doclint to maven-compiler-plugin configuration.

@Tibor17
Copy link
Contributor

Tibor17 commented Apr 27, 2014

@stefanbirkner
I am trying to repeat my lost comment, which i am not sure if it will be as good as before.

The point is to configure maven-compiler-plugin instead of maven-javadoc-plugin.
The same feature doclint was introduced in javadoc and javac because the Oracle want the developer to find Javadoc issues in compile time. This makes sense to me and therefore I modified configuration of maven-compiler-plugin.

                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.1</version>
                <configuration>
                    <encoding>${project.build.sourceEncoding}</encoding>
                    <source>${jdkVersion}</source>
                    <target>${jdkVersion}</target>
                    <showWarnings>true</showWarnings>
                    <debug>true</debug>
                    <verbose>true</verbose>
                    <fork>true</fork>
                    <compilerArgs>
                        <args>-Xlint:unchecked</args>
                    </compilerArgs>
                </configuration>

With profiles

        <profile>
            <id>java8</id>
            <activation>
                <jdk>[1.8,)</jdk>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <artifactId>maven-compiler-plugin</artifactId>
                        <configuration>
                            <compilerArgument>-Xdoclint:html,syntax,accessibility,reference</compilerArgument>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>
        <profile>
            <id>default</id>
            <activation>
                <activeByDefault>true</activeByDefault>
            </activation>
            <build>
                <plugins>
                    <plugin>
                        <artifactId>maven-compiler-plugin</artifactId>
                        <configuration>
                            <showDeprecation>true</showDeprecation>
                        </configuration>
                    </plugin>
                </plugins>
            </build>
        </profile>

You can use -Xdoclint:html,syntax,accessibility,reference, and the number of issues is:
html 23
syntax 9
accessibility 0
reference 2

Since the attribute html reports a lot of issues, I can add it and with it in next pull request.

@stefanbirkner
Copy link
Contributor Author

@Tibor17 I rebased the branch. Obviously this deleted your comment. I create an issue for the Github team.

@Tibor17
Copy link
Contributor

Tibor17 commented Apr 29, 2014

@stefanbirkner
Pls remove the default plugin groupId org.apache.maven.plugins. Maven will make a guess on groupId in case of default prefixed plugins. See how pom.xml started in early beginning - groupId was avoided as intended.

When I tested my pom.xml, the compiler's config <showDeprecation>true</showDeprecation> was in collision with -Xdoclint and the build faild with errors on deprecated classes with JDK8. I wanted to have only warnings on deprecations. That's the reason why i removed showDeprecation from plugin's config and introduced it in profile "default" activated by default.

Did you test it with JDK8, are there any errors or warnings?

@stefanbirkner
Copy link
Contributor Author

Thanks for the hint about the group id of default plugins. I change it in the evening.

The project is building without errors on JDK6, JDK7 and JDK8: https://travis-ci.org/stefanbirkner/junit/builds/23911515 Unfortunately Travis doesn't support JDK 5.

@Tibor17
Copy link
Contributor

Tibor17 commented May 6, 2014

@stefanbirkner
I am using your branch and launch the build mvn compile javadoc:javadoc -e -X with new jdk.
I can see new arguments in maven-compiler-plugin
[DEBUG] (f) compilerArgs = [-Xlint:unchecked, -Xdoclint:accessibility,reference,syntax]

  <compilerArgs>
    <arg>-Xlint:unchecked</arg>
    <arg>-Xdoclint:accessibility,reference,syntax</arg>
  </compilerArgs>

but the maven-javadoc-plugin has only one parameter:
-Xdoclint:reference
[DEBUG] (f) additionalparam = -Xdoclint:reference

Can you fix it?

@stefanbirkner
Copy link
Contributor Author

Now javadoc verifies accessibility and reference. Thanks for finding this issue.

With JDK 8, doclint has been added to Javac and Javadoc. It is enabled
by default but it is too strict for JUnit. The only groups that don't
report issues are accessibility, reference and syntax. But syntax has
to be disabled for Javadoc because the Hamcrest classes are reporting
syntax issues.

Additionally doclint is disabled for JDK < 8, because the -Xdoclint
argument causes a failing build in JDKs before JDK 8.
The build should fail for Javadoc issues.
The parameter compilerArguments is deprecated.
@Tibor17
Copy link
Contributor

Tibor17 commented May 7, 2014

+1

@stefanbirkner
Copy link
Contributor Author

This pull request is ready to be merged.

@marcphilipp
Copy link
Member

LGTM!

@kcooney Any objections?

@kcooney
Copy link
Member

kcooney commented May 9, 2014

@marcphilipp no objections at all.

@stefanbirkner Thanks for fixing the Javadoc!

marcphilipp added a commit that referenced this pull request May 12, 2014
@marcphilipp marcphilipp merged commit 5e692e4 into junit-team:master May 12, 2014
@marcphilipp
Copy link
Member

Thanks!

@stefanbirkner stefanbirkner deleted the doclint branch May 13, 2014 07:50
@stefanbirkner stefanbirkner added this to the 4.12 milestone May 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants