Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Code cleanup #3064

Merged
merged 36 commits into from
Apr 3, 2023
Merged

Code cleanup #3064

merged 36 commits into from
Apr 3, 2023

Conversation

jwiesler
Copy link
Contributor

@jwiesler jwiesler commented Mar 12, 2023 •
edited
Loading

This MR does some automated code cleanup.
Lots of changes, I grouped the changes in commits and the review should happen on a per commit basis.

Some of them might be debateable, please do so.
However, I think most of them really help with parsing of and reasoning about the code.

@jwiesler jwiesler self-assigned this Mar 12, 2023
@jwiesler jwiesler marked this pull request as draft March 13, 2023 00:58
@jwiesler
Copy link
Contributor Author

jwiesler commented Mar 13, 2023 •
edited
Loading

This PR already did some good, it resulted in a bug report for IntelliJ.

Edit: Fix is included in Build 232.2162.

@jwiesler jwiesler marked this pull request as ready for review March 15, 2023 22:40
@jwiesler jwiesler enabled auto-merge March 23, 2023 09:20
@jwiesler jwiesler disabled auto-merge March 24, 2023 13:07
@FliegendeWurst FliegendeWurst added the 🛠 Maintenance Code quality and related things w/o functional changes label Mar 24, 2023
@jwiesler jwiesler enabled auto-merge March 25, 2023 23:05
mattulbrich
mattulbrich requested changes Mar 26, 2023
View reviewed changes
Copy link
Member

@mattulbrich mattulbrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the spring clean!

I have a few comments and questions.

key.core/src/main/java/de/uka/ilkd/key/rule/AuxiliaryContractBuilders.java Show resolved Hide resolved
...core/src/main/java/de/uka/ilkd/key/control/instantiation_model/TacletInstantiationModel.java Show resolved Hide resolved
key.core/src/main/java/de/uka/ilkd/key/proof/io/ProofSaver.java Outdated Show resolved Hide resolved
key.core/src/main/java/de/uka/ilkd/key/util/MiscTools.java Outdated Show resolved Hide resolved
key.core/src/main/java/de/uka/ilkd/key/util/MiscTools.java Outdated Show resolved Hide resolved
...e/src/test/java/de/uka/ilkd/key/proof/runallproofs/proofcollection/ForkedTestFileRunner.java Show resolved Hide resolved
...olic_execution/src/main/java/de/uka/ilkd/key/symbolic_execution/AbstractUpdateExtractor.java Show resolved Hide resolved
@mattulbrich
Copy link
Member

Apparently, this PR reshuffles many import statements and puts .* rather than individual imports (or v.v.). Is this not something that should be taken care of automatically in spotless?

@WolframPfeifer @jwiesler @wadoon

@wadoon
Copy link
Member

wadoon commented Mar 26, 2023

Apparently, this PR reshuffles many import statements and puts .* rather than individual imports (or v.v.). Is this not something that should be taken care of automatically in spotless?

@WolframPfeifer @jwiesler @wadoon

Depends on the formatter. It seems that the eclipse formatter (that we currently use) can only reorder the imports.

@jwiesler
Copy link
Contributor Author

jwiesler commented Mar 27, 2023 •
edited
Loading

Depends on the formatter. It seems that the eclipse formatter (that we currently use) can only reorder the imports

I didn't find any option that orders the imports?

@wadoon
Copy link
Member

wadoon commented Mar 30, 2023

I didn't find any option that orders the imports?

I am not sure, whether this is a question or a statement. There seems no documentation on the Eclipse formatting settings (XML) and I am to lazy to install Eclipse to modify them.

@jwiesler
Copy link
Contributor Author

That was a question, I checked the file that loads the configuration and didn't find anything the like.

@wadoon
Copy link
Member

wadoon commented Mar 30, 2023

That was a question, I checked the file that loads the configuration and didn't find anything the like.

You can find some traces but in the end, the simplest thing is to configure the import via the eclipse GUI, and export an XML file.

@jwiesler
Static inner classes and toArray
cc980ff
@jwiesler
String concat in StringBuilder append
385b393
@jwiesler
String concat in StringBuilder append, String append in loop
a81c5a4
@jwiesler
Minor changes
7c7e06d
@jwiesler
Stream api, checks already covered by other checks, weak variable typ…
279f715 
…es leading to casts, bugs
@jwiesler
Fix returned stream
400f219
@jwiesler
Fix macro args generics
8e195c9
@jwiesler
Fix test walk dir
4a2157a
@jwiesler
Enforce proof script argument fields not being final
d2b7b6c
@jwiesler
Use parseInt instead of valueOf
daa21ce
@jwiesler
Fix some critical warnings by removing dead code
3056bfe
@jwiesler
Remove java.util logging
ab9f5ef
@jwiesler
Remove more manual logging
699f7ae
@jwiesler
Improve log output of RunAllProofsFunctional
4ad88f8
@jwiesler
Replace printStackTrace with logging
97de68f
@jwiesler
Minor changes
7c23bcb
@jwiesler
Obsolete collection types
4343ae4
@jwiesler
Code style
a2c926d
@jwiesler
Use slf4j.detectLoggerNameMismatch and fix issues
5af6617
@jwiesler
Fix exception cause being dropped in MiscTools::extractURI
476e31e
@jwiesler
Wrong logger class parameter used
d1d9fbf
@jwiesler
Spotless
b903cf2
@mattulbrich
Copy link
Member

Since there is a (partial?) solution to the import problem, this should be cleared after a merge with main.

@jwiesler jwiesler added this pull request to the merge queue Apr 3, 2023
Merged via the queue into KeYProject:main with commit d0047a4 Apr 3, 2023
@jwiesler jwiesler deleted the cleanup branch April 3, 2023 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattulbrich mattulbrich mattulbrich approved these changes

Assignees

@jwiesler jwiesler

Labels
🛠 Maintenance Code quality and related things w/o functional changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jwiesler @mattulbrich @wadoon @FliegendeWurst