Add PMD to core

[Pankraz76]

No description provided.

[quarkus-bot]

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

• description should not be empty, describe your intent or provide links to the issues this PR is fixing (using Fixes #NNNNN) or changelogs

_{This message is automatically generated by a bot.}

[geoand]

These are not changes we want

[Pankraz76]

[WARNING] PMD Failure: io.quarkus.logging.Log:27 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'java.lang': 'ClassNotFoundException' is already in scope because it is declared in java.lang.
[WARNING] PMD Failure: io.quarkus.runtime.ApplicationLifecycleManager:487 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'ApplicationLifecycleManager': 'QUARKUS_APPCDS_GENERATE_PROP' is already in scope because it is declared in an enclosing type.
[WARNING] PMD Failure: io.quarkus.runtime.BlockingOperationControl:8 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'BlockingOperationControl': 'ioThreadDetectors' is already in scope because it is declared in an enclosing type.
[WARNING] PMD Failure: io.quarkus.runtime.LaunchMode:46 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'LaunchMode': 'NORMAL' is already in scope because it is declared in an enclosing type.
[WARNING] PMD Failure: io.quarkus.runtime.configuration.NameIterator:175 Rule:EmptyControlStatement Priority:3 Empty if statement.
[WARNING] PMD Failure: io.quarkus.runtime.configuration.NameIterator:187 Rule:EmptyControlStatement Priority:3 Empty if statement.
[WARNING] PMD Failure: io/quarkus/runtime/configuration/QuarkusConfigBuilderCustomizer.java:63 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'io.smallrye.config': 'NameIterator' is already in scope because it is imported in this file.
[WARNING] PMD Failure: io/quarkus/runtime/configuration/QuarkusConfigBuilderCustomizer.java:63 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'io.smallrye.config': 'NameIterator' is already in scope because it is imported in this file.
[WARNING] PMD Failure: io/quarkus/runtime/configuration/QuarkusConfigBuilderCustomizer.java:68 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'io.smallrye.config': 'NameIterator' is already in scope because it is imported in this file.
[WARNING] PMD Failure: io.quarkus.runtime.logging.DecorateStackUtil:16 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'DecorateStackUtil': 'getDecoratedString' is already in scope.
[WARNING] PMD Failure: io.quarkus.runtime.logging.DecorateStackUtil:27 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'DecorateStackUtil': 'getDecoratedString' is already in scope.
[WARNING] PMD Failure: io.quarkus.runtime.logging.DecorateStackUtil:44 Rule:UnnecessaryFullyQualifiedName Priority:4 Unnecessary qualifier 'DecorateStackUtil': 'getRelatedLinesInSource' is already in scope.
[WARNING] PMD Failure: io.quarkus.runtime.logging.DecorateStackUtil:49 Rule:EmptyCatchBlock Priority:3 Avoid empty catch blocks.
[WARNING] PMD Failure: io.quarkus.runtime.logging.LogCleanupFilter:33 Rule:CollapsibleIfStatements Priority:3 This if statement could be combined with its parent.
[WARNING] PMD Failure: io.quarkus.runtime.metrics.MetricsFactory:16 Rule:UnnecessaryModifier Priority:3 Unnecessary modifier 'final' on field 'MP_METRICS': the field is declared in an interface type.
[WARNING] PMD Failure: io.quarkus.runtime.metrics.MetricsFactory:19 Rule:UnnecessaryModifier Priority:3 Unnecessary modifier 'final' on field 'MICROMETER': the field is declared in an interface type.
[WARNING] PMD Failure: io.quarkus.runtime.metrics.MetricsFactory:43 Rule:UnnecessarySemicolon Priority:3 Unnecessary semicolon.
[WARNING] PMD Failure: io.quarkus.runtime.types.GenericArrayTypeImpl:29 Rule:UselessParentheses Priority:4 Useless parentheses..

[Pankraz76]

PMD 7.7.0 has found 18 violations

[Pankraz76]

@geoand

What specifically don't you like about it? Is it too much, or is there something else going on? I applied the Boy Scout principle, cleaning up code wherever I found issues.

Do you guys care about best practices? If so, PMD is a great tool to help enforce them and prevent bugs or violations from the start, rather than letting technical debt accumulate.

Let me know what I can do to make this acceptable for you. And please don't shoot the messenger!

[cescoffier]

We tend to avoid static code analysis in Quarkus. Most of them are made for application developers, and it can be hard to tune them for frameworks with slightly more complex and low-level code.

If I look at the list of violations, none is critical.
First, the usage of fully qualified names is often made on purpose. It improves code review (as you know immediately which class you are talking about, especially when a class is defined multiple times (yes... it happens).
Regarding empty statements, the tool seems to ignore the comments in these blocks. Again, having these is about readability and maintenance. The same goes for the empty catch block (there is a comment - so it's not empty). The compiler will optimize for us - so it's a win/win.

The necessary usage of the final keyword is a question of taste. When you are reviewing a PR, you may not have the interface/class declaration - so, you may not know immediately if you are in an interface (where final can be omitted) or in a class (where final must not be omitted).

Same thing about parenthesis - in general, there are ways to improve the understandability of complex expressions.

So, basically, we would have only one violation in the list: the extra semi-colon (which I believe is a typo).

Quarkus codebase is quite large and somewhat complex. People writing and maintaining the code are not necessarily the same (and even where it is, you are always happy to find some explanations from pour previous-self). Thus, we keep "extra hints" in the code to improve understandability. It's often not an oversight, but a deliberate decision.

[maxandersen]

We tend to avoid static code analysis in Quarkus. Most of them are made for application developers, and it can be hard to tune them for frameworks with slightly more complex and low-level code.

+1 and just to make it clear - tools that actually can be tuned and run well on Quarkus projects we definitely do try and utilize. i.e. we have the importsort, code formatting and a few more.

If anyone interested in opening PR's on adding things please put in description what is being suggested, what the tool is doing and how its going to help improve Quarkus - and no, "have less warnings when running this tool" is NOT a valid reason. It should be because it solves and actual problem that helps us and/or users in way that is not super costly (i.e. getting to deal with lots of meaningless false positives over things that are trivially fixed otherwise).

[Pankraz76]

• [POC-FIX-PMD] maven-pmd-plugin: reactivate PMD - Best practices apache/maven#2331

writing and maintaining

how should they work together if not with norma and conventions ?

I think its called best practices for a reason. Having unused obsolete stuff is never a benefit. Not in application server or whatever you want to call it.

https://pmd.github.io/pmd/pmd_rules_java_bestpractices.html

[Pankraz76]

Its just bad code not begin fixed and not being reviews properly. Humans can never be as exact as PMD.