[nullsafe] Suppress _some_ errors related to synthetic/autogenerated code

Summary:
Synthetic/autogenerated methods/fields usually contain `$` in their names.
Reporting nullability violations on such code doesn't make much sense
since the violations are not actionable for users and likely need to be
resolved on another level.

This diff contributes:
1. Several test cases that involve synthetic code of different
   complexity.
2. Code that handles some particular types of errors (but not all!).

Reviewed By: mityal

Differential Revision: D22984578

fbshipit-source-id: d25806209
master
Artem Pianykh 4 years ago committed by Facebook GitHub Bot
parent cf29bc7aa2
commit 5bd78660ea

@ -18,6 +18,8 @@ let get_field_name {field_name} = field_name
let is_java {class_name} = Typ.Name.Java.is_class class_name let is_java {class_name} = Typ.Name.Java.is_class class_name
let is_java_synthetic t = is_java t && String.contains (get_field_name t) '$'
module T = struct module T = struct
type nonrec t = t [@@deriving compare] type nonrec t = t [@@deriving compare]
end end

@ -20,6 +20,9 @@ val get_field_name : t -> string
val is_java : t -> bool val is_java : t -> bool
val is_java_synthetic : t -> bool
(** Check if the field is autogenerated/synthetic **)
(** Set for fieldnames *) (** Set for fieldnames *)
module Set : Caml.Set.S with type elt = t module Set : Caml.Set.S with type elt = t

@ -121,12 +121,14 @@ let get_for_class_under_analysis tenv proc_attributes =
{result with nullsafe_mode} {result with nullsafe_mode}
let pp proc_name fmt annotated_signature = let pp_ia fmt ia = if not (List.is_empty ia) then F.fprintf fmt "%a " Annot.Item.pp ia
let pp_ia fmt ia = if not (List.is_empty ia) then F.fprintf fmt "%a " Annot.Item.pp ia in
let pp_annotated_param fmt {mangled; param_annotation_deprecated; param_annotated_type} = let pp_annotated_param fmt {mangled; param_annotation_deprecated; param_annotated_type} =
F.fprintf fmt " %a%a %a" pp_ia param_annotation_deprecated AnnotatedType.pp param_annotated_type F.fprintf fmt " %a%a %a" pp_ia param_annotation_deprecated AnnotatedType.pp param_annotated_type
Mangled.pp mangled Mangled.pp mangled
in
let pp proc_name fmt annotated_signature =
let {ret_annotation_deprecated; ret_annotated_type} = annotated_signature.ret in let {ret_annotation_deprecated; ret_annotated_type} = annotated_signature.ret in
F.fprintf fmt "[%a] %a%a %a (%a )" NullsafeMode.pp annotated_signature.nullsafe_mode pp_ia F.fprintf fmt "[%a] %a%a %a (%a )" NullsafeMode.pp annotated_signature.nullsafe_mode pp_ia
ret_annotation_deprecated AnnotatedType.pp ret_annotated_type ret_annotation_deprecated AnnotatedType.pp ret_annotated_type

