0

How would i set up using pmd and checkstyle results as advice only and disable them on the build server? And would it be bad practice to do so?

Both pmd and checkstyle offer valuable advice, and i want to keep on using them.

But (here comes the but) i find that my code collects a lot of lint trying to work around some of the warnings. To name a few examples:

  • Test-classes contain many mockito and junit static imports, invariably i have to add @SuppressWarnings("PMD.TooManyStaticImports").

  • A class under test needs its fields filled with mock objects, these are not used anywhere in the test but they need to be declared and annotated with @Mock for the class under test to work correctly. Add @SuppressWarnings("PMD.UnusedPrivateField").

  • In test classes i will have methods for creating objects from a long list of parameters, eg: createPerson(String firstname, String lastname, int shoesize, String favouritecolor, ...). These objects are normally created from a database or XML. Add @SuppressWarnings("PMD.ParameterNumberCheck").

  • Sometimes my documentation will be: "This method makes sure that X in the following 3 cases: \n ...". Apparently this is not allowed as the first sentence should end with a period.

  • Parent class X has some field y that all its children need and use, but checkstyle won't allow it unless the field is accessed through a method (getY()). This is just unnatural, IMO.

One option would be to turn the checks causing the most nuisance off permanently, however a check may be a nuisance or very useful depending on the context. I recognize that explicitly suppressing warnings in the code is also a way to document that only in the specific context, the check is irrelavant and annoying. It is the amount of suppresions that annoys me, almost every testclass needs suppressions, and some of the other classes need workarounds.

So would it be a solutions to generate the warings, but not allow checkstyle and pmd violations to fail te build?

Ivana
  • 643
  • 10
  • 27

1 Answers1

2

Test-classes contain ...
A class under test ...
In test classes ...

It seems to me, you should suppress these checks under your test code as you don't agree with them.

This is a common occurrence, like in Checkstyle we don't document our test code but our main code documents everything. To get around this for PMD, we split our configuration between test and main. To get around this for Checkstyle utility, we suppress violations for the test directory. You can also look at the options for the Checks, and see if there is anyway to configure it to ignore your cases.

Sometimes my documentation will be: "This method makes sure that X in the following 3 cases: \n ...".

I can't say for certain since I don't know the contents of your methods, but the first sentence should be a simple explanation of what the method does and it's goal. Then you can follow it by your specific cases you mentioned. Checkstyle just requires the first sentence to end with a period, not every sentence.

Parent class X has some field y that all its children need and use, but checkstyle won't allow it unless the field is accessed through a method (getY()). This is just unnatural, IMO.

Since you completely dislike this, then just disable the check for protected fields. If you look at the documentation for VisibilityModifier, you can change protectedAllowed to true and have it ignore these specific cases.

i find that my code collects a lot of lint trying to work around some of the warnings.

To me, it just seems you are not customizing these tools to your preferences and just trying to use a default configuration.

rveach
  • 2,081
  • 15
  • 25