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

New bug type: AA_ASSERTION_OF_ARGUMENTS #2125

Merged
merged 20 commits into from Jul 4, 2023

Conversation

baloghadamsoftware
Copy link
Contributor

@baloghadamsoftware baloghadamsoftware commented Jul 29, 2022

Using assertions to validate arguments of public functions is a bad and dangerous practice because the validation does not happen if assertions are disabled, thus the behavior of the program depends on this compilation option. Therefore, we introduced a new detector FindArgumentAssertions to detect such cases and report AA_ASSERTION_OFR_ARGUMENTS for each of them.


Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

Co-authored-by: Gábor Kutas <@vodorok>

Using assertions to validate arguments of public functions is a bad and dangerous practice because the validation does not happen if assertions are disabled, thus the behavior of the program depends on this compilation option. Therefore, we introduced a new detector `FindArgumentAssertions`  to detect such cases and report `AA_ASSERTION_OFR_ARGUMENTS` for each of them.

Co-authored-by: Gábor Kutas <@vodorok>
@iloveeclipse
Copy link
Member

I don't understand why assert for arguments is bad but assert for anything else is not.

I strongly believe that assert was the worst invention in Java and should be flagged as "BAD_PRACTICE" independently in which piece of code it is used.

@baloghadamsoftware
Copy link
Contributor Author

Thank you for you comment, @iloveeclipse!

Actually, I believe that assert was introduced for the same reason that it exsits in C and C++: to add invariants to some points of the code. This serves two different purposes: it is a comment to the programmer and it is checked in runtime during testing, when assertions are enabled. For example, in private methods you can validate the arguments using assertions, thus in these cases assertions define the precondition of the method. Also it can define the postcondition at the end of the method. However, in public methods we cannot ensure that the caller invokes the method with the correct arguments thus explicit checking must be made or in simple cases we also can make use of annotations such as @NonNull or @Nullable.

@baloghadamsoftware
Copy link
Contributor Author

Could someone take a look on this one, please? @ThrawnCA or @KengoTODA?

Fix FP in Argument Assert (MET01)
baloghadamsoftware and others added 3 commits June 15, 2023 09:29
# Conflicts:
#	CHANGELOG.md
#	spotbugs/etc/findbugs.xml
#	spotbugs/etc/messages.xml
@hazendaz
Copy link
Member

hazendaz commented Jul 4, 2023

Thanks @baloghadamsoftware

@hazendaz hazendaz merged commit 0cc1110 into spotbugs:master Jul 4, 2023
4 checks passed
@hazendaz hazendaz self-assigned this Jul 4, 2023
@hazendaz hazendaz added this to the SpotBugs 4.8.0 milestone Dec 18, 2023
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