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

docs: added JS doc comments and tsconfig #530

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions espree.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ import espreeVersion from "./lib/version.js";
import * as visitorKeys from "eslint-visitor-keys";
import { getLatestEcmaVersion, getSupportedEcmaVersions } from "./lib/options.js";

/**
* @typedef {import("acorn")} acorn
* @typedef {import("./lib/options").ParserOptions} ParserOptions
* @typedef {import("./lib/token-translator").EsprimaToken} EsprimaToken
* @typedef {import("./lib/token-translator").TokenRange} TokenRange
*/


// To initialize lazily.
const parsers = {
Expand Down Expand Up @@ -101,8 +108,8 @@ const parsers = {
/**
* Tokenizes the given code.
* @param {string} code The code to tokenize.
* @param {Object} options Options defining how to tokenize.
* @returns {Token[]} An array of tokens.
* @param {ParserOptions} [options] Options defining how to tokenize.
* @returns {EsprimaToken[]} An array of tokens.
* @throws {SyntaxError} If the input code is invalid.
* @private
*/
Expand All @@ -124,8 +131,8 @@ export function tokenize(code, options) {
/**
* Parses the given code.
* @param {string} code The code to tokenize.
* @param {Object} options Options defining how to tokenize.
* @returns {ASTNode} The "Program" AST node.
* @param {ParserOptions} options Options defining how to tokenize.
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @param {ParserOptions} options Options defining how to tokenize.
* @param {ParserOptions} [options] Options defining how to parse.

* @returns {acorn.Node} The "Program" AST node.
Copy link
Member

@mdjermanovic mdjermanovic Apr 14, 2022

Choose a reason for hiding this comment

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

Object returned from parse() also has sourceType property, and (optionally) comments and tokens properties.

https://github.com/eslint/espree/blob/b578a66991985d96d5e6ee4f388c4356ad0b3594/lib/espree.js#L154-L161

* @throws {SyntaxError} If the input code is invalid.
*/
export function parse(code, options) {
Expand Down
39 changes: 31 additions & 8 deletions lib/espree.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,24 @@ import TokenTranslator from "./token-translator.js";
import { normalizeOptions } from "./options.js";


/**
* @typedef {import("acorn")} acorn
* @typedef {import("./token-translator").Range} Range
*/

const STATE = Symbol("espree's internal state");
const ESPRIMA_FINISH_NODE = Symbol("espree's esprimaFinishNode");

/**
* @typedef {Object} EsprimaComment
* @property {"Block"|"Line"} type Type of the comment, can either be "Block" (multiline) or "Line" (single line).
* @property {string} text Contents of the comment.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @property {string} text Contents of the comment.
* @property {string} value Contents of the comment.

* @property {number|undefined} start Start column of a comment.
* @property {number|undefined} end End column of the comment.
Copy link
Member

Choose a reason for hiding this comment

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

* @property {Range|undefined} range The [start, end] range of a comment.
* @property {acorn.Position} startLoc Start location of the comment.
* @property {acorn.Position} endLoc End location of the comment.
Copy link
Member

Choose a reason for hiding this comment

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

There are no startLoc and endLoc properties on comments. They have loc, like tokens.

*/

/**
* Converts an Acorn comment to a Esprima comment.
Expand All @@ -15,7 +30,7 @@ const ESPRIMA_FINISH_NODE = Symbol("espree's esprimaFinishNode");
* @param {int} end The index at which the comment ends.
* @param {Location} startLoc The location at which the comment starts.
* @param {Location} endLoc The location at which the comment ends.
* @returns {Object} The comment object.
* @returns {EsprimaComment} The comment object.
* @private
*/
function convertAcornCommentToEsprimaComment(block, text, start, end, startLoc, endLoc) {
Expand All @@ -40,7 +55,12 @@ function convertAcornCommentToEsprimaComment(block, text, start, end, startLoc,
return comment;
}

export default () => Parser => {
/**
* Takes an acorn Parser class and returns a new Parser extending from it.
* @param {typeof acorn.Parser} Parser A base acorn parser class.
* @returns {typeof acorn.Parser} An espree parser extending the base acorn parser.
*/
function extendAcornParser(Parser) {
const tokTypes = Object.assign({}, Parser.acorn.tokTypes);

if (Parser.acornJsx) {
Expand Down Expand Up @@ -232,7 +252,7 @@ export default () => Parser => {

/**
* Overwrites the default raise method to throw Esprima-style errors.
* @param {int} pos The position of the error.
* @param {number} pos The position of the error.
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering, this isn't related to acorn.Position is it?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, this was presumably done to simplify the fact that TS (and JS) doesn't distinguish regular integers and the number type. I think it would be ideal to preserve this though so as to clarify the intent, e.g., for autocomplete.

Copy link
Author

Choose a reason for hiding this comment

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

But then passing a number wouldn't satisfy TS's typechecker, no?
I'd rather have it be integer too but I think we can mention that in the parameter description since TS/JS don't have native integers.

Copy link
Contributor

Choose a reason for hiding this comment

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

int would need to have its own typedef set to number (but if it's a problem because TypeScript will export the type, don't worry about it).

* @param {string} message The error message.
* @throws {SyntaxError} A syntax error.
* @returns {void}
Expand All @@ -249,7 +269,7 @@ export default () => Parser => {

/**
* Overwrites the default raise method to throw Esprima-style errors.
* @param {int} pos The position of the error.
* @param {number} pos The position of the error.
* @param {string} message The error message.
* @throws {SyntaxError} A syntax error.
* @returns {void}
Expand All @@ -260,7 +280,7 @@ export default () => Parser => {

/**
* Overwrites the default unexpected method to throw Esprima-style errors.
* @param {int} pos The position of the error.
* @param {number} pos The position of the error.
* @throws {SyntaxError} A syntax error.
* @returns {void}
*/
Expand Down Expand Up @@ -305,8 +325,8 @@ export default () => Parser => {

/**
* Performs last-minute Esprima-specific compatibility checks and fixes.
* @param {ASTNode} result The node to check.
* @returns {ASTNode} The finished node.
* @param {acorn.Node} result The node to check.
* @returns {acorn.Node} The finished node.
*/
[ESPRIMA_FINISH_NODE](result) {

Expand All @@ -325,4 +345,7 @@ export default () => Parser => {
return result;
}
};
};

}

export default () => extendAcornParser;
10 changes: 10 additions & 0 deletions lib/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@
// Public
//------------------------------------------------------------------------------

/**
* @typedef {Object} EcmaFeatures
* @property {boolean} [jsx]
* @property {boolean} [globalReturn]
* @property {boolean} [impliedStrict]
*/

/**
* @type {EcmaFeatures}
*/
export default {

// React JSX parsing
Expand Down
18 changes: 17 additions & 1 deletion lib/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,25 @@ function normalizeSourceType(sourceType = "script") {
throw new Error("Invalid sourceType.");
}

/**
* @typedef {import("./features").EcmaFeatures} EcmaFeatures
*/

/**
* @typedef {Object} ParserOptions
* @property {boolean} [range] Whether to include the range information for each node.
* @property {boolean} [loc] Whether to include the location information for every node.
* @property {boolean} [comment] Whether to include an array of all comments
* @property {boolean} [tokens] Whether to include an array of all tokens
* @property {number|"latest"} [ecmaVersion] The ECMAScript version to use (between 3 and 13, or 2015 and 2022, or "latest").
* @property {boolean} [allowReserved] Only allowed when `ecmaVersion` is set to 3.
* @property {"script"|"module"|"commonjs"} [sourceType] The kind of the source string being parsed.
* @property {EcmaFeatures} [ecmaFeatures] The additional features to enable.
*/

/**
* Normalize parserOptions
* @param {Object} options the parser options to normalize
* @param {ParserOptions} options the parser options to normalize
* @throws {Error} throw an error if found invalid option.
* @returns {Object} normalized options
*/
Expand Down
29 changes: 26 additions & 3 deletions lib/token-translator.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,29 @@
// Private
//------------------------------------------------------------------------------

/**
* @typedef {import("acorn")} acorn
*/

/**
* @typedef {Object} Location
* @property {acorn.Position} start The start position.
* @property {acorn.Position} end The end position.
*/

/**
* @typedef {[number, number]} Range
*/

/**
* @typedef {Object} EsprimaToken
* @property {string} type The type of this token.
* @property {string} value The string content of the token.
* @property {Location|undefined} loc Location in source text.
* @property {number|undefined} start start column.
* @property {number|undefined} end end column.
Copy link
Member

Choose a reason for hiding this comment

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

These are not columns, but indexes (like range[0] and range[1])

* @property {Range|undefined} range [start, end] range
*/

// Esprima Token Types
const Token = {
Expand All @@ -34,7 +57,7 @@ const Token = {

/**
* Converts part of a template into an Esprima token.
* @param {AcornToken[]} tokens The Acorn tokens representing the template.
* @param {acorn.Token[]} tokens The Acorn tokens representing the template.
* @param {string} code The source code.
* @returns {EsprimaToken} The Esprima equivalent of the template token.
* @private
Expand Down Expand Up @@ -94,7 +117,7 @@ TokenTranslator.prototype = {
* Translates a single Esprima token to a single Acorn token. This may be
* inaccurate due to how templates are handled differently in Esprima and
* Acorn, but should be accurate for all other tokens.
* @param {AcornToken} token The Acorn token to translate.
* @param {acorn.Token} token The Acorn token to translate.
* @param {Object} extra Espree extra object.
* @returns {EsprimaToken} The Esprima version of the token.
*/
Expand Down Expand Up @@ -174,7 +197,7 @@ TokenTranslator.prototype = {

/**
* Function to call during Acorn's onToken handler.
* @param {AcornToken} token The Acorn token.
* @param {acorn.Token} token The Acorn token.
* @param {Object} extra The Espree extra object.
* @returns {void}
*/
Expand Down
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
"files": [
"lib",
"dist/espree.cjs",
"dist/espree.d.ts",
"espree.js"
],
"types": "dist/espree.d.ts",
"engines": {
"node": "^12.22.0 || ^14.17.0 || >=16.0.0"
},
Expand Down Expand Up @@ -52,7 +54,9 @@
"mocha": "^8.3.1",
"npm-run-all": "^4.1.5",
"rollup": "^2.41.2",
"shelljs": "^0.3.0"
"shelljs": "^0.3.0",
"typescript": "^4.5.4",
"unicode-6.3.0": "^0.7.5"
Copy link
Member

Choose a reason for hiding this comment

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

I think we're not using this dependency.

Copy link
Author

Choose a reason for hiding this comment

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

We're using it in tests/lib/libraries.js

},
"keywords": [
"ast",
Expand All @@ -69,7 +73,7 @@
"test": "npm-run-all -p unit lint",
"lint": "eslint \"*.?(c)js\" lib/ tests/lib/",
"fixlint": "npm run lint -- --fix",
"build": "rollup -c rollup.config.js",
"build": "rollup -c rollup.config.js && tsc --build",
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the --build flag, what is the difference?

"update-version": "node tools/update-version.js",
"pretest": "npm run build",
"prepublishOnly": "npm run update-version && npm run build",
Expand Down
11 changes: 11 additions & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"include": ["lib/**/*", "espree.js"],
"compilerOptions": {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use "strict": true in here? https://www.typescriptlang.org/tsconfig#strict

Copy link
Author

Choose a reason for hiding this comment

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

Ah, good point

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it has any effects - we don't have ts code, and didn't enable "checkJs": true.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, you could be right, I thought there might be some stricter behavior/validation it might enable. Regardless, it probably wouldn't hurt to use it, especially in case we add "checkJs": true later.

"allowJs": true,
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
"declaration": true,
"emitDeclarationOnly": true,
"outDir": "dist",
"declarationMap": true,
"strict": true
}
}