Apply Java 17 automatic migration: records #3746
Conversation
Baltoli
requested review from
radumereuta,
dwightguth,
Scott-Guest and
Robertorosmaninho
There was a problem hiding this comment.
I skimmed through all the commits, and LGTM!
I like all the small fixes from IntelliJ - we should consider using Qodana (https://www.jetbrains.com/qodana/) to enforce these on CI.
haskell-backend/src/main/java/org/kframework/backend/haskell/HaskellRewriter.java
Show resolved
Hide resolved
kernel/src/main/java/org/kframework/compile/checks/CheckSortTopUniqueness.java
Show resolved
Hide resolved
There was a problem hiding this comment.
My main comments are that it looks like the automated transformations removed final and static from the classes that were made into records. I know records are automatically final, so that part is fine, but I'm less certain that inner records are automatically static. If you can find evidence that they are, ignore all those comments, otherwise, the code probably needs to be audited to add the static keyword back in and make sure I didn't miss any places where there were inner records added.
kernel/src/main/java/org/kframework/parser/inner/kernel/EarleyParser.java
Show resolved
Hide resolved
kernel/src/main/java/org/kframework/parser/inner/kernel/EarleyParser.java
Show resolved
Hide resolved
|
IntelliJ explicitly prompts for the removal of |
|
I have addressed and resolved all the comments left on this PR; none of them required any further changes to the code and so I'm going to merge the PR once CI passes with the merge conflicts resolved. |
This PR applies all automatic cleanups that IntelliJ is confident enough to apply silently with no further user input; a rough summary of the different kinds of change are: * Removal of redundant casts (`T t = (T) v` where `v` already has type `T`). * Removal of unnecessary calls to `toString()` * Insertion of `final` where possible * Collapsing of redundant boolean conditions `if (cond) return true else return false` * Syntactic fixes (`;;` etc.) * Use of `StandardCharsets.UTF_8` to simplify exception checks when encoding strings * Removal of redundant explicit key checks when removing values from hashmaps As ever, I'm happy to bikeshed these changes - I can redo the analysis and disable some subset of the checks if anyone has objections to the specific results we get. ~~To avoid merge conflicts, this PR is based on #3746 and should only be merged when that PR is merged.~~ --------- Co-authored-by: rv-jenkins <[email protected]>
This PR applies automatic fixes suggested by IntelliJ to refactor Java
classes intorecords, which reduces boilerplate code in a lot of different places.The changes made are large, but I have split the changes up so that each commit deals only with a single class at a time, along with any downstream changes required by making that class a record. In some cases, I have applied small fix-its suggested by IntelliJ as well as the record transformation.
I'm happy to bikeshed whether any of these cases should remain a class rather than a record, and the commit history means that reverting an individual case is easy should we decide to do so.