Reorganize functions for checking Java identifiers (#859)

Summary:


I made the misguided choice of refactoring and reusing some methods from ProguardLexer.cpp, and broke the build on Windows for a reason I cannot understand (`TokenType` enum values in ProguardLexer.h somehow were not parsed as valid anymore???? Dunno).

Revert that part of the change, and instead move the useful methods from ProguardLexer to the `java_names` namespace in DexUtil.h.

Differential Revision: D56921682

libredex/DexUtil.cpp

@@ -466,3 +466,29 @@ bool is_valid_identifier(std::string_view s) {
   }
   return true;
 }
+
+namespace java_names {
+
+namespace {
+bool is_not_idenfitier_character(char ch) {
+  return ch == '=' || ch == '+' || ch == '|' || ch == '@' || ch == '#' ||
+         ch == '^' || ch == '&' || ch == '"' || ch == '\'' || ch == '`' ||
+         ch == '~' || ch == '-';
+}
+} // namespace
+
+// Differs from above "is_valid_identifier" function since this is for external
+// names
+bool is_identifier(const std::string_view& ident) {
+  for (const char& ch : ident) {
+    // java identifiers can be multi-lingual so membership testing is complex.
+    // much simpler to test for what is definitely not an identifier and then
+    // assume everything else is a legal identifier char, accepting that we
+    // will have false positives.
+    if (is_deliminator(ch) || is_not_idenfitier_character(ch)) {
+      return false;
+    }
+  }
+  return true;
+}
+} // namespace java_names

libredex/DexUtil.h

@@ -9,6 +9,7 @@
 
 #include <algorithm>
 #include <boost/algorithm/string/predicate.hpp>
+#include <cctype>
 #include <functional>
 #include <string_view>
 #include <unordered_set>
@@ -387,4 +388,13 @@ inline std::string package_name(std::string_view type_name) {
     return nice_name;
   }
 }
+
+inline bool is_deliminator(char ch) {
+  return isspace(ch) || ch == '{' || ch == '}' || ch == '(' || ch == ')' ||
+         ch == ',' || ch == ';' || ch == ':' || ch == '#';
+}
+
+// An identifier can refer to a class name, a field name or a package name.
+// https://docs.oracle.com/javase/specs/jls/se16/html/jls-3.html#jls-JavaLetter
+bool is_identifier(const std::string_view& ident);
 } // namespace java_names

libredex/ProguardLexer.cpp

@@ -15,6 +15,7 @@
 #include <vector>
 
 #include "Debug.h"
+#include "DexUtil.h"
 #include "Macros.h"
 #include "ProguardLexer.h"
 
@@ -30,17 +31,6 @@ constexpr char kPathDelim =
     ':';
 #endif
 
-bool is_deliminator(char ch) {
-  return isspace(ch) || ch == '{' || ch == '}' || ch == '(' || ch == ')' ||
-         ch == ',' || ch == ';' || ch == ':' || ch == '#';
-}
-
-bool is_not_idenfitier_character(char ch) {
-  return ch == '=' || ch == '+' || ch == '|' || ch == '@' || ch == '#' ||
-         ch == '^' || ch == '&' || ch == '"' || ch == '\'' || ch == '`' ||
-         ch == '~' || ch == '-';
-}
-
 void skip_whitespace(std::string_view& data, unsigned int* line) {
   size_t index = 0;
   for (; index != data.size(); ++index) {
@@ -213,21 +203,6 @@ using UnorderedStringViewIndexableMap = multi_index_container<
 
 } // namespace
 
-// An identifier can refer to a class name, a field name or a package name.
-// https://docs.oracle.com/javase/specs/jls/se16/html/jls-3.html#jls-JavaLetter
-bool is_identifier(const std::string_view& ident) {
-  for (const char& ch : ident) {
-    // java identifiers can be multi-lingual so membership testing is complex.
-    // much simpler to test for what is definitely not an identifier and then
-    // assume everything else is a legal identifier char, accepting that we
-    // will have false positives.
-    if (is_deliminator(ch) || is_not_idenfitier_character(ch)) {
-      return false;
-    }
-  }
-  return true;
-}
-
 std::string Token::show() const {
   switch (type) {
   case TokenType::openCurlyBracket:
@@ -715,8 +690,8 @@ std::vector<Token> lex(const std::string_view& in) {
     // Check for commands.
     if (ch == '-') {
       data = data.substr(1);
-      auto command =
-          parse_part_fn</*kSkipWs=*/false>(data, &line, is_deliminator);
+      auto command = parse_part_fn</*kSkipWs=*/false>(
+          data, &line, java_names::is_deliminator);
 
       {
         auto it = simple_commands.find(command);
@@ -789,7 +764,8 @@ std::vector<Token> lex(const std::string_view& in) {
     // At this point:
     // * data is not empty
     // * it does not start with a deliminator
-    auto word = parse_part_fn</*kSkipWs=*/false>(data, &line, is_deliminator);
+    auto word = parse_part_fn</*kSkipWs=*/false>(
+        data, &line, java_names::is_deliminator);
 
     {
       auto it = word_tokens.find(word);
@@ -811,7 +787,7 @@ std::vector<Token> lex(const std::string_view& in) {
       continue;
     }
 
-    if (is_identifier(word)) {
+    if (java_names::is_identifier(word)) {
       add_token_data(TokenType::identifier, word);
       continue;
     }

libredex/ProguardLexer.h

@@ -26,10 +26,6 @@
 namespace keep_rules {
 namespace proguard_parser {
 
-// An identifier can refer to a class name, a field name or a package name.
-// https://docs.oracle.com/javase/specs/jls/se16/html/jls-3.html#jls-JavaLetter
-bool is_identifier(const std::string_view& ident);
-
 enum class TokenType {
   openCurlyBracket,
   closeCurlyBracket,

libredex/RedexResources.cpp

@@ -25,7 +25,6 @@
 #include "GlobalConfig.h"
 #include "IOUtil.h"
 #include "Macros.h"
-#include "ProguardLexer.h"
 #include "ReadMaybeMapped.h"
 #include "StringUtil.h"
 #include "Trace.h"
@@ -490,11 +489,8 @@ void gather_r_classes(const Scope& scope, std::vector<DexClass*>* vec) {
   }
 }
 
-bool valid_java_identifier(const std::string& ident) {
-  return !ident.empty() && keep_rules::proguard_parser::is_identifier(ident);
-}
-
 bool valid_xml_element(const std::string& ident) {
-  return valid_java_identifier(ident) && ident.find('.') != std::string::npos;
+  return java_names::is_identifier(ident) &&
+         ident.find('.') != std::string::npos;
 }
 } // namespace resources

libredex/RedexResources.h

@@ -25,6 +25,7 @@
 #include "androidfw/ResourceTypes.h"
 
 #include "Debug.h"
+#include "DexUtil.h"
 #include "GlobalConfig.h"
 #include "RedexMappedFile.h"
 
@@ -60,8 +61,6 @@ const inline std::set<std::string> POSSIBLE_CLASS_ATTRIBUTES = {
     "actionViewClass", "class", "controller",  "layout_behavior",
     "layoutManager",   "name",  "targetClass",
 };
-// Returns false if there are any characters that are not valid Java identifier.
-bool valid_java_identifier(const std::string& ident);
 // Returns false if there is no dot or it's not a Java identifier.
 bool valid_xml_element(const std::string& ident);
 
@@ -78,7 +77,7 @@ struct StringOrReference {
     if (ref != 0) {
       return true;
     }
-    return valid_java_identifier(str);
+    return java_names::is_identifier(str);
   }
 
   bool operator==(const StringOrReference& that) const {