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

Add end position to ParserError #157

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+206 −175
Draft

Add end position to ParserError #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
165 changes: 132 additions & 33 deletions src/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Token, KeywordDescTable } from './token';
import { Errors, report } from './errors';
import { Node, Comment, Decorator, SourceLocation } from './estree';
import { Node, Comment, Decorator, SourceLocation, Position, Location } from './estree';
import { nextToken } from './lexer/scan';

/**
Expand Down Expand Up @@ -133,7 +133,7 @@ export const enum Flags {
SimpleParameterList = 1 << 7,
HasStrictReserved = 1 << 8,
StrictEvalArguments = 1 << 9,
DisallowCall = 1 << 10,
DisallowCall = 1 << 10,
HasOptionalChaining = 1 << 11
}

Expand Down Expand Up @@ -200,34 +200,133 @@ export interface ScopeError {
* The parser interface.
*/
export interface ParserState {
source: string;
flags: Flags;
index: number;
line: number;
column: number;
tokenPos: number;
startPos: number;
startColumn: number;
startLine: number;
colPos: number;
linePos: number;
end: number;
token: Token;
onComment: any;
onToken: any;
tokenValue: any;
tokenRaw: string;
/**
* The source code to be parsed
*/
source: string
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the semicolon in interface def?

Copy link
Author

Choose a reason for hiding this comment

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

Not an actual reason. I'm just used to don't use semis.

Copy link
Member

Choose a reason for hiding this comment

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

But they are there for our other interface definitions. I suggest to put it back here for consistency.

I am surprised "prettier" does not have an opinion on this syntax, it accepted both. But there is semi: true in our prettier.config.js, a "prettier" bug?
If you want to cleanup the type def to remove semicolon, pls create another PR to touch all of the interfaces, if @KFlash has no strong opinion on this syntax thing.

Copy link
Member

Choose a reason for hiding this comment

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

@fisker should not semi:true in prettier.config.js enforce the semicolon here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, why don't we run prettier --check on CI?

Copy link
Member

@3cp 3cp Nov 22, 2020 •
edited

Choose a reason for hiding this comment

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

Got it, we got these in .prettierignore. @KFlash why we ignored those source files?

bench/v1.6.2/
bench/v2/
bench/v8-native-calls.js
node_modules/
dist/
src/token.ts
src/common.ts
src/chars.ts
src/lexer/charClassifier.ts
src/unicode.ts
package-lock.json

Copy link
Contributor

Choose a reason for hiding this comment

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

the style get fucked up with Prettier and not lined. Regarding unicode.ts it will be a long list (2000 lines)

Copy link
Contributor

Choose a reason for hiding this comment

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

@3cp Possible to use non-open source to replace Prettier? I got my own pretty printer we could use, but ain't going to open source that one.

Copy link
Member

Choose a reason for hiding this comment

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

It could be weird for open source project to depend on non-open project.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point!


/**
* The mutable parser flags, in case any flags need passed by reference.
*/
flags: Flags

/**
* The current index
*/
index: number

/**
* Beginning of current line
*/
line: number

/**
* Beginning of current column
*/
column: number

/**
* Start position of whitespace before current token
*/
startPos: number

/**
* The end of the source code
*/
end: number

/**
* Start position of text of current token
*/
tokenPos: number

/**
* Start position of the colum before newline
*/
startColumn: number

/**
* Position in the input code of the first character after the last newline
*/
colPos: number

/**
* The number of newlines
*/
linePos: number

/**
* Start position of text of current token
*/
startLine: number

/**
* Used together with source maps. File containing the code being parsed
*/
sourceFile: string | void

/**
* Holds the scanned token value
*/
tokenValue: any

/**
* The current token in the stream to consume
*/
token: Token

/**
* Holds the raw text that have been scanned by the lexer
*/
tokenRaw: string

/**
* Holds the regExp info text that have been collected by the lexer
*/
tokenRegExp: void | {
pattern: string;
flags: string;
};
sourceFile: string | void;
assignable: AssignmentKind | DestructuringKind;
destructible: AssignmentKind | DestructuringKind;
currentChar: number;
exportedNames: any;
exportedBindings: any;
leadingDecorators: Decorator[];
pattern: string
flags: string
}

/**
* The code point at the current index
*/
currentChar: number

/**
* https://tc39.es/ecma262/#sec-module-semantics-static-semantics-exportednames
*/
exportedNames: any

/**
* https://tc39.es/ecma262/#sec-exports-static-semantics-exportedbindings
*/
exportedBindings: any

/**
* Assignable state
*/
assignable: AssignmentKind | DestructuringKind

/**
* Destructuring state
*/
destructible: AssignmentKind | DestructuringKind

/**
* Holds either a function or array used on every comment
*/
onComment: any

/**
* Holds either a function or array used on every token
*/
onToken: any

/**
* Holds leading decorators before "export" or "class" keywords
*/
leadingDecorators: Decorator[]
}

