Noissue - Return value of filter match expression directly

lib/filterClasses.js

Index: lib/filterClasses.js
===================================================================
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -714,23 +714,20 @@
    * @param {string} [docDomain] domain name of the document that loads the URL
    * @param {boolean} [thirdParty] should be true if the URL is a third-party
    *                               request
    * @param {string} [sitekey] public key provided by the document
    * @return {boolean} true in case of a match
    */
   matches(location, typeMask, docDomain, thirdParty, sitekey)
   {
-    if (this.contentType & typeMask &&
-        (this.thirdParty == null || this.thirdParty == thirdParty) &&
-        this.isActiveOnDomain(docDomain, sitekey) && this.regexp.test(location))
-    {
-      return true;
-    }
-    return false;
+    return (this.contentType & typeMask) != 0 &&

[Manish Jethani, 2018/09/04 13:46:06]

The first condition here needs to evaluate to a boolean value, just so the
function returns false rather than 0 when the check fails.

[Jon Sonesen, 2018/09/05 14:16:38]

Yeah, I see what you did there. Stylistically I prefer this as well, it is
easier to read. I think it is not crazy hard to read the masking, as most
programmers are aware of masks but this just looks nicer and is more uniform
with how most programmers do things (my first choice would have been to return a
bool directly with the mask as you have done here had I implemented this)

Thanks!

LGTM

+           (this.thirdParty == null || this.thirdParty == thirdParty) &&
+           this.isActiveOnDomain(docDomain, sitekey) &&
+           this.regexp.test(location);
   }
 });
 
 /**
  * Creates a RegExp filter from its text representation
  * @param {string} text   same as in Filter()
  * @return {Filter}
  */