mark all APIs nullable/non-nullable by asolntsev · Pull Request #1764 · datafaker-net/datafaker

asolntsev · 2026-02-20T07:26:21Z

JSpecify is de-facto standard annotations for marking nullability of variables, parameters, return values and fields. Supported by all IDEs.

All remarkable libraries have migrated to JSpecify: Spring, JUnit etc.

By default, we mark everything as non-nullable (see @NullMarked in package-info.java)
When needed, we mark the nullable parameters/fields as @nullable.

JSpecify is de-facto standard annotations for marking nullability of variables, parameters, return values and fields. Supported by all IDEs. 1. By default, we mark everything as non-nullable (see @NullMarked in package-info.java) 2. When needed, we mark the nullable parameters/fields as @nullable.

bodiam · 2026-02-20T07:28:11Z

Awesome! Just wondering, could this start breaking things for libraries which depend on us, or Kotlin projects? Just wonder if we release this, should this be 2.x release or 3.x ?

codecov-commenter · 2026-02-20T07:28:24Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.59459% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.45%. Comparing base (4e4a5bc) to head (b51ccfe).

Files with missing lines	Patch %	Lines
...n/java/net/datafaker/annotations/FakeResolver.java	83.33%	2 Missing ⚠️
...ain/java/net/datafaker/providers/base/Vehicle.java	50.00%	0 Missing and 1 partial ⚠️
...java/net/datafaker/service/FakeValuesGrouping.java	80.00%	0 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1764      +/-   ##
============================================
+ Coverage     92.43%   92.45%   +0.02%     
+ Complexity     3450     3445       -5     
============================================
  Files           339      339              
  Lines          6794     6786       -8     
  Branches        670      661       -9     
============================================
- Hits           6280     6274       -6     
  Misses          351      351              
+ Partials        163      161       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

asolntsev · 2026-02-20T07:37:28Z

No, it doesn't need to be 3.0
These are just annotations that fix current state of things. They don't change the behavior, they will just help IDE to highlight problematic places in code.

bodiam · 2026-02-20T07:46:00Z

Okay. My understanding was that before the code is annotated, Kotlin will treat this as platform types. So, the following code would work:

var name: String? = faker.name().firstName()
var name: String = faker.name().firstName()

Both would work. But if I look here: https://resources.jetbrains.com/storage/products/kotlinconf-2025/may-23/JSpecify_%20Java%20Nullness%20Annotations%20and%20Kotlin%20_%20David%20Baker.pdf, they say:

The Kotlin compiler treats JSpecify-annotated Java code as null-safe types as of version 2.1.0.

So, if we'd add @nullable to something Kotlin clients assumed was non-null, they'll get compile errors if they're using it in a non-null context without null checks.

So, something like this might break Kotlin consumers, I think:

    @Nullable
    public String userId() {

Also, I think the code is perhaps incorrect, I don't think it should ever return a nullable userId. (and yes, I know what the documentation says, but it's also a bit sus) Wdyt?

asolntsev · 2026-02-20T08:43:43Z

The Kotlin compiler treats JSpecify-annotated Java code as null-safe types as of version 2.1.0.
So, something like this might break Kotlin consumers, I think:

@bodiam Hm... Yes, that's right: the new annotations can break compilation of existing Kotlin code.
For example:

  val userId1: String = faker.credentials().userId(null);

Previously: it compiled because Kotlin doesn't know if userId(null) can return null.
Now: this code doesn't compile because Kotlin knows that userId(null) can return null.

So yes, we need to release it as DataFaker 3.0.0 :)

P.S. And yes, good point: method faker.credentials.userId() never returns null. I've removed @Nullable annotation from this method.

method `faker.credentials.userId()` never returns null.

kingthorin

It's kind of a lot to review, especially with the unrelated typo fixes etc.

These are the only things I noticed.

kingthorin · 2026-02-20T10:38:43Z

src/main/java/net/datafaker/providers/base/Credentials.java

+        String regex = resolve("credentials.uid_pattern");
+        return faker.regexify(regex);


Why not inline it and avoid the intermediate storage?

kingthorin · 2026-02-20T10:46:12Z

src/test/java/net/datafaker/formats/YamlTest.java

            of(Schema.of(field("boolean", () -> true)), "boolean: true" + System.lineSeparator()),
            of(Schema.of(field("nullValue", () -> null)), "nullValue: null" + System.lineSeparator()),
