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

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

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