Skip to content

Commit 658c69f

Browse files
authored
mark wait.until as non-nullable (#17007)
They always return true/false, WebDriver or Webelement. They never return null.
1 parent 88412e3 commit 658c69f

15 files changed

Lines changed: 85 additions & 22 deletions

java/src/org/openqa/selenium/TimeoutException.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
package org.openqa.selenium;
1919

20+
import org.jspecify.annotations.Nullable;
21+
2022
/** Thrown when a command does not complete in enough time. */
2123
public class TimeoutException extends WebDriverException {
2224

@@ -30,7 +32,7 @@ public TimeoutException(Throwable cause) {
3032
super(cause);
3133
}
3234

33-
public TimeoutException(String message, Throwable cause) {
35+
public TimeoutException(String message, @Nullable Throwable cause) {
3436
super(message, cause);
3537
}
3638
}

java/src/org/openqa/selenium/firefox/FirefoxDriver.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.logging.Logger;
2828
import java.util.stream.Collectors;
2929
import java.util.stream.Stream;
30+
import org.jspecify.annotations.NonNull;
3031
import org.openqa.selenium.Beta;
3132
import org.openqa.selenium.Capabilities;
3233
import org.openqa.selenium.ImmutableCapabilities;
@@ -183,6 +184,7 @@ private static Capabilities checkCapabilitiesAndProxy(Capabilities capabilities)
183184
return caps;
184185
}
185186

187+
@NonNull
186188
@Override
187189
public Capabilities getCapabilities() {
188190
return capabilities;

java/src/org/openqa/selenium/remote/Augmenter.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static net.bytebuddy.matcher.ElementMatchers.anyOf;
2222
import static net.bytebuddy.matcher.ElementMatchers.named;
2323

24+
import java.lang.reflect.Constructor;
2425
import java.lang.reflect.Field;
2526
import java.lang.reflect.Modifier;
2627
import java.util.ArrayList;
@@ -212,7 +213,9 @@ private WebDriver augment(WebDriver driver, List<Augmentation<?>> matchingAugmen
212213
.asSubclass(driver.getClass());
213214

214215
try {
215-
WebDriver toReturn = definition.getDeclaredConstructor().newInstance();
216+
Constructor<? extends WebDriver> constructor = definition.getDeclaredConstructor();
217+
constructor.setAccessible(true);
218+
WebDriver toReturn = constructor.newInstance();
216219

217220
copyFields(driver.getClass(), driver, toReturn);
218221

@@ -285,7 +288,7 @@ private void copyFields(Class<?> clazz, Object source, Object target) {
285288
}
286289

287290
private void copyField(Object source, Object target, Field field) {
288-
if (Modifier.isFinal(field.getModifiers())) {
291+
if (Modifier.isStatic(field.getModifiers())) {
289292
return;
290293
}
291294

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import java.util.logging.Logger;
4747
import java.util.stream.Collectors;
4848
import java.util.stream.Stream;
49+
import org.jspecify.annotations.NonNull;
4950
import org.openqa.selenium.AcceptedW3CCapabilityKeys;
5051
import org.openqa.selenium.Alert;
5152
import org.openqa.selenium.Beta;
@@ -357,6 +358,7 @@ protected void setCommandExecutor(CommandExecutor executor) {
357358
this.executor = executor;
358359
}
359360

361+
@NonNull
360362
@Override
361363
public Capabilities getCapabilities() {
362364
if (capabilities == null) {

java/src/org/openqa/selenium/support/ui/ExpectedCondition.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.openqa.selenium.support.ui;
1919

2020
import com.google.common.base.Function;
21-
import org.jspecify.annotations.NullMarked;
2221
import org.jspecify.annotations.Nullable;
2322
import org.openqa.selenium.WebDriver;
2423

@@ -29,13 +28,12 @@
2928
*
3029
* <p>Note that it is expected that ExpectedConditions are idempotent. They will be called in a loop
3130
* by the {@link WebDriverWait} and any modification of the state of the application under test may
32-
* have unexpected side-effects.
31+
* have unexpected side effects.
3332
*
3433
* @param <T> The return type
3534
*/
3635
// NB: this originally extended Guava's Function interface since Java didn't have one. To avoid code
3736
// such as "com.google.common.base.Function condition = ExpectedConditions.elementFound(By);"
3837
// breaking at compile time, we continue to extend Guava's Function interface.
39-
@NullMarked
4038
public interface ExpectedCondition<T extends @Nullable Object>
4139
extends Function<WebDriver, T>, java.util.function.Function<WebDriver, T> {}

java/src/org/openqa/selenium/support/ui/ExpectedConditions.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.util.logging.Level;
2727
import java.util.logging.Logger;
2828
import java.util.regex.Pattern;
29-
import org.jspecify.annotations.NullMarked;
3029
import org.jspecify.annotations.Nullable;
3130
import org.openqa.selenium.Alert;
3231
import org.openqa.selenium.By;
@@ -42,7 +41,6 @@
4241
import org.openqa.selenium.WebElement;
4342

4443
/** Canned {@link ExpectedCondition}s which are generally useful within webdriver tests. */
45-
@NullMarked
4644
@SuppressWarnings("MismatchedJavadocCode")
4745
public class ExpectedConditions {
4846
private static final Logger LOG = Logger.getLogger(ExpectedConditions.class.getName());

java/src/org/openqa/selenium/support/ui/FluentWait.java

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.List;
3030
import java.util.function.Function;
3131
import java.util.function.Supplier;
32+
import org.jspecify.annotations.Nullable;
3233
import org.openqa.selenium.TimeoutException;
3334
import org.openqa.selenium.WebDriverException;
3435
import org.openqa.selenium.internal.Require;
@@ -76,7 +77,7 @@ public class FluentWait<T> implements Wait<T> {
7677

7778
protected Duration timeout = DEFAULT_WAIT_DURATION;
7879
protected Duration interval = DEFAULT_WAIT_DURATION;
79-
protected Supplier<String> messageSupplier = () -> null;
80+
protected Supplier<@Nullable String> messageSupplier = () -> null;
8081

8182
protected final List<Class<? extends Throwable>> ignoredExceptions = new ArrayList<>();
8283

@@ -117,6 +118,7 @@ public FluentWait<T> withTimeout(Duration timeout) {
117118
* @return A self reference.
118119
*/
119120
public FluentWait<T> withMessage(final String message) {
121+
Require.nonNull("Message", message);
120122
this.messageSupplier = () -> message;
121123
return this;
122124
}
@@ -128,7 +130,7 @@ public FluentWait<T> withMessage(final String message) {
128130
* @return A self reference.
129131
*/
130132
public FluentWait<T> withMessage(Supplier<String> messageSupplier) {
131-
this.messageSupplier = messageSupplier;
133+
this.messageSupplier = Require.nonNull("Message supplier", messageSupplier);
132134
return this;
133135
}
134136

@@ -165,7 +167,7 @@ public <K extends Throwable> FluentWait<T> ignoreAll(Collection<Class<? extends
165167
* @see #ignoreAll(Collection)
166168
*/
167169
public FluentWait<T> ignoring(Class<? extends Throwable> exceptionType) {
168-
return this.ignoreAll(List.<Class<? extends Throwable>>of(exceptionType));
170+
return this.ignoreAll(List.of(exceptionType));
169171
}
170172

171173
/**
@@ -198,7 +200,7 @@ public FluentWait<T> ignoring(
198200
* @throws TimeoutException If the timeout expires.
199201
*/
200202
@Override
201-
public <V> V until(Function<? super T, V> isTrue) {
203+
public <V> V until(Function<? super T, @Nullable V> isTrue) {
202204
Instant end = clock.instant().plus(timeout);
203205

204206
Throwable lastException;
@@ -220,7 +222,7 @@ public <V> V until(Function<? super T, V> isTrue) {
220222
// Check the timeout after evaluating the function to ensure conditions
221223
// with a zero timeout can succeed.
222224
if (end.isBefore(clock.instant())) {
223-
String message = messageSupplier != null ? messageSupplier.get() : null;
225+
String message = messageSupplier.get();
224226

225227
String timeoutMessage =
226228
String.format(
@@ -272,7 +274,7 @@ private Throwable propagateIfNotIgnored(Throwable e) {
272274
* on a function.
273275
* @return Nothing will ever be returned; this return type is only specified as a convenience.
274276
*/
275-
protected RuntimeException timeoutException(String message, Throwable lastException) {
277+
protected RuntimeException timeoutException(String message, @Nullable Throwable lastException) {
276278
throw new TimeoutException(message, lastException);
277279
}
278280
}

java/src/org/openqa/selenium/support/ui/WebDriverWait.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import java.time.Clock;
2121
import java.time.Duration;
22+
import org.jspecify.annotations.Nullable;
2223
import org.openqa.selenium.NotFoundException;
2324
import org.openqa.selenium.TimeoutException;
2425
import org.openqa.selenium.WebDriver;
@@ -79,7 +80,7 @@ public WebDriverWait(
7980
}
8081

8182
@Override
82-
protected RuntimeException timeoutException(String message, Throwable lastException) {
83+
protected RuntimeException timeoutException(String message, @Nullable Throwable lastException) {
8384
WebDriver exceptionDriver = driver;
8485
TimeoutException ex = new TimeoutException(message, lastException);
8586
ex.addInfo(WebDriverException.DRIVER_INFO, exceptionDriver.getClass().getName());
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Licensed to the Software Freedom Conservancy (SFC) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The SFC licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
@NullMarked
19+
package org.openqa.selenium.support.ui;
20+
21+
import org.jspecify.annotations.NullMarked;

java/test/org/openqa/selenium/remote/AugmenterTest.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.HashMap;
2929
import java.util.List;
3030
import java.util.Map;
31+
import org.jspecify.annotations.NonNull;
3132
import org.junit.jupiter.api.Tag;
3233
import org.junit.jupiter.api.Test;
3334
import org.openqa.selenium.By;
@@ -114,8 +115,7 @@ void proxyShouldNotAppearInStackTraces() {
114115
// This will force the class to be enhanced
115116
final Capabilities caps = new ImmutableCapabilities("magic.numbers", true);
116117

117-
DetonatingDriver driver = new DetonatingDriver();
118-
driver.setCapabilities(caps);
118+
DetonatingDriver driver = new DetonatingDriver(caps);
119119

120120
WebDriver returned =
121121
getAugmenter()
@@ -337,12 +337,17 @@ public interface MyInterface {
337337

338338
public static class DetonatingDriver extends RemoteWebDriver {
339339

340-
private Capabilities caps;
340+
private final Capabilities caps;
341341

342-
public void setCapabilities(Capabilities caps) {
342+
protected DetonatingDriver() {
343+
this(null);
344+
}
345+
346+
public DetonatingDriver(Capabilities caps) {
343347
this.caps = caps;
344348
}
345349

350+
@NonNull
346351
@Override
347352
public Capabilities getCapabilities() {
348353
return caps;
@@ -366,6 +371,7 @@ public interface HasNumbers {
366371

367372
public static class ChildRemoteDriver extends RemoteWebDriver implements HasMagicNumbers {
368373

374+
@NonNull
369375
@Override
370376
public Capabilities getCapabilities() {
371377
return new FirefoxOptions();
@@ -379,6 +385,7 @@ public int getMagicNumber() {
379385

380386
public static class WithFinals extends RemoteWebDriver {
381387

388+
@NonNull
382389
@Override
383390
public Capabilities getCapabilities() {
384391
return new ImmutableCapabilities();

0 commit comments

Comments
 (0)