Bump PMD with fixes #25
Conversation
brinxmat
commented
Sep 26, 2024
brinxmat
commented
- Bumps PMD to a version that actually works with Java 21
- Fixes the errors (some fixes may be ignorant/heavy handed, so do check
- Removes checkstyle errors
- Fixes a minor antipattern
| </rule> | ||
|
|
||
| <rule ref="category/java/multithreading.xml"> | ||
| <!-- Because it complains for the Map interface in the Resource class.--> | ||
| <!-- Because it complains about the Map interface in the Resource class.--> |
There was a problem hiding this comment.
We share Pub-API's PMD spec, but this isn't relevant for us…weird. Probably need to decide what to do about "Standard" and local colour in another meeting.
| @@ -24,10 +23,10 @@ public class ApiData { | |||
|
|
|||
| private final RestApi awsRestApi; | |||
| private final OpenAPI openapiApiGateway; | |||
| private Optional<OpenAPI> openapiApiGithub; | |||
There was a problem hiding this comment.
Optional fields are a no-no, I replaced this with a private getter…it serves little real purpose aside hiding null checks. I suspect we simply have a suboptimal design here (typically, we see this kind of thing when the OO is off), which should probably be rectified.
| @@ -93,27 +95,29 @@ public ApiData setEmptySchemasIfNull() { | |||
| } | |||
|
|
|||
| public ApiData overridePropsFromGithub() { | |||
| if (this.openapiApiGithub.isPresent()) { | |||
| var openApiGithub = getOpenapiApiGithub(); | |||
There was a problem hiding this comment.
It is here, it really adds nothing. Probably, there is a missing object.
| @@ -118,8 +117,8 @@ private void removeTags(OpenAPI api) { | |||
|
|
|||
| private void mergeSecurity(OpenAPI api) { | |||
| if (nonNull(api.getSecurity())) { | |||
| for (SecurityRequirement securityRequirement : api.getSecurity()) { | |||
| logger.info("adding security req {}", securityRequirement.toString()); | |||
| for (var securityRequirement : api.getSecurity()) { | |||
There was a problem hiding this comment.
This was a cheap trick to avoid the prescription to use the interface, which here would have been quite harmful. It is not just us that makes things difficult on ourselves.
| } | ||
|
|
||
| public ApiData setEmptySchemasIfNull() { | ||
| public ApiData applyEmptySchemasIfNull() { |
There was a problem hiding this comment.
Setters traditionally return void. I agree strongly with this pattern.