/**
Expand Down Expand Up @@ -406,11 +505,11 @@ export function validateFunctionName(

if (t === Token.EscapedFutureReserved) {
report(parser, Errors.InvalidEscapedKeyword);
}
}

if (t === Token.EscapedReserved) {
report(parser, Errors.InvalidEscapedKeyword);
}
}
}

if ((t & Token.Reserved) === Token.Reserved) {
Expand Down Expand Up @@ -769,7 +868,7 @@ export function addBindingToExports(parser: ParserState, name: string): void {
}

export function pushComment(context: Context, array: any[]): any {
return function(type: string, value: string, start: number, end: number, loc: SourceLocation) {
return function (type: string, value: string, start: number, end: number, loc: SourceLocation) {
const comment: any = {
type,
value
Expand All @@ -788,7 +887,7 @@ export function pushComment(context: Context, array: any[]): any {
}

export function pushToken(context: Context, array: any[]): any {
return function(token: string, start: number, end: number, loc: SourceLocation) {
return function (token: string, start: number, end: number, loc: SourceLocation) {
const tokens: any = {
token
};
Expand Down
53 changes: 39 additions & 14 deletions src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ParserState } from './common';
import { Location, Node, Position } from './estree';
import { Context, finishNode, ParserState } from './common';

export const enum Errors {
Unexpected,
Expand Down Expand Up @@ -352,26 +353,39 @@ export class ParseError extends SyntaxError {
line: ParseError['line'];
column: ParseError['column'];
};
public end?: Position;
public index: number;
public line: number;
public column: number;
public description: string;
/*eslint-disable*/
constructor(startindex: number, line: number, column: number, type: Errors, ...params: string[]) {
constructor(startindex: number, loc: { start: Position; end?: Position }, type: Errors, ...params: string[]) {
const message =
'[' + line + ':' + column + ']: ' + errorMessages[type].replace(/%(\d+)/g, (_: string, i: number) => params[i]);
'[' +
loc.start.line +
':' +
loc.start.column +
']: ' +
errorMessages[type].replace(/%(\d+)/g, (_: string, i: number) => params[i]);
super(`${message}`);
this.index = startindex;
this.line = line;
this.column = column;
this.line = loc.start.line;
this.column = loc.start.column;
this.description = message;
/* Acorn compat */
this.loc = {
line,
column
} as any;
line: this.line,
column: this.line
};
this.end = loc.end
? {
line: loc.end.line,
column: loc.end.column
}
: undefined;
}
}

/**
* Throws an error
*
Expand All @@ -381,11 +395,22 @@ export class ParseError extends SyntaxError {
* @param {...string[]} params
* @returns {never}
*/
export function report(parser: ParserState, type: Errors, ...params: string[]): never {
throw new ParseError(parser.index, parser.line, parser.column, type, ...params);
export function report(parser: ParserState, context: Context, node: Node, type: Errors, ...params: string[]): never {
const end = finishNode(parser, context, parser.index, parser.line, parser.column, node).loc?.end;
Copy link
Member

Choose a reason for hiding this comment

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

The finishNode mutates nothing on parser: ParserState. If you look into the implementation, the loc.end is just

end: {
        line: parser.startLine,
        column: parser.startColumn
      }

Which you can directly get from parser.
Also, the availability of loc.end in finishNode depends on OptionsLoc, but you can always get parser.startLine directly.


throw new ParseError(
parser.index,
{
start: { line: parser.line, column: parser.column },
end: end ? ({ line: end.line, column: end.column } as Position) : undefined
},
type,
...params
);
}

export function reportScopeError(scope: any): never {
// warn me about this pls
throw new ParseError(scope.index, scope.line, scope.column, scope.type, scope.params);
}

Expand All @@ -400,8 +425,8 @@ export function reportScopeError(scope: any): never {
* @param {Errors} type
* @param {...string[]} params
*/
export function reportMessageAt(index: number, line: number, column: number, type: Errors, ...params: string[]): never {
throw new ParseError(index, line, column, type, ...params);
export function reportMessageAt(index: number, loc: Location, type: Errors, ...params: string[]): never {
throw new ParseError(index, loc, type, ...params);
}

/**
Expand All @@ -414,6 +439,6 @@ export function reportMessageAt(index: number, line: number, column: number, typ
* @param {number} column
* @param {Errors} type
*/
export function reportScannerError(index: number, line: number, column: number, type: Errors): never {
throw new ParseError(index, line, column, type);
export function reportScannerError(index: number, loc: Location, type: Errors): never {
throw new ParseError(index, loc, type);
}
11 changes: 7 additions & 4 deletions src/estree.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,19 @@ export interface _Node {
loc?: SourceLocation | null;
}

export interface SourceLocation {
source?: string | null;
export interface Location {
start: Position;
end: Position;
}

export interface SourceLocation extends Location {
source?: string | null;
}

export interface Position {
/** >= 1 */
/** \>= 1 */
tscpp marked this conversation as resolved.
Show resolved Hide resolved
line: number;
/** >= 0 */
/** \>= 0 */
column: number;
}

Expand Down