Use String#isEmpty where possible

[blacelle]

Suggestion: Add Cleanthat as Spotless plugin

Like Spotless is used to format automatically the code-base, I suggest activating Cleanthat as a Spotless plugin to apply linting-rules.

The proposed set of mutators is restricted to safe and consensual ones:

• The changes may not be exciting
• We may activate additional mutators (amongst draft or not-consensual ones)
• I would be happy to get feedback about specific linting needs, and make them available through Cleanthat

Cleanthat changelog: CHANGES.MD

Testing done

No tests done. I reviewed the changes, which looks very safe.

Proposed changelog entries

• Entry 1: Activated Cleanthat through Spotless to apply automatically linting rules.

Proposed upgrade guidelines

N/A

Submitter checklist

• The Jira issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
  • Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

?

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[NotMyFault]

Please provide some information about what this plugin is, what it does and most notably, why it should be considered.

Thanks.

[blacelle]

Cleanthat is a spotless-plugin enabling automatic linting of Java code. Automatic linting enables leaner code (similarly to formatting, but extended to semantic changes).

I suggest integrating it in JenkinsCI as jenkinsCI already integrates Spotless, which makes Cleanthat integration very easy.

The initial suggested set of mutators of SafeAndConsensual which applies safe and consensual changes like :

• Keep consistant Modifier Ordering
• Remove unnecessary uses String.toString()
• Use Collection.isEmpty() instead of Collection.size() != 0

[daniel-beck]

On the "pro" side of things, the code changes here look mildly useful and I'm confident we'd integrate them without introduction of Cleanthat into the build.

There are a few things in the "con" column though:

• Your own lack of confidence.
• The lack of justification provided. We have PR templates for a reason.
• Apparent lack of project popularity (both users and contributors). Unfortunately this is something we have to consider in terms of long-term viability of our dependencies. In this case, it appears there are no contributors besides you and GH indicator of user popularity are also fairly low.

Overall this doesn't seem to bring enough benefit to offset the (fairly minor overall, but still) potential drawbacks.

[blacelle]

On the "pro" side of things, the code changes here look mildly useful

I would be happy to activate less consensual mutators (or get feedbacks about mutators yet-to-implement which would fit your needs.

and I'm confident we'd integrate them without introduction of Cleanthat into the build.

The point here is, like for formatting, to get this done automatically on any PR. Not requiring to apply them when you think about them (would the changes be simple or complex).

Your own lack of confidence.

Cleanthat is definitely in early stage, and looking for a broader audience. One project may encounter issues, while others would not. I open this PR after checking by myself Cleanthat runs smoothly in your project.

The lack of justification provided. We have PR templates for a reason.

Sorry for that. I should have comply to the guidelines. I'll update the initial comment ASAP. (Looking into this, fact is the template did not appear in when I open the PR, for any unknown reason).

Apparent lack of project popularity (both users and contributors). Unfortunately this is something we have to consider in terms of long-term viability of our dependencies.

Yes, definitely. However, I'd like to stress-out this is not a Runtime-dependency, but a build-time side-wheel dependency. Would cleanthat project die, and would you decide to drop it from JenkinsCI, you could do that by dropping the lint-option without impacting any users.

Overall this doesn't seem to bring enough benefit to offset the (fairly minor overall, but still) potential drawbacks.

I totally get your train of thoughts. I would be happy to get feedback from relevant linting rules you could expect from such a rule. More mutators are available (see MUTATORS.MD ) and I would be happy to implement some following your specific needs.

[NotMyFault]

While the underlying changes could be considered as code cleanup, but the first release of this plugin happened close to 2 months ago. Hence, I believe it's still in its early stages.
I believe, I could consider an addition at a later point, when more people in real-world projects started to use this plugin, and it offers broader functionality. I don't think the plugin offers enough value to add it to this repository at this point.

[blacelle]

Thanks a lot for the consideration.

when [...] it offers broader functionality

There is various available rules, which I did not activated in this PR for the sake of sticking to consensual changes. Anyway, coming functionalities may appear more interesting (like automatic refactoring, or covering more patterns, or converting more patterns to Stream API).

I don't think the plugin offers enough value to add it to this repository at this point.

If I may ask, do you have specific features related to Cleanthat proposal which may appear useful ? (e.g. converting some code manipulating a Collection to rely on Streams? (e.g. converting for (...) {if (...)} into collection.stream().filter(...)))

[github-actions]

Please take a moment and address the merge conflicts of your pull request. Thanks!

[basil]

The consensus among the Jenkins core maintainers is that we are not interested in adopting this tool. I have retained the String#isEmpty portions of this PR, as they have value as an independent cleanup. I have reverted everything related to Cleanthat. With that, this PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!