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

Re: Patch: Poor use of hasOwnProperty in JSLint

Expand Messages
  • Douglas Crockford
    ... Why are you doing that?
    Message 1 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 2 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 3 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 4 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.