[nullsafe] Treat signatures in the class under analysis differently

Summary:
See the comments in the code why it makes logical sense.
This diff is a step forward the state when list of type violations is
independent of the mode (and we use mode solely to decide re: whether to
report or not).

This fixes majority of cases in ModePromotions.java

Reviewed By: artempyanykh

Differential Revision: D20948656

fbshipit-source-id: 82c0d530b
master
Mitya Lyubarskiy 5 years ago committed by Facebook GitHub Bot
parent c2b512c227
commit ee9cf15d83

@ -127,6 +127,20 @@ let get ~is_trusted_callee ~nullsafe_mode proc_attributes : t =
{nullsafe_mode; model_source= None; ret; params}
let get_for_class_under_analysis tenv proc_attributes =
(* Signature makes special meaning when the method is inside the class we are currently analysing.
Various non-nullable levels (as dictated by nullsafe mode of the class)
make sense only for external (for the class under analysis) methods.
But in context of currently analyzed class we effectively have two levels of nullability for signatures:
nullable and (strict) non-null.
We achieve it via passing Strict mode to the signature extractor.
*)
let result = get ~is_trusted_callee:false ~nullsafe_mode:NullsafeMode.Strict proc_attributes in
(* Don't forget about the original mode *)
let nullsafe_mode = NullsafeMode.of_procname tenv proc_attributes.ProcAttributes.proc_name in
{result with nullsafe_mode}
let param_has_annot predicate pvar ann_sig =
List.exists
~f:(fun {mangled; param_annotation_deprecated} ->

@ -38,5 +38,8 @@ val set_modelled_nullability : Procname.t -> t -> model_source -> bool * bool li
val get : is_trusted_callee:bool -> nullsafe_mode:NullsafeMode.t -> ProcAttributes.t -> t
(** Get a method signature with annotations from a proc_attributes. *)
val get_for_class_under_analysis : Tenv.t -> ProcAttributes.t -> t
(** Signature of the method belonging to the currently analyzed class. *)
val pp : Procname.t -> Format.formatter -> t -> unit
(** Pretty print a method signature with annotations. *)

@ -206,8 +206,7 @@ let callback checks ({Callbacks.summary} as callback_args) : Summary.t =
let calls_this = ref false in
let tenv = Exe_env.get_tenv callback_args.exe_env proc_name in
let annotated_signature =
Models.get_modelled_annotated_signature ~is_trusted_callee:false tenv
(Procdesc.get_attributes proc_desc)
AnnotatedSignature.get_for_class_under_analysis tenv (Procdesc.get_attributes proc_desc)
in
L.debug Analysis Medium "Signature: %a@\n" (AnnotatedSignature.pp proc_name)
annotated_signature ;

@ -1065,19 +1065,29 @@ let typecheck_sil_call_function find_canonical_duplicate checks tenv instr_ref t
in
List.fold_right ~f:handle_et etl ~init:([], typestate)
in
let pname = callee_attributes.ProcAttributes.proc_name in
let callee_annotated_signature =
match
(Procname.get_class_type_name callee_pname, Procname.get_class_type_name curr_pname)
with
| Some callee_class, Some class_under_analysis
when Typ.Name.equal callee_class class_under_analysis ->
(* The method call is to the method in the same class as we are currently analyzing. *)
AnnotatedSignature.get_for_class_under_analysis tenv callee_attributes
| _ ->
(* The call is to external (relatively to the class under analysis) method. Lookup models, trust lists, etc. *)
let is_trusted_callee =
let caller_nullsafe_mode = NullsafeMode.of_procname tenv curr_pname in
let callee_class = Procname.get_class_type_name pname in
let callee_class = Procname.get_class_type_name callee_pname in
Option.value_map callee_class
~f:(NullsafeMode.is_trusted_name caller_nullsafe_mode)
~default:false
in
let callee_annotated_signature =
Models.get_modelled_annotated_signature ~is_trusted_callee tenv callee_attributes
in
if Config.write_html then
L.d_printfln "Callee signature: %a" (AnnotatedSignature.pp pname) callee_annotated_signature ;
L.d_printfln "Callee signature: %a"
(AnnotatedSignature.pp callee_pname)
callee_annotated_signature ;
let signature_params =
drop_unchecked_signature_params callee_attributes callee_annotated_signature
in

@ -36,7 +36,7 @@ class Strict_NoDeps_NoPromos {
}
}
// FIXME - promo is incorrectly calculated as Strict.
// FIXME - promo is incorrectly calculated as Trust None.
class Default_UsesDefault_CanBePromotedToTrustAll_FIXME {
static String f() {
// We use unknown default function. Since we don't support trust some in promotions,
@ -45,8 +45,19 @@ class Default_UsesDefault_CanBePromotedToTrustAll_FIXME {
}
}
// FIXME - promo is incorrectly calculated as Strict.
class Default_UsesLocal_CanBePromotedToTrustNone_FIXME {
class Default_UsesItself_CanBePromotedToStrict {
static String f() {
// We use only the function from its own class. The class can be promoted to strict staight
// ahead.
return g();
}
static String g() {
return "";
}
}
class Default_UsesLocal_CanBePromotedToTrustNone {
static String f() {
// We depend only on a nullsafe method.
// Hence the class can be promoted to "trust none" (but not to strict).
@ -62,17 +73,16 @@ class Default_UsesStrict_CanBePromotedToStrict {
}
}
// FIXME: promo is incorrectly calculated as strict
@Nullsafe(
value = Nullsafe.Mode.LOCAL,
trustOnly = @Nullsafe.TrustList({Default_NoDeps_CanBePromotedToStrict.class}))
class TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone_FIXME {
class TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone {
static String f() {
return Local_NoDeps_CanBePromotedToStrict.f();
}
}
// FIXME: promo is incorrectly calculated as strict
// FIXME: promo is incorrectly calculated as trust none
@Nullsafe(
value = Nullsafe.Mode.LOCAL,
trustOnly = @Nullsafe.TrustList({Default_NoDeps_CanBePromotedToStrict.class}))
@ -82,11 +92,10 @@ class TrustSome_UsesTrusted_NoPromo_FIXME {
}
}
// FIXME: promo is incorrectly calculated as strict
@Nullsafe(
value = Nullsafe.Mode.LOCAL,
trustOnly = @Nullsafe.TrustList({Local_NoDeps_CanBePromotedToStrict.class}))
class TrustSome_TrustToLocalIsNotNeeded_CanBePromotedToTrustNone_FIXME {
class TrustSome_TrustToLocalIsNotNeeded_CanBePromotedToTrustNone {
static String f() {
return Local_NoDeps_CanBePromotedToStrict.f();
}

@ -162,16 +162,17 @@ codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.null
codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.nullsafe_default.MapNullability$TestThatGetIsAllowedOnlyAfterContainsKeyWasChecked.usingGetAfterWrongKeyWasCheckedInWhileLoopIsBAD(java.util.Map):void, 3, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`m.get(...)` is nullable and is not locally checked for null when calling `isEmpty()`: call to Map.get(...) at line 44 (nullable according to nullsafe internal models).]
codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.nullsafe_default.MapNullability$TestThatGetIsAllowedOnlyAfterContainsKeyWasChecked.usingGetAfterWrongKeyWasCheckedIsBAD(java.util.Map):void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`m.get(...)` is nullable and is not locally checked for null when calling `isEmpty()`: call to Map.get(...) at line 29 (nullable according to nullsafe internal models).]
codetoanalyze/java/nullsafe-default/MapNullability.java, codetoanalyze.java.nullsafe_default.MapNullability$TestThatGetIsAllowedOnlyAfterContainsKeyWasChecked.usingGetWithoutCheckingKeyIsBAD(java.util.Map):void, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`m.get(...)` is nullable and is not locally checked for null when calling `isEmpty()`: call to Map.get(...) at line 24 (nullable according to nullsafe internal models).]
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesDefault_CanBePromotedToTrustAll_FIXME is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesDefault_CanBePromotedToTrustAll_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesLocal_CanBePromotedToTrustNone_FIXME is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesLocal_CanBePromotedToTrustNone_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesItself_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesItself_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesLocal_CanBePromotedToTrustNone is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesLocal_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesDefault_CanBePromotedToTrustAll_FIXME is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesDefault_CanBePromotedToTrustAll_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_UsesStrict_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_UsesStrict_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.Default_NoDeps_CanBePromotedToStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], Default_NoDeps_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], Local_NoDeps_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustAll", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_TrustStrictIsNotNeeded_CanBePromotedToStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], Strict_NoDeps_NoPromos, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_UsesTrusted_NoPromo_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_TrustToLocalIsNotNeeded_CanBePromotedToTrustNone_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_DoesNotUseTrusted_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_TrustToLocalIsNotNeeded_CanBePromotedToTrustNone, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/ModePromotions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], TrustSome_UsesTrusted_NoPromo_FIXME, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "LocalTrustSome", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/MyPreconditions.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.MyPreconditions is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], MyPreconditions, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/NestedFieldAccess.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NestedFieldAccess is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NestedFieldAccess, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/NestedFieldAccess.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NestedFieldAccess$C is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NestedFieldAccess$C, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
@ -327,7 +328,7 @@ codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.
codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.nullsafe_default.TestPropagatesNullable$TestSecondParameter.test(java.lang.String,java.lang.String):void, 11, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`nullable(...)` is nullable and is not locally checked for null when calling `length()`.]
codetoanalyze/java/nullsafe-default/PropagatesNullable.java, codetoanalyze.java.nullsafe_default.TestPropagatesNullable$TestSecondParameter.test(java.lang.String,java.lang.String):void, 15, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`nullable(...)` is nullable and is not locally checked for null when calling `length()`.]
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$E is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$E, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$Lambda$_9_1 is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$Lambda$_9_1, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.ReturnNotNullable$Lambda$_9_1 is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], ReturnNotNullable$Lambda$_9_1, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustNone"
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ReturnNotNullable$ConditionalAssignment, codetoanalyze.java.nullsafe_default, issues: 1, curr_mode: "Default"
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], ReturnNotNullable, codetoanalyze.java.nullsafe_default, issues: 9, curr_mode: "Default"
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable$ConditionalAssignment.test(boolean):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`test(...)`: return type is declared non-nullable but the method returns a nullable value: field f1 at line 199.]
@ -342,7 +343,7 @@ codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.n
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`return_null_in_catch()`: return type is declared non-nullable but the method returns `null`: null constant at line 160.]
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.return_null_in_catch_after_throw():java.lang.String, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`return_null_in_catch_after_throw()`: return type is declared non-nullable but the method returns `null`: null constant at line 172.]
codetoanalyze/java/nullsafe-default/ReturnNotNullable.java, codetoanalyze.java.nullsafe_default.ReturnNotNullable.tryWithResourcesReturnNullable(java.lang.String):java.lang.Object, 0, ERADICATE_RETURN_NOT_NULLABLE, no_bucket, WARNING, [`tryWithResourcesReturnNullable(...)`: return type is declared non-nullable but the method returns a nullable value: call to nullToNullableIsOK() at line 142.]
codetoanalyze/java/nullsafe-default/StrictMode.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NonStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NonStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "Strict"
codetoanalyze/java/nullsafe-default/StrictMode.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_CAN_BE_NULLSAFE, no_bucket, ADVICE, [Congrats! Class codetoanalyze.java.nullsafe_default.NonStrict is free of nullability issues. Mark it `@Nullsafe(Nullsafe.Mode.Local)` to prevent regressions.], NonStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default", promote_mode: "LocalTrustAll"
codetoanalyze/java/nullsafe-default/StrictMode.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_IS_NULLSAFE, no_bucket, INFO, [], OtherStrict, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Strict"
codetoanalyze/java/nullsafe-default/StrictMode.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], SomeEnum, codetoanalyze.java.nullsafe_default, issues: 0, curr_mode: "Default"
codetoanalyze/java/nullsafe-default/StrictMode.java, Linters_dummy_method, 1, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], Strict, codetoanalyze.java.nullsafe_default, issues: 17, curr_mode: "Strict"

Loading…
Cancel
Save