4

I have a doubt, I have a class in which I'm using different static import for import constants, my issue is that I'm getting the error message: Too many static imports may lead to messy code. But in the unit test it looks like it is not a bad practice. For example in a unit test class, I'm using this import with any problem:

import static com.rccl.middleware.kidsclub.engine.web.controller.KidController.KID_FIND_PATH;
import static com.rccl.middleware.kidsclub.engine.web.controller.KidController.KID_LIST_PATH;
import static com.rccl.middleware.kidsclub.engine.web.controller.KidController.KID_PATH;
import static com.rccl.middleware.kidsclub.engine.web.controller.KidController.KID_REGISTER_ALL_PATH;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.then;
import static org.mockito.Mockito.times;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.request;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;```

Conversely in my class:

import static org.springframework.http.HttpStatus.BAD_REQUEST;
import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
import static org.springframework.http.HttpStatus.NOT_FOUND;
import static org.springframework.http.HttpStatus.UNPROCESSABLE_ENTITY;

These imports makes me problems and I get the error message: Too many static imports may lead to messy code. I'm not pretty sure why.

I am not able to access the pmd file to figured out the real cause of this issue. Any clue?

Thanks!

Ahmad Abdelghany
  • 11,983
  • 5
  • 41
  • 36
rasilvap
  • 1,771
  • 3
  • 31
  • 70
  • No isn't. It is a pmd error. – rasilvap Oct 04 '19 at 18:58
  • The rule has a `maximumStaticImports` property to set how many are "too many". Tune that value to your liking, after all, this is a *code style* rule. Read the docs here: https://pmd.github.io/pmd-6.18.0/pmd_rules_java_codestyle.html#toomanystaticimports – Johnco Oct 05 '19 at 20:28
  • In production code (=non-test code) the code _quality_ is weighted a bit more than _verbosity_. I think the current settings came originally from Assert imports. Above there is nothing wrong using a qualified `HttpStatus.SAD_REQUEST`, especially as you still have `case SAD_REQUEST:`. – Joop Eggen Nov 19 '19 at 14:52

7 Answers7

8

I have used @SuppressWarnings("PMD.TooManyStaticImports") To avoid this issue in my class.

rasilvap
  • 1,771
  • 3
  • 31
  • 70
  • It would be nice to clarify if that solution is recommended – darw May 26 '20 at 15:33
  • 2
    I'd rather not use this solution, as it needs to be introduced in gazillion files... it's better to do it via exclusion of rules for the whole project, that wey you'll have just 1 place to control this rule instead of gazillion places – Dáve Oct 22 '20 at 07:41
  • I would also edit it in the pom.xml so that I have one place where I can change it. In this way you would always have to remember where you placed such annotations. – s33h Aug 24 '22 at 14:13
4

If you are using maven-pmd-plugin, it ignores tests by default. You could configure it to includeTests. See Docs

    <plugin>
        <artifactId>maven-pmd-plugin</artifactId>
        <executions>
            <execution>
                <phase>package</phase>
                    <goals>
                        <goal>check</goal>
                    </goals>
            </execution>
       </executions>
       <configuration>
           <rulesets>
               <ruleset>/my-custom-rules.xml</ruleset>  // Your own rules here
           </rulesets>
           <includeTests>true</includeTests> // Default value is false
       </configuration>
    </plugin>

Regarding the TooManyStaticImports rule, just like any other PMD rule, it can be subjective, and it won't necessarily fit all use cases or code styles. That is why it says "may lead to messy code". There are arguments for and against its usage.

In general, if you want to mute a rule for one class, you could use:

@SuppressWarnings("PMD.TooManyStaticImports")

Alternatively, if you want to fine-tune the rule, you could provide your own custom config e.g.:

<rule ref="category/java/codestyle.xml/TooManyStaticImports">
    <properties>
        <property name="maximumStaticImports" value="6" /> // Default is 4
    </properties>
</rule>
Ahmad Abdelghany
  • 11,983
  • 5
  • 41
  • 36
2

This seems to be an objectionable PMD configuration or rule; especially in testing, this sort of static import is to be expected. The PMD setup should be changed either to disable this inspection entirely or not to apply it to test code.

chrylis -cautiouslyoptimistic-
  • 75,269
  • 21
  • 115
  • 152
0

While this isn't an answer to the question, it does make me wonder if PMD could have an annotation that allows users to override rule configurations in one or more source files or even a package? The only issue is that this would introduce a compile-time dependency. Would it be worth it if introduced at the package-level? Are there any rules with that kind of irritability quotient? Law of Demeter comes to mind.

As for the question itself, use star imports for static imports when you're practically importing the whole class. It's less verbose and less ugly.

Linus Fernandes
  • 498
  • 5
  • 30
  • This does not provide an answer to the question. You can [search for similar questions](//stackoverflow.com/search), or refer to the related and linked questions on the right-hand side of the page to find an answer. If you have a related but different question, [ask a new question](//stackoverflow.com/questions/ask), and include a link to this one to help provide context. See: [Ask questions, get answers, no distractions](//stackoverflow.com/tour) – Das_Geek Nov 19 '19 at 14:53
  • That's great to know . I wanted to add it as a comment but I don't have the points to do that as yet. And I'm not interested in accumulating points just so that I can comment. I deleted this answer because I realized that SuppressWarnings annotation works well enough. There's no real need to add an overridable one. Another option would be to store runs and try and use that as training data to suggest modifying rule parameters. – Linus Fernandes Nov 21 '19 at 00:41
0

To me, it's still better to import all static explicitly and concretely because it loads faster than importing all (*), and one reason is we prefer explicit things in Java.

So, I recommend to disable this PMD rules check.

Cheers.

turong
  • 1,474
  • 9
  • 14
0

You can try importing one level up.
For example, Use Constant.MY_VAR instead of directly using MY_VAR. So you don't have to import each variable separately in Constant, you can just import Constant and access the variable in code with Constant.MY_VAR.

0

Basically this is bad practice, as you refere to static fields from another Class, in a way you would address a static property from the current file. To prevent this PMD error you have to make a qualified call.

Instead of:

BAD_REQUEST;
INTERNAL_SERVER_ERROR;
NOT_FOUND;
UNPROCESSABLE_ENTITY;

call:

HttpStatus.BAD_REQUEST;
HttpStatus.INTERNAL_SERVER_ERROR;
HttpStatus.NOT_FOUND;
HttpStatus.UNPROCESSABLE_ENTITY;