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

Bug Report: pos in lex()

Expand Messages
  • Chris
    I think I have located a bug in JSLint. This bug depends on a variable set in a closure inside JSLint. The variable is not reset between runs of JSLint, so
    Message 1 of 3 , Jun 20, 2011
    • 0 Attachment
      I think I have located a bug in JSLint. This bug depends on a variable set in a closure inside JSLint. The variable is not reset between runs of JSLint, so it is necessary to reload the JSLint page to observe this bug. Because of this, simply passing the example code through JSLint will not be enough to see the issue; please follow the complete steps below.

      This example input is designed to illustrate the bug in JSLint, and does not otherwise produce useful output. The <script> tags are a necessary part of the input. This should be tested with these options: /*jslint fragment: true */.

      Input:

          <script>
          /*global console */

          function test() {
              "use strict";
              var re = /x/;
              if ('abc'.test(re)) {
                  console.log('<\/p>');
              }
          }
          </script>

      Steps:

      1. Load JSLint in a new tab or window, or refresh the page to get a freshly initialized version. Do not test in a tab that has previously run JSLint without reloading.

      2. Paste the provided input into JSLint and execute with "Tolerate HTML fragments" turned on. Observe that there are no error messages.

      3. In the regular expression, change the "x" to a single space character: var re = / /;

      4. Run JSLint again without reloading. Observe that error messages are now reported.

      5. Change the regular expression back by replacing the single space character with an "x": var re = /x/;

      6. Run JSLint again without reloading. Observe that there are still error messages, even though the input is now identical to the input that JSLint reported as error-free in step 2.


      Examination:

      This behavior is caused by the state of the "pos" variable declared in the "lex" function. The lex function declares "pos" in a closure and uses it throughout. However, "pos" is only assigned to if JSLint is asked to parse a regular expression that contains a space character. If the source upon JSLint functions does not contain such a regular expression, then all uses of "pos" will use the "undefined" value.

      However, once a matching regular expression is found, "pos" receives a value. Since "pos" is only assigned to when a matching regex is found, its value will remain in use until another matching regex causes it to change. In other words, it is not reset between one run and another.

      This appears as buggy behavior, since it is unfortunate that running JSLint on one input can affect the results of JSLint on a subsequent input.
    • Chris
      ... Addendum: This bug was introduced in the cleanup commit on 2011-06-11:
      Message 2 of 3 , Jun 20, 2011
      • 0 Attachment
        --- In jslint_com@yahoogroups.com, "Chris" <Nielsen.Chris@...> wrote:
        >
        > I think I have located a bug in JSLint. This bug depends on a variable set in a closure inside JSLint. The variable is not reset between runs of JSLint, so it is necessary to reload the JSLint page to observe this bug. Because of this, simply passing the example code through JSLint will not be enough to see the issue; please follow the complete steps below.

        Addendum:

        This bug was introduced in the "cleanup" commit on 2011-06-11: https://github.com/douglascrockford/JSLint/commit/5a1064bc597f18f5bb5784ddd747da860c76cb73. The variable now known as "pos" was once called "j"; it appears that you went through and gave it a more meaningful name as part of your cleanup efforts.

        Prior to the cleanup, the "j" variable was defined in the "token" function on line 1652: https://github.com/douglascrockford/JSLint/commit/5a1064bc597f18f5bb5784ddd747da860c76cb73#L0L1652. "token" is a large function, and the "j" variable was used meaningfully only to parse regular expressions containing a space: the same usage that "pos" has today.

        However, "token" also contained a function named "string", which also defined a "j" variable, on line 1669: https://github.com/douglascrockford/JSLint/commit/5a1064bc597f18f5bb5784ddd747da860c76cb73#L0L1669. The "string" function inside "token" used its own "j" several times, in quite a different way than the "j" variable outside "string."

        Therefore, prior to the cleanup, you had this sort of structure:

        var token = function () {
            var j;

            function string() {
                var j;
                // one meaning of "j".
            }

            // different meaning of "j" out here!
        };

        When you cleaned this up, you became confused about the different meanings of "j". When you renamed the "j" variable in the "token" scope, you incorrectly renamed some (but not all) instances of "j" in the "string" scope (lines 1709, 1714, 1715, 1718, 1724 were renamed, many other lines were not; line numbers are pre-cleanup).

        There has previously been a suggestion that JSLint should warn about re-declaring variable names that are already accessible in a higher scope or closure. If you had implemented this suggestion, JSLint would have warned you about "j" having two different meanings: http://tech.groups.yahoo.com/group/jslint_com/message/2212.

        Thank you for your consideration!

        - Chris
      • Douglas Crockford
        ... Thanks. Please try it now.
        Message 3 of 3 , Jun 20, 2011
        • 0 Attachment
          --- In jslint_com@yahoogroups.com, "Chris" <Nielsen.Chris@...> wrote:
          >
          > I think I have located a bug in JSLint. This bug depends on a variable set in a closure inside JSLint. The variable is not reset between runs of JSLint, so it is necessary to reload the JSLint page to observe this bug.

          Thanks. Please try it now.
        Your message has been successfully submitted and would be delivered to recipients shortly.