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

Improve combability with cobertura #1014

Closed
halmafest opened this issue Feb 4, 2016 · 14 comments
Closed

Improve combability with cobertura #1014

halmafest opened this issue Feb 4, 2016 · 14 comments

Comments

@halmafest
Copy link

I use lombok to generate a lot of code and I really like it!
To measure the code coverage we use cobertura

We like to use the cobertura option ignoreAnnotations. Basically the feature allows to disable code coverage checks of methods annotated with a defined annotation. Unfortunately the annotation added by lombok javax.annotation.Generated does not work.

Reasons:

  • as I understand cobertura works on class files
  • javax.annotation.Generated has a @retention(value=SOURCE) definition (javadoc)
  • means the annotation gets removed from the java compiler and is not visible during the code coverage check
  • => each annotation having a @retention(value=SOURCE) can not be used to exclude code.

Possible solutions:

  • allow a user to define which annotation(s) are added to generated source
  • add the already defined annotation lombok.@Generated to generated source
@Maaartinus
Copy link
Contributor

Another solution idea: At the end, for every @javax.annotation.Generated annotation, add @lombok.Generated. This could be pretty simple and could help other code generation tools.

@halmafest
Copy link
Author

As a workaround I did a few modifications to the lombok code to write the lombok.Generated annotation in addition.

Changes:

  • lombok.Generated is missing the constructor flag in the @target
  • when javax.annotation.Generated is written I added to write in addition lombok.Generated to the code

Result:

  • + Code coverage reports looks nice now. Works together with cobertura
  • + only few changes required
  • - issues during delombok tests, annotation isn't known except you add lombok as dependency
  • - forked library, need to rebuild for every new version of lombok I want to use

@rspilker
Copy link
Collaborator

It is possible to add an extra option to FormatPreferences either generate or not generate source code using the lombok.Generated annotation.

I think by default the lombok.Generated should not be generated in delomboked code.

Regarding the forked library: we do accept pull requests...

@dabraham02124
Copy link

Yes please, @halmafest. As the person who reported the dupe issue #1102, I'd love to see this pulled in to mainline lombok. Thank you very much.

@halmafest
Copy link
Author

I will see if I can find some time the next weeks o make a clean pull request.
@rspilker, @ALL What is the preferred solution regarding FormatPreferences. I see two different options how to implement it:

  • on/off switch to toggle generation of the lombock.Generated (same as for SuppressWarnings), default would be false (skip)
    • follows the current implementation
    • minimal changes required
  • allow to define in the FormatPreferences a set of additional annotations to be added, defined using package and classname
    • more flexibility
    • added complexity to code
    • validation of the input might be tricky

@rspilker
Copy link
Collaborator

Thanks for offering to implement it. I prefer to have an on/off toggle in the FormatPreferences, defaulting to not generate it when delombokking.

@halmafest
Copy link
Author

I started to implement this and I stuck a bit in the JavacHandlerUtil class. Maybe its only an understanding issue.

As I understand there are two ways to configure lombok.

  • FormatPreferences
  • LombokConfiguration/ConfigurationKeys

The FormatPreferences are checked first, ConfigurationKeys second.
Example:

public static void addGenerated(JCModifiers mods, JavacNode node, int pos, JCTree source, Context context) {
        if (!LombokOptionsFactory.getDelombokOptions(context).getFormatPreferences().generateGenerated()) return;

        if (!Boolean.FALSE.equals(node.getAst().readConfiguration(ConfigurationKeys.ADD_GENERATED_ANNOTATIONS))) {
            addAnnotation(mods, node, pos, source, context, "javax.annotation.Generated", node.getTreeMaker().Literal("lombok"));
        }
    }

This works fine as long the default value for an option in the FormatPreferences is set to true. For the new feature I like to set the default to false. This will lead to situation that for a normal javac run the FormatPreferences will be set to the defaults and the ConfigurationKeys are ignored.

I'm not sure how to solve this issue in the best way. Maybe there is a way to merge both configuration worlds.
In general I think the FormatPreferences class is located wrongly. As it is used in core, it should also be part of the core package otherwise you have a strong link to the delombok package. An alternative solution might be to remove all references to the FormatPreferences from the core classes and init LombokConfiguration values when running delombok.

@dabraham02124
Copy link

I was wondering if there was any motion on this. Or if there was any way that I could help with it. I'd love to have this, and I'm willing to at least try to help. Thank you very much.

@Gaibhne
Copy link

Gaibhne commented Dec 23, 2016

I forked Lombok, added a config key to specify the name of the annotation (like "lombok.generatedAnnotationName = com.whatever.test.LombokGenerated") which can then have a retain value of 'class', tested it with Cobertura and it worked perfectly fine. Since the ignore features in Jacoco are still in development I didn't bother testing them, but I see no reason it wouldn't work with that as well. I could make a PR, it's only like five lines of code changes, but I don't know how the Lombok team feels about adding config keys. @rspilker ?

@rspilker
Copy link
Collaborator

rspilker commented Jan 6, 2017

From now on, whenever @javax.annotation.Generated("lombok") was added, @lombok.Generated is also added.

To turn them both on or off, use the (original) configuration key lombok.addGeneratedAnnotation.

To override this general setting, use the keys lombok.addJavaxGeneratedAnnotation and lombok.addLombokGeneratedAnnotation.

For delombok you can turn them both off using the -f generated:skip command line parameter. This is only necessary if they were turned on in the configuration system in the first place.

@rspilker rspilker closed this as completed Jan 6, 2017
@rspilker
Copy link
Collaborator

rspilker commented Jan 6, 2017

We've just created an edge release. Can someone try this out on cobertura and/or Jacoco?

@Gaibhne
Copy link

Gaibhne commented Jan 6, 2017

I can confirm it working with Cobertura and:

<build>
	<plugins>
		<!-- Code coverage plugin to produce test reports -->
		<plugin>
			<groupId>org.codehaus.mojo</groupId>
			<artifactId>cobertura-maven-plugin</artifactId>
			<version>2.7</version>
			<configuration>
				<instrumentation>
					<ignoreMethodAnnotations>
						<ignoreMethodAnnotation>lombok.Generated</ignoreMethodAnnotation>
					</ignoreMethodAnnotations>
				</instrumentation>
			</configuration>
			<executions>
				<execution>
					<goals>
						<goal>clean</goal>
					</goals>
				</execution>
			</executions>
		</plugin>
	</plugins>
</build>

I have no Jacoco test environment set up since their filtering stuff is still in a dev branch as far as I can tell. Thanks for fixing/implementing this!

@seifertm
Copy link

seifertm commented Jan 9, 2017

This looks very promising! Thanks for your efforts.
Is there any preliminary date when this will make it into a release available on Maven central?

@rspilker
Copy link
Collaborator

rspilker commented Feb 9, 2017

Slight change of plans. @lombok.Generated needs to be explicitly turned on to cater for the scenario that the lombok version in the project is much older than the one used in Eclipse. This is the safest option.

The (original) configuration key lombok.addGeneratedAnnotation is now deprecated,
It is replaced by lombok.addJavaxGeneratedAnnotation

To generate @lombok.Generated use lombok.addLombokGeneratedAnnotation = true

We expect the release a new version of lombok within a few days.

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

No branches or pull requests

6 participants