Skip to content

Add possibility to check only public coverage #10

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

Open
wants to merge 6 commits into
base: master
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 pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@
<scope>system</scope>
<systemPath>${java.home}/../lib/tools.jar</systemPath>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.16.20</version>
Copy link
Owner

@manoelcampos manoelcampos Feb 15, 2018

Choose a reason for hiding this comment

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

I really don't like lombok because it uses some JDK internals which may be removed. It also requires IDE plugins for some features. At this time, I prefer to keep dependencies as minimal as possible.

<scope>provided</scope>
</dependency>
</dependencies>

<build>
Expand Down
141 changes: 18 additions & 123 deletions src/main/java/com/manoelcampos/javadoc/coverage/CoverageDoclet.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,10 @@
*/
package com.manoelcampos.javadoc.coverage;

import com.manoelcampos.javadoc.coverage.exporter.ConsoleExporter;
import com.manoelcampos.javadoc.coverage.exporter.DataExporter;
import com.manoelcampos.javadoc.coverage.exporter.HtmlExporter;
import com.sun.javadoc.DocErrorReporter;
import com.sun.javadoc.Doclet;
import com.sun.javadoc.LanguageVersion;
import com.sun.javadoc.RootDoc;
import com.sun.tools.doclets.standard.Standard;

import java.io.*;
import com.manoelcampos.javadoc.coverage.configuration.Configuration;
import com.manoelcampos.javadoc.coverage.exporter.*;
import com.manoelcampos.javadoc.coverage.stats.JavaDocsStats;
import com.sun.javadoc.*;

