Fix new PMD warnings

[stoecker]

After PMD upgrade a bunch of new warnings cam up.

Anyone time to help fixing them?

[gaben]

Anyone time..?

I don't have any at the moment, but hopefully I will have some in the following weeks.

[GerdP]

Sorry, the recent changes caused another build failure in my Eclipse installation, so atm I try to fix that.

[stoecker]

I found that an "svn cleanup --remove-ignored" helped. I had to do this for the CI to work.

[stoecker]

Also note, that while I tried to keep downwards compatibility intact I may have created some places where Java >= 21 is required. I don't plan to actively search such issues as long as the JOSM server stuff for the needed builds does still work with Java 11.

[GerdP]

My problems with Eclipse are fixed. No idea what exactly was wrong, it used an older version of the compression library.

[GerdP]

Where do I see the bunch of PMD warnings?

[GerdP]

Ah, didn't use svn update today. Now I see when running ant pmd

[stoecker]

Replying to GerdP:

My problems with Eclipse are fixed. No idea what exactly was wrong

That's the reason why I don't use eclipse. That "No idea why" happened too often to me...

[GerdP]

I can take care about the "Public member ... declared in a non-public type" messages. My understanding is that I just have to remove the public key word.

[stoecker]

Replying to GerdP:

I can take care about the "Public member ... declared in a non-public type" messages. My understanding is that I just have to remove the public key word.

Or change to "protected"? I'm not totally sure. Had only a short look at the PMD description.

[GerdP]

In ​https://github.com/pmd/pmd/pull/6231 I see that either public should be removed or private should be added.
Anyhow there seem to be more to this because when I change the flagged places in src\org\openstreetmap\josm\data\coor\Coordinate.java I can no longer compile. Not yet sure if this is a PMD error or if I have to change more code.

[stoecker]

In 19520/josm:

ignore rule PublicMemberInNonPublicType, see #24635

[stoecker]

In 19535/josm:

remove a lot of PMD warnings, especially outdated ignores, see #24635

[stoecker]

In 19536/josm:

remove PMD ImplicitFunctionalInterface, see #24635

[stoecker]

In 19537/josm:

fix one more PMD, see #24635

[stoecker]

In 19538/josm:

fix one more PMD, see #24635, drop java.util.Date in SyncEditorLayerIndex

[stoecker]

Only 2 types missing:

• ReplaceJavaUtilDate
• OverrideBothEqualsAndHashCodeOnComparable

[stoecker]

In 19539/josm:

see #24635 - remove usage of Date

[stoecker]

In 19541/josm:

remove outdated expiry data check, see #24635

[stoecker]

Milestone renamed

[gaben]

Are core plugins in the scope?

[gaben]

So as you see I used PMD 7.27 (not updated in Ant and Ivy yet), but with the current 7.22 the config change not causing anything meaningful change, maybe one more false positive.

PMD config: I removed the obsolete excludes that are no longer in version 7.x. I also removed e.g. the InefficientEmptyStringCheck performance rule exclusion, since Java 11+ has the isBlank() method, and a few more, which do not report violations on the trunk version.

What remains that I don't like is the rule ExhaustiveSwitchHasDefault; it's a bit noisy in my opinion. There are also a few OverrideBothEqualsAndHashCodeOnComparable.

[stoecker]

What remains that I don't like

Disable it? :-)

[gaben]

Will we have a release this month? If not, I would like to start applying the fixes for the warnings and update the config/upgrade PMD to latest version.

[stoecker]

Actually I planned to have a release. But it seems I have no time. In any case don't delay any changes.

[gaben]

In 19628/josm:

see #24635 - update PMD to 7.28.0 and re-enable several rules, remove deprecated and not reporting ones

[gaben]

In 19629/josm:

see #24635 - fix PMD violations

• use String.isBlank() instead of trim().isEmpty(),
• method references,
• EnumMap for enum keys
• drop redundant initializers/toString() calls and unused annotations,
• remove unused private getDefensiveDate()
• use an explicit charset for Content-length

[gaben]

In 19630/josm:

see #24635 - drop protected modifier on nested types of final classes

[gaben]

In 19631/josm:

see #24635 - re-enable SimplifiedTernary rule and fix the warning

[gaben]

In 19632/josm:

see #24635 - fix one more SimplifyConditional violation

[stoecker]

In 19633/josm:

see #24635 - disable wrong warnings

[gaben]

I don't see UnusedPrivateMethod on PMD 7.28.0. I updated it in r19628. Are you using this version?

In fact, there are two new UnnecessaryWarningSuppression.

[stoecker]

In 19635/josm:

see #24635 - fix some PMD warnings

[stoecker]

Ah. Seems GitHub runs another version? ​https://github.com/JOSM/josm/actions/workflows/pmd-analysis.yml

[stoecker]

Well, It's running 7.28 as well...

But it seems the results differ.

[stoecker]

Wonderful, another one in last change: GitHub PMD complains about "@SuppressWarnings" and "ant pmd" issues "CloseResource" for MapboxVectorCachedTileLoader.java

Can we get that unified?

[stoecker]

In 19636/josm:

readd suppress, see #24635

[gaben]

So ant pmd includes the script/ folder, while maven pmd:pmd does not. I used the latter one, CI uses ant pmd for the PMD action.

However, this still does not explain the issue. I also suspect that there are only 10 errors displayed on GitHub due to some kind of limitation.

Can we get that unified?

Sure. I'll look into that as well besides ticket:24234#comment:53, after I finished the Java 17 migration locally, if that's okay :) Already made changes in over 600 files summed and over 10 branches. Mostly mechanical replacements.