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

Re: [jslint] Re: Patch: Poor use of hasOwnProperty in JSLint

Expand Messages
  • 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 1 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 2 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 3 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.