Use classic enum and focus on code readability

Author: mathieucarbou

src/ESPAsyncWebServer.h

@@ -731,6 +731,19 @@ class AsyncWebServerRequest {
 };
 
 class AsyncURIMatcher {
+private:
+    // Matcher types are internal, not part of public API
+  enum class Type {
+    None,                 // default state: matcher does not match anything
+    Auto,                 // parse _uri at construct time and infer match type(s): _uri may be transformed to remove wildcards
+    All,                  // matches everything
+    Exact,                // matches equivalent to regex: ^{_uri}$
+    Prefix,               // matches equivalent to regex: ^{_uri}.*
+    Extension,            // non-regular match: /pattern../*.ext
+    BackwardCompatible,   // matches equivalent to regex: ^{_uri}(/.*)?$
+    Regex,                // matches _url as regex
+  };
+
 public:
   /**
    * @brief No special matching behavior (default)
@@ -762,9 +775,9 @@ class AsyncURIMatcher {
   static constexpr uint16_t CaseInsensitive = (1 << 0);
 
   // public constructors
-  AsyncURIMatcher() : _flags(All) {}
-  AsyncURIMatcher(String uri, uint16_t modifiers = None) : AsyncURIMatcher(std::move(uri), Auto, modifiers) {}
-  AsyncURIMatcher(const char *uri, uint16_t modifiers = None) : AsyncURIMatcher(String(uri), Auto, modifiers) {}
+  AsyncURIMatcher() : _flags(intptr_t(Type::None)) {}
+  AsyncURIMatcher(String uri, uint16_t modifiers = None) : AsyncURIMatcher(std::move(uri), Type::Auto, modifiers) {}
+  AsyncURIMatcher(const char *uri, uint16_t modifiers = None) : AsyncURIMatcher(String(uri), Type::Auto, modifiers) {}
 
 #ifdef ASYNCWEBSERVER_REGEX
   AsyncURIMatcher(const AsyncURIMatcher &c) : _value(c._value), _flags(c._flags) {
@@ -774,7 +787,7 @@ class AsyncURIMatcher {
   }
 
   AsyncURIMatcher(AsyncURIMatcher &&c) : _value(std::move(c._value)), _flags(c._flags) {
-    c._flags = 0;
+    c._flags = intptr_t(Type::None);
   }
 
   ~AsyncURIMatcher() {
@@ -810,7 +823,7 @@ class AsyncURIMatcher {
     _flags = r._flags;
     if (r._isRegex()) {
       // We have adopted it
-      r._flags = 0;
+      r._flags = intptr_t(Type::None);
     }
     return *this;
   }
@@ -825,11 +838,6 @@ class AsyncURIMatcher {
 #endif
 
   bool matches(AsyncWebServerRequest *request) const {
-    // Match-all is tested first
-    if (_flags & All) {
-      return true;
-    }
-
 #ifdef ASYNCWEBSERVER_REGEX
     if (_isRegex()) {
       std::smatch matches;
@@ -843,34 +851,33 @@ class AsyncURIMatcher {
       return false;
     }
 #endif
-    String path = request->url();
-    if (_flags & (CaseInsensitive << 16)) {
-      path.toLowerCase();
-    }
 
-    // Exact match (should be the most common case)
-    if ((_flags & Exact) && (_value == path)) {
-      return true;
-    }
+    // extract matcher type from _flags
+    const intptr_t flags = _flags >> 1;  // shift off disambiguation bit;
+    const Type type = static_cast<Type>(flags & 0xFFFF);  // Type is lower 16 bits
+    const uint16_t modifiers = flags >> 16;  // Modifiers are upper 16 bits
 
-    // Prefix match types
-    if ((_flags & Prefix) && path.startsWith(_value)) {
-      return true;
-    }
-    if ((_flags & PrefixFolder) && path.startsWith(_value + "/")) {
-      return true;
+    // apply modifiers
+    String path = request->url();
+    if (modifiers & CaseInsensitive) {
+      path.toLowerCase();
     }
 
-    // Extension match
-    if (_flags & Extension) {
-      int split = _value.lastIndexOf("/*.");
-      if (split >= 0 && path.startsWith(_value.substring(0, split)) && path.endsWith(_value.substring(split + 2))) {
-        return true;
+    switch (type) {
+      case Type::All:   return true;
+      case Type::Exact:  return (_value == path);
+      case Type::Prefix: return path.startsWith(_value);
+      case Type::Extension:
+      {
+        int split = _value.lastIndexOf("/*.");
+        return (split >= 0 && path.startsWith(_value.substring(0, split)) && path.endsWith(_value.substring(split + 2)));
       }
+      case Type::BackwardCompatible: return (_value == path) || path.startsWith(_value + "/");
+      default:
+        // Should never happen - programming error
+        assert("Invalid type");
+        return false;
     }
-
-    // we did not match
-    return false;
   }
 
   // static factory methods for common match types
@@ -882,7 +889,7 @@ class AsyncURIMatcher {
    * Usage: server.on(AsyncURIMatcher::all(), handler);
    */
   static inline AsyncURIMatcher all() {
-    return AsyncURIMatcher{{}, All, None};
+    return AsyncURIMatcher{{}, Type::All, None};
   }
 
   /**
@@ -897,7 +904,7 @@ class AsyncURIMatcher {
    * Doesn't match: "/LOGIN" (unless CaseInsensitive flag used)
    */
   static inline AsyncURIMatcher exact(String c, uint16_t modifiers = None) {
-    return AsyncURIMatcher{std::move(c), Exact, modifiers};
+    return AsyncURIMatcher{std::move(c), Type::Exact, modifiers};
   }
 
   /**
@@ -911,7 +918,7 @@ class AsyncURIMatcher {
    * Note: This is pure prefix matching - does NOT require folder separator
    */
   static inline AsyncURIMatcher prefix(String c, uint16_t modifiers = None) {
-    return AsyncURIMatcher{std::move(c), Prefix, modifiers};
+    return AsyncURIMatcher{std::move(c), Type::Prefix, modifiers};
   }
 
   /**
@@ -929,12 +936,12 @@ class AsyncURIMatcher {
   static inline AsyncURIMatcher dir(String c, uint16_t modifiers = None) {
     // Pre-calculate folder for efficiency
     if (!c.length()) {
-      return AsyncURIMatcher{"/", Prefix, modifiers};
+      return AsyncURIMatcher{"/", Type::Prefix, modifiers};
     }
     if (c[c.length() - 1] != '/') {
       c.concat('/');
     }
-    return AsyncURIMatcher{std::move(c), Prefix, modifiers};
+    return AsyncURIMatcher{std::move(c), Type::Prefix, modifiers};
   }
 
   /**
@@ -951,7 +958,7 @@ class AsyncURIMatcher {
    * The path before "/\*." must match exactly, and the URI must end with the extension.
    */
   static inline AsyncURIMatcher ext(String c, uint16_t modifiers = None) {
-    return AsyncURIMatcher{std::move(c), Extension, modifiers};
+    return AsyncURIMatcher{std::move(c), Type::Extension, modifiers};
   }
 
 #ifdef ASYNCWEBSERVER_REGEX
@@ -970,33 +977,15 @@ class AsyncURIMatcher {
    * Performance note: Regex matching is slower than other match types.
    */
   static inline AsyncURIMatcher regex(String c, uint16_t modifiers = None) {
-    return AsyncURIMatcher{std::move(c), Regex, modifiers};
+    return AsyncURIMatcher{std::move(c), Type::Regex, modifiers};
   }
 #endif
 
 private:
-  // Matcher types
-  enum Type : uint16_t {
-    // Meta types - low bits
-    Auto = (1 << 0),  // parse _uri at construct time and infer match type(s)
-                      // (_uri may be transformed to remove wildcards)
-
-    All = (1 << 1),           // No type set
-    Exact = (1 << 2),         // matches equivalent to regex: ^{_uri}$
-    Prefix = (1 << 3),        // matches equivalent to regex: ^{_uri}.*
-    PrefixFolder = (1 << 4),  // matches equivalent to regex: ^{_uri}/.*
-    Extension = (1 << 5),     // non-regular match: /pattern../*.ext
-
-#ifdef ASYNCWEBSERVER_REGEX
-    NonRegex = (1 << 0),  // bit to use as pointer tag
-    Regex = (1 << 15),    // matches _url as regex
-#endif
-  };
-
   // fields
   String _value;
   union {
-    intptr_t _flags;
+    intptr_t _flags; // type and flags packed together
 #ifdef ASYNCWEBSERVER_REGEX
     // Overlay the pattern pointer storage with the type.  It is treated as a tagged pointer:
     // if any of the LSBs are set, it stores type, as a valid object must be aligned and so
@@ -1018,40 +1007,42 @@ class AsyncURIMatcher {
 #endif
 
   // Core private constructor
-  AsyncURIMatcher(String uri, Type type = Auto, uint16_t modifiers = None) : _value(std::move(uri)), _flags(uint32_t(modifiers) << 16 | type) {
+  AsyncURIMatcher(String uri, Type type, uint16_t modifiers) : _value(std::move(uri)) {
 #ifdef ASYNCWEBSERVER_REGEX
-    if ((type & Regex) || ((type & Auto) && _value.startsWith("^") && _value.endsWith("$"))) {
+    if ((type == Type::Regex) || ((type == Type::Auto) && _value.startsWith("^") && _value.endsWith("$"))) {
       pattern = new std::regex(_value.c_str(), (modifiers & CaseInsensitive) ? (std::regex::icase | std::regex::optimize) : (std::regex::optimize));
-      return;  // no additional processing - modifiers are overwritten
+      return;  // no additional processing - flags are overwritten by pattern pointer
     }
 #endif
     if (modifiers & CaseInsensitive) {
       _value.toLowerCase();
     }
-    if (type & Auto) {
+    if (type == Type::Auto) {
       // Inspect _value to set flags
       // empty URI matches everything
       if (!_value.length()) {
-        _flags = All;
-        return;  // Does not require extra bit for regex disambiguation
-      }
-      if (_value.endsWith("*")) {
+        type = Type::All;
+      } else if (_value.endsWith("*")) {
         // wildcard match with * at the end
-        _flags |= Prefix;
+        type = Type::Prefix;
         _value = _value.substring(0, _value.length() - 1);
       } else if (_value.lastIndexOf("/*.") >= 0) {
         // prefix match with /*.ext
         // matches any path ending with .ext
         // e.g. /images/*.png will match /images/pic.png and /images/2023/pic.png but not /img/pic.png
-        _flags |= Extension;
+        type = Type::Extension;
       } else {
-        // No special values - use default of folder and exact
-        _flags |= PrefixFolder | Exact;
+        // backward compatible use case: exact match or prefix with trailing /
+        type = Type::BackwardCompatible;
       }
     }
-#ifdef ASYNCWEBSERVER_REGEX
-    _flags |= NonRegex;  // disambiguate regex case
-#endif
+    _flags = uint32_t(modifiers) << 16 | uint16_t(type);
+    // Use lsb to disambiguate from regex pointer in the case where someone has regex activated but uses a non-regex type.
+    // We always do this shift, even if regex is not enabled, to keep the layout identical and also catch programmatic errors earlier.
+    // For example a mistake is to set a modifier flag to (1 << 15), which is the msb of the uint16_t.
+    // This msb is discarded during this shift operation.
+    // So pay attention to not have more than 15 modifier flags.
+    _flags = (_flags << 1) + 1;
   }
 };