From 064e211e39fbc6ac98ca878d8f01e11763c45961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ezgi=20=C3=87i=C3=A7ek?= Date: Thu, 10 Oct 2019 03:27:10 -0700 Subject: [PATCH] [inefficient-keyset] Add androidx.collections.SimpleArrayMap to eligible maps Summary: [androidx.collection.SimpleArrayMap](https://developer.android.com/reference/androidx/collection/SimpleArrayMap.html) also has `keySet` and `entrySet` methods which make them eligible for inefficient keyset checker. Let's add it. Title Reviewed By: skcho Differential Revision: D17831594 fbshipit-source-id: 32e831e18 --- infer/src/absint/PatternMatch.ml | 2 ++ infer/src/absint/PatternMatch.mli | 3 +++ infer/src/checkers/inefficientKeysetIterator.ml | 9 ++++++--- .../java/inefficientKeysetIterator/Test.java | 8 ++++++++ .../java/inefficientKeysetIterator/issues.exp | 1 + 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/infer/src/absint/PatternMatch.ml b/infer/src/absint/PatternMatch.ml index 954be1504..e7cad8017 100644 --- a/infer/src/absint/PatternMatch.ml +++ b/infer/src/absint/PatternMatch.ml @@ -95,6 +95,8 @@ let implements_nio class_name = implements ("java.nio." ^ class_name) let implements_map = implements "java.util.Map" +let implements_androidx_map = implements "androidx.collection.SimpleArrayMap" + let implements_set = implements "java.util.Set" let implements_map_entry = implements "java.util.Map$Entry" diff --git a/infer/src/absint/PatternMatch.mli b/infer/src/absint/PatternMatch.mli index 6f279ccc9..b2e4e90f9 100644 --- a/infer/src/absint/PatternMatch.mli +++ b/infer/src/absint/PatternMatch.mli @@ -70,6 +70,9 @@ val implements_nio : string -> Tenv.t -> string -> bool val implements_map : Tenv.t -> string -> bool (** Check whether class implements a Java's Map *) +val implements_androidx_map : Tenv.t -> string -> bool +(** Check whether class implements a AndroidX's Map *) + val implements_set : Tenv.t -> string -> bool (** Check whether class implements a Java's Set *) diff --git a/infer/src/checkers/inefficientKeysetIterator.ml b/infer/src/checkers/inefficientKeysetIterator.ml index ae5415819..2c1d17589 100644 --- a/infer/src/checkers/inefficientKeysetIterator.ml +++ b/infer/src/checkers/inefficientKeysetIterator.ml @@ -26,6 +26,10 @@ let find_first_arg_id ~fun_name ~class_name_f ~lhs_f = function None +let implements_map tenv s = + PatternMatch.implements_map tenv s || PatternMatch.implements_androidx_map tenv s + + (** If given a node that has 4 instructions and calls fun_name, pickup bcvarY, i.e. variable for the first argument n$X = *&$bcvarY @@ -52,8 +56,7 @@ let report_matching_get tenv summary pvar loop_nodes : unit = let instrs = Procdesc.Node.get_instrs node in if Instrs.count instrs >= 5 then let instr_arr = Instrs.get_underlying_not_reversed instrs in - find_first_arg_id ~fun_name:"get" - ~class_name_f:(PatternMatch.implements_map tenv) + find_first_arg_id ~fun_name:"get" ~class_name_f:(implements_map tenv) ~lhs_f:(fun _ -> true) instr_arr.(3) |> Option.iter ~f:(fun arg_id -> @@ -113,7 +116,7 @@ let checker Callbacks.{summary; exe_env} : Summary.t = when_dominating_preds_satisfy idom loop_head ~fun_name:"iterator" ~class_name_f:(PatternMatch.implements_set tenv) ~f:(fun itr_node _ -> when_dominating_preds_satisfy idom itr_node ~fun_name:"keySet" - ~class_name_f:(PatternMatch.implements_map tenv) ~f:(fun _keySet_node get_pvar -> + ~class_name_f:(implements_map tenv) ~f:(fun _keySet_node get_pvar -> report_matching_get tenv summary get_pvar loop_nodes ) ) ) loop_head_to_loop_nodes ; summary diff --git a/infer/tests/codetoanalyze/java/inefficientKeysetIterator/Test.java b/infer/tests/codetoanalyze/java/inefficientKeysetIterator/Test.java index 58f29fb07..74dad08d5 100644 --- a/infer/tests/codetoanalyze/java/inefficientKeysetIterator/Test.java +++ b/infer/tests/codetoanalyze/java/inefficientKeysetIterator/Test.java @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ import android.os.Bundle; +import androidx.collection.ArrayMap; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -92,4 +93,11 @@ class Test { Object t = extras.get(key); } } + + // ArrayMap extends SimpleMap. + void inefficient_arraymap_loop_bad(ArrayMap arrayMap) { + for (String key : arrayMap.keySet()) { + arrayMap.get(key); + } + } } diff --git a/infer/tests/codetoanalyze/java/inefficientKeysetIterator/issues.exp b/infer/tests/codetoanalyze/java/inefficientKeysetIterator/issues.exp index ce0b6d014..a6d371010 100644 --- a/infer/tests/codetoanalyze/java/inefficientKeysetIterator/issues.exp +++ b/infer/tests/codetoanalyze/java/inefficientKeysetIterator/issues.exp @@ -1,3 +1,4 @@ +codetoanalyze/java/inefficientKeysetIterator/Test.java, Test.inefficient_arraymap_loop_bad(androidx.collection.ArrayMap):void, 2, INEFFICIENT_KEYSET_ITERATOR, no_bucket, ERROR, [Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.] codetoanalyze/java/inefficientKeysetIterator/Test.java, Test.inefficient_loop_bad(java.util.HashMap):void, 2, INEFFICIENT_KEYSET_ITERATOR, no_bucket, ERROR, [Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.] codetoanalyze/java/inefficientKeysetIterator/Test.java, Test.inefficient_loop_itr_bad(java.util.HashMap):void, 5, INEFFICIENT_KEYSET_ITERATOR, no_bucket, ERROR, [Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.] codetoanalyze/java/inefficientKeysetIterator/Test.java, Test.inefficient_loop_itr_heur_bad(java.util.HashMap):void, 6, INEFFICIENT_KEYSET_ITERATOR, no_bucket, ERROR, [Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.]