-            of(Schema.of(field("array", () -> new String[]{null, "test", "123"})),
+            of(Schema.of(field("array", () -> new @Nullable String[]{null, "test", "123"})),


Are users of datafaker going to have to do this?

Why is it needed here but not the one just before?

I don't know either, but a nullable string[] which is not null? looks weird. I think this could be removed?

snuyanzin · 2026-02-20T10:57:28Z

src/main/java/net/datafaker/internal/helper/SingletonLocale.java

@@ -19,23 +21,16 @@ private SingletonLocale(Locale locale) {
        this.locale = locale;
    }

-    public static SingletonLocale get(Locale locale) {
+    @Nullable
+    public static SingletonLocale get(@Nullable Locale locale) {
        if (locale == null) {
            return null;
        }
-        SingletonLocale res = LOCALE2SINGLETON_LOCALE.get(locale);
-        if (res != null) {
-            return res;
-        }
-        synchronized (SingletonLocale.class) {
-            res = LOCALE2SINGLETON_LOCALE.get(locale);
-            if (res != null) {
-                return res;
-            }
-            res = new SingletonLocale(locale);
-            LOCALE2SINGLETON_LOCALE.put(locale, res);
-            return res;
-        }
+        return getRequired(locale);
+    }
+
+    public static SingletonLocale getRequired(Locale locale) {
+        return LOCALE2SINGLETON_LOCALE.computeIfAbsent(locale, (__) -> new SingletonLocale(locale));


doesn't match PR description

snuyanzin · 2026-02-20T11:06:07Z

is there any enforcer that could fail the build if missed?
Or otherwise it always be in a phase of "eventual consistency"

bodiam · 2026-02-20T11:08:55Z

So yes, we need to release it as DataFaker 3.0.0 :)

P.S. And yes, good point: method faker.credentials.userId() never returns null. I've removed @Nullable annotation from this method.

Just to be clear on this: I do really like the PR, I think we should absolutely do it, just may need a bit of checking on the impact, and how we're communicating this, that's all. I have no issue with releasing DF 3.x either.

bodiam · 2026-02-20T11:11:17Z

src/test/java/net/datafaker/service/FakeValuesServiceTest.java

    }

    @Test
+    @SuppressWarnings("deprecation")


Why is this deprecation warning suppressed, instead of using the Locale.of method?

bodiam · 2026-02-20T11:13:07Z

src/test/java/net/datafaker/script/ProvidersDocsGenerator.java

+        private final @Nullable Function<Matcher, String> extractor;

-        Column(String columnName, int length, Pattern pattern2extract, Function<Matcher, String> extractor) {
+        Column(String columnName, int length, @Nullable Pattern pattern2extract, @Nullable Function<Matcher, String> extractor) {


I don't think this is correct. pattern2extract cannot be null, it would throw an NPE at line 246.

bodiam · 2026-02-20T11:14:09Z

src/test/java/net/datafaker/script/ProvidersDocsGenerator.java

+        private final @Nullable Function<Matcher, String> extractor;

-        Column(String columnName, int length, Pattern pattern2extract, Function<Matcher, String> extractor) {
+        Column(String columnName, int length, @Nullable Pattern pattern2extract, @Nullable Function<Matcher, String> extractor) {


same for extractor, if it's null, it would throw an exception at line 249/250. There's no guards/optional stuff at all on this.

bodiam · 2026-02-20T11:15:41Z

src/test/java/net/datafaker/integration/FakerIntegrationTest.java

    @ParameterizedTest
    @MethodSource("dataParameters")
-    void testAllFakerMethodsThatReturnStrings(Locale locale, Random random) throws Exception {
+    void testAllFakerMethodsThatReturnStrings(@Nullable Locale locale, @Nullable Random random) throws Exception {


I have no idea if these should be nullable or not. Cannot image they should be.

snuyanzin · 2026-02-20T11:19:58Z

src/main/java/net/datafaker/providers/base/Lorem.java

        int numberOfWordsToAdd = randomWordsToAdd == 0 ? 0 : faker.random().nextInt(randomWordsToAdd);
        final int totalWordCount = wordCount + numberOfWordsToAdd;
-        StringBuilder sb = new StringBuilder();
+        StringBuilder sb = new StringBuilder(wordCount * 9);


what is this magic 9 ?

bodiam · 2026-02-20T11:21:52Z

src/main/java/net/datafaker/transformations/JavaObjectTransformer.java

    private Object getObject(Schema<Object, ?> schema, Object result, Constructor<?> recordConstructor) {
        final Field<Object, ?>[] fields = schema.getFields();
-        final Object[] values = new Object[fields.length];
+        final @Nullable Object[] values = new Object[fields.length];


It's not really clear to me here wheat the intention is. Why can a final reference to values which is filled by an object be nullable? I don't think it can?

bodiam · 2026-02-20T11:22:57Z

src/main/java/net/datafaker/transformations/CompositeField.java

    private final String name;

-    public CompositeField(String name, Field<MyObject, MyType>[] fields) {
+    public CompositeField(@Nullable String name, Field<MyObject, MyType>[] fields) {


I don't understand why a compositeField can have a nullable name.

bodiam · 2026-02-20T11:23:21Z

src/main/java/net/datafaker/transformations/sql/SqlTransformer.java

    }

-    private String generateBatchModeStatements(Schema<IN, ?> schema, List<IN> inputs, int limit) {
+    private String generateBatchModeStatements(Schema<IN, ?> schema, @Nullable List<IN> inputs, int limit) {


I don't think collection types should ever be nullable.

snuyanzin · 2026-02-20T11:31:29Z

src/main/java/net/datafaker/transformations/TomlTransformer.java

+    private void writeSchema(StringBuilder sb, @Nullable IN input, Schema<IN, ?> schema, String pathPrefix) {
        Field<IN, ?>[] fields = schema.getFields();
-        Set<String> seen = new HashSet<>();
+        Set<@Nullable String> seen = new HashSet<>();


do you have a case when it might be null?

It looks like annotations sometimes were placed very randomly

snuyanzin · 2026-02-20T11:32:24Z

@asolntsev can you explain how you put annotations?
It looks like in some cases it makes some in some cases there are placed very randomly

bodiam · 2026-02-20T11:40:32Z

src/main/java/net/datafaker/service/FakeValuesService.java

     * @throws IllegalArgumentException in case of invalid path
     */
-    public void addPath(Locale locale, Path path) {
+    public void addPath(Locale locale, @Nullable Path path) {


this doesn't seem to be correct. It's nullable, but when it is null, we thorw an IllegalArgumentException. Don't think that's correct?

bodiam · 2026-02-20T11:42:31Z

I used Claude Code to review it as well:

Summary

This branch introduces JSpecify (@NullMarked, @nullable) and Error Prone (@CheckReturnValue) annotations across the entire codebase. It adds package-info.java files to 22 packages, making non-null the default for
all parameters and return types, and explicitly marks nullable locations with @nullable. Along the way, it includes several cleanup improvements: thread-safety fixes, dead-code removal, raw type elimination, and
typo corrections.

Findings

Critical

None found.

Improvements

Vehicle.licensePlate(String) - Javadoc still references old behavior (Vehicle.java:147)
The Javadoc at line 147 still says "If the regex is null or invalid, it returns null" - wait, that's actually on Credentials. For Vehicle.licensePlate, the method now returns @nullable but the null check on
licensePlatesByState was removed. This is correct since resolve() throws rather than returning null - but this is a subtle contract dependency. A brief comment explaining why the null check was removed would help
future readers.
Credentials.userId() - stale Javadoc (Credentials.java:146-149)
The no-arg userId() Javadoc still says "If the regex is null or invalid, it returns null" (line 147), but the method no longer delegates to the nullable overload and will now throw if resolve() fails. The Javadoc
should be updated to match.
SingletonLocale.get() still returns @nullable (SingletonLocale.java:22-27)
The get(Locale) method accepts @nullable and returns @nullable. Several call sites now wrap it with requireNonNull(SingletonLocale.get(locale), ...). Consider whether the @nullable accepting overload is still
needed. Currently FakerContext has a private singletonLocale() helper, and FakeValuesContext has an inline requireNonNull. Since get(null) is only useful for the one place that truly passes a nullable locale, it
might be cleaner to have callers handle the null check themselves and make get() non-null-in-non-null-out - but this is a judgment call given this is already the chosen direction.
FakeValuesService.addPath accepts @nullable Path (FakeValuesService.java:212)
The @nullable on the path parameter is odd for a public API since the next line immediately throws on null. Consider keeping it non-null in the signature and letting NullMarked enforce it at compile time, rather
than accepting null only to immediately reject it. Same goes for addUrl where url is now enforced via requireNonNull but the parameter isn't marked @nullable (which is actually the better approach).
TomlTransformer.writeSchema - Set<@nullable String> (TomlTransformer.java:68)
The Set<@nullable String> for seen looks intentional (since f.getName() can return null), but adding null to a HashSet and then calling add(null) again to detect duplicates could lead to confusing "duplicate key"
errors for unnamed fields. This seems pre-existing but worth a thought.
@SuppressWarnings("ConstantValue") in WeightedRandomSelector (WeightedRandomSelector.java:59,67,75)
These are appropriate since the validation methods guard against null at the API boundary. Just confirming this was intentional and correct.
LazyEvaluated.get() uses requireNonNull (LazyEvaluated.java:26)
The requireNonNull(value, "Null value not allowed") is correct for the contract (supplier shouldn't return null), but the message "Null value not allowed" could be more descriptive, e.g., "Supplier returned null"
or "LazyEvaluated value was null after evaluation".

Nitpicks

Lorem.java:116: new StringBuilder(wordCount * 9) - the initial capacity heuristic (9 chars per word) is a reasonable optimization. Minor: a comment explaining the magic number would be nice.
FakeValuesGrouping.java: Nice cleanup removing raw types and adding the null check before putAll.
Internet.BotUserAgent.any: Good simplification using nextEnum().
Typo fixes ("estonian" -> "Estonian", "summ" -> "sum", "criterias" -> "criteria", "a algorithm" -> "an algorithm") are all welcome.
The CsvTest exception type change from IllegalArgumentException to NullPointerException correctly tracks the behavioral change in SimpleField.transform().

Conclusion

Approved. This is a high-quality, thorough nullability annotation pass. The @NullMarked + @nullable approach with JSpecify is the modern best practice for Java, the provided scope is correct, and the various code
cleanups (thread-safety in SingletonLocale, dead code removal in Vehicle, raw type cleanup in FakeValuesGrouping) are all improvements. The setLocale being made final to avoid virtual dispatch from the
constructor is a particularly nice detail.

The only item I'd suggest addressing before merge is the stale Javadoc on Credentials.userId() (finding #2).

asolntsev · 2026-02-20T13:11:15Z

@asolntsev can you explain how you put annotations?
It looks like in some cases it makes some in some cases there are placed very randomly

As I explained in the PR description:

By default, we mark everything as non-nullable (see @NullMarked in package-info.java)
When needed, we mark the nullable parameters/fields as @nullable.

So I added @nullable in places where i see that null is actually expected. Of course, I might make a mistake in some of these places (but I hope very few).

asolntsev added this to the 2.6.0 milestone Feb 20, 2026

asolntsev added the enhancement New feature or request label Feb 20, 2026

asolntsev self-assigned this Feb 20, 2026

asolntsev requested review from bodiam and snuyanzin February 20, 2026 08:41

remove unneeded @nullable

b51ccfe

method `faker.credentials.userId()` never returns null.

asolntsev requested a review from kingthorin February 20, 2026 08:49

asolntsev modified the milestones: 2.6.0, 3.0.0 Feb 20, 2026

kingthorin reviewed Feb 20, 2026

View reviewed changes

snuyanzin reviewed Feb 20, 2026

View reviewed changes

bodiam reviewed Feb 20, 2026

View reviewed changes

snuyanzin reviewed Feb 20, 2026

View reviewed changes

bodiam reviewed Feb 20, 2026

View reviewed changes

snuyanzin reviewed Feb 20, 2026

View reviewed changes

bodiam reviewed Feb 20, 2026

View reviewed changes

		String regex = resolve("credentials.uid_pattern");
		return faker.regexify(regex);

Comments

Conversation

asolntsev commented Feb 20, 2026

Uh oh!

bodiam commented Feb 20, 2026

Uh oh!

codecov-commenter commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

asolntsev commented Feb 20, 2026

Uh oh!

bodiam commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asolntsev commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kingthorin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin commented Feb 20, 2026

Uh oh!

bodiam commented Feb 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

snuyanzin commented Feb 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bodiam commented Feb 20, 2026

Summary

Findings

Critical

Improvements

Nitpicks

Conclusion

Uh oh!

asolntsev commented Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Feb 20, 2026 •

edited

Loading

bodiam commented Feb 20, 2026 •

edited

Loading

asolntsev commented Feb 20, 2026 •

edited

Loading