Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False-positive in @Contract check when a nullImpliesNullParameters-derived value is used in a conditional #1104

Open
agrieve opened this issue Dec 20, 2024 · 0 comments

Comments

@agrieve
Copy link

agrieve commented Dec 20, 2024

In the context of this code review, I found:

// Complains that the contract is not satisfied

    @Contract("_, !null -> !null")
    public @Nullable Set<String> readStringSet(String key, @Nullable Set<String> defaultValue) {
        checkIsKeyInUse(key);
        Set<String> values = ContextUtils.getAppSharedPreferences().getStringSet(key, defaultValue);
        return (values != null) ? Collections.unmodifiableSet(values) : null;
    }

// Also complains:

    @Contract("_, !null -> !null")
    public @Nullable Set<String> readStringSet(String key, @Nullable Set<String> defaultValue) {
        checkIsKeyInUse(key);
        Set<String> values = ContextUtils.getAppSharedPreferences().getStringSet(key, defaultValue);
        if (values != null) {
            return Collections.unmodifiableSet(values);
        }
        return null;
    }

The error is:

../../base/android/java/src/org/chromium/base/shared_preferences/SharedPreferencesManager.java:206: warning: [NullAway] Method readStringSet has @Contract(_, !null -> !null), but this appears to be violated, as a @Nullable value may be returned when parameter defaultValue is non-null.
        return null;
        ^
    (see http://t.uber.com/nullaway )
1 warning

However, if I remove the unmodifiableSet part, it succeeds:

// No warnings:
    @Contract("_, !null -> !null")
    public @Nullable Set<String> readStringSet(String key, @Nullable Set<String> defaultValue) {
        checkIsKeyInUse(key);
        Set<String> values = ContextUtils.getAppSharedPreferences().getStringSet(key, defaultValue);
        return values;
    }

Note that values is determined to be non-null as a result of a nullImpliesNullParameters() library model here. I'm guessing the data flow analysis is not recognizing that if (values != null) is always true when defaultValue != null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant