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.

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

Issue a warning when final field is overwritten #414 #420

Closed
wants to merge 2 commits into from
Closed

Conversation

alamar
Copy link
Contributor

@alamar alamar commented Mar 23, 2022

@alamar alamar marked this pull request as draft April 18, 2022 11:49
@alamar
Copy link
Contributor Author

alamar commented Apr 18, 2022

Turns out we have more than 10 tests which do exactly that - overwrite final fields.

What should we do - should we update them all?

@rogersimmons @RobAustin

@JerryShea
Copy link
Contributor

What should we do - should we update them all?

IMO yes we should - our code should set a good example

@rogersimmons
Copy link
Contributor

+1. Yes, please fix any tests which overwrite final fields.

@alamar alamar force-pushed the Issue414 branch 2 times, most recently from db92898 to 828b5e4 Compare April 22, 2022 13:07
Ilya Kaznacheev added 2 commits April 22, 2022 17:56
De-finalize marshallable fields to remove warnings

closes #414
@sonarcloud
Copy link

sonarcloud bot commented Apr 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@@ -641,6 +650,14 @@ protected void readValue(Object o, Object defaults, ValueIn read, boolean overwr
}
}

protected void triggerFinalWarning() {
Copy link
Contributor

@minborg minborg Apr 25, 2022

Choose a reason for hiding this comment

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

Suggest assertNonFinal(). In this version, there will be a warning but in later, an Exception would be thrown.

final List<Integer> numbers = new ArrayList<>();
final Map<String, String> map1 = new LinkedHashMap<>();
final Map<String, Double> map2 = new LinkedHashMap<>();
Set<String> words = new LinkedHashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't mutable objects be final?

@peter-lawrey
Copy link
Member

It's important to recognise how Java Serialization does this currently, as we can't change that behaviour. Java Serialization doesn't produce a warning when setting final fields, nor does it expect you to make a field that can be set non-final

@alamar alamar closed this Apr 21, 2023
@alamar alamar deleted the Issue414 branch April 21, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants