Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 2.10.0 (in progress) #3273

Merged
merged 39 commits into from Feb 5, 2019
Merged

Version 2.10.0 (in progress) #3273

merged 39 commits into from Feb 5, 2019

Conversation

jugglinmike
Copy link
Member

We have been hard at work on JSHint version 2.10.0 for over six months now. We're still working, but I'm opening this pull request today in order to give folks a better view into our progress.

Here's a high-level summary of what the next version contains and what we hope to implement before releasing:

  • New esVersion: 2016
    • Exponentiation operator
    • Restrictions on the function form of the "use strict" directive
  • New esVersion: 2017
    • Async functions
    • Trailing commas in parameter lists
  • unstable configuration
    • Generic support
    • Options
      • Object "rest" properties
  • New linting option: leanswitch

If you're interested in adding a new feature to JSHint, we encourage you to work from this branch. However, since we continue to publish bug fixes under the 2.9 release train, we reserve the right to rebase this branch. You should also be prepared to rebase your pull request as requested. (This is a more advanced workflow for git, and we'll be happy to assist you as necessary.) Those following along should also note that commits referenced on pages for merged pull requests may be present here under a different SHA.

@coveralls
Copy link

coveralls commented Apr 23, 2018

Coverage Status

Coverage decreased (-0.2%) to 98.471% when pulling f80e049 on v2.10.0 into 00f83c9 on master.

donalffons added a commit to donalffons/i2Configurator that referenced this pull request Jul 23, 2018
@jugglinmike
Copy link
Member Author

This branch now supports trailing commas in parameter lists, making async functions the final big hurdle

The `inES5` method is never invoked with a parameter, so the branch in
question is never executed. Remove the parameter, the parameter's
description, and the associated branch.
This change simply enables the configuration settings `esversion: 7` and
`esversion: 2016` without introducing any new parsing/linting
functionality.
ECMAScript 2016 introduced an early error for any function situated in
non-strict code whose parameter list is non-simple and whose body
contains a "use strict" directive. Enforce this restriction when
interpreting input according to that revision of the specification.
- Indroduce a JSDoc-style comment describing the function and its
  parameters
- Re-name parameters with more descriptive names
- Introduce a local alias for the next token to be consumed
This condition was previously reported as a warning. It unambiguously
describes a syntax error, meaning that an error is more appropriate than
a warning. Reporting it as such impoves the interpretation of Test262
tests. Unit tests do not require modification because W116 and E021
share identical messages ("Expected '{a}' and instead saw '{b}'.").

This change triggers failures in four Test262 tests that were previously
interpreted as "valid." These tests concern the "object spread/rest"
proposal, but support for this proposal has not yet been implemented in
JSHint. Therefor, their previous "passing" status was circumstantial,
and the newly-identified error should be marked as "expected."
Previously, the spread/rest operator was implemented as a "prefix"
operator. This incorrectly enabled its use in generic expression
contexts, as in:

    ...x;

Re-implement as an "infix" operator and implement a helper function for
parsing it in only those contexts where it is valid:

- parameter lists
- call expressions
- array initializers
- destructuring element patterns

This change triggers failures in a number of Test262 tests that were
previously interpreted as "valid." These tests concern the "object
spread/rest" proposal, but support for this proposal has not yet been
implemented in JSHint. Therefor, their previous "passing" status was
circumstantial, and the newly-identified error should be marked as
"expected."
Describe the expected language feature more precisely based on the
context in which the "..." is encountered.
Implement a new namespace for unstable features--`jshint.unstable`.
Taken on its own, this change is not useful to end users, but it defines
the necessary structure for the future implementation of unstable
linting options.
Ensure that the "token" object is consistently returned by the "prefix"
`(` operator's null denotation function. This alleviates calling code
from having to account for "functor" objects and instead consistently
interpret operands as token objects.
A previous commit removed all references to an internal function,
introducing a linting error. This was not reported by the project's
continuous integration system because that system was not configured to
run the project's linting check.

Correct the linting error and enable linting in the continuous
integration environment.
The second parameter of the `expression` function accepts a value that
modifies how the next token is parsed. When set to `true`, the token is
interpreted as the beginning of a statement; otherwise, it is
interpreted as the beginning of an expression.

Two call sites within the `for` statement's parsing logic specified the
string value `"for"`. Due to inconsistencies in how the parameter was
interpreted internally, this causes `expression` to mix parsing
strategies. Specifically:

- It used the "statement" parsing strategy to mark the token value as
  beginning a statement. This could be observed in the application of
  the `singleGroups` linting option. Further, it disqualify it for
  consideration as a Mozilla "let expression"
