Skip to content

Commit edc8754

Browse files
artempyanykhfacebook-github-bot
authored andcommitted
[nullsafe] Fix behaviour of --no-nullsafe-optimistic-third-party-in-default-mode
Summary: The problem is that in `AnnotatedField.special_case_nullability` we first check the _generic_ nullability and if it is `nonnullish` we apply refinements for enums, synthetic fields, etc. The problem is that the definition of `is_nonnullish` changed in D25186043 (facebook@7dcbacf) to a stricter one `UncheckedNonnull`, but generic nullability stayed the same `ThirdPartyNonnull`. Therefore enum elements were not considered `nonnullish` under `--no-nullsafe-optimistic-third-party-in-default-mode` and the enum refinements were not applied, which led to bogus errors. **Example:** There's a third-party enum ``` enum EnumClass { ENUM_ELEMENT } ``` `ENUM_ELEMENT` is represented as a private static field of `EnumClass`. Then we have first party code that does ``` EnumClass.ENUM_ELEMENT ``` If this first party class is not `Nullsafe` and the checker is ran with `--no-nullsafe-optimistic-third-party-in-default-mode`, the user gets an incorrect warning about `ENUM_ELEMENT` being unvetted third party. Reviewed By: ngorogiannis Differential Revision: D25560119 fbshipit-source-id: 4ad0760c5
1 parent f779ed8 commit edc8754

File tree

4 files changed

+24
-9
lines changed

4 files changed

+24
-9
lines changed

infer/src/nullsafe/AnnotatedField.ml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ let get tenv field_name ~class_typ ~class_under_analysis =
7575
~is_third_party field_typ annotations
7676
in
7777
let special_case_nullability =
78-
if Nullability.is_nonnullish (AnnotatedNullability.get_nullability nullability) then
78+
if
79+
Nullability.is_subtype
80+
~subtype:(AnnotatedNullability.get_nullability nullability)
81+
~supertype:Nullability.ThirdPartyNonnull
82+
then
7983
if
8084
is_enum_value
8185
(* Enum values are the special case - they can not be null. So we can strengten nullability.

infer/tests/codetoanalyze/java/nullsafe/NullsafeMode.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ UncheckedParams OK_passUncheckedToUnchecked() {
118118
UncheckedParams second = new UncheckedParams(first.copy());
119119
return second;
120120
}
121+
122+
int OK_enumElementsAreNotNull() {
123+
return ThirdPartyTestClass.InnerEnum.EA.ordinal();
124+
}
121125
}
122126

123127
@Nullsafe(value = Nullsafe.Mode.LOCAL, trustOnly = @Nullsafe.TrustList({NonNullsafe.class}))