/**
* A {@link Doclet} that computes coverage of JavaDoc documentation.
Expand All @@ -42,13 +36,6 @@
* @since 1.0.0
*/
public class CoverageDoclet {
/**
* A command line parameter to enable defining the name of the coverage report.
* The first value is the long version of the parameter name and the second
* is the short one.
*/
public static final String OUTPUT_NAME_OPTION[] = {"-outputName", "-o"};

/**
* The {@link DataExporter} object to export the coverage report to a file
* in a specific format.
Expand All @@ -57,8 +44,8 @@ public class CoverageDoclet {
private final RootDoc rootDoc;

/**
* Starts the actual parsing or JavaDoc documentation and generation of the coverage report.
* This is the entry point for the JavaDoc tool to start the Doclet.
* Starts the actual parsing or JavaDoc documentation and generation of the coverage report. This is the entry point
* for the JavaDoc tool to start the Doclet.
*
* @param rootDoc root element which enables reading JavaDoc documentation
* @return true if the Doclet was started successfully, false otherwise
Expand All @@ -75,51 +62,12 @@ public static boolean start(final RootDoc rootDoc) {
*/
public CoverageDoclet(final RootDoc rootDoc) {
this.rootDoc = rootDoc;
this.exporter = new HtmlExporter(this);
}
Configuration config = new Configuration(rootDoc.options());
Copy link
Owner

Choose a reason for hiding this comment

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

The idea in my previous message was to create config as an attribute. This way, you don't need to pass an additional parameter for some methods which already receive the doclet.
I think the code will be clearer if you pass the RootDoc instead of an matrix of String for the Configuration constructor. Inside the constructor you can get the options from the rootDoc.
It's not obvious where this [][] String has to be got from. On the other hand, it's straighforward to understand where to get a rootDoc object from. This way, we move the complexity into the Configuration class.


/**
* Checks if a given parameter is a valid custom parameter accepted by this doclet.
* @param paramName the name of the parameter to check
* @return true if it's a valid custom parameter, false otherwise
*/
private static boolean isCustomParameter(final String paramName) {
return isParameter(paramName, OUTPUT_NAME_OPTION);
}
JavaDocsStats stats = new JavaDocsStats(rootDoc, config);
Copy link
Owner

Choose a reason for hiding this comment

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

Changing the Configuration constructor as suggested above makes unnecessary to add one more parameter to the JavaDocsStats constructor. You can pass only the config parameter and get the rootDoc from it.
For a cleaner and more functional code, the fewer parameters the better.

The stats object was being created inside the AbstractDataExporter class and you moved it out here. The object is then passed to the HtmlExporter constructor below, also increasing its number of parameters. This moves the complexity out of the AbstractDataExporter classes. Encapsulating the stats inside the AbstractDataExporter avoids duplicating its instantiation for every exporter class and also hides the creation of an object used just internally by such exporter classes.


/**
* Checks if the name of a given parameter corresponds to either its long or short form.
*
* @param paramName the name of the parameter to check
* @param validNames the list of accepted names for that parameter
* @return true if the given name corresponds to one of the valid names, false otherwise
*/
private static boolean isParameter(final String paramName, final String[] validNames) {
for (String validName : validNames) {
if (validName.equalsIgnoreCase(paramName)) {
return true;
}
}

return false;
}

/**
* Validates command line options.
*
* @param options the array of given options
* @param errorReporter an object that allows printing error messages for invalid options
* @return true if the options are valid, false otherwise
* @see Doclet#validOptions(String[][], DocErrorReporter)
*/
public static boolean validOptions(final String[][] options, final DocErrorReporter errorReporter) {
for (final String[] opt : options) {
if (isCustomParameter(opt[0])) {
return true;
}
}

return Standard.validOptions(options, errorReporter);
// this needs to be the last part as it already accesses some stuff from the doclet
this.exporter = new HtmlExporter(config, stats);
Copy link
Owner

Choose a reason for hiding this comment

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

Check out the comment above.

}

/**
Expand All @@ -130,32 +78,19 @@ public static boolean validOptions(final String[][] options, final DocErrorRepor
* @see Doclet#optionLength(String)
*/
public static int optionLength(final String option) {
/*The custom outputName parameter accepts one argument.
* The name of the param counts as the one argument.*/
if (isCustomParameter(option)) {
return 2;
}

return Standard.optionLength(option);
return Configuration.getOptionLength(option);
}

/**
* Gets the values associated to a given command line option.
* Checks that all given option are valid
Copy link
Owner

Choose a reason for hiding this comment

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

"Checks that all given options are valid"

*
* @param optionNames an array containing the valid names for the command line option to get its associated values.
* This array may include the long and short versions of the option name,
* for instance {@code {-outputName, -o}}.
* @return the values associated to the option, where the 0th element is the option itself;
* or an empty array if the option is invalid.
* @param options the options to be checked on validity
* @param errorReporter
* @return true if the options are valid
* @see Doclet#validOptions(String[][], DocErrorReporter)
*/
public String[] getOptionValues(final String[] optionNames) {
for (final String[] optionValues : rootDoc.options()) {
if (isParameter(optionValues[0], optionNames)) {
return optionValues;
}
}

return new String[]{};
public static boolean validOptions(final String[][] options, final DocErrorReporter errorReporter) {
return Configuration.areValidOptions(options, errorReporter);
}

/**
Expand Down Expand Up @@ -186,44 +121,4 @@ private boolean render() {
public RootDoc getRootDoc() {
return rootDoc;
}

/**
* Gets a {@link PrintWriter} used by the {@link #exporter} to write
* the coverage report to.
*
* @param file the file to which the coverage report will be saved to
*/
public PrintWriter getWriter(final File file) throws FileNotFoundException {
return new PrintWriter(new OutputStreamWriter(new FileOutputStream(file)));
}

/**
* Gets a {@link File} object from a given file name.
*
* @param fileName the name of the file to get a {@link File} object.
* @return the {@link File} object
*/
public File getOutputFile(final String fileName) {
final File dir = new File(getOutputDir());
if (!dir.exists() && !dir.mkdirs()) {
throw new RuntimeException("The directory '" + getOutputDir() + "' was not created due to unknown reason.");
}

return new File(dir, fileName);
}

/**
* Gets the output directory passed as a command line argument to javadoc tool.
*
* @return the output directory to export the JavaDocs
*/
private String getOutputDir() {
for (final String[] option : rootDoc.options()) {
if (option.length == 2 && option[0].equals("-d")) {
return Utils.includeTrailingDirSeparator(option[1]);
}
}

return "";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package com.manoelcampos.javadoc.coverage.configuration;

import java.util.*;

import com.manoelcampos.javadoc.coverage.Utils;
import com.sun.javadoc.DocErrorReporter;
import com.sun.tools.doclets.standard.Standard;

public class Configuration {

private static final NoValueOption computePublicCoverageOnly = new NoValueOption("-po", "-publicOnly",
"Indicates if coverage should be computed only for the public parts of a project.");
private static final StringOption coverageFileName = new StringOption("-o", "-outputName",
"Set the filename for the created coverage file. If nothing is given, the default is \"javadoc-coverage\".",
Optional.of("javadoc-coverage"));

private static final List<Option<?>> ALL_OPTIONS = new ArrayList<>();

/**
* Static constructor for initializing the options list
*/
static {
ALL_OPTIONS.add(computePublicCoverageOnly);
ALL_OPTIONS.add(coverageFileName);
}

/**
* The raw options which need to be parsed to our configuration
*/
private final String[][] rawOptions;

public Configuration(String[][] rawOptions) {
this.rawOptions = rawOptions;
}

public boolean computePublicCoverageOnly() {
return isOptionContained(computePublicCoverageOnly);
}

public String getCoverageFileName() {
if (isOptionContained(coverageFileName)) {
return getOptionValue(coverageFileName);
}
return coverageFileName.getDefaultValue();
}

/**
* Gets the output directory passed as a command line argument to javadoc tool.
*
* @return the output directory to export the JavaDocs
*/
public String getOutputDirectory() {
// This is no option of the doclet, but instead of the javadoc tool
// so we don't declare it as an option here directly)

for (final String[] option : rawOptions) {
if (option.length == 2 && option[0].equals("-d")) {
return Utils.includeTrailingDirSeparator(option[1]);
}
}

return "";
}

private boolean isOptionContained(Option<?> option) {
for (final String[] opt : rawOptions) {
if (option.isOption(opt[0])) {
return true;
}
}
return false;
}

private <T> T getOptionValue(Option<T> option) {
if (!isOptionContained(option) && !option.hasDefaultValue()) {
throw new IllegalStateException("Option is not contained and has no default value");
}

if (!isOptionContained(option)) {
return option.getDefaultValue();
}

for (final String[] opt: rawOptions) {
if (option.isOption(opt[0])) {
return option.parseValue(opt);
}
}

throw new IllegalStateException("no valid option found although it should be there");
}

/**
* This method is necessary for validating the doclet parameters, which happens before instantiating the
* CoverageDoclet and the configuration container.
*
* @param option
* @return the length of the option
*/
public static int getOptionLength(final String option) {
Optional<Option<?>> opt = ALL_OPTIONS.stream().filter(o -> o.isOption(option)).findFirst();
if (opt.isPresent()) {
return opt.get().getLength();
}
return Standard.optionLength(option);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Using isPresent is a bad practice also. You could replace the lines starting at the if by:

return opt.map(Option::getLength).orElseGet(() -> Standard.optionLength(option))


/**
* This method is necessary for validating the doclet parameters, which happens before instantiating the
* CoverageDoclet and the configuration container.
*
* @param options
* @param errorReporter
* @return indicates if the given options are valid
*/
public static boolean areValidOptions(final String[][] options, final DocErrorReporter errorReporter) {
for (final String[] opt : options) {
if (ALL_OPTIONS.stream().anyMatch(o -> o.isOption(opt[0]))) {
return true;
}
}

return Standard.validOptions(options, errorReporter);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package com.manoelcampos.javadoc.coverage.configuration;

import lombok.AllArgsConstructor;
import lombok.Getter;

@Getter
@AllArgsConstructor
public class CoverageLimits {

private static final int PACKAGE_INDEX = 1;
private static final int INTERFACE_INDEX = 2;
private static final int CLASS_INDEX = 3;
private static final int METHOD_INDEX = 4;

private final double minPackageCoverage;
private final double minInterfaceCoverage;
private final double minClassCoverage;
private final double minMethodCoverage;

public static CoverageLimits noLimits() {
return new CoverageLimits(0,0,0,0);
}

public static CoverageLimits fromOptionsArray(String[] arr) {
double packageCov = safeParse(arr, PACKAGE_INDEX, "package");
double interfaceCov = safeParse(arr, INTERFACE_INDEX, "interface");
double classCov = safeParse(arr, CLASS_INDEX, "class");
double methodCov = safeParse(arr, METHOD_INDEX, "method");

return new CoverageLimits(packageCov, interfaceCov, classCov, methodCov);
}

private static double safeParse(String[] arr, int index, String exceptionMessagePrefix) {
try {
return Double.parseDouble(arr[index]);
} catch (NumberFormatException e) {
throw new IllegalArgumentException(exceptionMessagePrefix + " min coverage number was not a double: " + arr[index]);
}
}
}
Loading