Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions DESIGN_GUIDELINES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -334,12 +333,11 @@ void callback_updateOtherSignal_signalUpdated() {
var dependency = createDependency();
SharedValueSignal<String> 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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
Loading