[autofix] Try to find proper autofix from list

Summary:
This diff enables it to try multiple autofix candiates and find a
successful one.

Reviewed By: jvillard

Differential Revision:
D63896024

Privacy Context Container: L1208441

fbshipit-source-id: a1cb78ec1bd71aa541059aac29222c7ff4b9ac5b

infer/src/pulse/PulseDiagnostic.ml

@@ -926,7 +926,7 @@ let get_message_and_suggestion diagnostic =
           , Some (get_suggestion_msg source_opt) ) )
 
 
-let make_autofix {Location.file; line; col} ~original ~replacement =
+let make_autofix {Location.file; line; col} candidates =
   let open IOption.Let_syntax in
   let* line_str =
     let file_path = SourceFile.to_abs_path file in
@@ -943,13 +943,14 @@ let make_autofix {Location.file; line; col} ~original ~replacement =
                 if Int.equal line_number line then Stop (Some line_str)
                 else Continue (line_number + 1) ) )
   in
-  if String.is_substring_at line_str ~pos:(col - 1) ~substring:original then
-    Some {Jsonbug_t.original= Some original; replacement= Some replacement; additional= None}
-  else
-    Option.map (String.substr_index line_str ~pattern:original) ~f:(fun idx ->
-        { Jsonbug_t.original= None
-        ; replacement= None
-        ; additional= Some [{line; column= idx + 1; original; replacement}] } )
+  List.find_map candidates ~f:(fun (original, replacement) ->
+      if String.is_substring_at line_str ~pos:(col - 1) ~substring:original then
+        Some {Jsonbug_t.original= Some original; replacement= Some replacement; additional= None}
+      else
+        Option.map (String.substr_index line_str ~pattern:original) ~f:(fun idx ->
+            { Jsonbug_t.original= None
+            ; replacement= None
+            ; additional= Some [{line; column= idx + 1; original; replacement}] } ) )
 
 
 let get_autofix pdesc diagnostic =
@@ -970,17 +971,22 @@ let get_autofix pdesc diagnostic =
         when Procname.is_constructor (Procdesc.get_proc_name pdesc) && is_formal pvar ->
           let param = Pvar.to_string pvar in
           make_autofix location
-            ~original:(F.asprintf "%a(%s)" Fieldname.pp field param)
-            ~replacement:(F.asprintf "%a(std::move(%s))" Fieldname.pp field param)
+            [ ( F.asprintf "%a(%s)" Fieldname.pp field param
+              , F.asprintf "%a(std::move(%s))" Fieldname.pp field param )
+            ; ( F.asprintf "%a{%s}" Fieldname.pp field param
+              , F.asprintf "%a{std::move(%s)}" Fieldname.pp field param )
+            ; ( F.asprintf "%a = %s;" Fieldname.pp field param
+              , F.asprintf "%a = std::move(%s);" Fieldname.pp field param ) ]
       | IntoField {field}, Some (DecompilerExpr.PVar pvar, []) when is_local pvar ->
           let param = Pvar.to_string pvar in
           make_autofix location
-            ~original:(F.asprintf "%a = %s;" Fieldname.pp field param)
-            ~replacement:(F.asprintf "%a = std::move(%s);" Fieldname.pp field param)
+            [ ( F.asprintf "%a = %s;" Fieldname.pp field param
+              , F.asprintf "%a = std::move(%s);" Fieldname.pp field param )
+            ; ( F.asprintf ".%a = %s," Fieldname.pp field param
+              , F.asprintf ".%a = std::move(%s)," Fieldname.pp field param ) ]
       | IntoVar {copied_var= ProgramVar pvar}, Some (DecompilerExpr.PVar _, [MethodCall _]) ->
           let tgt = Pvar.to_string pvar in
-          make_autofix location ~original:(F.asprintf "auto %s = " tgt)
-            ~replacement:(F.asprintf "auto& %s = " tgt)
+          make_autofix location [(F.asprintf "auto %s = " tgt, F.asprintf "auto& %s = " tgt)]
       | _ ->
           None )
   | _ ->

infer/tests/codetoanalyze/cpp/pulse-17/issues.exp

@@ -1,5 +1,5 @@
 codetoanalyze/cpp/pulse-17/closures.cpp, generic_lambda_ok_FP, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is assigned to the null pointer,assigned,invalid access occurs here]
-codetoanalyze/cpp/pulse-17/make_shared.cpp, make_shared_ptr::SharedPtr::SharedPtr, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)]
+codetoanalyze/cpp/pulse-17/make_shared.cpp, make_shared_ptr::SharedPtr::SharedPtr, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)], "s_ptr0 = _s_ptr;"=>"s_ptr0 = std::move(_s_ptr);"
 codetoanalyze/cpp/pulse-17/make_shared.cpp, make_shared_ptr::SharedPtr::SharedPtr, 0, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `std::shared_ptr<float>&`)], "s_ptr1(_s_ptr)"=>"s_ptr1(std::move(_s_ptr))"
 codetoanalyze/cpp/pulse-17/make_shared.cpp, make_shared_ptr::make_shared0_bad, 5, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is assigned to the null pointer,assigned,invalid access occurs here]
 codetoanalyze/cpp/pulse-17/make_shared.cpp, make_shared_ptr::make_shared1_bad, 7, NULLPTR_DEREFERENCE, no_bucket, ERROR, [is assigned to the null pointer,assigned,invalid access occurs here]

infer/tests/codetoanalyze/cpp/pulse/issues.exp

