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.

Sign up for GitHub

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

Jump to bottom

docs: added JS doc comments and tsconfig #530

Closed
wants to merge 4 commits into from
Closed

docs: added JS doc comments and tsconfig #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f8ce0d2
docs: add jsdoc types to espree
srijan-paul Jan 18, 2022
41d48d8
docs: add jsdoc types to espree
srijan-paul Jan 18, 2022
b7a1e85
fix: fix some inconsistent type definitions
srijan-paul May 6, 2022
63f5b65
fix: add missing type definition files
srijan-paul May 6, 2022
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
16 changes: 12 additions & 4 deletions espree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ 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").Range} TokenRange
* @typedef {import("estree").Program} Program
*/


// To initialize lazily.
const parsers = {
Expand Down Expand Up @@ -101,8 +109,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 +132,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 parse.
* @returns {Program} The "Program" AST node.
* @throws {SyntaxError} If the input code is invalid.
*/
export function parse(code, options) {
Expand Down
39 changes: 31 additions & 8 deletions lib/espree.js
View file
Open in desktop
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
* @typedef {import("./token-translator").Location} Location
*/

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 index of the comment in source code.
* @property {number|undefined} end End index of the comment in source code.
* @property {Range|undefined} range The [start, end] range of a comment.
* @property {Location} loc Location of the comment.
*/

/**
* 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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 index of the token in source code.
* @property {number|undefined} end end index of the token in source code.
* @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
17 changes: 12 additions & 5 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@
"require": "./dist/espree.cjs",
"default": "./dist/espree.cjs"
},
"./dist/espree.cjs"
"./dist/espree.cjs",
"./dist/lib"
],
"./package.json": "./package.json"
},
"version": "9.3.1",
"files": [
"lib",
"dist/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 All @@ -31,14 +35,15 @@
},
"license": "BSD-2-Clause",
"dependencies": {
"acorn": "^8.7.0",
"acorn-jsx": "^5.3.1",
"acorn": "^8.7.1",
"acorn-jsx": "^5.3.2",
"eslint-visitor-keys": "^3.3.0"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^17.1.0",
"@rollup/plugin-json": "^4.1.0",
"@rollup/plugin-node-resolve": "^11.2.0",
"@types/estree": "^0.0.51",
"c8": "^7.11.0",
"chai": "^4.3.4",
"eslint": "^7.22.0",
Expand All @@ -52,7 +57,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 +76,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
View file
Open in desktop
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
}
}