- It used the "expression" parsing strategy to ignore the token's `fud`
  method

Since the grammar does not allow a statement in either location, the
"expression" parsing strategy should be used consistently.

Achieve this by removing the argument from the call site, thereby
falling back to the default behavior of the `expression` function.
Update `expression` to consistently interpret the parameter for
"truthiness" in order to discourage similar ambiguity in the future.
Extend in-line documentation to help future contributors orient
themselves to the code base. Avoid any modification to executable code
in order to ensure that program behavior is unchanged.
@jugglinmike
Copy link
Member Author

A lot has happened in master since I first opened this pull request. In order to limit the amount of divergence (and to ease the process of merging in the future), I've rebased this branch. My earlier offer to help folks rebase their own work stands--just let me know!

And in case it's helpful, I've made the prior version of this branch available here:

https://github.com/jshint/jshint/tree/v2.10.0-2018-12-22

jugglinmike and others added 2 commits January 13, 2019 19:00
JSHint's class-parsing logic is derived from code first contributed by
GitHub user "usrbincc." That user could not be reached to sign the
project's Contributor License Agreement. Revert the current
implementation in its entirety so that it may be re-written under the
terms of the agreement.
Previously, the instructions for parsing Mozilla's "let expression"
feature were included within JSHint's core `expression` function. This
factoring was somewhat inefficient due to the `expression` function's
central role in JSHint's operation. More importantly, it made the
`expression` function harder to read and understand.

Relocate the logic to a dedicated `nud` method of the `let` token, where
it will be invoked by the Pratt parsing semantics.
Tolerate the use of `let` as an Identifier outside of strict mode.

This change makes error messages less helpful when the input code
contains the non-standard "`let` expression" language extension but
omits the required `moz` option. However, this change does not effect
JSHint's ability to parse that language extension and is therefore
compatible with prior releases of JSHint.
Re-use the `(hasSimpleParams)` property of the `funct` object (recently
introduced to implement ES2016 restrictions on the "use strict"
directive) to honor the early error for duplicate parameters.
This change simply enables the configuration settings `esversion: 8` and
`esversion: 2017` without introducing any new parsing/linting
functionality.
@jugglinmike
Copy link
Member Author

Rebased once again. This now includes a re-write of JSHint's class-parsing logic.

Here's the prior version of the branch:

https://github.com/jshint/jshint/tree/v2.10.0-2019-01-13

Copy link
Member

@caitp caitp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks cool so far, ill take a closer look on monday

// += -= *= %= &= |= ^= /=
if (ch1 === ch2 && ("+-<>&|".indexOf(ch1) >= 0)) {
// 2-character punctuators: ++ -- << >> && || **
if (ch1 === ch2 && ("+-<>&|*".indexOf(ch1) >= 0)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this easier to read than /[+-<>&|*]{2}/.test(ch1 + ch2) ?

* https://github.com/tc39/proposal-object-rest-spread
*/
objspreadrest: true
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t see anything in changes other than tests and error messages where this is actually used. (However, reading this from a phone, so might have missed it)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub is collapsing the changes to jshint.js for me, so it might not be immediately visible from a phone. After expanding that file, though, I am seeing the relevant changes.

At this point, I think we'll want to re-implement this as a stable feature enabled via esversion: 9 rather than an unstable option, though.

That this correction does not require a modification to the Test262
expectations file is an indication of missing coverage in that project.
New tests have been submitted there, as well [1].

[1] tc39/test262#2043
The "Object rest/spread properties" feature was standardized in ES2018.
Update enabling mechanism to use a new `esversion` value dedicated to
that edition.
Introduce a new warning to alert users of RegExp literals that include
the new flag without using the "dot" atom.
The `expression` function was recently extended to support a second
parameter describing the parsing context. All other internal functions
that accept this value do so via the first parameter. Refactor the
`expression` function to match that pattern.
@jugglinmike
Copy link
Member Author

Great news, folks! Following some stellar work by a couple first-time contributors, I finished up support for async functions. @rwaldron provided excellent review, so everything we set out to do for version 2.10.0 is complete. More than that, actually: we have asynchronous iteration done, too!

You can expect a release on 2019-02-04

@oskarkrawczyk
Copy link

I'm so happy about the headway made on this and the contribution you've received to!

I'd be lying if I'd say the first thing that I've done upon receiving this comment was not integrate the current code state into JSFiddle, happy to say it works perfectly fine!

Again, such a fantastic job!

@jugglinmike jugglinmike merged commit f80e049 into master Feb 5, 2019
@jugglinmike
Copy link
Member Author

Thanks so much, Oskar! JSHint version 2.10.0 is now published.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants