Add readability-container-size-empty

Summary: Add `readability-container-size-empty` check. Makes code more explicit and usually shorter. Reviewed By: justinjhendrick Differential Revision: D19857775 fbshipit-source-id: f1c12bbeaad060106461792a962d7606c9dad1da
arkdnz · Feb 14, 2020 · ba53109 · ba53109
1 parent 8a497dc
commit ba53109
Show file tree

Hide file tree

Showing 81 changed files with 213 additions and 208 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,2 +1,2 @@
-Checks: '-*,modernize-make-shared,performance-*,-performance-inefficient-string-concatenation,readability-const-return-type'
+Checks: '-*,modernize-make-shared,performance-*,-performance-inefficient-string-concatenation,readability-const-return-type,readability-container-size-empty'
 WarningsAsErrors: '*'
diff --git a/libredex/ConfigFiles.cpp b/libredex/ConfigFiles.cpp
@@ -29,7 +29,7 @@ ConfigFiles::ConfigFiles(const Json::Value& config, const std::string& outdir)
         config.get("default_coldstart_classes", "").asString();
   }
 
-  if (m_profiled_methods_filename != "") {
+  if (!m_profiled_methods_filename.empty()) {
     load_method_to_weight();
   }
   load_method_sorting_whitelisted_substrings();
@@ -52,7 +52,7 @@ const std::unordered_set<DexType*>& ConfigFiles::get_no_optimizations_annos() {
     Json::Value no_optimizations_anno;
     m_json.get("no_optimizations_annotations", Json::nullValue,
                no_optimizations_anno);
-    if (no_optimizations_anno != Json::nullValue) {
+    if (!no_optimizations_anno.empty()) {
       for (auto const& config_anno_name : no_optimizations_anno) {
         std::string anno_name = config_anno_name.asString();
         DexType* anno = DexType::get_type(anno_name.c_str());
@@ -70,7 +70,7 @@ const std::unordered_set<DexMethodRef*>& ConfigFiles::get_pure_methods() {
   if (m_pure_methods.empty()) {
     Json::Value pure_methods;
     m_json.get("pure_methods", Json::nullValue, pure_methods);
-    if (pure_methods != Json::nullValue) {
+    if (!pure_methods.empty()) {
       for (auto const& method_name : pure_methods) {
         std::string name = method_name.asString();
         DexMethodRef* method = DexMethod::get_method(name);
@@ -171,7 +171,7 @@ void ConfigFiles::load_method_sorting_whitelisted_substrings() {
   Json::Value json_result;
   json_cfg.get("method_sorting_whitelisted_substrings", Json::nullValue,
                json_result);
-  if (json_result != Json::nullValue) {
+  if (!json_result.empty()) {
     for (auto const& json_element : json_result) {
       m_method_sorting_whitelisted_substrings.insert(json_element.asString());
     }
@@ -194,10 +194,10 @@ void ConfigFiles::ensure_agg_method_stats_loaded() {
 void ConfigFiles::load_inliner_config(inliner::InlinerConfig* inliner_config) {
   Json::Value config;
   m_json.get("inliner", Json::nullValue, config);
-  if (config == Json::nullValue) {
+  if (config.empty()) {
     m_json.get("MethodInlinePass", Json::nullValue, config);
   }
-  if (config == Json::nullValue) {
+  if (config.empty()) {
     fprintf(stderr, "WARNING: No inliner config\n");
     return;
   }

diff --git a/libredex/ControlFlow.cpp b/libredex/ControlFlow.cpp
@@ -570,7 +570,7 @@ ControlFlowGraph::ControlFlowGraph(IRList* ir,
                                    reg_t registers_size,
                                    bool editable)
     : m_registers_size(registers_size), m_editable(editable) {
-  always_assert_log(ir->size() > 0, "IRList contains no instructions");
+  always_assert_log(!ir->empty(), "IRList contains no instructions");
 
   BranchToTargets branch_to_targets;
   TryEnds try_ends;
@@ -921,7 +921,7 @@ void ControlFlowGraph::remove_empty_blocks() {
 
     const auto& succs = get_succ_edges_if(
         b, [](const Edge* e) { return e->type() != EDGE_GHOST; });
-    if (succs.size() > 0) {
+    if (!succs.empty()) {
       always_assert_log(succs.size() == 1,
                         "too many successors for empty block %d:\n%s",
                         it->first, SHOW(*this));

diff --git a/libredex/DexClass.cpp b/libredex/DexClass.cpp
@@ -416,7 +416,7 @@ int DexCode::encode(DexOutputIdx* dodx, uint32_t* output) {
     opc->encode(dodx, insns);
   }
   code->insns_size = (uint32_t)(insns - ((uint16_t*)(code + 1)));
-  if (m_tries.size() == 0)
+  if (m_tries.empty())
     return ((code->insns_size * sizeof(uint16_t)) + sizeof(dex_code_item));
   /*
    * Now the tries..., obscenely messy encoding :(
@@ -836,8 +836,8 @@ bool DexClass::has_class_data() const {
 int DexClass::encode(DexOutputIdx* dodx,
                      dexcode_to_offset& dco,
                      uint8_t* output) {
-  if (m_sfields.size() == 0 && m_ifields.size() == 0 &&
-      m_dmethods.size() == 0 && m_vmethods.size() == 0) {
+  if (m_sfields.empty() && m_ifields.empty() && m_dmethods.empty() &&
+      m_vmethods.empty()) {
     opt_warn(PURE_ABSTRACT_CLASS,
              "'%s' super '%s' flags 0x%08x\n",
              m_self->get_name()->c_str(),

diff --git a/libredex/DexOutput.cpp b/libredex/DexOutput.cpp
@@ -787,7 +787,7 @@ void DexOutput::generate_typelist_data() {
   uint32_t tl_start = m_offset;
   size_t num_tls = 0;
   for (DexTypeList* tl : typel) {
-    if (tl->get_type_list().size() == 0) {
+    if (tl->get_type_list().empty()) {
       m_tl_emit_offsets[tl] = 0;
       continue;
     }
@@ -1090,7 +1090,7 @@ void DexOutput::generate_static_values() {
       }
     }
   }
-  if (m_static_values.size() || m_call_site_items.size()) {
+  if (!m_static_values.empty() || !m_call_site_items.empty()) {
     insert_map_item(TYPE_ENCODED_ARRAY_ITEM, (uint32_t)enc_arrays.size(),
                     sv_start, m_offset - sv_start);
   }
@@ -1438,7 +1438,7 @@ uint32_t emit_instruction_offset_debug_info(
     //   2.1.2) Filter out methods who increase uncompressed APK size
 
     auto& sizes = pts.second;
-    always_assert(sizes.size() > 0);
+    always_assert(!sizes.empty());
 
     // 2.1.1) In Android 8+ there's a background optimizer service that
     // automatically runs dex2oat with a profile collected by the runtime JIT.

diff --git a/libredex/DexPosition.cpp b/libredex/DexPosition.cpp
@@ -46,7 +46,7 @@ uint32_t RealPositionMapper::position_to_line(DexPosition* pos) {
 }
 
 void RealPositionMapper::write_map() {
-  if (m_filename_v2 != "") {
+  if (!m_filename_v2.empty()) {
     write_map_v2();
   }
 }
@@ -133,7 +133,7 @@ void RealPositionMapper::write_map_v2() {
 }
 
 PositionMapper* PositionMapper::make(const std::string& map_filename_v2) {
-  if (map_filename_v2 == "") {
+  if (map_filename_v2.empty()) {
     // If no path is provided for the map, just pass the original line numbers
     // through to the output. This does mean that the line numbers will be
     // incorrect for inlined code.

diff --git a/libredex/DexTypeDomain.h b/libredex/DexTypeDomain.h
@@ -110,7 +110,8 @@ class DexTypeDomain final
       break;
     }
     case AbstractValueKind::Value: {
-      out << show(x.get_dex_type());
+      auto type = x.get_dex_type();
+      out << (type ? show(*type) : std::string("<NONE>"));
       break;
     }
     }

diff --git a/libredex/HierarchyUtil.cpp b/libredex/HierarchyUtil.cpp
@@ -22,7 +22,7 @@ std::unordered_set<const DexMethod*> find_non_overridden_virtuals(
       for (auto* method : cls->get_vmethods()) {
         const auto& overrides =
             mog::get_overriding_methods(*override_graph, method);
-        if (overrides.size() == 0) {
+        if (overrides.empty()) {
           non_overridden_virtuals.emplace(method);
         }
       }

diff --git a/libredex/IRAssembler.cpp b/libredex/IRAssembler.cpp
@@ -156,7 +156,7 @@ std::vector<s_expr> to_s_exprs(
       }
     }
     auto result = to_s_exprs(snay, positions_emitted);
-    always_assert(positions_emitted->size() > 0);
+    always_assert(!positions_emitted->empty());
     auto parent_idx = positions_emitted->size() - 1;
     positions_emitted->push_back(pos);
     auto pos_idx = positions_emitted->size() - 1;
@@ -617,7 +617,7 @@ s_expr to_s_expr(const IRCode* code) {
         auto prev = std::prev(it);
         if (can_merge(prev, it)) {
           auto& label_strs = label_refs.at(prev->target->src->insn);
-          if (label_strs.size() > 0) {
+          if (!label_strs.empty()) {
             const auto& label_name = label_strs.back();
             label_refs[bt->src->insn].push_back(label_name);
             break;
@@ -758,7 +758,7 @@ std::unique_ptr<IRCode> ircode_from_s_expr(const s_expr& e) {
         std::string catch_name;
         s_patn({s_patn(&catch_name)})
             .must_match(tail, "try marker is missing a name");
-        always_assert(catch_name != "");
+        always_assert(!catch_name.empty());
         auto try_marker = new MethodItemEntry(is_start ? TRY_START : TRY_END,
                                               catches.at(catch_name));
         code->push_back(*try_marker);
@@ -780,7 +780,8 @@ std::unique_ptr<IRCode> ircode_from_s_expr(const s_expr& e) {
               .must_match(tail, "catch marker is missing a name");
         }
         // Get the type name, for example: "LCatchType;"
-        always_assert_log(this_catch != "", "catch marker is missing a name");
+        always_assert_log(!this_catch.empty(),
+                          "catch marker is missing a name");
         std::string type_name;
         // nullptr is a valid catch type. It means catch all exceptions.
         DexType* catch_type = nullptr;
@@ -789,7 +790,7 @@ std::unique_ptr<IRCode> ircode_from_s_expr(const s_expr& e) {
         }
         MethodItemEntry* catch_marker = catches.at(this_catch);
         catch_marker->centry->catch_type = catch_type;
-        if (next_catch != "") {
+        if (!next_catch.empty()) {
           catch_marker->centry->next = catches.at(next_catch);
         }
         code->push_back(*catch_marker);

diff --git a/libredex/JsonWrapper.cpp b/libredex/JsonWrapper.cpp
@@ -67,6 +67,7 @@ void JsonWrapper::get(const char* name,
                       const std::vector<std::string>& dflt,
                       std::vector<std::string>& param) const {
   auto it = m_config[name];
+  // NOLINTNEXTLINE(readability-container-size-empty)
   if (it == Json::nullValue) {
     param = dflt;
   } else {
@@ -82,6 +83,7 @@ void JsonWrapper::get(const char* name,
                       std::unordered_set<std::string>& param) const {
   auto it = m_config[name];
   param.clear();
+  // NOLINTNEXTLINE(readability-container-size-empty)
   if (it == Json::nullValue) {
     param.insert(dflt.begin(), dflt.end());
   } else {
@@ -97,6 +99,7 @@ void JsonWrapper::get(
     std::unordered_map<std::string, std::vector<std::string>>& param) const {
   auto cfg = m_config[name];
   param.clear();
+  // NOLINTNEXTLINE(readability-container-size-empty)
   if (cfg == Json::nullValue) {
     param = dflt;
   } else {
@@ -142,4 +145,4 @@ const Json::Value& JsonWrapper::operator[](const char* name) const {
 
 bool JsonWrapper::contains(const char* name) const {
   return m_config.isMember(name);
-}
+}
diff --git a/libredex/MethodOverrideGraph.cpp b/libredex/MethodOverrideGraph.cpp
@@ -296,7 +296,7 @@ bool is_true_virtual(const Graph& graph, const DexMethod* method) {
     return true;
   }
   const auto& node = graph.get_node(method);
-  return node.parents.size() != 0 || node.children.size() != 0;
+  return !node.parents.empty() || !node.children.empty();
 }
 
 std::unordered_set<DexMethod*> get_non_true_virtuals(const Graph& graph,

diff --git a/libredex/MethodProfiles.cpp b/libredex/MethodProfiles.cpp
@@ -23,7 +23,7 @@ constexpr double MINIMUM_APPEAR_PERCENT = 80.0;
 
 bool MethodProfiles::parse_stats_file(const std::string& csv_filename) {
   TRACE(METH_PROF, 3, "input csv filename: %s", csv_filename.c_str());
-  if (csv_filename == "") {
+  if (csv_filename.empty()) {
     TRACE(METH_PROF, 2, "No csv file given");
     return false;
   }

diff --git a/libredex/PointsToSemantics.cpp b/libredex/PointsToSemantics.cpp
@@ -1236,7 +1236,7 @@ class PointsToActionGenerator final {
       return PointsToVariable();
     }
     auto anchors = s.elements();
-    if (anchors.size() == 0) {
+    if (anchors.empty()) {
       // The denotation of the anchor set is just the `null` reference. This is
       // represented by a special points-to variable.
       return PointsToVariable::null_variable();

diff --git a/libredex/ProguardLexer.cpp b/libredex/ProguardLexer.cpp
@@ -253,15 +253,15 @@ vector<unique_ptr<Token>> lex(istream& config) {
       if (command == "include") {
         tokens.push_back(unique_ptr<Token>(new Include(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
       }
       if (command == "basedirectory") {
         tokens.push_back(unique_ptr<Token>(new BaseDirectory(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
@@ -296,31 +296,31 @@ vector<unique_ptr<Token>> lex(istream& config) {
       if (command == "printmapping") {
         tokens.push_back(unique_ptr<Token>(new PrintMapping(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
       }
       if (command == "printconfiguration") {
         tokens.push_back(unique_ptr<Token>(new PrintConfiguration(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
       }
       if (command == "printseeds") {
         tokens.push_back(unique_ptr<Token>(new PrintSeeds(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
       }
       if (command == "target") {
         tokens.push_back(unique_ptr<Token>(new Target(line)));
         string version = read_target_version(config, &line);
-        if (version != "") {
+        if (!version.empty()) {
           tokens.push_back(unique_ptr<Token>(new TargetVersion(line, version)));
         }
         continue;
@@ -370,7 +370,7 @@ vector<unique_ptr<Token>> lex(istream& config) {
       if (command == "printusage") {
         tokens.push_back(unique_ptr<Token>(new PrintUsage(line)));
         string path = read_path(config, &line);
-        if (path != "") {
+        if (!path.empty()) {
           tokens.push_back(unique_ptr<Token>(new Filepath(line, path)));
         }
         continue;
@@ -418,7 +418,7 @@ vector<unique_ptr<Token>> lex(istream& config) {
       if (command == "repackageclasses") {
         tokens.push_back(unique_ptr<Token>(new RepackageClasses(line)));
         string package_name = parse_package_name(config, &line);
-        if (package_name != "") {
+        if (!package_name.empty()) {
           tokens.push_back(
               unique_ptr<Token>(new Identifier(line, package_name)));
         }

diff --git a/libredex/ProguardMatcher.cpp b/libredex/ProguardMatcher.cpp
@@ -756,7 +756,7 @@ void ProguardMatcher::process_keep(const KeepSpecSet& keep_rules,
 
     // This is also very fast. Process it in the main thread, too.
     const auto& extendsClassName = keep_rule.class_spec.extendsClassName;
-    if (extendsClassName != "" &&
+    if (!extendsClassName.empty() &&
         !classname_contains_wildcard(extendsClassName)) {
       DexClass* super = find_single_class(extendsClassName);
       if (super != nullptr) {

diff --git a/libredex/Purity.cpp b/libredex/Purity.cpp
@@ -400,7 +400,7 @@ size_t compute_locations_closure(
   for (const auto& p : method_lads) {
     auto method = p.first;
     auto& lads = p.second;
-    if (lads.dependencies.size()) {
+    if (!lads.dependencies.empty()) {
       for (auto d : lads.dependencies) {
         inverse_dependencies[d].insert(method);
       }
@@ -418,7 +418,7 @@ size_t compute_locations_closure(
   // dependencies, and have the Locations as the abstract domain.
 
   size_t iterations = 0;
-  while (impacted_methods.size()) {
+  while (!impacted_methods.empty()) {
     iterations++;
     // We order the impacted methods in a deterministic way that's likely
     // helping to reduce the number of needed iterations.
@@ -496,7 +496,7 @@ size_t compute_locations_closure(
         }
       }
 
-      if (!entries.size()) {
+      if (entries.empty()) {
         // remove inverse dependency
         inverse_dependencies.erase(changed_method);
       } else {

diff --git a/libredex/Reachability.cpp b/libredex/Reachability.cpp
@@ -341,7 +341,7 @@ void TransitiveClosureMarker::gather_and_push(DexMethod* meth) {
   auto* cls = type_class(type);
   auto refs = gather(meth);
   bool check_strings = m_ignore_sets.keep_class_in_string;
-  if (!check_strings && refs.strings.size() > 0 && has_class_forname(meth)) {
+  if (!check_strings && !refs.strings.empty() && has_class_forname(meth)) {
     check_strings = true;
   }
   if (m_ignore_sets.string_literals.count(type)) {
@@ -395,7 +395,7 @@ void TransitiveClosureMarker::visit_cls(const DexClass* cls) {
     } else if (method::is_init(m)) {
       // Push the parameterless constructor, in case it's constructed via
       // .class or Class.forName()
-      if (m->get_proto()->get_args()->get_type_list().size() == 0) {
+      if (m->get_proto()->get_args()->get_type_list().empty()) {
         push(cls, m);
       }
     }