@@ -163,7 +163,7 @@ codetoanalyze/cpp/pulse/path.cpp, FP_only_bad_on_42_latent, 2, NULLPTR_DEREFEREN
 codetoanalyze/cpp/pulse/path.cpp, FN_faulty_call_bad, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [when calling `FP_only_bad_on_42_latent` here,in call to `may_return_null`,is assigned to the null pointer,returned,return from call to `may_return_null`,assigned,invalid access occurs here]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, read_shared_ptr_param_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, read_shared_ptr_param_cond_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used,used]
-codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, SharedPtrField1_Bad::SharedPtrField1_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)]
+codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, SharedPtrField1_Bad::SharedPtrField1_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)], "field = x;"=>"field = std::move(x);"
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, copy_raw_ptr_to_global_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, copy_shared_ptr_to_global_bad, 1, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)]
 codetoanalyze/cpp/pulse/reference_stability.cpp, folly_fastmap_bad, 9, PULSE_REFERENCE_STABILITY, no_bucket, ERROR, [invalidation part of the trace starts here,variable `map` declared here,in call to `folly::F14FastMap::emplace` (modelled),was potentially invalidated by `folly::F14FastMap::emplace`,use-after-lifetime part of the trace starts here,variable `map` declared here,in call to `folly::F14FastMap::begin` (modelled),in call to `folly::f14::detail::ValueContainerIterator::operator->` (modelled),assigned,invalid access occurs here]
@@ -521,7 +521,7 @@ codetoanalyze/cpp/pulse/unnecessary_copy.cpp, ClassWithoutConstructDef::field_se
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, foo, 0, PULSE_CONST_REFABLE, no_bucket, ERROR, [Parameter my_vec with type `std::vector<int,std::allocator<int>>`]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, foo, 0, PULSE_UNNECESSARY_COPY, no_bucket, ERROR, [macro expanded here,copied here (with type `std::vector<int,std::allocator<int>>&`)]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField1_Bad::CopiedToField1_Bad, 0, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `Arr&`)], "field(a)"=>"field(std::move(a))"
-codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField2_Bad::CopiedToField2_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)]
+codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField2_Bad::CopiedToField2_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)], "field = a;"=>"field = std::move(a);"
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField3_Last_Bad::CopiedToField3_Last_Bad, 2, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, PassedToUnknown_Bad::PassedToUnknown_Bad, 0, PULSE_CONST_REFABLE, no_bucket, ERROR, [Parameter a with type `Arr`]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, PassedToUnknown_Bad::PassedToUnknown_Bad, 0, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `Arr&`)]

infer/tests/codetoanalyze/cpp/pulse/issues.exp-11

@@ -176,7 +176,7 @@ codetoanalyze/cpp/pulse/path.cpp, FP_only_bad_on_42_latent, 2, NULLPTR_DEREFEREN
 codetoanalyze/cpp/pulse/path.cpp, FN_faulty_call_bad, 0, NULLPTR_DEREFERENCE, no_bucket, ERROR, [when calling `FP_only_bad_on_42_latent` here,in call to `may_return_null`,is assigned to the null pointer,returned,return from call to `may_return_null`,assigned,invalid access occurs here]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, read_shared_ptr_param_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, read_shared_ptr_param_cond_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used,used]
-codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, SharedPtrField1_Bad::SharedPtrField1_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)]
+codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, SharedPtrField1_Bad::SharedPtrField1_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)], "field = x;"=>"field = std::move(x);"
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, copy_raw_ptr_to_global_bad, 0, PULSE_READONLY_SHARED_PTR_PARAM, no_bucket, ERROR, [Parameter x with type `std::shared_ptr<int>`,used]
 codetoanalyze/cpp/pulse/readonly_shared_ptr_param.cpp, copy_shared_ptr_to_global_bad, 1, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `std::shared_ptr<int>&`)]
 codetoanalyze/cpp/pulse/reference_stability.cpp, folly_fastmap_bad, 9, PULSE_REFERENCE_STABILITY, no_bucket, ERROR, [invalidation part of the trace starts here,variable `map` declared here,in call to `folly::F14FastMap::emplace` (modelled),was potentially invalidated by `folly::F14FastMap::emplace`,use-after-lifetime part of the trace starts here,variable `map` declared here,in call to `folly::F14FastMap::begin` (modelled),in call to `folly::f14::detail::ValueContainerIterator::operator->` (modelled),assigned,invalid access occurs here]
@@ -532,7 +532,7 @@ codetoanalyze/cpp/pulse/unnecessary_copy.cpp, ClassWithoutConstructDef::field_se
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, foo, 0, PULSE_CONST_REFABLE, no_bucket, ERROR, [Parameter my_vec with type `std::vector<int,std::allocator<int>>`]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, foo, 0, PULSE_UNNECESSARY_COPY, no_bucket, ERROR, [macro expanded here,copied here (with type `std::vector<int,std::allocator<int>>&`)]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField1_Bad::CopiedToField1_Bad, 0, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `Arr&`)], "field(a)"=>"field(std::move(a))"
-codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField2_Bad::CopiedToField2_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)]
+codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField2_Bad::CopiedToField2_Bad, 0, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)], "field = a;"=>"field = std::move(a);"
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, CopiedToField3_Last_Bad::CopiedToField3_Last_Bad, 2, PULSE_UNNECESSARY_COPY_ASSIGNMENT, no_bucket, ERROR, [copy assigned here (with type `Arr&`)]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, PassedToUnknown_Bad::PassedToUnknown_Bad, 0, PULSE_CONST_REFABLE, no_bucket, ERROR, [Parameter a with type `Arr`]
 codetoanalyze/cpp/pulse/unnecessary_copy.cpp, PassedToUnknown_Bad::PassedToUnknown_Bad, 0, PULSE_UNNECESSARY_COPY_INTERMEDIATE, no_bucket, ERROR, [copied here (with type `Arr&`)]