[Discussion] Reconsider the use of `assert` for input validation in our codebase

[zhangt2333]

Background

Java's assert keyword has specific intended use cases and runtime behavior that may not align with our current usage patterns in this repository.

Java assert Conventions and Behavior

According to Java best practices and Oracle documentation:

• Intended use: Internal invariant checking, debugging, and development-time verification
• Runtime behavior: Assertions are disabled by default and require explicit JVM flags (-ea/-enableassertions) to activate
• Purpose: Help developers catch bugs during development and testing, not for production error handling
• Expected outcome: When disabled (default), assert statements are completely ignored and have zero performance impact

Current Issues in Our Codebase

I've identified several instances where we're using assert for input validation and precondition checking in public APIs. This approach has several significant drawbacks:

1. Inconsistent behavior: Same input produces different results depending on JVM assertion settings
2. Silent failures: Invalid inputs may be silently accepted when assertions are disabled (default behavior)
3. Unclear error handling: Users don't know what exceptions to expect from our APIs
4. Testing gaps: Our tests might pass in development (with -ea) but fail silently in production

Examples in Our Codebase

https://github.com/pascal-lab/Tai-e/blob/12e7c432c16fd5db0aa66aae5f6af2aa1bc96077/src/main/java/pascal/taie/util/collection/AbstractHybridSet.java#L167-L169

https://github.com/pascal-lab/Tai-e/blob/12e7c432c16fd5db0aa66aae5f6af2aa1bc96077/src/main/java/pascal/taie/analysis/bugfinder/nullpointer/IsNullConditionDecision.java#L42-L47

https://github.com/pascal-lab/Tai-e/blob/12e7c432c16fd5db0aa66aae5f6af2aa1bc96077/src/main/java/pascal/taie/ir/exp/BitwiseExp.java#L58-L62

https://github.com/pascal-lab/Tai-e/blob/12e7c432c16fd5db0aa66aae5f6af2aa1bc96077/src/main/java/pascal/taie/analysis/pta/plugin/reflection/MetaObjHelper.java#L141-L147

... more than 50 assert statements.

Proposed Solution

I suggest we replace assertion-based validation with explicit exception throwing:

Before (Current Approach)

public void processData(String input) {
    assert input != null : "Input cannot be null";
    assert !input.isEmpty() : "Input cannot be empty";
    // processing logic
}

After (Recommended Approach)

public void processData(String input) {
    if (input == null) {
        throw new IllegalArgumentException("Input cannot be null");
    }
    if (input.isEmpty()) {
        throw new IllegalArgumentException("Input cannot be empty");
    }
    // processing logic
}

Alternative: Utility Methods

public void processData(String input) {
    Objects.requireNonNull(input, "Input cannot be null");
    if (input.isEmpty()) {
        throw new IllegalArgumentException("Input cannot be empty");
    }
    // processing logic
}

Real-World Best Practices

• Spring Framework: Created its own org.springframework.util.Assert utility class that throws explicit exceptions (IllegalArgumentException, IllegalStateException) instead of using native assert keyword

• Google Guava: Developed Preconditions class with methods like checkArgument() and checkState() that throw IllegalArgumentException and IllegalStateException respectively, completely avoiding the native assert keyword

• Apache Commons: Provides Validate.isTrue() and similar methods that throw IllegalArgumentException for input validation, explicitly designed for cases where assertions would be inappropriate due to their ability to be disabled at runtime

Benefits

• ✅ Consistent behavior regardless of JVM settings
• ✅ Clear API contracts with documented exceptions
• ✅ Better error messages for library users
• ✅ Proper separation between debugging aids and production error handling

Discussion Points

• Should we establish coding guidelines or checkstyle rules about when to use assert vs explicit validation?
• Do we want to introduce a utility class (like Guava's Preconditions) for common validations?
• Are there any performance-critical paths where we might want to keep assertions for internal checks?