From ae1ebb312160d60af95fcc3d8e01b0546a2d761f Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Wed, 26 Aug 2026 11:05:50 +0300 Subject: [PATCH] chore: Drop obsolete NullAway workarounds NullAway reads @Nullable off record components, so GeolocationOptions uses a compact constructor instead of a spelled-out canonical one. @Nullable on the declared type is enough for NullAway to infer the constructor and generic method type arguments, so the repeated type arguments and explicit type witnesses in the signal tests are gone. DESIGN_GUIDELINES.md described both patterns as requirements. --- DESIGN_GUIDELINES.md | 12 +++++----- .../geolocation/GeolocationOptions.java | 16 +++----------- .../flow/signals/impl/ComputedSignalTest.java | 22 +++++++++---------- .../flow/signals/local/ValueSignalTest.java | 9 +++----- 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/DESIGN_GUIDELINES.md b/DESIGN_GUIDELINES.md index d4a9175f667..d6edb1854e7 100644 --- a/DESIGN_GUIDELINES.md +++ b/DESIGN_GUIDELINES.md @@ -50,9 +50,9 @@ is a DX hazard: for example, `setGeolocationAvailability` lives on For tunable knobs (accuracy, timeout, cache age), prefer an immutable Java record over a long parameter list: -- Public canonical constructor with `@Nullable` params and validation - (reject negative durations, etc.) — NullAway needs the canonical ctor - spelled out explicitly rather than the record-derived one. +- Compact constructor with validation (reject negative durations, etc.). + NullAway reads `@Nullable` off the record components, so there is no + need to spell out the full canonical constructor. - Builder for ergonomics. Offer both `Duration` and `int`-ms overloads on time-related setters where the wire format is ms; applications get a fluent `Duration` API and the record stores the int. @@ -148,9 +148,9 @@ public sealed interface Foo permits FooA, FooB, FooC {} fields may be `@Nullable` because the wire format permits omissions. Keep the wire record private and translate to a non-null public shape at the boundary. -- NullAway requires `@Nullable` on constructor type arguments for - nullable-parameterised signals: - `new ValueSignal<@Nullable X>(null)`. +- `@Nullable` belongs on the declared type — `ValueSignal<@Nullable X>` — + and NullAway infers it for the constructor, so `new ValueSignal<>(null)` + and `Signal.cached(...)` need no repeated type argument or type witness. ### Lifecycle and cleanup diff --git a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationOptions.java b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationOptions.java index b40304c65d6..123b2922ef8 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationOptions.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/geolocation/GeolocationOptions.java @@ -75,20 +75,13 @@ public record GeolocationOptions(@Nullable Boolean enableHighAccuracy, @Nullable Integer maximumAge) implements Serializable { /** - * Canonical constructor. Rejects negative {@code timeout} and - * {@code maximumAge} values — both must be non-negative or {@code null}. + * Rejects negative {@code timeout} and {@code maximumAge} values — both + * must be non-negative or {@code null}. * - * @param enableHighAccuracy - * see the record component - * @param timeout - * see the record component - * @param maximumAge - * see the record component * @throws IllegalArgumentException * if {@code timeout} or {@code maximumAge} is negative */ - public GeolocationOptions(@Nullable Boolean enableHighAccuracy, - @Nullable Integer timeout, @Nullable Integer maximumAge) { + public GeolocationOptions { if (timeout != null && timeout < 0) { throw new IllegalArgumentException( "timeout must be non-negative, was " + timeout); @@ -97,9 +90,6 @@ public GeolocationOptions(@Nullable Boolean enableHighAccuracy, throw new IllegalArgumentException( "maximumAge must be non-negative, was " + maximumAge); } - this.enableHighAccuracy = enableHighAccuracy; - this.timeout = timeout; - this.maximumAge = maximumAge; } /** diff --git a/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java b/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java index 1e13fad0bfe..97821746a3c 100644 --- a/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java @@ -179,12 +179,11 @@ void cached_constantCallback_throws() { void cached_constantCallback_runOnceAndConstantSignalValue() { var dependency = createDependency(); AtomicInteger count = new AtomicInteger(); - Signal<@Nullable Object> signal = Signal - .<@Nullable Object> cached(() -> { - dependency.get(); - count.incrementAndGet(); - return null; - }); + Signal<@Nullable Object> signal = Signal.cached(() -> { + dependency.get(); + count.incrementAndGet(); + return null; + }); assertNull(signal.peek()); assertEquals(1, count.intValue()); @@ -334,12 +333,11 @@ void callback_updateOtherSignal_signalUpdated() { var dependency = createDependency(); SharedValueSignal other = new SharedValueSignal<>("value"); - Signal<@Nullable String> signal = Signal - .<@Nullable String> cached((() -> { - dependency.get(); - other.set("update"); - return null; - })); + Signal<@Nullable String> signal = Signal.cached(() -> { + dependency.get(); + other.set("update"); + return null; + }); // Trigger running the callback signal.peek(); diff --git a/flow-server/src/test/java/com/vaadin/flow/signals/local/ValueSignalTest.java b/flow-server/src/test/java/com/vaadin/flow/signals/local/ValueSignalTest.java index f8e4aa24d06..14273fd7731 100644 --- a/flow-server/src/test/java/com/vaadin/flow/signals/local/ValueSignalTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/signals/local/ValueSignalTest.java @@ -207,8 +207,7 @@ void update_sameValueEqualObjects_noChangeDetected() { @Test void update_nullToNull_noChangeDetected() { - ValueSignal<@Nullable String> signal = new ValueSignal<@Nullable String>( - null); + ValueSignal<@Nullable String> signal = new ValueSignal<>(null); Usage usage = UsageTracker.track(() -> { signal.get(); @@ -251,8 +250,7 @@ void update_differentValue_changeDetected() { @Test void update_nullToDifferent_changeDetected() { - ValueSignal<@Nullable String> signal = new ValueSignal<@Nullable String>( - null); + ValueSignal<@Nullable String> signal = new ValueSignal<>(null); Usage usage = UsageTracker.track(() -> { signal.get(); @@ -317,8 +315,7 @@ void usageTracker_setSameValue_noChangeDetected() { @Test void set_nullToNull_noChangeDetected() { - ValueSignal<@Nullable String> signal = new ValueSignal<@Nullable String>( - null); + ValueSignal<@Nullable String> signal = new ValueSignal<>(null); Usage usage = UsageTracker.track(() -> { signal.get();