Code cleanup in progress

[hazendaz]

I ran a full code cleanup via Eclipse 2021-12 a while back but had not gotten around to bringing it in. Making many passes trying to break this up in more easily read passes. We can debate finer points later. I have not fully checked if any other items can come in and/or adjustments to the detection as of yet but will be doing so afterwards. Main point here was to get this code off my machine and moving so I can follow up with better caching fix.

[hazendaz]

In Eclipse 2021-12, the code cleanup for java is set true for 109 of 114. Only extra plugin in there is jautodoc so that will differ slightly from stock Eclipse. What I skipped was teh 'source fixing' (3) as more experimental with warnings, sort members, and jautodoc header given we already license header. With recent Eclipse versions, due to some of the rewriting, it takes a couple of passes to fully complete. I'm basically completed at this point. Please discuss any concerns or issues here and things to look at.

I'll start.

Yes some things are now a bit more verbose but its clear to intent (static vs instance). Finals are in place to prevent changes of data which is already a problem in numerous spots. Static usage in tests are usually highly contested. I don't personally use them at all if there are very few to start with which ironically is most of the time. Eclipse on that one unless I'm missing something dislikes them.

[ctubbsii]

Some of these changes are good, but some of them are actually quite irritating, and make the code substantially worse. They also touched nearly everything and made it impossible to review to see if any mistakes were made in the process. Even if they were all done entirely by IDE auto-refactoring, that's no guarantee they are good. I've seen Eclipse's IDE auto-refactoring make mistakes before.

Some things that I think are quite excessive that I think should be reverted:

1. The overuse of var everywhere. var should be used much more conservatively, and only where it improves the readability of the code. Even the developers who created it agree on that point. A good example: var shouldn't be used in place of int. It makes the code less readable and literally has no benefit whatsoever... it's not even fewer bytes.
2. The forced use of javadocs everywhere, even on trivial methods and fields that aren't improved by adding javadocs. It would be better to have less noise in the files in many cases, rather than to have lines and lines and lines to scroll past to get to the actual code.
3. Static imports for some things are very helpful in ensuring the code is readable. assert* methods, StandardCharsets, and EasyMock methods are just a few examples.

I kinda gave up looking at most of the rest of the changes in detail. There was just way too much. Just because an IDE has a button, it doesn't mean it should be used. And, even if it should be used, it doesn't mean it should always be used.

Honestly, some of these changes make me want to fork this plugin again, and start over. I'm sure I can get used to some of them in time, but others are going to irritate me every time I look at the code. I think you went a bit too overboard here. You should have discussed some of these changes and tried to get feedback and/or consensus before turning the knobs all the way up, and pushing them in.

[hazendaz]

Guess you would really dislike working with me in the office. I force it all on 800+ repos with bots doing most of the work. Its 50/50. Some love it, some hate it. Its all perspective.

[hazendaz]

and yes agree Eclipse has made mistakes before. Only recently though that I've ever seen. I've been using Eclipse since 2008. The last few releases in 2021 before 12 had a lot of issues - broke code - etc. But that is all fixed now. The ones they have marked as be super careful are ones of concern. It did javadocs when I ran the souce cleanup so I replayed those separately on its own commit. The others were mainly for me reviewing to make sure I could review each PR and see no mistakes there. Not sure I totally agree on var, every other language has it. The right hand side already tells us what it is so its not making it less readable IMHO. Its just new and takes some time getting use to seeing. Its just really diamon operation in the reverse. ANd eclipse did leave the vaste majority of them alone. Only the clear ones - and of course the code I wrote that was for jsoup I didn't use it at all but could have. Its not ingrained in my mind set yet. One day it will be and was part of the reasont to run the scan here. There is a lot this repo needs. Its missing a lot of CI stuff in general. Lacks proper integration tests. Etc. To get us there, we have to work together. This repo has significantly improved since it came from google code when neither of us were involved. Just a little give and take and discussion. And anything can be adjusted as we go. As I noted, I'll be in this code here for probably most of the week and will be loooking at everyhing in it including fixing issues rom sonar. Some methods are too, too big. But I always feel like there is this want to break this into dozens of plugins (exagerating) but when I have OSS base parents and ones at work that are 100s of plugins deep, personally don't want more when so many overlap for same feature set. Like all formatters IMHO should be part of some overall plugin not just for each file type. I don't think that is exactly what you mean with making them separate mojos but feel it wasn't liked that json, html, and css joined into this. Those are vital and used at least at my job, hense tryign to get jsoup to fix their massive amount of bugs and no other solution that works well for that (that I'm aware of).

[ctubbsii]

I don't remember the specific error, but I saw errors with automated refactoring recently. Also, I can't use 2021-12 because of https://bugs.eclipse.org/bugs/show_bug.cgi?id=574425 . But, in general, it's pretty dangerous thinking that Eclipse has fixed all its errors in automated refactoring. Regardless of how the changes were made, the patch should be subject to review.

Besides, that's not the point. The point is that this is a community project, and your voice is one of many. What you pushed was way too disruptive. You should not have imposed your personal preferences on this project like that without first attempting to get feedback/consensus. The quantity makes it difficult to review. The imposition to cater to one person's preferences is impolite. We should be working together. If this was a personal project, it would live in your personal space on GitHub, not here. When I volunteered to take over hosting it here, I did so under the premise that it would be a collaborative effort. All I'm asking is that you slow your pace, and give chance for community feedback and consensus before imposing personal preferences, especially ones that are so massively disruptive.