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();