From b0191435775186b27fbf51d1ecf0f6d8f5370521 Mon Sep 17 00:00:00 2001 From: Mitya Lyubarskiy Date: Wed, 23 Sep 2020 03:39:13 -0700 Subject: [PATCH] [nullsafe] Record relevant third party method in case of bad param passed. Summary: The previous diffs recorded it for the case when the unvetted value is dereferenced or otherwise used wrongly. This case finishes the work, recording the needed signature for the remaining case (when the offending third party has a non-nullable param with nullable passed inside) Reviewed By: ngorogiannis Differential Revision: D23706679 fbshipit-source-id: e6f641223 --- infer/src/nullsafe/AssignmentRule.ml | 15 ++++++++------- infer/src/nullsafe/AssignmentRule.mli | 4 ++-- infer/src/nullsafe/eradicateChecks.ml | 4 ++-- infer/src/nullsafe/typeErr.ml | 4 ++-- .../tests/codetoanalyze/java/nullsafe/issues.exp | 4 ++-- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/infer/src/nullsafe/AssignmentRule.ml b/infer/src/nullsafe/AssignmentRule.ml index aa003cdb3..e83e30b68 100644 --- a/infer/src/nullsafe/AssignmentRule.ml +++ b/infer/src/nullsafe/AssignmentRule.ml @@ -19,10 +19,10 @@ module ReportableViolation = struct and function_info = { param_signature: AnnotatedSignature.param_signature - ; kind: AnnotatedSignature.kind ; actual_param_expression: string ; param_position: int - ; function_procname: Procname.Java.t } + ; annotated_signature: AnnotatedSignature.t + ; procname: Procname.Java.t } let from nullsafe_mode ({lhs; rhs} as violation) = let falls_under_optimistic_third_party = @@ -74,7 +74,7 @@ module ReportableViolation = struct let mk_issue_for_bad_param_passed - {kind; param_signature; actual_param_expression; param_position; function_procname} + {annotated_signature; param_signature; actual_param_expression; param_position; procname} ~param_nullability_kind ~nullability_evidence ~(make_issue_fn : description:string -> issue_type:IssueType.t -> NullsafeIssue.t) = let nullability_evidence_as_suffix = @@ -110,7 +110,7 @@ module ReportableViolation = struct let suggested_third_party_sig_file = ThirdPartyAnnotationInfo.lookup_related_sig_file_for_proc (ThirdPartyAnnotationGlobalRepo.get_repo ()) - function_procname + procname in let where_to_add_signature = Option.value_map suggested_third_party_sig_file @@ -120,7 +120,7 @@ module ReportableViolation = struct (* this can happen when third party is registered in a deprecated way (not in third party repository) *) ~default:"the third party signature storage" in - let procname_str = Procname.Java.to_simplified_string ~withclass:true function_procname in + let procname_str = Procname.Java.to_simplified_string ~withclass:true procname in let description = Format.asprintf "Third-party %a is missing a signature that would allow passing a nullable to param \ @@ -130,12 +130,13 @@ module ReportableViolation = struct where_to_add_signature in make_issue_fn ~description ~issue_type + |> NullsafeIssue.with_third_party_dependent_methods [(procname, annotated_signature)] | Nullability.LocallyCheckedNonnull | Nullability.LocallyTrustedNonnull | Nullability.UncheckedNonnull | Nullability.StrictNonnull -> let nonnull_evidence = - match kind with + match annotated_signature.kind with | FirstParty | ThirdParty Unregistered -> "" | ThirdParty ModelledInternally -> @@ -149,7 +150,7 @@ module ReportableViolation = struct let description = Format.asprintf "%a: parameter #%d%a is declared non-nullable%s but the argument %s%s." MF.pp_monospaced - (Procname.Java.to_simplified_string ~withclass:true function_procname) + (Procname.Java.to_simplified_string ~withclass:true procname) param_position pp_param_name param_signature.mangled nonnull_evidence argument_description nullability_evidence_as_suffix in diff --git a/infer/src/nullsafe/AssignmentRule.mli b/infer/src/nullsafe/AssignmentRule.mli index 780f57529..323ea3d4f 100644 --- a/infer/src/nullsafe/AssignmentRule.mli +++ b/infer/src/nullsafe/AssignmentRule.mli @@ -32,10 +32,10 @@ module ReportableViolation : sig and function_info = { param_signature: AnnotatedSignature.param_signature - ; kind: AnnotatedSignature.kind ; actual_param_expression: string ; param_position: int - ; function_procname: Procname.Java.t } + ; annotated_signature: AnnotatedSignature.t + ; procname: Procname.Java.t } val make_nullsafe_issue : assignment_location:Location.t -> assignment_type -> t -> NullsafeIssue.t diff --git a/infer/src/nullsafe/eradicateChecks.ml b/infer/src/nullsafe/eradicateChecks.ml index 8f9f35961..ff5c45abe 100644 --- a/infer/src/nullsafe/eradicateChecks.ml +++ b/infer/src/nullsafe/eradicateChecks.ml @@ -422,10 +422,10 @@ let check_call_parameters ({IntraproceduralAnalysis.tenv; _} as analysis_data) ~ ; assignment_type= PassingParamToFunction { param_signature= formal - ; kind= callee_annotated_signature.AnnotatedSignature.kind ; actual_param_expression ; param_position - ; function_procname= callee_pname } }) + ; annotated_signature= callee_annotated_signature + ; procname= callee_pname } }) (Some instr_ref) ~nullsafe_mode in if PatternMatch.type_is_class formal.param_annotated_type.typ then diff --git a/infer/src/nullsafe/typeErr.ml b/infer/src/nullsafe/typeErr.ml index 34c7922c2..8b45e3771 100644 --- a/infer/src/nullsafe/typeErr.ml +++ b/infer/src/nullsafe/typeErr.ml @@ -103,10 +103,10 @@ let is_synthetic_err = function | Bad_assignment {assignment_type; _} -> ( AssignmentRule.ReportableViolation.( match assignment_type with - | PassingParamToFunction {function_procname; _} -> + | PassingParamToFunction {procname; _} -> (* NOTE: this currently doesn't cover the case when the actual argument involves syntethic code *) - Procname.Java.is_autogen_method function_procname + Procname.Java.is_autogen_method procname | AssigningToField fieldname -> Fieldname.is_java_synthetic fieldname | _ -> diff --git a/infer/tests/codetoanalyze/java/nullsafe/issues.exp b/infer/tests/codetoanalyze/java/nullsafe/issues.exp index f4c70ace0..42025ed9f 100644 --- a/infer/tests/codetoanalyze/java/nullsafe/issues.exp +++ b/infer/tests/codetoanalyze/java/nullsafe/issues.exp @@ -128,7 +128,7 @@ codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$La codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$Lambda$_27_1.apply(java.lang.Object):java.lang.Object, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `$bcvar1` of method `Lambdas$Lambda$_27_1.apply(...)` is missing `@Nullable` declaration when overriding `Lambdas$NullableFunction.apply(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.], Lambdas$Lambda$_27_1, codetoanalyze.java.nullsafe codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$Lambda$_28_1.apply(java.lang.Object):java.lang.Object, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `$bcvar1` of method `Lambdas$Lambda$_28_1.apply(...)` is missing `@Nullable` declaration when overriding `Lambdas$NullableFunction.apply(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.], Lambdas$Lambda$_28_1, codetoanalyze.java.nullsafe codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$Lambda$_29_1.apply(java.lang.Object):java.lang.Object, 0, ERADICATE_INCONSISTENT_SUBCLASS_PARAMETER_ANNOTATION, no_bucket, WARNING, [First parameter `$bcvar1` of method `Lambdas$Lambda$_29_1.apply(...)` is missing `@Nullable` declaration when overriding `Lambdas$NullableFunction.apply(...)`. The parent method declared it can handle `null` for this param, so the child should also declare that.], Lambdas$Lambda$_29_1, codetoanalyze.java.nullsafe -codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$NullsafeClass.useJavaUtilFunction_UNSUPPORTED(java.util.function.Function):java.lang.String, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, ERROR, [Third-party `Function.apply(...)` is missing a signature that would allow passing a nullable to param #1. Actual argument `getNullableString()` is nullable. Consider adding the correct signature of `Function.apply(...)` to nullsafe/third-party-signatures/java.sig.], Lambdas$NullsafeClass, codetoanalyze.java.nullsafe +codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$NullsafeClass.useJavaUtilFunction_UNSUPPORTED(java.util.function.Function):java.lang.String, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, ERROR, [Third-party `Function.apply(...)` is missing a signature that would allow passing a nullable to param #1. Actual argument `getNullableString()` is nullable. Consider adding the correct signature of `Function.apply(...)` to nullsafe/third-party-signatures/java.sig.], Lambdas$NullsafeClass, codetoanalyze.java.nullsafe, unvetted_3rd_party:[java.util.function.Function#apply(java.lang.Object)] codetoanalyze/java/nullsafe/Lambdas.java, codetoanalyze.java.nullsafe.Lambdas$NullsafeClass.useJavaUtilFunction_UNSUPPORTED(java.util.function.Function):java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`Function.apply(...)`: `@NullsafeLocal(trust=all)` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 143. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/java.sig.], Lambdas$NullsafeClass, codetoanalyze.java.nullsafe, unvetted_3rd_party:[java.util.function.Function#apply(java.lang.Object)] codetoanalyze/java/nullsafe/LibraryCalls.java, Linters_dummy_method, 16, ERADICATE_META_CLASS_NEEDS_IMPROVEMENT, no_bucket, INFO, [], LibraryCalls, codetoanalyze.java.nullsafe, issues: 5, curr_mode: "Default" codetoanalyze/java/nullsafe/LibraryCalls.java, codetoanalyze.java.nullsafe.LibraryCalls.badAtomicReferenceDereference(java.util.concurrent.atomic.AtomicReference):java.lang.String, 1, ERADICATE_NULLABLE_DEREFERENCE, no_bucket, WARNING, [`ref.get()` is nullable and is not locally checked for null when calling `toString()`: call to AtomicReference.get() at line 35 (nullable according to nullsafe internal models).], LibraryCalls, codetoanalyze.java.nullsafe @@ -357,7 +357,7 @@ codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nul codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.dereferenceUnspecifiedIsBAD():void, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.returnUnspecified()`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 42. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#returnUnspecified()] codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.genericExtendsStringRepresentation(java.lang.String,java.util.List):java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.genericString(...)`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 97. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#genericString(java.lang.String, java.util.List)] codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.genericObjectRepresentation(java.lang.String,java.util.List):java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.generic(...)`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 90. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#generic(java.lang.Object, java.util.List)] -codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.passingNullableParamToUnspecifiedIsBAD():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, ERROR, [Third-party `ThirdPartyTestClass.paramUnspecified(...)` is missing a signature that would allow passing a nullable to param #1(`param`). Actual argument `getNullable()` is nullable. Consider adding the correct signature of `ThirdPartyTestClass.paramUnspecified(...)` to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe +codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.passingNullableParamToUnspecifiedIsBAD():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, ERROR, [Third-party `ThirdPartyTestClass.paramUnspecified(...)` is missing a signature that would allow passing a nullable to param #1(`param`). Actual argument `getNullable()` is nullable. Consider adding the correct signature of `ThirdPartyTestClass.paramUnspecified(...)` to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#paramUnspecified(java.lang.String)] codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.passingNullableToParamSpecifiedAsNonnullIsBAD():void, 1, ERADICATE_PARAMETER_NOT_NULLABLE, no_bucket, ERROR, [`ThirdPartyTestClass.secondParamSpecifiedAsNonnull(...)`: parameter #2(`specifiedAsNonnull`) is declared non-nullable (see nullsafe/third-party-signatures/some.test.pckg.sig at line 3) but the argument `getNullable()` is nullable.], StrictModeForThirdParty, codetoanalyze.java.nullsafe codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.varargGenericRepresentation(java.lang.String):java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.varargGeneric(...)`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 116. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#varargGeneric(java.lang.Object, java.lang.Object[])] codetoanalyze/java/nullsafe/StrictModeForThirdParty.java, codetoanalyze.java.nullsafe.StrictModeForThirdParty.varargRepresentation(java.lang.String):java.lang.String, 1, ERADICATE_UNVETTED_THIRD_PARTY_IN_NULLSAFE, no_bucket, ERROR, [`ThirdPartyTestClass.vararg(...)`: `@NullsafeStrict` mode prohibits using values coming from not vetted third party methods without a check. Result of this call is used at line 109. Either add a local check for null or assertion, or add the correct signature to nullsafe/third-party-signatures/some.test.pckg.sig.], StrictModeForThirdParty, codetoanalyze.java.nullsafe, unvetted_3rd_party:[some.test.pckg.ThirdPartyTestClass#vararg(java.lang.String, java.lang.String[])]