@ -82,6 +82,34 @@ type err_instance =
; assignment_type: AssignmentRule.ReportableViolation.assignment_type } ; assignment_type: AssignmentRule.ReportableViolation.assignment_type }
[@@deriving compare] [@@deriving compare]
(** Returns whether we can be certain that an [err_instance] is related to synthetic code,
autogen/codegen).
NOTE: This implementation is very naive and handles only simplest cases (which is actually
enough for our use-case at the moment). The problem here is that in general:
- any expression that has synthetic code inside can be considered "tainted",
- any expression with a "tainted" sub-expression is also "tainted",
- with "tainted" expressions all bets are off and error reporting should be suppressed.
We could handle this properly by introducing [Nullability.Synthetic] but this is quite involved
(and hopefully won't be needed). *)
let is_synthetic_err = function
| Bad_assignment {assignment_type; _} -> (
AssignmentRule.ReportableViolation.(
match assignment_type with
| PassingParamToFunction {function_procname; _} ->
(* NOTE: this currently doesn't cover the case when the actual
argument involves syntethic code *)
Procname.is_java_autogen_method function_procname
| AssigningToField fieldname ->
Fieldname.is_java_synthetic fieldname
| _ ->
false) )
| _ ->
false
let pp_err_instance fmt err_instance = let pp_err_instance fmt err_instance =
match err_instance with match err_instance with
| Condition_redundant _ -> | Condition_redundant _ ->
@ -216,6 +244,8 @@ let get_nonnull_explanation_for_condition_redudant (nonnull_origin : TypeOrigin.
(** If error is reportable to the user, return its information. Otherwise return [None]. *) (** If error is reportable to the user, return its information. Otherwise return [None]. *)
let get_error_info_if_reportable_lazy ~nullsafe_mode err_instance = let get_error_info_if_reportable_lazy ~nullsafe_mode err_instance =
let open IOption.Let_syntax in let open IOption.Let_syntax in
if is_synthetic_err err_instance then None
else
match err_instance with match err_instance with
| Condition_redundant {is_always_true; condition_descr; nonnull_origin} -> | Condition_redundant {is_always_true; condition_descr; nonnull_origin} ->
Some Some

@ -0,0 +1,63 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package codetoanalyze.java.nullsafe;
import android.annotation.SuppressLint;
import javax.annotation.Nullable;
public class SyntheticErrorSuppressions {
static class Fragment {
@SuppressLint("ReturnOverAnnotated")
@Nullable
public static Object getContext() {
return new Object();
}
public static void setContext(final Object context) {}
}
static class ClassWithSyntheticCode {
@Nullable private Object $ul_fieldNullable;
private Object $ul_fieldNotNull;
// This method contains $ which is *extremely* rarely used in user code, but
// used extensively in autogenerated code, therefore is good marker.
private static void $ul_iAmAutogen(
final ClassWithSyntheticCode instance, final Object context) {}
@Nullable
private Object $ul_iAmNullableAutogen() {
return $ul_fieldNullable;
}
public void passingIncorrectParamToSyntheticMethod_OK() {
$ul_iAmAutogen(this, Fragment.getContext());
}
public void assigningAnythingToSyntheticField_OK() {
$ul_fieldNotNull = null;
}
// Following cases are hard to support since synthetic code can be a part of
// a complex expression, and we need some more sophisticated mechanisms to
// handle that. On the bright side, this is not something that happens often
// in the wild.
public void passingSyntheticParamToAnyMethod_OK_FP() {
Fragment.setContext($ul_fieldNullable);
}
public String dereferencingSyntheticNullableField_OK_FP() {
return $ul_fieldNullable.toString();
}
public String dereferencingNullableSyntheticMethodCall_OK_FP() {
return $ul_iAmNullableAutogen().toString();
}
}
}

@ -352,6 +352,10 @@ codetoanalyze/java/nullsafe/SwitchCase.java, Linters_dummy_method, 63, ERADICATE
codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.getNullableColor():codetoanalyze.java.nullsafe.Color, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, ADVICE, [Method `getNullableColor()` is annotated with `@Nullable` but never returns null.] codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.getNullableColor():codetoanalyze.java.nullsafe.Color, 0, ERADICATE_RETURN_OVER_ANNOTATED, no_bucket, ADVICE, [Method `getNullableColor()` is annotated with `@Nullable` but never returns null.]
codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.switchOnNullIsBad():java.lang.String, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [NullPointerException will be thrown at this line! `color` is `null` and is dereferenced via calling `ordinal()`: null constant at line 14.] codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.switchOnNullIsBad():java.lang.String, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [NullPointerException will be thrown at this line! `color` is `null` and is dereferenced via calling `ordinal()`: null constant at line 14.]
codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.switchOnNullableIsBad():java.lang.String, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`color` is nullable and is not locally checked for null when calling `ordinal()`: call to getNullableColor() at line 28.] codetoanalyze/java/nullsafe/SwitchCase.java, codetoanalyze.java.nullsafe.SwitchCase.switchOnNullableIsBad():java.lang.String, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`color` is nullable and is not locally checked for null when calling `ordinal()`: call to getNullableColor() at line 28.]
codetoanalyze/java/nullsafe/SyntheticErrorSuppressions.java, Linters_dummy_method, 13, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], SyntheticErrorSuppressions, codetoanalyze.java.nullsafe, issues: 3, curr_mode: "Default"
codetoanalyze/java/nullsafe/SyntheticErrorSuppressions.java, codetoanalyze.java.nullsafe.SyntheticErrorSuppressions$ClassWithSyntheticCode.dereferencingNullableSyntheticMethodCall_OK_FP():java.lang.String, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`$ul_iAmNullableAutogen(...)` is nullable and is not locally checked for null when calling `toString()`: call to $ul_iAmNullableAutogen() at line 60.]
codetoanalyze/java/nullsafe/SyntheticErrorSuppressions.java, codetoanalyze.java.nullsafe.SyntheticErrorSuppressions$ClassWithSyntheticCode.dereferencingSyntheticNullableField_OK_FP():java.lang.String, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`SyntheticErrorSuppressions$ClassWithSyntheticCode.$ul_fieldNullable` is nullable and is not locally checked for null when calling `toString()`.]
codetoanalyze/java/nullsafe/SyntheticErrorSuppressions.java, codetoanalyze.java.nullsafe.SyntheticErrorSuppressions$ClassWithSyntheticCode.passingSyntheticParamToAnyMethod_OK_FP():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, WARNING, [`SyntheticErrorSuppressions$Fragment.setContext(...)`: parameter #1(`context`) is declared non-nullable but the argument `SyntheticErrorSuppressions$ClassWithSyntheticCode.$ul_fieldNullable` is nullable.]
codetoanalyze/java/nullsafe/TrueFalseOnNull.java, Linters_dummy_method, 17, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], TrueFalseOnNull, codetoanalyze.java.nullsafe, issues: 17, curr_mode: "Default" codetoanalyze/java/nullsafe/TrueFalseOnNull.java, Linters_dummy_method, 17, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], TrueFalseOnNull, codetoanalyze.java.nullsafe, issues: 17, curr_mode: "Default"
codetoanalyze/java/nullsafe/TrueFalseOnNull.java, codetoanalyze.java.nullsafe.TrueFalseOnNull$EarlyReturn.testEarlyReturn(java.lang.CharSequence,java.lang.CharSequence):void, 6, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`s2` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe/TrueFalseOnNull.java, codetoanalyze.java.nullsafe.TrueFalseOnNull$EarlyReturn.testEarlyReturn(java.lang.CharSequence,java.lang.CharSequence):void, 6, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`s2` is nullable and is not locally checked for null when calling `toString()`.]
codetoanalyze/java/nullsafe/TrueFalseOnNull.java, codetoanalyze.java.nullsafe.TrueFalseOnNull$TestNonStaticOneParam.falseOnNullNegativeBranchIsBAD(java.lang.String):void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`s` is nullable and is not locally checked for null when calling `toString()`.] codetoanalyze/java/nullsafe/TrueFalseOnNull.java, codetoanalyze.java.nullsafe.TrueFalseOnNull$TestNonStaticOneParam.falseOnNullNegativeBranchIsBAD(java.lang.String):void, 2, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`s` is nullable and is not locally checked for null when calling `toString()`.]

Loading…
Cancel
Save