infer/tests/codetoanalyze/java/nullsafe/issues.exp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,19 +217,19 @@ codetoanalyze/java/nullsafe/NullMethodCall.java, codetoanalyze.java.nullsafe.Nul
217217
codetoanalyze/java/nullsafe/NullMethodCall.java, codetoanalyze.java.nullsafe.NullMethodCall.withConditionalAssignemnt(codetoanalyze.java.nullsafe.NullMethodCall$AnotherI,boolean,java.lang.Object,java.lang.Object):void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`i` is nullable and is not locally checked for null when calling `withObjectParameter(...)`.], NullMethodCall, codetoanalyze.java.nullsafe
218218
codetoanalyze/java/nullsafe/NullMethodCall.java, codetoanalyze.java.nullsafe.NullMethodCall.withConjuction(codetoanalyze.java.nullsafe.NullMethodCall$AnotherI,boolean,boolean):void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`i` is nullable and is not locally checked for null when calling `withBooleanParameter(...)`.], NullMethodCall, codetoanalyze.java.nullsafe
219219
codetoanalyze/java/nullsafe/NullsafeMode.java, Linters_dummy_method, 16, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeMode, codetoanalyze.java.nullsafe, issues: 13, curr_mode: "Default"
220-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$NullsafeWithStrictMode.BAD_returnFromNonStrict():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@Nullsafe(STRICT)` prohibits using values coming from non-strict classes without a check. Result of this call is used at line 174. Either add a local check for null or assertion, or make `NullsafeMode$VariousMethods` nullsafe strict.], NullsafeMode$NullsafeWithStrictMode, codetoanalyze.java.nullsafe
221-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_dereferenceNotAnnotatedThirdParty():void, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.returnUnspecified()`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 218. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#returnUnspecified()]
222-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_passThirdPartyToUnchecked():codetoanalyze.java.nullsafe.NullsafeMode$UncheckedParams, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.getUncheckedLong(...)`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 214. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#getUncheckedLong(long)]
223-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_returnFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.returnVal()`: `@Nullsafe(STRICT)` prohibits using values coming from non-strict classes without a check. Result of this call is used at line 197. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` nullsafe strict.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe
220+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$NullsafeWithStrictMode.BAD_returnFromNonStrict():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@Nullsafe(STRICT)` prohibits using values coming from non-strict classes without a check. Result of this call is used at line 178. Either add a local check for null or assertion, or make `NullsafeMode$VariousMethods` nullsafe strict.], NullsafeMode$NullsafeWithStrictMode, codetoanalyze.java.nullsafe
221+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_dereferenceNotAnnotatedThirdParty():void, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.returnUnspecified()`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 222. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#returnUnspecified()]
222+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_passThirdPartyToUnchecked():codetoanalyze.java.nullsafe.NullsafeMode$UncheckedParams, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.getUncheckedLong(...)`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 218. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#getUncheckedLong(long)]
223+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$StrictNullsafe.BAD_returnFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.returnVal()`: `@Nullsafe(STRICT)` prohibits using values coming from non-strict classes without a check. Result of this call is used at line 201. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` nullsafe strict.], NullsafeMode$StrictNullsafe, codetoanalyze.java.nullsafe
224224
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustAllNullsafe.BAD_passThirdPartyToUnchecked():codetoanalyze.java.nullsafe.NullsafeMode$UncheckedParams, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.getUncheckedLong(...)`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 113. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$TrustAllNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#getUncheckedLong(long)]
225225
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustAllNullsafe.BAD_returnFromUnvettedThirdParty():java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.returnUnspecified()`: `@Nullsafe` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 92. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], NullsafeMode$TrustAllNullsafe, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#returnUnspecified()]
226226
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustAllNullsafe.BAD_returnNonNullableFieldFromThirdParty():java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.nonNullableField`: `@Nullsafe` mode prohibits using values coming from not vetted third party fields without a check. This field is used at line 100. Either add a local check for null or assertion, or access `nonNullableField` via a nullsafe getter.], NullsafeMode$TrustAllNullsafe, codetoanalyze.java.nullsafe
227227
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustAllNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 89.], NullsafeMode$TrustAllNullsafe, codetoanalyze.java.nullsafe, nullable_methods:codetoanalyze.java.nullsafe.NullsafeMode$VariousMethods.returnNull at 89
228228
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustAllNullsafe.BAD_returnNullableFieldFromThirdParty():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`BAD_returnNullableFieldFromThirdParty()`: return type is declared non-nullable but the method returns a nullable value: field nullableField at line 97.], NullsafeMode$TrustAllNullsafe, codetoanalyze.java.nullsafe
229-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustNoneNullsafe.BAD_returnFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.returnVal()`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. Result of this call is used at line 154. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` @Nullsafe (or add it to trust list).], NullsafeMode$TrustNoneNullsafe, codetoanalyze.java.nullsafe
230-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.BAD_returnFromUntrustedNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. Result of this call is used at line 134. Either add a local check for null or assertion, or make `NullsafeMode$VariousMethods` @Nullsafe (or add it to trust list).], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe
231-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 144.], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe, nullable_methods:codetoanalyze.java.nullsafe.NullsafeMode$VariousMethods.returnNull at 144
232-
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.FP_OK_accessFieldFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.valField`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. This field is used at line 147. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` @Nullsafe (or add it to trust list).], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe
229+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustNoneNullsafe.BAD_returnFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.returnVal()`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. Result of this call is used at line 158. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` @Nullsafe (or add it to trust list).], NullsafeMode$TrustNoneNullsafe, codetoanalyze.java.nullsafe
230+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.BAD_returnFromUntrustedNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$VariousMethods.returnVal()`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. Result of this call is used at line 138. Either add a local check for null or assertion, or make `NullsafeMode$VariousMethods` @Nullsafe (or add it to trust list).], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe
231+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.BAD_returnNullFromNonNulsafe():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, ERROR, [`BAD_returnNullFromNonNulsafe()`: return type is declared non-nullable but the method returns a nullable value: call to returnNull() at line 148.], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe, nullable_methods:codetoanalyze.java.nullsafe.NullsafeMode$VariousMethods.returnNull at 148
232+
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.FP_OK_accessFieldFromNonNullsafe():java.lang.String, 1, ERADICATE_UNCHECKED_USAGE_IN_NULLSAFE, no_bucket, ERROR, [`NullsafeMode$NonNullsafe.valField`: `@Nullsafe(trust={...})` prohibits using values coming from non-`@Nullsafe` classes without a check, unless the class is in the trust list. This field is used at line 151. Either add a local check for null or assertion, or make `NullsafeMode$NonNullsafe` @Nullsafe (or add it to trust list).], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe
233233
codetoanalyze/java/nullsafe/NullsafeMode.java, codetoanalyze.java.nullsafe.NullsafeMode$TrustSomeNullsafe.OK_returnFromUntrustedNonNullsafeAsNullable():java.lang.String, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, ADVICE, [Method `OK_returnFromUntrustedNonNullsafeAsNullable()` is annotated with `@Nullable` but never returns null.], NullsafeMode$TrustSomeNullsafe, codetoanalyze.java.nullsafe
234234
codetoanalyze/java/nullsafe/NullsafeModeNestedClasses.java, Linters_dummy_method, 14, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], NullsafeLocal, codetoanalyze.java.nullsafe, issues: 5, curr_mode: "LocalTrustAll"
235235
codetoanalyze/java/nullsafe/NullsafeModeNestedClasses.java, Linters_dummy_method, 49, ERADICATE_REDUNDANT_NESTED_CLASS_ANNOTATION, no_bucket, ADVICE, [`NullsafeLocal$NestedExplicitLocal`: the same @Nullsafe mode is already specified in the outer class, so this annotation can be removed.], NullsafeLocal$NestedExplicitLocal, codetoanalyze.java.nullsafe

infer/tests/codetoanalyze/java/nullsafe/third-party-test-code/some/test/pckg/ThirdPartyTestClass.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
public class ThirdPartyTestClass {
1818

19+
// Inner classes
20+
1921
public static class UncheckedLong {
2022
public long mInner;
2123

@@ -24,6 +26,11 @@ public UncheckedLong(long inner) {
2426
}
2527
}
2628

29+
public enum InnerEnum {
30+
EA,
31+
EB,
32+
}
33+
2734
// Fields.
2835

2936
public String nonNullableField;

0 commit comments

Comments
 (0)