Loading ...
Sorry, an error occurred while loading the content.

Re: Patch: Poor use of hasOwnProperty in JSLint

Expand Messages
  • Daniel Cassidy
    Hi List, On Mon, Oct 20, 2008 at 6:50 PM, Daniel Cassidy ... Patch here: http://www.danielcassidy.me.uk/misc/fulljslint-hasOwnProperty.patch Thanks, Dan.
    Message 1 of 6 , Oct 20, 2008
    • 0 Attachment
      Hi List,

      On Mon, Oct 20, 2008 at 6:50 PM, Daniel Cassidy
      <mail@...> wrote:
      > (This is the first time I've used Yahoo Groups and I'm not sure if
      > attachments are allowed. If not, I will upload the patch somewhere
      > when I get home).

      Patch here: http://www.danielcassidy.me.uk/misc/fulljslint-hasOwnProperty.patch

      Thanks,
      Dan.
    • Douglas Crockford
      ... Why are you doing that?
      Message 2 of 6 , Oct 23, 2008
      • 0 Attachment
        --- In jslint_com@yahoogroups.com, "Daniel Cassidy" <mail@...> wrote:
        >
        > Hi List,
        >
        > I was surprised to run JSLint on some of my code today only to see the
        > following message:
        >
        > 'hasOwnProperty' is a really bad name.
        > Stopping, unable to continue. (26% scanned).
        >
        >
        > While I agree that hasOwnProperty is, in many ways, a uniquely bad
        > choice of name in JavaScript, it's not a terrible choice for the
        > hasOwnProperty function itself, which is what I was (re-)implementing.

        Why are you doing that?
      • Daniel Cassidy
        Hi Douglas, On Thu, Oct 23, 2008 at 11:25 PM, Douglas Crockford ... Two reasons. The first reason is that I keep seeing very subtle bugs being introduced as a
        Message 3 of 6 , Oct 23, 2008
        • 0 Attachment
          Hi Douglas,

          On Thu, Oct 23, 2008 at 11:25 PM, Douglas Crockford
          <douglas@...> wrote:
          > --- In jslint_com@yahoogroups.com, "Daniel Cassidy" <mail@...> wrote:
          >> While I agree that hasOwnProperty is, in many ways, a uniquely bad
          >> choice of name in JavaScript, it's not a terrible choice for the
          >> hasOwnProperty function itself, which is what I was (re-)implementing.
          >
          > Why are you doing that?

          Two reasons.


          The first reason is that I keep seeing very subtle bugs being
          introduced as a result of hasOwnProperty being used in the form
          obj.hasOwnProperty(p).

          Imagine that some user input is being used as a key in obj. Although
          in most scenarios it is unlikely that the user will enter
          "hasOwnProperty", it is possible, and it is absurd that this input
          should break an otherwise robust program.

          In other scenarios, the problem is worse because it is quite likely
          that the user will enter "hasOwnProperty". For example this is the
          case if the user's input is a JavaScript program. Two examples of
          JavaScript programs that have this problem in practice are JsDoc
          Toolkit and JSLint (although the latter handles the problem with more
          grace than the former, which simply crashes with an unhelpful error
          message).

          To discourage my colleagues from making this error, I wrote a helper
          function roughly as follows:

          (function () {
          var hasOwnProperty; // JSLint chokes on this line
          if (Object.prototype.hasOwnProperty === undefined) {
          hasOwnProperty = function (object, key) {
          return object[key] !== undefined &&
          object[key] !== object.prototype[key];
          };
          } else {
          hasOwnProperty = function (object, key) {
          return {}.hasOwnProperty.call(object, key);
          };
          }
          COMPANY_NAME.utils.hasOwnProperty = hasOwnProperty;
          })();

          By simply instructing my colleagues to call
          COMPANY_NAME.utils.hasOwnProperty(obj, key) instead of
          obj.hasOwnProperty(key), we avoid instances of this bug in our own
          code.

          JSLint is rejecting the above snippet because of the variable named
          "hasOwnProperty". I don't think this is because of any bad practice in
          my code. Rather, JSLint is rejecting it in order to hide a bad
          practice in its own code -- ironically an instance of the very same
          bad practice I was trying to avoid in the first place.


          The second reason is evidenced in the code snippet above. As you know,
          some browsers don't provide a hasOwnProperty function so it's wise to
          implement your own.


          Thanks,
          Dan.
        • Douglas Crockford
          ... I don t want to modify JSLint to satisfy a single, unnecessary, dubious use. I recommend that you change your variable name. And if you are trying to
          Message 4 of 6 , Oct 23, 2008
          • 0 Attachment
            --- In jslint_com@yahoogroups.com, "Daniel Cassidy" <mail@...> wrote:
            > Two reasons.
            >
            >
            > The first reason is that I keep seeing very subtle bugs being
            > introduced as a result of hasOwnProperty being used in the form
            > obj.hasOwnProperty(p).
            >
            > Imagine that some user input is being used as a key in obj. Although
            > in most scenarios it is unlikely that the user will enter
            > "hasOwnProperty", it is possible, and it is absurd that this input
            > should break an otherwise robust program.
            >
            > In other scenarios, the problem is worse because it is quite likely
            > that the user will enter "hasOwnProperty". For example this is the
            > case if the user's input is a JavaScript program. Two examples of
            > JavaScript programs that have this problem in practice are JsDoc
            > Toolkit and JSLint (although the latter handles the problem with more
            > grace than the former, which simply crashes with an unhelpful error
            > message).
            >
            > To discourage my colleagues from making this error, I wrote a helper
            > function roughly as follows:
            >
            > (function () {
            > var hasOwnProperty; // JSLint chokes on this line
            > if (Object.prototype.hasOwnProperty === undefined) {
            > hasOwnProperty = function (object, key) {
            > return object[key] !== undefined &&
            > object[key] !== object.prototype[key];
            > };
            > } else {
            > hasOwnProperty = function (object, key) {
            > return {}.hasOwnProperty.call(object, key);
            > };
            > }
            > COMPANY_NAME.utils.hasOwnProperty = hasOwnProperty;
            > })();
            >
            > By simply instructing my colleagues to call
            > COMPANY_NAME.utils.hasOwnProperty(obj, key) instead of
            > obj.hasOwnProperty(key), we avoid instances of this bug in our own
            > code.
            >
            > JSLint is rejecting the above snippet because of the variable named
            > "hasOwnProperty". I don't think this is because of any bad practice in
            > my code. Rather, JSLint is rejecting it in order to hide a bad
            > practice in its own code -- ironically an instance of the very same
            > bad practice I was trying to avoid in the first place.
            >
            >
            > The second reason is evidenced in the code snippet above. As you know,
            > some browsers don't provide a hasOwnProperty function so it's wise to
            > implement your own.


            I don't want to modify JSLint to satisfy a single, unnecessary,
            dubious use. I recommend that you change your variable name. And if
            you are trying to implement hasOwnProperty, I think you intended
            object.constructor.prototype[key]. But I don't think it works for
            mischievous objects that have altered or inherited constructor
            properties.
          • Daniel Cassidy
            On Fri, Oct 24, 2008 at 12:40 AM, Douglas Crockford ... I agree. But you misunderstand my complaint. It is trivial to change my program to work with JSLint,
            Message 5 of 6 , Oct 23, 2008
            • 0 Attachment
              On Fri, Oct 24, 2008 at 12:40 AM, Douglas Crockford
              <douglas@...> wrote:
              > I don't want to modify JSLint to satisfy a single, unnecessary,
              > dubious use. I recommend that you change your variable name.

              I agree. But you misunderstand my complaint.

              It is trivial to change my program to work with JSLint, and indeed I
              have already done so.

              However, I believe that calling hasOwnProperty in the form
              obj.hasOwnProperty(p) is a bad practice. The reason it is bad is that
              naive, lazy or tired developers will assume that obj is a generic
              container which can contain any key/value pair, whereas in fact it is
              unable to safely contain the key "hasOwnProperty". I have seen this
              exact problem several times in the wild.

              JSLint itself does not make this assumption, so this is not really a
              bug. But I believe that bad practices are best avoided even when the
              developer knows they are safe -- unless there is no alternative.

              My complaint is that there is a trivial, safe alternative to the bad
              practice and that JSLint does not use it. That is all.


              > And if
              > you are trying to implement hasOwnProperty, I think you intended
              > object.constructor.prototype[key].

              You're right. I was retyping from memory -- it's a typo and not
              present in the original code, honest :).


              > But I don't think it works for
              > mischievous objects that have altered or inherited constructor
              > properties.

              I agree, it's impossible to correctly reimplement hasOwnProperty on
              implementations where it is not present, which is a shame. That's why
              my snippet defers to the built-in hasOwnProperty whenever possible.

              That said, you have raised the rather important point that, on
              platforms with no hasOwnProperty, I'm exchanging one problem for
              another in that now objects which are assumed to be generic containers
              will be able to contain members named "hasOwnProperty", but unable to
              contain members named "constructor". I have no idea how I missed that
              -- thanks for the observation.

              That isn't true on modern JavaScript platforms, though, and my dodgy
              attempt at reimplementing hasOwnProperty is very much irrelevant to
              the point that I was trying to raise :).

              Thanks,
              Dan.
            Your message has been successfully submitted and would be delivered to recipients shortly.