[CALCITE-6552] Enable CheckerFramework in 'server' module

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.38.0
• Labels: pull-request-available
• Assignee: Julian Hyde
• Reporter: Julian Hyde
• Created: 28/Aug/24 16:37
• Updated: 17/Oct/24 22:41
• Resolved: 17/Oct/24 22:41

Description

In CALCITE-4199 we enabled CheckerFramework (via annotations and CI jobs) in the 'core' and 'linq4j' modules; this change further enables CheckerFramework in the 'server' module, and fixes all violations.

There are also a large number of 'cosmetic' modifications to improve code quality without changing behavior, including:

• Replace 'this.x = x; assert x != null;' by 'this.x = requireNonNull(x);' in constructors
• Replace assert in other code locations where it implements an invariant. We don't use requireNonNull because it throws NullPointerException; we would prefer to throw AssertionError or IllegalStateException
• Replace x.equals("") and x.size() == 0 with x.isEmpty()
• Make fields final where possible
• Make private methods and inner classes static where possible
• In class Pair<K, V> make the type variables K and V no longer @Nullable by default (you can make each of them nullable if you need)

Issue Links

• is related to: CALCITE-4199 Add nullability annotations to the methods and fields, ensure consistency with checkerframework
• is related to: CALCITE-6553 In server, several CREATE and DROP commands are incorrect if schema does not exist
• relates to: CALCITE-4524 Make some fields non-nullable (SqlSelect.selectList, DataContext.getTypeFactory)
• relates to: CALCITE-6408 Not-null ThreadLocal
• links to: GitHub Pull Request #3939