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

Improve parsing Issues reporting #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
All notable changes to this project from version 1.2.0 upwards are documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [Unreleased]

### Changed
- `Position`'s method `isEmpty` is deprecated in favour of `isFlat` to ensure consistency across the [StarLasu](https://github.com/Strumenta/StarLasu) libraries collection.
- `Issue`'s messages are capitalized.
- Parsing `Issue`s' position uses the token's length.

## [1.6.29] – 2024-07-09

### Fixed
Expand Down
12 changes: 10 additions & 2 deletions src/model/position.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,18 @@ export class Position {

/**
* If start and end are the same,
* then this Position is considered empty.
* then this Position is considered flat.
*/
isFlat(): boolean {
return this.start.equals(this.end);
}

/**
* @deprecated
* Use `this.isFlat()` instead.
*/
isEmpty(): boolean {
return this.start.equals(this.end)
return this.isFlat();
}

/**
Expand Down
23 changes: 18 additions & 5 deletions src/parsing/tylasu-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {
Recognizer,
TerminalNode,
Token,
TokenStream
TokenStream,
CommonToken
} from "antlr4ng";
import {Issue, IssueSeverity} from "../validation";
import {Point, Position, Source, StringSource} from "../model/position";
Expand Down Expand Up @@ -120,14 +121,20 @@ export abstract class TylasuANTLRLexer<T extends TylasuToken> implements TylasuL
reportAttemptingFullContext() {},
reportContextSensitivity() {},
syntaxError<S extends Token, T extends ATNSimulator>(recognizer: Recognizer<T>, offendingSymbol: S | null, line: number, charPositionInLine: number, msg: string) {
const startPoint = new Point(line, charPositionInLine);
let endPoint = new Point(line, charPositionInLine);
if (offendingSymbol instanceof CommonToken) {
const tokenLength = offendingSymbol.stop - offendingSymbol.start + 1;
endPoint = new Point(offendingSymbol.line, offendingSymbol.column + tokenLength);
};
const regex = /token recognition error at: '(.+)'/
if (regex.test(msg)){
const match = msg.match(regex) as string[];
issues.push(
Issue.lexical(
msg || "unspecified",
IssueSeverity.ERROR,
Position.ofPoint(new Point(line, charPositionInLine)),
new Position(startPoint, endPoint),
undefined,
TOKEN_RECOGNITION_ERROR,
[
Expand All @@ -141,7 +148,7 @@ export abstract class TylasuANTLRLexer<T extends TylasuToken> implements TylasuL
Issue.lexical(
msg || "unspecified",
IssueSeverity.ERROR,
Position.ofPoint(new Point(line, charPositionInLine)),
new Position(startPoint, endPoint),
undefined,
SYNTAX_ERROR));
}
Expand Down Expand Up @@ -301,6 +308,12 @@ export abstract class TylasuParser<
reportAttemptingFullContext() {},
reportContextSensitivity() {},
syntaxError<S extends Token, T extends ATNSimulator>(recognizer: Recognizer<T>, offendingSymbol: S | null, line: number, charPositionInLine: number, msg: string) {
const startPoint = new Point(line, charPositionInLine);
let endPoint = new Point(line, charPositionInLine);
if (offendingSymbol instanceof CommonToken) {
const tokenLength = offendingSymbol.stop - offendingSymbol.start + 1;
endPoint = new Point(offendingSymbol.line, offendingSymbol.column + tokenLength);
};
const mismatchedRegex = /^mismatched input '(<EOF>|.+)' expecting {([a-zA-Z_]+(, [a-zA-Z_]+)*)}$/
if (mismatchedRegex.test(msg)) {
const match = msg.match(mismatchedRegex) as string[];
Expand All @@ -316,7 +329,7 @@ export abstract class TylasuParser<
Issue.syntactic(
msg,
IssueSeverity.ERROR,
Position.ofPoint(new Point(line, charPositionInLine)),
new Position(startPoint, endPoint),
undefined,
MISMATCHED_INPUT,
args));
Expand All @@ -325,7 +338,7 @@ export abstract class TylasuParser<
Issue.syntactic(
msg || "unspecified",
IssueSeverity.ERROR,
Position.ofPoint(new Point(line, charPositionInLine)),
new Position(startPoint, endPoint),
undefined,
SYNTAX_ERROR));
}
Expand Down
6 changes: 6 additions & 0 deletions src/utils/capitalize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* Capitalize the first letter of a string
*/
export function capitalize(str: string) {
return str.charAt(0).toUpperCase() + str.slice(1);
}
3 changes: 3 additions & 0 deletions src/validation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {Node} from "./model/model";
import {Position} from "./model/position";
import { capitalize } from "./utils/capitalize";

export enum IssueType { LEXICAL, SYNTACTIC, SEMANTIC}

Expand All @@ -21,6 +22,8 @@ export class Issue {
public readonly code?: string,
public readonly args: IssueArg[] = []
) {
this.message = capitalize(message);

if (!position) {
this.position = node?.position;
}
Expand Down
6 changes: 6 additions & 0 deletions tests/issues.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,10 @@ describe('Issues', function() {
SOURCE_NODE_NOT_MAPPED, [{ name: "nodeType", value: "SomeNode" }]);
expect(i18next.t(issue.code!, { type: issue.args[0].value })).to.equal("Source node not mapped: SomeNode");
});

it("has capitalized messages",
function () {
let issue = Issue.syntactic("unexpected token: foo", IssueSeverity.ERROR, undefined, undefined, SYNTAX_ERROR);
expect(issue.message).to.equal("Unexpected token: foo");
});
});
20 changes: 19 additions & 1 deletion tests/parsing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('Parsing', function() {
IssueType.SYNTACTIC,
"mismatched input '+' expecting {INT_LIT, DEC_LIT, STRING_LIT, BOOLEAN_LIT}",
IssueSeverity.ERROR,
new Position(new Point(1, 11), new Point(1, 11)),
new Position(new Point(1, 11), new Point(1, 12)),
undefined,
"parser.mismatchedinput",
[
Expand Down Expand Up @@ -225,4 +225,22 @@ describe('Parsing', function() {
]
)])
})
it("produces issues with non-flat positions",
function() {
const code =
"set set a = 10\n" +
"|display c\n";
const parser = new SLParser(new ANTLRTokenFactory());
const result = parser.parse(code);

expect(result.issues.length).to.not.eq(0)

const extraneousInput = result.issues.find(issue => issue.message.startsWith("Extraneous input 'set'"))
expect(!(extraneousInput?.position?.isFlat()))
expect(extraneousInput?.position).to.eql(new Position(new Point(1, 4), new Point(1, 7)))

const mismatchedInput = result.issues.find(issue => issue.message.startsWith("Mismatched input 'c'"))
expect(!(mismatchedInput?.position?.isFlat()))
expect(mismatchedInput?.position).to.eql(new Position(new Point(2, 9), new Point(2, 10)))
})
});
6 changes: 3 additions & 3 deletions tests/transformation/transformation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ describe("AST Transformers", function () {
transformer.addIssue("warning", IssueSeverity.WARNING);
transformer.addIssue("info", IssueSeverity.INFO, pos(1, 0, 1, 2));

expect(transformer.issues[0].message).to.be.equal("error");
expect(transformer.issues[1].message).to.be.equal("warning");
expect(transformer.issues[2].message).to.be.equal("info");
expect(transformer.issues[0].message).to.be.equal("Error");
expect(transformer.issues[1].message).to.be.equal("Warning");
expect(transformer.issues[2].message).to.be.equal("Info");
});
it("transform function does not accept collections as source", function () {
const transformer = new ASTTransformer();
Expand Down